Skip to content

Commit

Permalink
Fix a different aspect of the not-decoded keys propagation bug
Browse files Browse the repository at this point in the history
When creating multiple containers and only using the last, you could end
up propagating not-decoded keys from the original container anyway. I think
it's ok to assume that only the last container was used to decode the value?
  • Loading branch information
stackotter committed Dec 3, 2024
1 parent ce6f4ca commit 18a7db4
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 2 deletions.
17 changes: 16 additions & 1 deletion Sources/TOMLKit/Decoder/InternalTOMLDecoder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ final class InternalTOMLDecoder: Decoder {
var dataDecoder: (TOMLValueConvertible) -> Data?
var strictDecoding: Bool = false
var notDecodedKeys: NotDecodedKeys
let originalNotDecodedKeys: [String: [CodingKey]]

let tomlValue: TOMLValue
init(
Expand All @@ -28,6 +29,7 @@ final class InternalTOMLDecoder: Decoder {
self.dataDecoder = dataDecoder
self.strictDecoding = strictDecoding
self.notDecodedKeys = notDecodedKeys
self.originalNotDecodedKeys = notDecodedKeys.keys
}

func container<Key>(keyedBy type: Key.Type) throws -> KeyedDecodingContainer<Key> where Key: CodingKey {
Expand All @@ -42,6 +44,10 @@ final class InternalTOMLDecoder: Decoder {
)
}

// Assume that any previous container creation was related to a failed decoding
// attempt, and reset the not-decoded keys back to the value that the parent
// decoder passed to us.
self.notDecodedKeys.keys = self.originalNotDecodedKeys
return KeyedDecodingContainer<Key>(
KDC(
table: table,
Expand All @@ -64,6 +70,11 @@ final class InternalTOMLDecoder: Decoder {
)
)
}

// Assume that any previous container creation was related to a failed decoding
// attempt, and reset the not-decoded keys back to the value that the parent
// decoder passed to us.
self.notDecodedKeys.keys = self.originalNotDecodedKeys
return UDC(
array,
codingPath: self.codingPath,
Expand All @@ -75,7 +86,11 @@ final class InternalTOMLDecoder: Decoder {
}

func singleValueContainer() throws -> SingleValueDecodingContainer {
SVDC(
// Assume that any previous container creation was related to a failed decoding
// attempt, and reset the not-decoded keys back to the value that the parent
// decoder passed to us.
self.notDecodedKeys.keys = self.originalNotDecodedKeys
return SVDC(
self.tomlValue,
codingPath: self.codingPath,
dataDecoder: self.dataDecoder,
Expand Down
41 changes: 40 additions & 1 deletion Tests/TOMLKitTests/TOMLKitTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,46 @@ final class TOMLKitTests: XCTestCase {
// decoding attempts, breaking the common 'try one thing then retry another'
// decoding pattern. This bug was specific to decoders with
// `strictDecoding: true`.
func testRetry() throws {
func testRetryKeyedOrDictionary() throws {
enum StructOrDictionary: Decodable, Equatable {
case `struct`(value: Int)
case dictionary([String: Int])

enum CodingKeys: String, CodingKey {
case value
}

init(from decoder: Decoder) throws {
if let container = try? decoder.container(keyedBy: CodingKeys.self) {
if let value = try? container.decode(Int.self, forKey: .value) {
self = .struct(value: value)
}
}

if let container = try? decoder.singleValueContainer() {
self = try .dictionary(container.decode([String: Int].self))
} else {
throw DecodingError.dataCorrupted(.init(
codingPath: [],
debugDescription: "Expected int or keyedInt"
))
}
}
}

let toml = "other_value = 1"

// Before the bug got fixed, this line would throw an unused key error.
let value = try TOMLDecoder(strictDecoding: true).decode(StructOrDictionary.self, from: toml)

// We don't actually expect this to fail, it's not the point of the test, but
// might as well assert it just in case.
XCTAssertEqual(value, .dictionary(["other_value": 1]))
}

// This is related to the test above, but tests for a different aspect of the
// bug (I originally fixed one but not the other so they are kind of independent).
func testRetryKeyedOrInt() throws {
struct SimpleCodableStruct: Codable, Equatable {
var value: Int
}
Expand Down

0 comments on commit 18a7db4

Please sign in to comment.