-
Notifications
You must be signed in to change notification settings - Fork 107
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
Encodable and Decodable support for choice elements #119
Conversation
Add CodingKey conformance to ChoiceKey
Add SingleElementEncodingContainer Fix def Expand ReferencingEncoder Complete SingleElementEncodingContainer Add decode-side hacks Add special diversion for SingleElementContainer Add hacky runtime bifurcation Add case for XMLEncoder Add SingleElementBox based XMLCoderElement initializer Change open permissions to public Add usage Add usage Add Nested cases within UnkeyedContainer Make use of SingleElementBox key Add UnkeyedBox of SingleElementBox fix Add static check for array of SingleElementBox type Implement nesting for keyed container Fix top level container func Implement nesting for SingleElementKeyed Container Clean up SingleElementBox initialization Fix formatting
Create interface for branching off XMLChoiceKeys Add XMLSingleElementDecodingContainer copy pasta Add XMLDecoderImplementation.singleElementContainer(keyedBy:) copy pasta Push XMLChoiceKeys through singleElementContainer Add XMLUnkeyedDecodingContainer.nestedSingleElementContainer copy pasta Add XMLKeyedDecodingContainer.nestedSingleElementContainer copy pasta Add XMLSingleElementDecodingContainer.nestedSingleElementContainer copy pasta Remove print statement from test Make IntOrStringWrapper.CodingKeys: XMLChoiceKey Make Entry.CodingKeys: XMLChoiceKey Only allow KeyedBoxes pass through to SingleElementDecodingContainer Actually use XMLSingleElementDecodingContainer Make tests pass Rename XMLSingleElementDecodingContainer -> XMLChoiceDecodingContainer Use ChoiceBox Get rid of some prints Unimplement singleElementContainer Unimplement singleElementContainer Tidy xcscheme Unimplement nestedSingleElementContainer Remove dump Replace fatalError with thrown error Omit type label Omit type label Fix formatting
Rename singleElementBox -> choiceBox Create privileged path for Choices Sweep away commented-out code Add comment Don't treat top-level choice Tighten up impl Rename singleElementContainer method -> choiceContainer Whoops that was the Encoder Add unkeyed single element container et al. Add messages to fatal errors Omit type label Switch to ChoiceBox based implementation Revert pretty printing special casing Add passing encode tests for choice elements with attributes Add xcodeproj debris Remove use of XMLUnkeyedSingleElementDecodingContainer Remove unreached code in XMLChoiceDecodingContainer Remove superDecoder methods because enums ain't classes Put all the decode impl in one place Whitespace
Add doc comment for XMLChoiceCodingKey
Remove unused SingleKeyedBox.init?(_: KeyedBox) Change internal property name from singleElementBox to singleKeyedBox Remove internal property names singleElement -> singleKeyed
Rename header XMLSingleElementEncodingContainer -> XMLChoiceEncodingContainer Fix formatting
Slim down SingleKeyedBox Remove Foundation imports where not necessary
Add nested array encoding test Add array within dictionary test Formatting Add keyed within unkeyed test Add roundtrip test for keyed within keyed case Update test to roundtrip for robustness Add wrapped tests for encoding Fix formatting
Resolve merge conflict Replace xcodeproj
Reorder key decoding strategy cases Touch less Mutate directly
Ah, it looks like we lost some |
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.
This is great work @jsbean! I especially like the proposal format 👏
A few nitpicky requests here, but overall good to merge in my opinion as soon as those are resolved.
topContainer = storage.pushChoiceContainer() | ||
} else { | ||
guard let container = storage.lastContainer as? SharedBox<ChoiceBox> else { | ||
preconditionFailure("Attempt to push new (single element) keyed encoding container when already previously encoded at this path.") |
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.
can this string be split into multiple lines with """
to bring it closer to 80 characters, or at least 100?
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.
Again, this looks like a copy-the-style-as-it-is job. Do you want me to tidy up the other instances as well or just stick to the new code?
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.
yes please, the overall cleanup is much appreciated!
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.
Is there a strict line length for the repo? The .swiftlint.yml
declares violations at 150
and 200
. I've been sticking to 100 out of habit, but I can constrain these to something else if it is so desired.
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 been trying to get some balance between changing the whole repository and maintaining readability. 80 doesn't work well when indentation is 4 spaces instead of 2, but I decided to stick with 4 to preserve the changes history. You can clean up as much as you don't find tedious. I think last time I tried to tighten the limit in .swiftlint.yml
(even to 120) it required too many code changes and reformatting XML snippets in the unit tests, which I just decided to postpone, but never got back to it. Maybe if there too many changes of this type, it can be made in a separate PR.
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.
All seems fair to me! Probably worth waiting for the official swift-format to roll out to act on anything?
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.
sure, makes sense
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.
For now, I just touched the long preconditionFailure
messages.
38c01f3
to
d4bd9f4
Compare
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.
One more thing if you don't mind: can you add a subsection to README.md
in the advanced features section with a short description of the feature specifying that this is available since version 0.8.0, with the link to this PR there as well please? The new README section would contain a simple explanation of the feature, while the link to the PR would make the proposal itself visible for anyone curious.
A couple things, @MaxDesiatov.
|
@jsbean throwing an error is preferrable, since the error case can be unit-tested later if/when we want to increase the code coverage. As long as this doesn't add |
Let me know if you'd like the README subsection to be more in-depth. |
Before this makes it in, 👏 to @bwetherfield for the insight to cut this problem at the |
(It seems that Homebrew is having a hard time only on the |
CI passes now, thank you @jsbean and @bwetherfield! I plan to tag 0.8.0 with this feature and Linux support in the next few days. |
## Introduction In merging in #119, we fixed most but not quite all of #91! Decoding of _null_ choice elements (represented as enums with empty struct associated values on the Swift side) still results in errors. This PR adds a test to demonstrate and fixes this issue by wrapping each `NullBox` inside of a `SingleKeyedBox` at the `merge` phase (called by `transformToBoxTree`). ## Motivation One of the main lessons from #119 was that we have to wrap choice elements in the decoding phase to hold onto their keys. The keys are needed for directing us to the correct branch of the do-catch pyramid used for decoding. ```swift private enum Entry: Equatable { case run(Run) case properties(Properties) case br(Break) } extension Entry: Decodable { private enum CodingKeys: String, XMLChoiceCodingKey { case run, properties, br } public init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) do { self = .run(try container.decode(Run.self, forKey: .run)) } catch { do { self = .properties(try container.decode(Properties.self, forKey: .properties)) } catch { self = .br(try container.decode(Break.self, forKey: .br)) } } } } ``` where one of the associated values could be an empty struct (represented by null): ```swift private struct Break: Decodable {} ``` Although we _can_ throw out keys for non-choice null elements, a mechanism is needed for holding onto the keys while transforming from the `XMLCoderElement` tree to the `boxTree`. Only later will we know if the key is needed (if this wrapped element is transformed to a `ChoiceBox`); if not, we will be able to throw out the key. ## Proposed solution The Public API is unchanged. On the implementation side, we catch `NullBox` values in `merge` and wrap them in `SingleKeyedBox` instances. ## Detailed Design In `merge`, we wrap each `NullBox` in a `SingleKeyedBox` with the appropriate key bundled in. An `XMLChoiceDecodingContainer` can be constructed from the `SingleKeyedBox` by converting it to a `ChoiceBox` (just transferring over the contents) - as normal. In `XMLKeyedDecodingContainer`, when preparing the `elements` for concrete decoding, we unwrap all `SingleKeyedBox` values that may be contained therein, as any choice elements contained would have already been transformed to a `ChoiceBox` by this point in decoding: any stray `SingleKeyedBox` wrappers can thus be thrown out. ## Source compatibility This is purely an additive change.
## Overview Fixes bug encountered when encoding structs that hold a mixture of choice-element and non-choice-element (or multiple choice-element) properties. ## Example Given a structure that stores both a choice and non-choice property, ```swift private struct IntOrStringAndDouble: Equatable { let intOrString: IntOrString let decimal: Double } ``` the natural encoding approach (now available) is ```swift extension IntOrStringAndDouble: Encodable { enum CodingKeys: String, CodingKey { case decimal } func encode(to encoder: Encoder) { try intOrString.encode(to: encoder) var container = encoder.container(keyedBy: CodingKeys.self) try container.encode(decimal, forKey: .decimal) } } ``` The following `encode` implementation also works: ```swift extension IntOrStringAndDouble: Encodable { enum CodingKeys: String, CodingKey { case decimal } func encode(to encoder: Encoder) { var singleValueContainer = encoder.singleValueContainer() try singleValueContainer.encode(intOrString) var container = encoder.container(keyedBy: CodingKeys.self) try container.encode(decimal, forKey: .decimal) } } ``` `IntOrString` as defined in #119: ```swift enum IntOrString: Equatable { case int(Int) case string(String) } extension IntOrString: Encodable { enum CodingKeys: String, CodingKey { case int case string } func encode(to encoder: Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) switch self { case let .int(value): try container.encode(value, forKey: .int) case let .string(value): try container.encode(value, forKey: .string) } } } extension IntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `IntOrString` is a choice element ``` ## Implementation Details In cases where choice and non-choice elements (or multiple choice elements) co-exist in a keyed container, we merge them into a single `XMLKeyedEncodingContainer` (wrapping a `SharedBox<KeyedBox>`). Arrays of choice elements (using `XMLUnkeyedEncodingContainer` under the hood) are encoded the same way as before, as we do not hit the merging cases. For the array case, we still need the `XMLChoiceEncodingContainer` structure. ## Source Compatibility This is an additive change. * Add breaking case * Add choice and keyed merging encode functionality * Refactor * Fix commented code * Fix misnamed file * Fix xcode project * Fix precondition catch * Use switch syntax * Add multiple choice element case * Add explicit types in KeyedBox initialization * Add explicitly empty parameter to KeyedBox initializer * Use more concise type inference * Unify switch syntax * Cut down code duplication * Fix formatting
## Introduction This PR introduces the `XMLChoiceCodingKey` protocol, which enables the encoding and decoding of union-type–like enums with associated values to and from `XML` choice elements. Resolves CoreOffice#25. Resolves CoreOffice#91. ## Motivation XML schemas support [choice](https://www.w3schools.com/xml/el_choice.asp) elements, which constrain their contents to a single instance of a member of a known set of types. Choice elements exhibit the properties of [union types](https://en.wikipedia.org/wiki/Union_type) and can be represented in Swift as enums with associated values, wherein each case of the enum carries with it a single associated value that is one of the types representable. An example of how such a type is implemented in Swift: ```Swift enum IntOrString { case int(Int) case string(String) } ``` There is currently no automatic synthesis of the `Codable` protocol requirements for enums with assocated types in today's Swift. As such, it is required to provide custom implementations of the `init(from: Decoder)` initializer and the `encode(to: Encoder)` method to conform to the `Encodable` and `Decodable` protocols, respectively. When encoding to and decoding from `JSON`, a single-element keyed container is created that uses the enum case name as the single key. An example of adding Codable conformance to such a type when working with JSON ```Swift extension IntOrString: Codable { enum CodingKeys: String, CodingKey { case int, string } init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) do { self = .int(try container.decode(Int.self, forKey: .int)) } catch { self = .string(try container.decode(String.self, forKey: .string)) } } func encode(to encoder: Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) switch self { case let .int(value): try container.encode(value, forKey: .int) case let .string(value): try container.encode(value, forKey: .string) } } } ``` This may not be the most handsome approach, but it does the job without imprinting any format-specfic logic onto otherwise format-agnostic types. This pattern works out of the box with the `JSONEncoder` and `JSONDecoder` provided by the `Foundation` framework. However, due to syntactic characteristics of the `XML` format, this pattern will **_not_** work automatically for encoding and decoding `XML`-formatted data, regardless of the tool used. ## Proposed solution The proposed solution is to define a new `XMLChoiceCodingKey` protocol: ```Swift /// An empty marker protocol that can be used in place of `CodingKey`. /// It must be used when conforming a union-type–like enum with associated values to `Codable` /// when the encoded format is `XML`. public protocol XMLChoiceCodingKey: CodingKey {} ``` The `XMLChoiceCodingKey` protocol inherits from `CodingKey` and adds no new requirements. This conformance can be made retroactively, without additional implementation. An example usage: ```Swift extension IntOrString.CodingKeys: XMLChoiceCodingKey {} ``` ## Detailed design This proposal adds a single `public` `protocol` `XMLChoiceCodingKey`, as well as several `internal` types. Under the hood, the `XMLChoiceEncodingContainer` and `XMLChoiceDecodingContainer` are used to provide `encode` and `decode` methods tuned for `XML` choice elements. Because of the characteristics of the `XML` format, there are some ambiguities (from an encoding and decoding perpsective) between unkeyed container elements that contain choice elements and those that contain nested unkeyed container elements. In order to untangle these ambiguities, the new container types utilize a couple of new `Box` types to redirect elements along the encoding and decoding process. ## Source compatibility This is purely an additive change.
## Introduction In merging in CoreOffice#119, we fixed most but not quite all of CoreOffice#91! Decoding of _null_ choice elements (represented as enums with empty struct associated values on the Swift side) still results in errors. This PR adds a test to demonstrate and fixes this issue by wrapping each `NullBox` inside of a `SingleKeyedBox` at the `merge` phase (called by `transformToBoxTree`). ## Motivation One of the main lessons from CoreOffice#119 was that we have to wrap choice elements in the decoding phase to hold onto their keys. The keys are needed for directing us to the correct branch of the do-catch pyramid used for decoding. ```swift private enum Entry: Equatable { case run(Run) case properties(Properties) case br(Break) } extension Entry: Decodable { private enum CodingKeys: String, XMLChoiceCodingKey { case run, properties, br } public init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) do { self = .run(try container.decode(Run.self, forKey: .run)) } catch { do { self = .properties(try container.decode(Properties.self, forKey: .properties)) } catch { self = .br(try container.decode(Break.self, forKey: .br)) } } } } ``` where one of the associated values could be an empty struct (represented by null): ```swift private struct Break: Decodable {} ``` Although we _can_ throw out keys for non-choice null elements, a mechanism is needed for holding onto the keys while transforming from the `XMLCoderElement` tree to the `boxTree`. Only later will we know if the key is needed (if this wrapped element is transformed to a `ChoiceBox`); if not, we will be able to throw out the key. ## Proposed solution The Public API is unchanged. On the implementation side, we catch `NullBox` values in `merge` and wrap them in `SingleKeyedBox` instances. ## Detailed Design In `merge`, we wrap each `NullBox` in a `SingleKeyedBox` with the appropriate key bundled in. An `XMLChoiceDecodingContainer` can be constructed from the `SingleKeyedBox` by converting it to a `ChoiceBox` (just transferring over the contents) - as normal. In `XMLKeyedDecodingContainer`, when preparing the `elements` for concrete decoding, we unwrap all `SingleKeyedBox` values that may be contained therein, as any choice elements contained would have already been transformed to a `ChoiceBox` by this point in decoding: any stray `SingleKeyedBox` wrappers can thus be thrown out. ## Source compatibility This is purely an additive change.
## Overview Fixes bug encountered when encoding structs that hold a mixture of choice-element and non-choice-element (or multiple choice-element) properties. ## Example Given a structure that stores both a choice and non-choice property, ```swift private struct IntOrStringAndDouble: Equatable { let intOrString: IntOrString let decimal: Double } ``` the natural encoding approach (now available) is ```swift extension IntOrStringAndDouble: Encodable { enum CodingKeys: String, CodingKey { case decimal } func encode(to encoder: Encoder) { try intOrString.encode(to: encoder) var container = encoder.container(keyedBy: CodingKeys.self) try container.encode(decimal, forKey: .decimal) } } ``` The following `encode` implementation also works: ```swift extension IntOrStringAndDouble: Encodable { enum CodingKeys: String, CodingKey { case decimal } func encode(to encoder: Encoder) { var singleValueContainer = encoder.singleValueContainer() try singleValueContainer.encode(intOrString) var container = encoder.container(keyedBy: CodingKeys.self) try container.encode(decimal, forKey: .decimal) } } ``` `IntOrString` as defined in CoreOffice#119: ```swift enum IntOrString: Equatable { case int(Int) case string(String) } extension IntOrString: Encodable { enum CodingKeys: String, CodingKey { case int case string } func encode(to encoder: Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) switch self { case let .int(value): try container.encode(value, forKey: .int) case let .string(value): try container.encode(value, forKey: .string) } } } extension IntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `IntOrString` is a choice element ``` ## Implementation Details In cases where choice and non-choice elements (or multiple choice elements) co-exist in a keyed container, we merge them into a single `XMLKeyedEncodingContainer` (wrapping a `SharedBox<KeyedBox>`). Arrays of choice elements (using `XMLUnkeyedEncodingContainer` under the hood) are encoded the same way as before, as we do not hit the merging cases. For the array case, we still need the `XMLChoiceEncodingContainer` structure. ## Source Compatibility This is an additive change. * Add breaking case * Add choice and keyed merging encode functionality * Refactor * Fix commented code * Fix misnamed file * Fix xcode project * Fix precondition catch * Use switch syntax * Add multiple choice element case * Add explicit types in KeyedBox initialization * Add explicitly empty parameter to KeyedBox initializer * Use more concise type inference * Unify switch syntax * Cut down code duplication * Fix formatting
Introduction
This PR introduces the
XMLChoiceCodingKey
protocol, which enables the encoding and decoding of union-type–like enums with associated values to and fromXML
choice elements.Resolves #25.
Resolves #91.
Motivation
XML schemas support choice elements, which constrain their contents to a single instance of a member of a known set of types. Choice elements exhibit the properties of union types and can be represented in Swift as enums with associated values, wherein each case of the enum carries with it a single associated value that is one of the types representable.
An example of how such a type is implemented in Swift
There is currently no automatic synthesis of the
Codable
protocol requirements for enums with assocated types in today's Swift. As such, it is required to provide custom implementations of theinit(from: Decoder)
initializer and theencode(to: Encoder)
method to conform to theEncodable
andDecodable
protocols, respectively.When encoding to and decoding from
JSON
, a single-element keyed container is created that uses the enum case name as the single key.An example of adding
Codable
conformance to such a type when working withJSON
This may not be the most handsome approach, but it does the job without imprinting any format-specfic logic onto otherwise format-agnostic types.
This pattern works out of the box with the
JSONEncoder
andJSONDecoder
provided by theFoundation
framework. However, due to syntactic characteristics of theXML
format, this pattern will not work automatically for encoding and decodingXML
-formatted data, regardless of the tool used.Proposed solution
The proposed solution is to define a new
XMLChoiceCodingKey
protocol:The
XMLChoiceCodingKey
protocol inherits fromCodingKey
and adds no new requirements. This conformance can be made retroactively, without additional implementation.An example usage
Detailed design
This proposal adds a single
public
protocol
XMLChoiceCodingKey
, as well as severalinternal
types.Under the hood, the
XMLChoiceEncodingContainer
andXMLChoiceDecodingContainer
are used to provideencode
anddecode
methods tuned forXML
choice elements.Because of the characteristics of the
XML
format, there are some ambiguities (from an encoding and decoding perpsective) between unkeyed container elements that contain choice elements and those that contain nested unkeyed container elements.In order to untangle these ambiguities, the new container types utilize a couple of new
Box
types to redirect elements along the encoding and decoding process.Source compatibility
This is purely an additive change.