swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.49k stars 10.35k forks source link

[SR-6890] Can't synthesize Decodable implementation for NSObject subclass #49439

Open mdiep opened 6 years ago

mdiep commented 6 years ago
Previous ID SR-6890
Radar rdar://problem/37119603
Original Reporter @mdiep
Type Bug
Environment Swift 4.0.2
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 3 | |Component/s | Compiler | |Labels | Bug, Codable | |Assignee | @itaiferber | |Priority | Medium | md5: e659b73eb1675cdd0a354ee6f38ab113

Issue Description:

import Foundation

public final class Object: NSObject, Swift.Decodable {
    public let property: String
}

gives this error:

test.swift:3:54: error: property 'self.property' not initialized at implicitly generated super.init call
public final class Object: NSObject, Swift.Decodable {
                                                     ^
itaiferber commented 6 years ago

Okay, looks like the issue here is real, but the diagnostic doesn't make it clear enough.

The core problem is that property is declared as let without an initial value. If you take off Decodable conformance, you can see the issue with that:

Untitled 2.swift:3:20: error: class 'Object' has no initializers
public final class Object: NSObject {
                   ^
Untitled 2.swift:4:16: note: stored property 'property' without initial value prevents synthesized initializers
    public let property: String
               ^
                                = ""

Because of this, Object can neither inherit init() nor synthesize a new one. Normally, this wouldn't be a problem:

// Note: not inheriting from anything.
public final class Object: Decodable, CustomStringConvertible {
    public let property: String

    public var description: String {
        return "{\"property:\" \"\(property)\"}"
    }
}

let json = """
{"property": "Hello!"}
""".data(using: .utf8)!

let decoder = JSONDecoder()
let object = try decoder.decode(Object.self, from: json)
print(object) // => {"property": "Hello!"}

but since Object inherits from NSObject, we need to call super.init().

Note that this error is present for any class inheriting from another class (even if the property is a var):

public class Super { init() {} }
public final class Object : Super {
    public var property: String
}

produces

Untitled 2.swift:4:20: error: class 'Object' has no initializers
public final class Object : Super {
                   ^
Untitled 2.swift:5:16: note: stored property 'property' without initial value prevents synthesized initializers
    public var property: String
               ^
                                = ""

The two simplest solutions to this are either:

  1. Give the property a default value to assign so that init() can be synthesized:

    public final class Object : NSObject, Decodable {
        public private(set) var property: String = ""
    }
    
    let json = """
    {"property": "Hello!"}
    """.data(using: .utf8)!
    
    let decoder = JSONDecoder()
    let object = try decoder.decode(Object.self, from: json)
    print(object.property) // => Hello!
  2. Provide an override for init that assigns to property:

    import Foundation
    
    public final class Object : NSObject, Decodable {
        public let property: String
    
        override init() {
            property = "Hi"
        }
    }
    
    let json = """
    {"property": "Hello!"}
    """.data(using: .utf8)!
    
    let decoder = JSONDecoder()
    let object = try decoder.decode(Object.self, from: json)
    print(object.property) // => Hello
itaiferber commented 6 years ago

What is worth investigating is what is producing this specific diagnostic, which normally indicates something like this:

import Foundation

public final class Object : NSObject, Decodable {
    public let property: String

    override init() {
        super.init()
    }
}

which produces

Untitled 2.swift:7:15: error: property 'self.property' not initialized at super.init call
        super.init()
              ^
itaiferber commented 6 years ago

@swift-ci Create

mdiep commented 6 years ago

I'm confused because if I were to hand-write a Decodable implementation for this, I'd write:

import Foundation

public final class Object: NSObject, Swift.Decodable {
    private enum CodingKeys: String, CodingKey {
        case property
    }

    public let property: String

    public override var description: String {
        return "{\"property:\" \"\(property)\"}"
    }

    public init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        property = try container.decode(String.self, forKey: .property)
        super.init()
    }
}

let json = """
{"property": "Hello!"}
""".data(using: .utf8)!

let decoder = JSONDecoder()
let object = try decoder.decode(Object.self, from: json)
print(object) // => {"property": "Hello!"}

In this case, I'm setting the property before calling init. It seems like Swift could do the same thing for me.

If the error said "we can't know that you don't want to encode properties from the superclass", then I'd be disappointed but satisfied. But it seems to me like Swift could satisfy its own error by setting the properties before calling init.

itaiferber commented 6 years ago

@mdiep That's what bears investigation here — it should be doing that; we don't skip over decoding properties unless they're let s which have a default value (in which case we couldn't assign to them), so I need to figure out where this diagnostic is coming from. The code you have above is exactly the code that Codable should be generating.

mattneub commented 6 years ago

I don't get why there's an issue here. If you hadn't adopted Decodable, you'd have been told, rightly, that you needed an initializer, and you'd have written this:

final class Object: NSObject {
    let property: String
    init(property:String) {
        self.property = property
    }
}

That compiles fine. Then you adopt Decodable and all is well.

itaiferber commented 6 years ago

@mattneub The issue is that the diagnostic isn't clear enough about what's going on — we're producing both a message that is misleading (the issue shouldn't be a super.init() call) and not informative enough.

mdiep commented 6 years ago

The synthesized Decodable provides an initializer. Why do I need to provide a separate one?

itaiferber commented 6 years ago

@mdiep Because NSObject provides -[NSObject init] (NSObject.init() in Swift) which your subclass must either inherit, or override, regardless of Codable presence. If you can't inherit it (because inheriting it would leave your property uninitialized), you must override it. Codable here provides an additional initializer, but you still must handle init().

mattneub commented 6 years ago

See, that's my point. The error message may not have been very helpful, but that's true of a lot of Swift compiler error messages. It seems to me that the real problem here is just a matter of knowing the basic rules of how to write an object in Swift. Regardless of the error message, what @mdiep wrote was invalid, and the compiler rightly stopped him.

mdiep commented 6 years ago

Because NSObject provides -[NSObject init] (NSObject.init() in Swift) which your subclass must either inherit, or override, regardless of Codable presence. If you can't inherit it (because inheriting it would leave your property uninitialized), you must override it.

I don't think that's true.

NSObject.init() isn't a required init. I only need any init, not init() specifically. In the code sample in my comment above, I write a custom init(from: Decoder) that acts as the sole init for the NSObject subclass. This compiles and runs fine, without inheriting or overriding NSObject.init().

What I'm suggesting is that the synthesized init(from: Decoder) should able to fill the same role, acting is the only initializer for the class.

Codable here provides an additional initializer, but you still must handle init().

I can't tell if this is supposed to be a statement of fact or a conclusion based on the previous arguments.

Is this behavior part of the specficiation? i.e. is it the specified behavior for synthesized Decodable implementations to not be the only init in subclasses where the superclass provides a init()?

mattneub commented 6 years ago

@mdiep init is not required, but it is a designated initializer. So your argument should run like this:

swift-ci commented 6 years ago

Comment by Dave Poirier (JIRA)

Not sure if this can help the discussion, but Apple in their documentation seems to showcase no initializer being required if you use Codable/Decodable only: https://developer.apple.com/documentation/foundation/archives_and_serialization/encoding_and_decoding_custom_types

Its quite a different beast if you also inherit from NSObject.