swiftlang / swift-corelibs-foundation

The Foundation Project, providing core utilities, internationalization, and OS independence
swift.org
Apache License 2.0
5.29k stars 1.13k forks source link

[SR-6629] JSONEncoder with convertToSnakeCase encodes myURLProperty into my_url_property, but JSONDecoder with convertFromSnakeCase does not decode myURLProperty from my_url_property. #3752

Open norio-nomura opened 6 years ago

norio-nomura commented 6 years ago
Previous ID SR-6629
Radar rdar://problem/41038908
Original Reporter @norio-nomura
Type Bug
Environment swift-4.1-DEVELOPMENT-SNAPSHOT-2017-12-15-a
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 1 | |Component/s | Foundation | |Labels | Bug, Codable | |Assignee | @itaiferber | |Priority | Medium | md5: fee6142a597261d7125a4f80b8b8ceb2

Issue Description:

Because JSONDecoder coverts my_url_property to myUrlProperty.

import Foundation

struct S: Codable {
    var myURLProperty: String
}

let encoder = JSONEncoder()
encoder.keyEncodingStrategy = .convertToSnakeCase
let data = try encoder.encode(S(myURLProperty: "property"))
let decoder = JSONDecoder()
decoder.keyDecodingStrategy = .convertFromSnakeCase
do {
    let decoded = try decoder.decode(S.self, from: data)
} catch {
    print(error)
// keyNotFound(CodingKeys(stringValue: "myURLProperty", intValue: nil), Swift.DecodingError.Context(codingPath: [], debugDescription: "No value associated with key CodingKeys(stringValue: \"myURLProperty\", intValue: nil) (\"myURLProperty\"), converted to my_url_property.", underlyingError: nil))
}
norio-nomura commented 6 years ago

Perhaps JSONDecoder should not convert keys of JSON from snake_case to camelCase, but should query JSON with CodingKeys that converted to snake_case.

belkadan commented 6 years ago

JSONDecoder doesn't have a list of words that are normally written as initials in Swift style, so the camel-ification of my_url_property would be myUrlProperty. @itaiferber, what's the recommendation here?

norio-nomura commented 6 years ago

I'm trying to implement my idea on YAMLDecoder in https://github.com/jpsim/Yams/issues/84.
However, since the implementation of JSONDecoder.KeyDecodingStrategy.convertFromSnakeCase supports too many variations, my implementation can not pass the same test cases. 🙁

norio-nomura commented 6 years ago

Rather than supporting various notation fluctuations, I would like to prioritize that data encoded with convertToSnakeCase can be decoded reliably with convertFromSnakeCase.

itaiferber commented 6 years ago

Sorry for the delay in responding — I was on vacation. My recommendation for values like this would be to provide a key name of something like myUrlProperty (manually in the CodingKeys enum) so the value can round-trip. There are indeed edge cases like this where we couldn't possibly tell what the original key was, so we can't round-trip. @norio-nomura what tests are you unable to pass?

itaiferber commented 6 years ago

(In any case, I don't think this is necessarily a bug, just a behavior that exists here because we can't know what the original key was; it's an imperfect conversion.)

norio-nomura commented 6 years ago

Sorry for delay.
My test implementation on Yams is here.
The YAMLDecoder converts CodingKey.stringValue into snake_case on accessing YAML contents, instead of converting keys in YAML contents from snake_case to camelCase.

what tests are you unable to pass?

Since used String conversion is snakecased only, the value can round-trip.
Instead, it does not pass tests that require special logic when converting snake_case to camelCase.

itaiferber commented 6 years ago

@norio-nomura I don't currently have the time to read through the full implementation of Yams, but how do you handle custom conversions on decode? The .custom block goes from key in JSON to CodingKey, but it sounds like your snake case conversion goes the other way. We do our conversion in the same direction both ways, which is why we end up with cases like this.

norio-nomura commented 6 years ago

I opened a PR that implemented my idea.