Skip to content

Commit 35cc5e9

Browse files
authored
Error on duplicate keys (#294)
I can't see a reason that you'd want to allow these in maps, but let me know if you think this needs an extension to the schema to optionally allow.
1 parent 2a18d58 commit 35cc5e9

2 files changed

Lines changed: 69 additions & 3 deletions

File tree

crates/quarto-yaml-validation/src/error.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,9 @@ pub enum ValidationErrorKind {
149149
/// Unresolved schema reference
150150
UnresolvedReference { ref_id: String },
151151

152+
/// A mapping key appears more than once
153+
DuplicateKey { key: String },
154+
152155
/// Other validation error
153156
///
154157
/// **WARNING**: This is a last-resort variant for errors that don't fit any other category.
@@ -176,6 +179,7 @@ impl ValidationErrorKind {
176179
ValidationErrorKind::ObjectPropertyCountInvalid { .. } => "Q-1-16",
177180
ValidationErrorKind::UnresolvedReference { .. } => "Q-1-17",
178181
ValidationErrorKind::UnknownProperty { .. } => "Q-1-18",
182+
ValidationErrorKind::DuplicateKey { .. } => "Q-1-20",
179183
ValidationErrorKind::ArrayItemsNotUnique => "Q-1-19",
180184
ValidationErrorKind::StringLengthInvalid { .. } => "Q-1-29",
181185
ValidationErrorKind::Other { .. } => "Q-1-99",
@@ -286,6 +290,9 @@ impl ValidationErrorKind {
286290
ValidationErrorKind::UnresolvedReference { ref_id } => {
287291
format!("Unresolved schema reference: {}", ref_id)
288292
}
293+
ValidationErrorKind::DuplicateKey { key } => {
294+
format!("Duplicate key '{}'", key)
295+
}
289296
ValidationErrorKind::Other { message } => message.clone(),
290297
}
291298
}
@@ -757,6 +764,22 @@ mod tests {
757764
assert_eq!(kind.error_code(), "Q-1-18");
758765
}
759766

767+
#[test]
768+
fn test_error_code_duplicate_key() {
769+
let kind = ValidationErrorKind::DuplicateKey {
770+
key: "examples".to_string(),
771+
};
772+
assert_eq!(kind.error_code(), "Q-1-20");
773+
}
774+
775+
#[test]
776+
fn test_message_duplicate_key() {
777+
let kind = ValidationErrorKind::DuplicateKey {
778+
key: "examples".to_string(),
779+
};
780+
assert_eq!(kind.message(), "Duplicate key 'examples'");
781+
}
782+
760783
#[test]
761784
fn test_error_code_array_items_not_unique() {
762785
let kind = ValidationErrorKind::ArrayItemsNotUnique;

crates/quarto-yaml-validation/src/validator.rs

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -588,11 +588,20 @@ fn validate_object(
588588
}
589589
};
590590

591-
// Extract keys
591+
// Extract keys, rejecting duplicates. The source-tracked hash view
592+
// preserves every entry (the collapsed `Yaml::Hash` would not), so a
593+
// repeated key is detectable here. Report it on the second occurrence,
594+
// pointing at that key's span.
592595
let mut keys = HashSet::new();
593596
for entry in entries {
594-
if let Yaml::String(ref key) = entry.key.yaml {
595-
keys.insert(key.clone());
597+
if let Yaml::String(ref key) = entry.key.yaml
598+
&& !keys.insert(key.clone())
599+
{
600+
context.add_error(
601+
ValidationErrorKind::DuplicateKey { key: key.clone() },
602+
&entry.key,
603+
);
604+
return Err(context.errors[0].clone());
596605
}
597606
}
598607

@@ -1800,6 +1809,40 @@ mod tests {
18001809
assert!(validate(&yaml, &schema, &registry, &source_ctx).is_err());
18011810
}
18021811

1812+
#[test]
1813+
fn test_validate_object_duplicate_keys() {
1814+
let registry = SchemaRegistry::new();
1815+
let source_ctx = SourceContext::new();
1816+
let schema = Schema::Object(ObjectSchema {
1817+
annotations: SchemaAnnotations::default(),
1818+
properties: HashMap::new(),
1819+
pattern_properties: HashMap::new(),
1820+
additional_properties: None,
1821+
required: vec![],
1822+
min_properties: None,
1823+
max_properties: None,
1824+
closed: false,
1825+
property_names: None,
1826+
naming_convention: None,
1827+
base_schema: None,
1828+
});
1829+
1830+
// A mapping with a repeated key must be rejected, not silently deduped.
1831+
let yaml = yaml_object(vec![
1832+
("examples", Yaml::String("a".to_string())),
1833+
("examples", Yaml::String("c".to_string())),
1834+
]);
1835+
let result = validate(&yaml, &schema, &registry, &source_ctx);
1836+
let err = result.expect_err("duplicate key should fail validation");
1837+
assert_eq!(
1838+
err.kind,
1839+
ValidationErrorKind::DuplicateKey {
1840+
key: "examples".to_string(),
1841+
}
1842+
);
1843+
assert_eq!(err.error_code(), "Q-1-20");
1844+
}
1845+
18031846
// ==================== AnyOf Tests ====================
18041847

18051848
#[test]

0 commit comments

Comments
 (0)