[SR-9385] Encoded data was lost when trying to encode with 2 separated nested keys container with 2 difference Coding Keys on the same key with JSONEncoder and PListEncoder

[pitiphong-p]

Previous ID	SR-9385
Radar	rdar://46368053
Original Reporter	@pitiphong-p
Type	Bug
Status	Resolved
Resolution	Done

Attachment: Download

Environment

Xcode 10.1 Swift 4.2 macOS 10.14.1

Additional Detail from JIRA

Votes	0
Component/s	Standard Library
Labels	Bug, Codable
Assignee	@itaiferber
Priority	Medium

md5: f5f48b9464d016e83e4a5e7f0faf4821

Issue Description:

I think I found an edge case bug of the JSONEncoder and the PListEncoder in Swift Standard Library

I have a model for an API of our service which will return a JSON data in a specific format. I'm trying to design a Swift model to conform to the Swift API Design guideline. In order to achieve that, we want to flatten the properties up to the type. We can use the nestedKey variant for that. However since we have some specific properties based on the type of the data, we decide to have another type for holding those information. The bug is that we need to call nestedKey variant on the same key but for 2 Coding Keys types while encoding the type, the second container will always replace the data encoding by the first container. I believe that it's due to This line of code where the JSONEncoder will create a new container when the nestedKeys method is called.

However the decoding works correctly

{
  "installment_boa": {
    "currencies": [
      "USD"
    ],
    "allowed_installment_terms": [
      3,
      4,
      6,
      9,
      10
    ],
    "amount": {
      "min": 2000,
      "max": 100000000
    }
  }
}

We want to convert the above JSON data to be something like this

public struct Payment: Codable, Equatable {
  public let channel: Channel
  public let limit: Limit?
  public let supportedCurrencies: Set<Currency>
  
  public enum Channel : Equatable {
    case card(Set<CardBrand>)
    case installment(InstallmentBrand, availableNumberOfTerms: IndexSet)
    case mobile(Mobile)
    case alipay
  }
  
  public struct Limit : Codable, Equatable, Hashable {
    public let max: Int64
    public let min: Int64
    
    public var range: ClosedRange<Int64> {
      return min...max
    }
  }
}

I think we should fix the Encoders logic not to replace the value encoded by the prior container, this will match with the behavior of the Decoder

[itaiferber]

Indeed, we should be looking up and reusing any existing containers if possible.

[pitiphong-p]

@itaiferber I think I can try to help on fixing this bug if you want me to do

[itaiferber]

@pitiphong-p If you have the time, by all means! That would be much appreciated. The fix should be relatively simple as you note; instead of always writing into self.container, we need to look up whether self.container[key] already exists.

One complication: previously, asking for multiple containers overwrote, and it didn't matter what type they were. Now, if you ask for a keyed container, then an unkeyed for the same key, we can't reuse it. So we need to preconditionFailure descriptively if this happens.

[pitiphong-p]

@itaiferber I try to fix this bug and open a #20951. Please have a look and review it

[itaiferber]

Pitiphong did this back in #20951