-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Decoding of Arrays of Empty Elements #152
Conversation
…element-empty-string Final Fix for empty element / empty string !
Transform precondition to where clause in switch statement
…empty-string-test
…-array-empty-string-test Empty Strings and Empty Elements Clarification Sweep
Refactor XMLKeyedDecodingContainer.decodeConcrete elements massaging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, but with regards to compatibility concerns I wonder if there's a way to distinguish <empty/>
from <empty></empty>
, so that the former is decoded as nil
, but the latter as ""
?
I can't quite figure out if func parser(_: XMLParser,
didStartElement elementName: String,
namespaceURI: String?,
qualifiedName: String?,
attributes attributeDict: [String: String] = [:]) and func parser(_: XMLParser,
didEndElement _: String,
namespaceURI _: String?,
qualifiedName _: String?) in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just tested this with CoreXLSX, and while changes were required, they did make sense. Distinguishing between <element/>
and <element></element>
wasn't required after all. On balance, it's probably worth breaking compatibility here for the sake of general improvements.
@bwetherfield thank you for polishing this and finding new edge cases! 🙏 |
## Overview Fixes CoreOffice#123 and extends CoreOffice#145 to accommodate decoding of arrays of empty strings or mixed arrays of non-empty and empty strings. ## Example We may now decode ```xml <container> <empty/> <empty/> <empty/> </container> ``` into the following type ```swift struct EmptyArray: Equatable, Codable { enum CodingKeys: String, CodingKey { case empties = "empty" } let empties: [Empty] } ``` where ```swift struct Empty: Equatable, Codable { } ``` We can also decode a value of the following type ```swift struct EmptyWrapper { let empty: Empty } ``` from ```xml <wrapper> <empty/> </wrapper> ``` Further, following from CoreOffice#145 we can decode arrays of empty strings ```xml <container> <string-type/> <string-type>My neighbors are empty<string-type> <string-type/> </container> ``` as ```swift struct EmptyArray: Equatable, Codable { enum CodingKeys: String, CodingKey { case stringType = "string-type" } let stringType: [String] } ``` and variants. ## Source Compatibility In cases where we decode an array of type `[String?]`, an empty element is now decoded as `""` rather than `nil`, the justification being that `String` can itself take empty values. We use the convention that `nil` signifies the absence of an element (if 0 or 1 of the element are allowed) as in the updated [BreakfastTest](https://github.com/bwetherfield/XMLCoder/blob/0d20614e47df98d1a10174e992c585edf629c9b9/Tests/XMLCoderTests/BreakfastTest.swift) and in the following error-throwing [test case](https://github.com/MaxDesiatov/XMLCoder/blob/2855777ff868e8a4c1d944c7da0ddb49a8b3893e/Tests/XMLCoderTests/Minimal/NullTests.swift#L56-L68). * Add nested choice unkeyed container decoding test * Fix nested unkeyed container implementation for nested keyed box * Clean up unwrapping syntax * Treat corner case of empty string value intrinsic * Remove xcodeproj junk * Add some logging to assess where we're at * Add cases for empty string as null element decoding * Swiftformat * Transform precondition to where clause in switch statement * Remove print statements * Add failing test for a nested array of empty-string value intrinsic elements * Do a little cleanup of single keyed box passing around * Refactor XMLKeyedDecodingContainer.decodeConcrete elements massaging * Remove xcscheme junk * Add fix for empty arrays, wrapped empties etc * Clean up * Revert singleKeyed dive change * Accommodate singleKeyed reading options * Alter Border Test * Add test case that returns [nil] (exists a non-optional property) * Eliminate possibly empty Int from Breakfast test * Fix formatting * Fix forcecast * Fix formatting * Update LinuxMain * Fix tests such that null elements read as empty strings * Fix linux main * Add nested array of empty strings decoding in the explicit style * Add mixed case empty and non-empty string cases * Reinstate missing test * Add test for decoding a null element into an optional type
Overview
Fixes #123 and extends #145 to accommodate decoding of arrays of empty strings or mixed arrays of non-empty and empty strings.
Example
We may now decode
into the following type
where
We can also decode a value of the following type
from
Further, following from #145 we can decode arrays of empty strings
as
and variants.
Source Compatibility
In cases where we decode an array of type
[String?]
, an empty element is now decoded as""
rather thannil
, the justification being thatString
can itself take empty values. We use the convention thatnil
signifies the absence of an element (if 0 or 1 of the element are allowed) as in the updated BreakfastTest and in the following error-throwing test case.