Skip to content
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

[SR-4772] Classes conforming to Codable need to synthesize overrides instead of inheriting conformance #47349

Open
itaiferber opened this issue May 2, 2017 · 4 comments

Comments

@itaiferber
Copy link
Contributor

@itaiferber itaiferber commented May 2, 2017

Previous ID SR-4772
Radar rdar://problem/31919734
Original Reporter @itaiferber
Type Bug
Additional Detail from JIRA
Votes 2
Component/s Compiler
Labels Bug, DerivedConformance
Assignee @itaiferber
Priority Medium

md5: 9f437e13a1f41076d594d332117a57db

is duplicated by:

  • SR-5431 JSON does not Encode/Decode subclass properties
  • SR-5125 Encodable not decoding properties with subclasses

Issue Description:

Classes which inherit from superclasses conforming to Encodable or Decodable should not inherit their superclass's implementations in most cases:

class A1 : Encodable {
    // Derives Encodable automatically
    let name: String
    init(name: String) { self.name = name }
}

class B1 : A1 {
    // Silently inherits A1's encode(to:); encodes just {"name": ...}, with no email.
    // This is bad.
    let email: String
    init(name: String, email: String) { self.email = email; super.init(name: name) }
}

class A2 : Codable {
    // Derives Codable automatically
    let name: String
    init(name: String) { self.name = name }
}

// Inherits A2's methods. This is correct.
class B2 : A2 {}

class C2 : A2 {
    // Error: since init(from:) is required, C2 must provide one.
    // It doesn't though, so we get a diagnostic — this is wrong; we should derive in this case.
    let email: String
    init(name: String, email: String) { self.email = email; super.init(name: name) }
}

Really, the only case where we want to inherit conformance instead of derive is when the class does not add any valid Encodable/Decodable properties.

@belkadan
Copy link
Contributor

@belkadan belkadan commented May 2, 2017

I'm uncomfortable with the idea of hacking the compiler like this at this point. At the very least, please limit this to "synthesize overrides" rather than "derive a new conformance".

"It's no worse than NSCoding."

@itaiferber
Copy link
Contributor Author

@itaiferber itaiferber commented May 2, 2017

@belkadan I think I'm just using the wrong terminology here; what I'm looking to here is that just because a class inherits from a type that is Encodable or Decodable doesn't mean we shouldn't synthesize overrides on its behalf.

@itaiferber
Copy link
Contributor Author

@itaiferber itaiferber commented Jul 19, 2017

@belkadan In working on PR-11045 I understood what you meant about hacking the compiler like this. Yeah, it made me uncomfortable too, and even synthesizing overrides was a stretch. I've left things as they are, but added a note to improve the diagnostic that you get if you do something like this:

class Super : Codable {
    var value = 5
}

// Confusing to beginners: why does this not synthesize Codable for sub???
class Sub : Super {
    var superValue: Double
}

You now get a note suggesting you override init(from🙂 (and possibly encode(to🙂).

@itaiferber
Copy link
Contributor Author

@itaiferber itaiferber commented Jul 19, 2017

Unfortunately, we can't at the moment do much about cases where you add storage that has an initial value — without additional hackiness, we never really detect that case.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants