Skip to content

[4.0] Optionality updates to Codable API #10706

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

Merged
merged 6 commits into from
Jun 29, 2017

Conversation

itaiferber
Copy link
Contributor

What's in this pull request?
Cherry-picks #10667 to swift-4.0-branch.

Explanation: As laid out in a proposed update to SE-0166 and SE-0167, there are optionality updates that we wanted to introduce to the Codable API. This implements the remainder of those updates which had not yet been implemented:

  • encodeNil(forKey:) and decodeNil(forKey:) have been added to keyed containers
  • encodeNil() and decodeNil() have been added to unkeyed containers
  • The default implementation of decode(...) in terms of decodeIfPresent(...) on all containers has been reversed: with decodeNil(...) in place, a default implementation of decodeIfPresent(...) is now given on all containers, and decode(...) must be implemented instead
    • These methods are now properly exposed on KeyedEncodingContainer and KeyedDecodingContainer so it should be possible to give actual implementations instead of calling into the defaults
  • Types should now be able to encode and decode from null values through single value containers

Scope: Affects Encoder/Decoder writers as well as users of the Codable API with Optional values.
Radars:

  • rdar://problem/32571881
  • rdar://problem/32650311

Risk: Low
Testing: This adds unit tests to confirm that decoding from null values works as expected.

Itai Ferber added 6 commits June 29, 2017 13:21
* Add encodeNil(forKey:)/encodeNil() for keyed/unkeyed encoding
  containers
* Add decodeNil(forKey:)/decodeNil() for keyed/unkeyed decoding
  containers
* Give default implementation of decodeIfPresent(forKey:)/
  decodeIfPresent(_:) on keyed/unkeyed decoding containers instead of
  decode(forKey:)/decode(_:)
* Expose all encode/decode methods on keyed encoding & decoding
  containers to allow overriding default methods (which were previously
  not forwarding)
* Add encodeNil/decodeNil variants on keyed and unkeyed containers
* Give implementations of decode() variants instead of decodeIfPresent()
* Add encodeNil/decodeNil variants on keyed and unkeyed containers
* Give implementations of decode() variants instead of decodeIfPresent()
To make it slightly easier to tell at a glance if a method is
implemented as part of API (public/open), shared implementation detail
(fileprivate), or implementation detail meaningful only to a given type
(private).
* Update {JSON,PropertyList}Decoder to allow superDecoder()s to wrap
  null value so current implementations of collections can decode
  contained objects from nil
* Eliminate null checks from unboxing in the general case (so types
  can at least attempt to decode from nil)
* Add unit tests to confirm this behavior
@itaiferber
Copy link
Contributor Author

@swift-ci Please test

@itaiferber
Copy link
Contributor Author

@shahmishal When I merged in #10667, I also merged in swiftlang/swift-corelibs-foundation#1087 simultaneously. Since these have to go in together, I think this will end up breaking folks' Linux 4.0 CI builds until this is in. Just a heads-up; we're looking to merge this in ASAP to fix that.

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 205e2cb
Test requested by - @itaiferber

@parkera parkera self-requested a review June 29, 2017 20:35
@itaiferber
Copy link
Contributor Author

@swift-ci Please clean test Linux

@itaiferber itaiferber merged commit fe1a2da into swiftlang:swift-4.0-branch Jun 29, 2017
@itaiferber itaiferber deleted the swift-4.0-branch branch June 29, 2017 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants