Skip to content

Commit 0506e46

Browse files
authored
Fix key encoding/decoding strategy applied to CodingKeyRepresentable dictionary keys (#1526)
- Fix JSONEncoder incorrectly applying key encoding strategy to CodingKeyRepresentable-keyed dictionary keys; only nested Encodable properties should be affected - Fix JSONDecoder incorrectly applying key decoding strategy to CodingKeyRepresentable-keyed dictionary keys; only nested Decodable properties should be affected - Extend _JSONCodingKeyRepresentableDictionaryEncodableMarker to replace the String-only marker, avoiding unnecessary AnyHashable boxing - Extend decoder's dictionary marker protocol to cover all CodingKeyRepresentable keys - Add tests verifying keys bypass the strategy while nested struct properties still apply it
1 parent 1f52be2 commit 0506e46

3 files changed

Lines changed: 102 additions & 17 deletions

File tree

Sources/FoundationEssentials/JSON/JSONDecoder.swift

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,44 @@ import Darwin
1919
internal import _FoundationCShims
2020
internal import Synchronization
2121

22-
/// A marker protocol used to determine whether a value is a `String`-keyed `Dictionary`
22+
/// A marker protocol used to determine whether a value is a `CodingKeyRepresentable`-keyed `Dictionary`
2323
/// containing `Decodable` values (in which case it should be exempt from key conversion strategies).
2424
///
25-
/// The marker protocol also provides access to the type of the `Decodable` values,
26-
/// which is needed for the implementation of the key conversion strategy exemption.
27-
private protocol _JSONStringDictionaryDecodableMarker {
25+
/// The protocol provides `_fromStringKeyedDictionary` to convert a `[String: Any]` dictionary
26+
/// to the proper `[Key: Value]` type using `Key.init(codingKey:)`.
27+
private protocol _JSONCodingKeyRepresentableDictionaryDecodableMarker {
2828
static var elementType: Decodable.Type { get }
29+
static func _fromStringKeyedDictionary(_ dict: [String: Any]) -> Self?
2930
}
3031

31-
extension Dictionary : _JSONStringDictionaryDecodableMarker where Key == String, Value: Decodable {
32+
extension Dictionary : _JSONCodingKeyRepresentableDictionaryDecodableMarker where Key: CodingKeyRepresentable, Value: Decodable {
3233
static var elementType: Decodable.Type { return Value.self }
34+
35+
static func _fromStringKeyedDictionary(_ dict: [String: Any]) -> Self? {
36+
// Fast path for String keys - no conversion needed
37+
if Key.self == String.self {
38+
return dict as? Self
39+
}
40+
// Convert keys for other CodingKeyRepresentable types
41+
var result = Self()
42+
result.reserveCapacity(dict.count)
43+
for (stringKey, value) in dict {
44+
guard let key = Key(codingKey: _DictionaryCodingKey(stringValue: stringKey)),
45+
let typedValue = value as? Value else {
46+
return nil
47+
}
48+
result[key] = typedValue
49+
}
50+
return result
51+
}
52+
}
53+
54+
/// A simple CodingKey implementation for string-to-key conversion.
55+
private struct _DictionaryCodingKey: CodingKey {
56+
var stringValue: String
57+
var intValue: Int? { nil }
58+
init(stringValue: String) { self.stringValue = stringValue }
59+
init?(intValue: Int) { nil }
3360
}
3461

3562
//===----------------------------------------------------------------------===//
@@ -611,7 +638,7 @@ extension JSONDecoderImpl: Decoder {
611638
if type == Decimal.self {
612639
return try self.unwrapDecimal(from: mapValue, for: codingPathNode, additionalKey) as! T
613640
}
614-
if !options.keyDecodingStrategy.isDefault, T.self is _JSONStringDictionaryDecodableMarker.Type {
641+
if !options.keyDecodingStrategy.isDefault, T.self is _JSONCodingKeyRepresentableDictionaryDecodableMarker.Type {
615642
return try self.unwrapDictionary(from: mapValue, as: type, for: codingPathNode, additionalKey)
616643
}
617644

@@ -770,8 +797,8 @@ extension JSONDecoderImpl: Decoder {
770797
private func unwrapDictionary<T: Decodable>(from mapValue: JSONMap.Value, as type: T.Type, for codingPathNode: _CodingPathNode, _ additionalKey: (some CodingKey)? = nil) throws -> T {
771798
try checkNotNull(mapValue, expectedType: [String:Any].self, for: codingPathNode, additionalKey)
772799

773-
guard let dictType = type as? (_JSONStringDictionaryDecodableMarker & Decodable).Type else {
774-
preconditionFailure("Must only be called if T implements __JSONStringDictionaryDecodableMarker")
800+
guard let dictType = type as? _JSONCodingKeyRepresentableDictionaryDecodableMarker.Type else {
801+
preconditionFailure("Must only be called if T implements _JSONCodingKeyRepresentableDictionaryDecodableMarker")
775802
}
776803

777804
guard case let .object(region) = mapValue else {
@@ -781,8 +808,8 @@ extension JSONDecoderImpl: Decoder {
781808
))
782809
}
783810

784-
var result = [String: Any]()
785-
result.reserveCapacity(region.count / 2)
811+
var stringKeyedResult = [String: Any]()
812+
stringKeyedResult.reserveCapacity(region.count / 2)
786813

787814
let dictCodingPathNode = codingPathNode.appending(additionalKey)
788815

@@ -791,10 +818,17 @@ extension JSONDecoderImpl: Decoder {
791818
// We know these values are keys, but UTF-8 decoding could still fail.
792819
let key = try self.unwrapString(from: keyValue, for: dictCodingPathNode, _CodingKey?.none)
793820
let value = try self.unwrap(value, as: dictType.elementType, for: dictCodingPathNode, _CodingKey(stringValue: key)!)
794-
result[key]._setIfNil(to: value)
821+
stringKeyedResult[key]._setIfNil(to: value)
795822
}
796823

797-
return result as! T
824+
// Convert [String: Any] to [Key: Value] using the marker protocol
825+
guard let result = dictType._fromStringKeyedDictionary(stringKeyedResult) as? T else {
826+
throw DecodingError.dataCorrupted(DecodingError.Context(
827+
codingPath: codingPathNode.path(byAppending: additionalKey),
828+
debugDescription: "Failed to create dictionary with CodingKeyRepresentable keys"
829+
))
830+
}
831+
return result
798832
}
799833

800834
private func unwrapString(from value: JSONMap.Value, for codingPathNode: _CodingPathNode, _ additionalKey: (some CodingKey)? = nil) throws -> String {

Sources/FoundationEssentials/JSON/JSONEncoder.swift

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,8 +1216,8 @@ private extension __JSONEncoder {
12161216
return self.wrap(url.absoluteString)
12171217
} else if let decimal = value as? Decimal {
12181218
return .number(decimal.description)
1219-
} else if !options.keyEncodingStrategy.isDefault, let encodable = value as? _JSONStringDictionaryEncodableMarker {
1220-
return try self.wrap(encodable as! [String:Encodable], for: additionalKey)
1219+
} else if !options.keyEncodingStrategy.isDefault, let encodable = value as? _JSONCodingKeyRepresentableDictionaryEncodableMarker {
1220+
return try self.wrap(encodable._stringKeyedDictionary, for: additionalKey)
12211221
} else if let array = _asDirectArrayEncodable(value) {
12221222
if options.outputFormatting.contains(.prettyPrinted) {
12231223
let (bytes, lengths) = try array.individualElementRepresentation(encoder: self, additionalKey)
@@ -1400,11 +1400,30 @@ extension JSONEncoder : @unchecked Sendable {}
14001400
// Special-casing Support
14011401
//===----------------------------------------------------------------------===//
14021402

1403-
/// A marker protocol used to determine whether a value is a `String`-keyed `Dictionary`
1403+
/// A marker protocol used to determine whether a value is a `CodingKeyRepresentable`-keyed `Dictionary`
14041404
/// containing `Encodable` values (in which case it should be exempt from key conversion strategies).
1405-
private protocol _JSONStringDictionaryEncodableMarker { }
1405+
///
1406+
/// The protocol provides `_stringKeyedDictionary` to convert keys using their `codingKey.stringValue`,
1407+
/// allowing the encoder to use the optimized String-keyed encoding path.
1408+
private protocol _JSONCodingKeyRepresentableDictionaryEncodableMarker {
1409+
var _stringKeyedDictionary: [String: Encodable] { get }
1410+
}
14061411

1407-
extension Dictionary : _JSONStringDictionaryEncodableMarker where Key == String, Value: Encodable { }
1412+
extension Dictionary: _JSONCodingKeyRepresentableDictionaryEncodableMarker where Key: CodingKeyRepresentable, Value: Encodable {
1413+
var _stringKeyedDictionary: [String: Encodable] {
1414+
// Fast path for String keys - return self without creating a new dictionary
1415+
if Key.self == String.self {
1416+
return self as! [String: Encodable]
1417+
}
1418+
// Convert other CodingKeyRepresentable keys to String
1419+
var result = [String: Encodable]()
1420+
result.reserveCapacity(count)
1421+
for (key, value) in self {
1422+
result[key.codingKey.stringValue] = value
1423+
}
1424+
return result
1425+
}
1426+
}
14081427

14091428
/// A protocol used to determine whether a value is an `Array` containing values that allow
14101429
/// us to bypass UnkeyedEncodingContainer overhead by directly encoding the contents as

Tests/FoundationEssentialsTests/JSONEncoderTests.swift

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,10 @@ private struct JSONEncoderTests {
470470
let outerValue: EncodeNested
471471
}
472472

473+
private struct CodingKeyRepresentableKey: RawRepresentable, CodingKeyRepresentable, Hashable, Codable {
474+
let rawValue: String
475+
}
476+
473477
@Test func encodingKeyStrategyPath() throws {
474478
// Make sure a more complex path shows up the way we want
475479
// Make sure the path reflects keys in the Swift, not the resulting ones in the JSON
@@ -548,6 +552,18 @@ private struct JSONEncoderTests {
548552

549553
#expect(["leave_me_alone": "test"] == result)
550554
}
555+
556+
@Test func decodingDictionaryCodingKeyRepresentableKeyConversionUntouched() throws {
557+
// CodingKeyRepresentable dictionary keys should NOT be converted by key decoding strategy,
558+
// but nested struct properties should still be converted.
559+
let input = "{\"leave_me_alone\":{\"this_is_camel_case\":\"test\"}}".data(using: .utf8)!
560+
let decoder = JSONDecoder()
561+
decoder.keyDecodingStrategy = .convertFromSnakeCase
562+
let result = try decoder.decode([CodingKeyRepresentableKey: DecodeMe3].self, from: input)
563+
564+
let value = try #require(result[CodingKeyRepresentableKey(rawValue: "leave_me_alone")])
565+
#expect(value.thisIsCamelCase == "test")
566+
}
551567

552568
@Test func decodingDictionaryFailureKeyPath() {
553569
let input = "{\"leave_me_alone\":\"test\"}".data(using: .utf8)!
@@ -2333,6 +2349,22 @@ extension JSONEncoderTests {
23332349

23342350
#expect(expected == resultString)
23352351
}
2352+
2353+
@Test func encodingDictionaryCodingKeyRepresentableKeyConversionUntouched() throws {
2354+
// CodingKeyRepresentable dictionary keys should NOT be converted by key encoding strategy,
2355+
// but nested struct properties should still be converted.
2356+
let expected = "{\"leaveMeAlone\":{\"this_is_camel_case\":\"test\"}}"
2357+
let toEncode: [CodingKeyRepresentableKey: DecodeMe3] = [
2358+
CodingKeyRepresentableKey(rawValue: "leaveMeAlone"): DecodeMe3(thisIsCamelCase: "test")
2359+
]
2360+
2361+
let encoder = JSONEncoder()
2362+
encoder.keyEncodingStrategy = .convertToSnakeCase
2363+
let resultData = try encoder.encode(toEncode)
2364+
let resultString = String(bytes: resultData, encoding: .utf8)
2365+
2366+
#expect(expected == resultString)
2367+
}
23362368

23372369
@Test func keyStrategySnakeGeneratedAndCustom() throws {
23382370
// Test that this works with a struct that has automatically generated keys

0 commit comments

Comments
 (0)