Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail on duplicate keys in a mapping #416

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
7 changes: 6 additions & 1 deletion .github/workflows/pod_lib_lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ jobs:
runs-on: macos-14
env:
DEVELOPER_DIR: /Applications/Xcode_15.4.app
strategy:
matrix:
platform: [macOS, iOS, tvOS, visionOS]
steps:
- uses: actions/checkout@v4
- run: bundle install --path vendor/bundle
- run: bundle exec pod lib lint --verbose
- if: matrix.platform == 'visionOS'
run: xcodebuild -downloadPlatform visionOS
- run: bundle exec pod lib lint --platforms=${{ matrix.platform }} --verbose
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@

##### Bug Fixes

* None.
* Yams will now correctly error when it tries to decode a mapping with duplicate keys.
Copy link
Contributor Author

@tejassharma96 tejassharma96 May 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is technically a bug fix but it may be breaking if people have malformed yaml they are trying to decode with this. imo this is the correct spot for this but happy to move it to "Breaking" instead if you feel that would be more apt

[Tejas Sharma](https://github.com/tejassharma96)
[#415](https://github.com/jpsim/Yams/issues/415)

## 5.1.3

Expand Down
9 changes: 9 additions & 0 deletions Sources/Yams/Parser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -355,13 +355,22 @@ private extension Parser {
pairs.append((key, value))
event = try parse()
}
let keys = pairs.map { $0.0 }
try checkDuplicates(mappingKeys: keys)
let node = Node.mapping(.init(pairs, tag(firstEvent.mappingTag), event.mappingStyle, firstEvent.startMark))
if let anchor = firstEvent.mappingAnchor {
anchors[anchor] = node
}
return node
}

private func checkDuplicates(mappingKeys: [Node]) throws {
let duplicates: [Node: [Node]] = Dictionary(grouping: mappingKeys) { $0 }.filter { $1.count > 1 }
guard duplicates.isEmpty else {
throw YamlError.duplicatedKeysInMapping(duplicates: duplicates, yaml: yaml)
}
}

func tag(_ string: String?) -> Tag {
let tagName = string.map(Tag.Name.init(rawValue:)) ?? .implicit
return Tag(tagName, resolver, constructor)
Expand Down
20 changes: 20 additions & 0 deletions Sources/Yams/YamlError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ public enum YamlError: Error {
/// - parameter encoding: The string encoding used to decode the string data.
case dataCouldNotBeDecoded(encoding: String.Encoding)

/// Multiple uses of the same key detected in a mapping
///
/// - parameter duplicates: A dictionary keyed by the duplicated node value, with all nodes that duplicate the value
/// - parameter yaml: YAML String which the problem occured while reading.
case duplicatedKeysInMapping(duplicates: [Node: [Node]], yaml: String)

/// The error context.
public struct Context: CustomStringConvertible {
/// Context text.
Expand Down Expand Up @@ -175,6 +181,20 @@ extension YamlError: CustomStringConvertible {
return problem
case .dataCouldNotBeDecoded(encoding: let encoding):
return "String could not be decoded from data using '\(encoding)' encoding"
case let .duplicatedKeysInMapping(duplicates, yaml):
return duplicates.duplicatedKeyErrorDescription(yaml: yaml)
}
}
}

private extension Dictionary where Key == Node, Value == [Node] {
func duplicatedKeyErrorDescription(yaml: String) -> String {
var error = "error: parser: expected all keys to be unique but found the following duplicated key(s):"
for key in self.keys.sorted() {
let duplicatedNodes = self[key]!
let marks = duplicatedNodes.compactMap { $0.mark }
error += "\n\(key.any) (\(marks)):\n\(marks.map { $0.snippet(from: yaml) }.joined(separator: "\n"))"
}
return error
}
}
46 changes: 46 additions & 0 deletions Tests/YamsTests/YamlErrorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,52 @@ class YamlErrorTests: XCTestCase {
)
}
}

func testDuplicateKeysCannotBeParsed() throws {
let yamlString = """
a: value
a: different_value
"""
XCTAssertThrowsError(try Parser(yaml: yamlString).singleRoot()) { error in
XCTAssertTrue(error is YamlError)
XCTAssertEqual("\(error)", """
error: parser: expected all keys to be unique but found the following duplicated key(s):
a ([1:5, 2:5]):
a: value
^
a: different_value
^
""")
}
}

func testDuplicatedKeysCannotBeParsed_MultipleDuplicates() throws {
let yamlString = """
a: value
a: different_value
b: value
b: different_value
b: different_different_value
"""
XCTAssertThrowsError(try Parser(yaml: yamlString).singleRoot()) { error in
XCTAssertTrue(error is YamlError)
XCTAssertEqual("\(error)", """
error: parser: expected all keys to be unique but found the following duplicated key(s):
a ([1:5, 2:5]):
a: value
^
a: different_value
^
b ([3:5, 4:5, 5:5]):
b: value
^
b: different_value
^
b: different_different_value
^
""")
}
}
}

extension YamlErrorTests {
Expand Down
Loading