swiftlang / swift

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

[SR-9545] Decodable gotcha: "let" with default + coding key #51995

Open stephencelis opened 5 years ago

stephencelis commented 5 years ago
Previous ID SR-9545
Radar None
Original Reporter @stephencelis
Type Bug
Environment Swift 4.2.1.
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, Codable, DiagnosticsQoI | |Assignee | None | |Priority | Medium | md5: 408dcb6091fdb216039c20402dda3a46

Issue Description:

Structs with default let properties that are codable will always prefer the default.

When an explicit coding keys is provided, Swift probably should not compile:

struct Foo: Decodable {
  let bar = true

  private enum CodingKeys: String, CodingKey {
    case bar
  }
}

The bar coding key should be a red flag since that key will never be decoded in a synthesized decodable initializer.

try JSONDecoder().decode(Foo.self, from: Data("""
{"bar" : false}
""".utf8).bar == false
belkadan commented 5 years ago

I don't think "default" is the right word, since that just is the value and it can't be overridden even in a normal initializer. It might still be relevant for encoding, though—what if it's a "version" field?

stephencelis commented 5 years ago

I don't really care about naming. The behavior is very problematic. If I explicitly specify a coding key for a field that cannot be decoded, something is wrong.

belkadan commented 5 years ago

But it's a field that you may want to _en_code.

…I guess the diagnostic could check to see if you only conformed to Decodable and not Encodable, but that definitely limits the applicability.

stephencelis commented 5 years ago

Yeah, sorry for the short reply. I guess the problem needs to be fleshed out. Think I should create a pitch on evolution describing why we may wanna rethink requirements for the auto-derived Decodable initializer? It's way too easy to accidentally introduce bugs into decoding at the moment.

Ideally Swift would not auto-derive Decodable initializer for structures with lets with predefined values. Either: specify coding keys explicitly and omit this field (this could be applied with a fix-it), or define the initializer by hand (fix-it to stub initializer).

stephencelis commented 5 years ago

Pitch: https://forums.swift.org/t/pitch-strengthen-requirements-for-synthesizing-decodable/18977