From 23681144e77be8019beba737b2d571dd0cc9a8cd Mon Sep 17 00:00:00 2001 From: Matt Stavola Date: Wed, 15 Apr 2026 00:51:19 -0400 Subject: [PATCH] Treat required and nullable as unordered sets in tests --- tests/real_world/roundtrip.rs | 90 +++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 36 deletions(-) diff --git a/tests/real_world/roundtrip.rs b/tests/real_world/roundtrip.rs index 6437bc5..8910769 100644 --- a/tests/real_world/roundtrip.rs +++ b/tests/real_world/roundtrip.rs @@ -285,10 +285,9 @@ fn compare_lexicon_json( ) -> ComparisonResult { let mut acceptable_diffs = Vec::new(); - // Lexicon authors use any of three syntactic forms for references - // (`#foo`, `ns.path#foo`, bare `ns.path` for implicit-main), so we - // rewrite both sides to a single canonical form before comparing. - // This folds away C7/C8-style diffs that are purely stylistic. + // Fold away purely-stylistic differences before comparing: the three + // ATProto-equivalent ref forms, and set-shaped fields like `required` + // and `nullable` whose element order carries no semantic meaning. let lexicon_id = original .get("id") .and_then(|v| v.as_str()) @@ -296,8 +295,8 @@ fn compare_lexicon_json( .to_string(); let mut original = original.clone(); let mut generated = generated.clone(); - canonicalize_refs(&mut original, &lexicon_id); - canonicalize_refs(&mut generated, &lexicon_id); + canonicalize_lexicon(&mut original, &lexicon_id); + canonicalize_lexicon(&mut generated, &lexicon_id); let original_stripped = strip_dollar_type(&original); let generated_stripped = strip_dollar_type(&generated); @@ -316,51 +315,70 @@ fn compare_lexicon_json( ComparisonResult::Failure("Structural differences detected".to_string()) } -/// Rewrite every reference string inside `value` to its canonical -/// `authority#defName` form, so the three syntactic variants compare -/// equal: +/// Walk `value` and rewrite every node that carries semantic-equivalence +/// noise into a single canonical form. This exists because the +/// authoritative lexicons we roundtrip against exercise author-chosen +/// styles that ATProto treats as equivalent; byte comparison would flag +/// them as diffs even though the lexicons are identical in meaning. /// -/// * `#foo` → `#foo` -/// * `ns.path#foo` → `ns.path#foo` (unchanged) -/// * `ns.path` (bare NSID) → `ns.path#main` (ATProto spec shorthand) +/// Currently folds: /// -/// The walk descends into every value, but only rewrites strings found -/// at well-known ref positions (`{"type":"ref", "ref": ...}` and the -/// `refs` array of a union). -fn canonicalize_refs(value: &mut serde_json::Value, lexicon_id: &str) { +/// * The three ATProto reference forms — local `#foo`, explicit +/// `ns#foo`, bare `ns` (= `ns#main`) — into `ns#foo` at every ref site. +/// * Set-shaped string arrays (`required`, `nullable`) — ATProto defines +/// these as sets, so their element order carries no meaning — into a +/// sorted order at every object. +fn canonicalize_lexicon(value: &mut serde_json::Value, lexicon_id: &str) { match value { serde_json::Value::Object(obj) => { - let kind = obj.get("type").and_then(|v| v.as_str()).map(str::to_owned); - match kind.as_deref() { - Some("ref") => { - if let Some(serde_json::Value::String(s)) = obj.get_mut("ref") { - *s = canonicalize_ref_string(s, lexicon_id); - } - } - Some("union") => { - if let Some(serde_json::Value::Array(arr)) = obj.get_mut("refs") { - for item in arr.iter_mut() { - if let serde_json::Value::String(s) = item { - *s = canonicalize_ref_string(s, lexicon_id); - } - } - } - } - _ => {} - } + canonicalize_ref_site(obj, lexicon_id); + sort_set_arrays(obj); for (_, v) in obj.iter_mut() { - canonicalize_refs(v, lexicon_id); + canonicalize_lexicon(v, lexicon_id); } } serde_json::Value::Array(arr) => { for v in arr.iter_mut() { - canonicalize_refs(v, lexicon_id); + canonicalize_lexicon(v, lexicon_id); } } _ => {} } } +/// If `obj` is a ref or union node, rewrite its ref strings to canonical +/// `authority#defName` form. +fn canonicalize_ref_site(obj: &mut serde_json::Map, lexicon_id: &str) { + match obj.get("type").and_then(|v| v.as_str()) { + Some("ref") => { + if let Some(serde_json::Value::String(s)) = obj.get_mut("ref") { + *s = canonicalize_ref_string(s, lexicon_id); + } + } + Some("union") => { + if let Some(serde_json::Value::Array(arr)) = obj.get_mut("refs") { + for item in arr.iter_mut() { + if let serde_json::Value::String(s) = item { + *s = canonicalize_ref_string(s, lexicon_id); + } + } + } + } + _ => {} + } +} + +/// ATProto defines `required` and `nullable` as sets of field names. +/// Sort them in place so two lexicons that list the same field names in +/// different orders compare equal. +fn sort_set_arrays(obj: &mut serde_json::Map) { + for key in ["required", "nullable"] { + if let Some(serde_json::Value::Array(arr)) = obj.get_mut(key) { + arr.sort_by(|a, b| a.as_str().unwrap_or("").cmp(b.as_str().unwrap_or(""))); + } + } +} + fn canonicalize_ref_string(ref_str: &str, lexicon_id: &str) -> String { if let Some(fragment) = ref_str.strip_prefix('#') { format!("{}#{}", lexicon_id, fragment) -- 2.51.2