From 78b739a62dc3955daf254d499018b1aa3d93d831 Mon Sep 17 00:00:00 2001 From: edochi Date: Thu, 20 Aug 2026 15:15:36 +0200 Subject: [PATCH] fix(schema): reject duplicate [[fields.field]] names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two entries sharing a `name` were accepted silently and the last one won. Downstream lookup keys on the bare name — `cmd/check/validate.rs` collects into a `HashMap<&str, _>`, as does `FieldValidators::build` — so `collect()` kept the final entry and discarded the earlier one without a word. The failure was actively misleading rather than merely lossy. Declaring `status` as String under `blog/**` and Integer under `projects/**` produced two violations against `blog/post.md`, both artifacts of the collapse: status │ Wrong type │ type Integer │ blog/post.md (got String) status │ Not allowed │ allowed in ["projects/**"] │ blog/post.md Adds invariant 10: no two entries may share a name. Rejected on the name alone, regardless of whether the entries agree on type — scoping a field per directory is a separate feature, not something a repeated entry should back into. Checked before the per-field loop so the structural problem is reported ahead of any per-field complaint. Names are matched exactly. YAML keys are case-sensitive, so `status` and `Status` are genuinely different frontmatter fields and both stay legal; invariant 7 already canonicalises dotted names, so no normalisation ambiguity remains. Also documents invariant 9 (`Array(Object)` not representable on disk), which was enforced in the body but missing from the docstring — the contract promised eight rules while the code enforced nine. Refs TODO-0196. Co-Authored-By: Claude --- crates/mdvs/src/schema/config.rs | 103 +++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/crates/mdvs/src/schema/config.rs b/crates/mdvs/src/schema/config.rs index abb984a..24096a3 100644 --- a/crates/mdvs/src/schema/config.rs +++ b/crates/mdvs/src/schema/config.rs @@ -295,6 +295,9 @@ impl MdvsToml { /// segments (`..`). Names without dots are unaffected. /// 8. No shape conflicts: a name cannot be declared both as a leaf and /// as a parent of nested leaves (e.g., `foo` *and* `foo.bar`). + /// 9. `Array` of `Object` is not representable on disk — use parallel + /// scalar arrays instead (per TODO-0155). + /// 10. No two `[[fields.field]]` entries may share a `name`. pub fn validate(&self) -> anyhow::Result<()> { // Invariant 1: ignore and [[fields.field]] are mutually exclusive for ignored in &self.fields.ignore { @@ -306,6 +309,22 @@ impl MdvsToml { } } + // Invariant 10: field names are unique. Lookup downstream keys on the + // bare name (`cmd/check/validate.rs` collects into a `HashMap<&str, _>`), + // so a repeated name would silently discard the earlier entry rather + // than scoping it. Rejected regardless of whether the two entries agree + // on type — see TODO-0196. + let mut seen: std::collections::HashSet<&str> = + std::collections::HashSet::with_capacity(self.fields.field.len()); + for field in &self.fields.field { + if !seen.insert(field.name.as_str()) { + anyhow::bail!( + "field '{}' is declared more than once — each [[fields.field]] name must be unique. Merge the entries into one, or rename one of them.", + field.name + ); + } + } + for field in &self.fields.field { // Invariant 7: dotted field name well-formedness if let Err(msg) = validate_field_name(&field.name) { @@ -1114,6 +1133,90 @@ nullable = false assert!(config.validate().is_ok()); } + // --- Invariant 10: field names are unique --- + + #[test] + fn validate_rejects_duplicate_field_names() { + let config = full_toml(vec![ + TomlField { + name: "status".into(), + field_type: FieldTypeSerde::Scalar("String".into()), + allowed: vec!["blog/**".into()], + required: vec![], + nullable: false, + constraints: None, + preprocess: vec![], + }, + TomlField { + name: "status".into(), + field_type: FieldTypeSerde::Scalar("Integer".into()), + allowed: vec!["projects/**".into()], + required: vec![], + nullable: false, + constraints: None, + preprocess: vec![], + }, + ]); + let err = config.validate().unwrap_err(); + assert!( + err.to_string() + .contains("field 'status' is declared more than once"), + "unexpected error: {}", + err + ); + } + + /// Duplicates are rejected on the name alone — agreeing on type does not + /// make a repeated entry acceptable. Scoping a field per directory is a + /// separate feature; see TODO-0196. + #[test] + fn validate_rejects_duplicate_names_even_with_matching_type() { + let field = |allowed: &str| TomlField { + name: "status".into(), + field_type: FieldTypeSerde::Scalar("String".into()), + allowed: vec![allowed.into()], + required: vec![], + nullable: false, + constraints: None, + preprocess: vec![], + }; + let config = full_toml(vec![field("blog/**"), field("projects/**")]); + assert!(config.validate().is_err()); + } + + /// Field names are matched exactly. YAML keys are case-sensitive, so + /// `status` and `Status` are genuinely different frontmatter fields and + /// must both remain declarable. + #[test] + fn validate_allows_names_differing_only_by_case() { + let field = |name: &str| TomlField { + name: name.into(), + field_type: FieldTypeSerde::Scalar("String".into()), + allowed: vec!["**".into()], + required: vec![], + nullable: true, + constraints: None, + preprocess: vec![], + }; + let config = full_toml(vec![field("status"), field("Status")]); + assert!(config.validate().is_ok()); + } + + #[test] + fn validate_distinct_field_names_pass() { + let field = |name: &str| TomlField { + name: name.into(), + field_type: FieldTypeSerde::Scalar("String".into()), + allowed: vec!["**".into()], + required: vec![], + nullable: true, + constraints: None, + preprocess: vec![], + }; + let config = full_toml(vec![field("title"), field("status"), field("author")]); + assert!(config.validate().is_ok()); + } + // --- Invariant 2: valid glob format --- #[test]