SE-0295: Codable synthesis for enums with associated values

[drexin (Dario Rexin)]

@beccadax

I think it's great that we're planning to define this stuff, but I'd like to suggest a couple refinements to the behavior.

Case with a single unlabeled associated value

The proposal suggests:

enum Command: Codable {
  case load(String)
  case store(String, value: Int)
}

would encoded to

{
  "load": [
    "MyKey"
  ]
}

Instead, I would recommend encoding this without the array wrapping it:

{
  "load": "MyKey"
}

This is a closer match for Serde's behavior (see the E::Y example). It also makes more sense with Swift's type system—the surrounding array implies that the associated value of Command.load is a 1-tuple, but there's no such thing as a 1-tuple in Swift. And it is just a cleaner representation of the value generally—internal objects, numbers, strings, or variable-length collections won't need to be wrapped in square brackets.

I think that makes sense and should be straightforward to implement.

@beccadax

Case with no associated value

The proposal says:

An enum case without associated values would encode the same as one where all values have labels,
i.e.

enum Command: Codable {
  case dumpToDisk
}

would encode to:

{
  "dumpToDisk": {}
}

You justify this design by saying:

This is done for compatibility reasons. If associated values are added to a case later on, the structure
would not change, unless those values are unlabeled.

But I find this justification pretty thin. There is no particular reason to favor labeled values over unlabeled ones, and I don't think you've proposed any fallback behavior that would allow e.g. case dumpToDisk(volume: String) to decode { "dumpToDisk": {} } without an error.

Instead, I would recommend encoding it as a single flat string, not in a keyed container at all:

"dumpToDisk"

This again matches Serde's behavior more closely (see the E::Z example) and again is a cleaner expression of the value generally. However, it does have a couple of problems: it can't be used at the top level of a JSON document (which would need an object or array) and it wouldn't be expressed by CodingKeys. If you think you need something that works with a keyed container, an alternative would be:

{
  "dumpToDisk": true
}

The true here is a bit odd, as we will never write false there instead, but I think it captures the intent better than empty-object or empty-array.

The plain value would indeed be problematic. Not necessarily because of top-level values, because those are actually allowed since RFC-8259. The bigger problem is that Decoder does not define functions to check for this. We have to know upfront whether we have a KeyedContainer, UnkeyedContainer, or SingleValueContainer. Adding the required functionality would break ABI.

Another alternative would be to use null as the value, i.e.

{
  "dumpToDisk": null
}

This is unbiased and not less confusing than a boolean value.

@beccadax

Sub-coding keys

The proposal says:

// contains keys for all cases of the enum
enum CodingKeys: CodingKey {
  case load
  case store
}

// contains keys for all associated values of `case load`
enum CodingKeys_load: CodingKey {
  case key
}

// contains keys for all associated values of `case store`
enum CodingKeys_store: CodingKey {
  case key
  case value
}

I'm not sure if the compiler has the right capitalization routines to pull this off, but I'd love it if we could instead do:

// contains keys for all cases of the enum
enum CodingKeys: CodingKey {
  case load
  case store

  // contains keys for all associated values of `case load`
  enum Load: CodingKey {
    case key
  }

  // contains keys for all associated values of `case store`
  enum Store: CodingKey {
    case key
    case value
  }
}

Or at least:

// contains keys for all cases of the enum
enum CodingKeys: CodingKey {
  case load
  case store
}

// contains keys for all associated values of `case load`
enum LoadCodingKeys: CodingKey {
  case key
}

// contains keys for all associated values of `case store`
enum StoreCodingKeys: CodingKey {
  case key
  case value
}

My reasons here are purely aesthetic: I think these names are more like the names I would choose if I were writing the synthesized code by hand.

I think this makes sense. I slightly prefer the last version, because that still has the CodingKeys part in it, which is less likely to to collide with user defined names.