Skip to content

Enhancements to Codable API #9791

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
May 23, 2017
Merged

Enhancements to Codable API #9791

merged 6 commits into from
May 23, 2017

Conversation

@itaiferber
Copy link
Contributor

@itaiferber itaiferber commented May 19, 2017 •
edited
Loading

What's in this pull request?

  • Adds conformance of Optional to Codable
  • encode(...) arguments are no longer Optional; Optional values go through generic version
  • encodeIfPresent added to KeyedEncodingContainerProtocol to mirror decodeIfPresent
  • Optional values synthesize encodeIfPresent(...) and decodeIfPresent(...) calls instead of encode(...) and decode(...)
  • encodeNil() and decodeNil() added as proper primitives on SingleValueEncodingContainer and SingleValueDecodingContainer, as well as encode<T : Encodable>(_: T) and decode<T : Decodable>(_: T) throws -> T
  • JSONEncoder and PropertyListEncoder updated to reflect these changes

NOTE: These changes are currently going through API review, but we want to have them up and ready unless something changes. These changes will be reflected in the swift-evolution proposal, and will go out to swift-evolution.

@itaiferber itaiferber changed the title Update Codable API for Optional values Enhancements to Codable API May 19, 2017
@itaiferber
Copy link
Contributor Author

itaiferber commented May 19, 2017

@swift-ci Please smoke test

@jrose-apple
Copy link
Contributor

jrose-apple commented May 19, 2017

I don't think we want to take the unconditional conformance of Optional to Codable. Someone could accidentally depend on that in a case where the conditional conformance would said no, and then us changing it would break source compatibility. Please continue to special-case Optional.

@itaiferber
Copy link
Contributor Author

itaiferber commented May 19, 2017 •
edited
Loading

@jrose-apple The same case can be made for Array, Set, and Dictionary, which are already in (and that we need)... We cannot change the rest of the API once this is in, and this is a really important part of the feature.

Encoding/decoding will fail at runtime (so you can't actually encode a non-Codable Optional value and rely on it in any way), but the commit I just pushed will prevent derived conformance if you've got properties of any of these types if they won't actually be Codable at runtime.

@jrose-apple
Copy link
Contributor

jrose-apple commented May 19, 2017

I don't think it's correct for Array, Set, or Dictionary either, and I'm sorry I didn't catch that the first time around. That still doesn't mean you can introduce a pitfall like this, though. It's much easier to add the API that handles these wrapping types, and then later (or even now) provide a default implementation for them.

@jrose-apple jrose-apple requested a review from airspeedswift May 19, 2017 23:43
@itaiferber
Copy link
Contributor Author

itaiferber commented May 19, 2017 •
edited
Loading

@jrose-apple This is a stop-gap until conditional conformance lands. We can't prevent people from encoding arrays, sets, dictionaries, and optionals. If we add a combinatorial explosion of overloads supporting these types, we cannot remove them later, nor can we handle recursive types with them.

Let's take this to email though. Please CC @parkera too.

@tkremenek
Copy link
Member

tkremenek commented May 20, 2017

@swift-ci test

@swift-ci
Copy link
Contributor

swift-ci commented May 20, 2017

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 222266d1bce5a042bac8451ec6c4593808226ad2
Test requested by - @tkremenek

@swift-ci
Copy link
Contributor

swift-ci commented May 20, 2017

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 222266d1bce5a042bac8451ec6c4593808226ad2
Test requested by - @tkremenek

@parkera parkera self-requested a review May 21, 2017 19:36
Itai Ferber added 3 commits May 22, 2017 09:29
Introduces changes to Codable stdlib API
d16297d 
* Adds conformance of Optional to Codable
* encode(...) arguments are no longer Optional; Optional values go
  through generic version
* encodeIfPresent added to KeyedEncodingContainerProtocol to mirror
  decodeIfPresent
* JSONEncoder and PropertyListEncoder updated to reflect these changes
Prevent synthesis for non-Codable stdlib types
91a8aaa 
Optional, Array, Set, and Dictionary currently all conform to Codable
regardless of the type they are generic on. Until conditional
conformance lands and we can rely on their conditional conformance, we
want to prevent Codable derived conformance if a type contains a
property of one of these types when we know it's actually going to fail
at runtime.
Synthesize *IfPresent calls for Optional values
26d0d5d 
Optional properties now get synthesized encodeIfPresent(...) and
decodeIfPresent(...) calls (to avoid encoding needless information, and
to be more accepting on input).
@itaiferber itaiferber force-pushed the swift-archival-serialization-updates branch from 2dd134f to 26d0d5d Compare May 22, 2017 19:17
Clean up Codable synthesis slightly
dd0bf0e 
Following feedback in swiftlang#9758:
* Use a struct type for the result of hasValidCodingKeysEnum
* Use an enum type for the result of
* Skip fetching a canonical type before calling getAnyNominal()
  (getAnyNominal() looks through sugar already)
* No need to cast to IterableDeclContext (or DeclContext, even)
@itaiferber
Copy link
Contributor Author

itaiferber commented May 22, 2017

@swift-ci Please test

@swift-ci
Copy link
Contributor

swift-ci commented May 22, 2017

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 2dd134f12239df7f492b41603298a2ab2983586d
Test requested by - @itaiferber

@swift-ci
Copy link
Contributor

swift-ci commented May 22, 2017

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 2dd134f12239df7f492b41603298a2ab2983586d
Test requested by - @itaiferber

Itai Ferber added 2 commits May 22, 2017 14:05
Add explicit support for IUOs
9988624 
Since implicitly unwrapped optional types do not reflect conformances
added to Optional, we need to explicitly allow and handle them.
Fix failing autocomplete IDE unit test
a0f5c10
@itaiferber
Copy link
Contributor Author

itaiferber commented May 22, 2017

@swift-ci Please test

@swift-ci
Copy link
Contributor

swift-ci commented May 22, 2017

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - dd0bf0e
Test requested by - @itaiferber

@swift-ci
Copy link
Contributor

swift-ci commented May 22, 2017

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

@tkremenek tkremenek merged commit dbe7760 into swiftlang:master May 23, 2017
@itaiferber itaiferber mentioned this pull request May 23, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@parkera parkera parkera approved these changes

@airspeedswift airspeedswift Awaiting requested review from airspeedswift

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@itaiferber @jrose-apple @tkremenek @swift-ci @parkera