From 1e59547a8f98e5573b7db3ef2217717ac9c37bdb Mon Sep 17 00:00:00 2001 From: Matt Stavola Date: Wed, 15 Apr 2026 00:40:12 -0400 Subject: [PATCH] emit canonicalized refs --- mlf-codegen/src/lib.rs | 52 ++++++--------- mlf-lang/src/lib.rs | 2 +- mlf-lang/src/workspace.rs | 116 ++++++++++++++++++++++++---------- tests/real_world/roundtrip.rs | 73 ++++++++++++++++++++- 4 files changed, 174 insertions(+), 69 deletions(-) diff --git a/mlf-codegen/src/lib.rs b/mlf-codegen/src/lib.rs index 0e29261..9b2f68c 100644 --- a/mlf-codegen/src/lib.rs +++ b/mlf-codegen/src/lib.rs @@ -1,5 +1,5 @@ use mlf_lang::ast::*; -use mlf_lang::Workspace; +use mlf_lang::{ResolvedRef, Workspace}; use serde_json::{json, Map, Value}; use std::collections::HashMap; @@ -412,43 +412,31 @@ fn build_params_object_opt(properties: Map, required: &[String]) } } -/// Resolve a typed reference path to the ATProto NSID string it names -/// (e.g. `#foo`, `app.bsky.actor.defs#profileViewBasic`, or a bare -/// `com.atproto.repo.strongRef` for implicit-main references). Used by -/// both direct `ref` types and union members — any place that needs to -/// emit the string form of a reference. +/// Resolve a typed reference path to the ATProto NSID string it names. /// -/// When workspace resolution fails (unresolved import, malformed path, -/// etc.) we fall back to a best-effort rendering rather than erroring: -/// the semantic check still happens in the resolver, so emitting a -/// recognisable-looking ref keeps the JSON self-describing for tooling. +/// The three valid output forms — local `#foo`, external `ns.path#foo`, +/// and bare `ns.path` (implicit-main) — map one-to-one onto the +/// [`ResolvedRef`] variants, so there's no guessing about the def name +/// or whether to append a fragment. Unresolvable paths (typically a +/// typo caught elsewhere by validation) fall through to a best-effort +/// rendering so the JSON stays self-describing for tooling. fn resolve_ref_nsid(path: &Path, workspace: &Workspace, current_namespace: &str) -> String { - if let Some(full_namespace) = workspace.resolve_reference_namespace(path, current_namespace) { - let last_segment = path.segments.last().unwrap().name.as_str(); - if full_namespace == current_namespace { - // Sibling def in the current lexicon. - return format!("#{}", last_segment); - } - return format!("{}#{}", full_namespace, last_segment); + match workspace.resolve_ref(path, current_namespace) { + Some(ResolvedRef::Local { def_name }) => format!("#{}", def_name), + Some(ResolvedRef::ImplicitMain { namespace, .. }) => namespace, + Some(ResolvedRef::External { namespace, def_name }) => format!("{}#{}", namespace, def_name), + None => unresolved_ref_fallback(path), } +} +/// Render a reference path the workspace couldn't resolve. Multi- +/// segment paths are emitted as `ns.path#def_name` on the assumption +/// that the author wrote a full NSID; single-segment paths render as a +/// local `#name` fragment. +fn unresolved_ref_fallback(path: &Path) -> String { if path.segments.len() == 1 { - let name = &path.segments[0].name; - // Single-segment path that didn't resolve via the workspace — - // maybe an imported symbol whose original path we can look up. - let imports = workspace.get_imports(current_namespace); - if let Some((_, original_path)) = imports.iter().find(|(local, _)| local == name) { - if original_path.len() > 1 { - let namespace = original_path[..original_path.len() - 1].join("."); - let type_name = original_path.last().unwrap(); - return format!("{}#{}", namespace, type_name); - } - } - // Not imported either — assume it names a sibling def. - return format!("#{}", name); + return format!("#{}", path.segments[0].name); } - - // Multi-segment unresolved path: trust the author's NSID as written. let namespace = path.segments[..path.segments.len() - 1] .iter() .map(|s| s.name.as_str()) diff --git a/mlf-lang/src/lib.rs b/mlf-lang/src/lib.rs index a21f3d1..cb72860 100644 --- a/mlf-lang/src/lib.rs +++ b/mlf-lang/src/lib.rs @@ -14,7 +14,7 @@ pub use ast::Lexicon; pub use error::{ParseError, ValidationError, ValidationErrors}; pub use parser::parse_lexicon; pub use validate::validate_lexicon; -pub use workspace::Workspace; +pub use workspace::{ResolvedRef, Workspace}; // Standard library directory use include_dir::{include_dir, Dir}; diff --git a/mlf-lang/src/workspace.rs b/mlf-lang/src/workspace.rs index ff7d9c6..b1efad2 100644 --- a/mlf-lang/src/workspace.rs +++ b/mlf-lang/src/workspace.rs @@ -8,6 +8,30 @@ pub struct Workspace { modules: BTreeMap, } +/// How a reference path resolves in the workspace. +/// +/// ATProto names a def with three syntactically distinct forms — a local +/// fragment (`#foo`), an explicit external reference (`ns.path#foo`), and +/// a bare NSID (`ns.path`, shorthand for `#main`). MLF source collapses +/// these into path expressions, and downstream code (codegen, LSP, +/// diagnostics) needs to know which form to emit or display for the +/// canonical output. `ResolvedRef` carries that discrimination so callers +/// don't have to re-derive it from implementation details of the resolver. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ResolvedRef { + /// The reference names a def in the same lexicon as the reference + /// site. Codegen emits this as `#def_name`. + Local { def_name: String }, + /// The reference names a def in a different lexicon. Codegen emits + /// this as `namespace#def_name`. + External { namespace: String, def_name: String }, + /// The reference resolves via MLF's implicit-main convention: the + /// whole path names a lexicon whose "main" def is conventionally the + /// def matching the last segment. In ATProto this corresponds to the + /// bare NSID form (`namespace`, equivalent to `namespace#main`). + ImplicitMain { namespace: String, def_name: String }, +} + #[derive(Debug, Clone, PartialEq)] struct Module { namespace: String, @@ -767,67 +791,91 @@ impl Workspace { } /// Resolve a type reference to its actual namespace - /// Returns the namespace where the type is defined, or None if not found - pub fn resolve_reference_namespace(&self, path: &Path, current_namespace: &str) -> Option { + /// Resolve a reference path (evaluated from inside `current_namespace`) + /// to the lexicon and def it names. Returns a structured + /// [`ResolvedRef`] so callers can distinguish local, external, and + /// implicit-main references without re-deriving the resolution + /// decision from a bare namespace string. + pub fn resolve_ref(&self, path: &Path, current_namespace: &str) -> Option { if path.segments.len() == 1 { let name = &path.segments[0].name; - // Check current module first if let Some(module) = self.modules.get(current_namespace) { - // Check if it's a local type if module.symbols.types.contains_key(name) { - return Some(current_namespace.to_string()); + return Some(ResolvedRef::Local { def_name: name.clone() }); } - // Check if it's imported if let Some(imported) = module.imports.mappings.get(name) { - // Build the full namespace from the original path - // original_path is Vec like ["place", "stream", "key", "key"] - // We want "place.stream.key" (drop the last segment which is the type name) + // `original_path` is the fully-qualified import path + // (e.g. `["com", "atproto", "label", "defs", "label"]`). + // All but the last segment form the namespace. if imported.original_path.len() > 1 { let namespace = imported.original_path[..imported.original_path.len() - 1].join("."); - return Some(namespace); + let def_name = imported.original_path.last().unwrap().clone(); + return Some(ResolvedRef::External { namespace, def_name }); } else { - // Edge case: just a single segment, assume it's in the current namespace - return Some(current_namespace.to_string()); + // Pathological single-segment import: treat as local. + return Some(ResolvedRef::Local { def_name: name.clone() }); } } } - // Check prelude if let Some(module) = self.modules.get("prelude") { if module.symbols.types.contains_key(name) { - return Some("prelude".to_string()); + return Some(ResolvedRef::External { + namespace: "prelude".to_string(), + def_name: name.clone(), + }); } } return None; - } else { - // Multi-segment path: resolve normally - let target_namespace = path.segments[..path.segments.len() - 1] - .iter() - .map(|s| s.name.as_str()) - .collect::>() - .join("."); - let type_name = &path.segments[path.segments.len() - 1].name; + } - // First try: normal resolution - if let Some(module) = self.modules.get(&target_namespace) { - if module.symbols.types.contains_key(type_name) { - return Some(target_namespace); + // Multi-segment path: try the explicit `namespace.def_name` form first. + let target_namespace = path.segments[..path.segments.len() - 1] + .iter() + .map(|s| s.name.as_str()) + .collect::>() + .join("."); + let type_name = &path.segments[path.segments.len() - 1].name; + + if let Some(module) = self.modules.get(&target_namespace) { + if module.symbols.types.contains_key(type_name) { + let namespace = target_namespace; + if namespace == current_namespace { + return Some(ResolvedRef::Local { def_name: type_name.clone() }); } + return Some(ResolvedRef::External { namespace, def_name: type_name.clone() }); } + } - // Second try: implicit main resolution - let full_namespace = path.to_string(); - if let Some(module) = self.modules.get(&full_namespace) { - let namespace_suffix = full_namespace.split('.').last().unwrap_or(&full_namespace); - if namespace_suffix == type_name && module.symbols.types.contains_key(type_name) { - return Some(full_namespace); - } + // Fall back to the implicit-main convention: the whole path names a + // lexicon whose "main" def is conventionally the def matching the + // last segment. In ATProto this corresponds to a bare NSID. + let full_namespace = path.to_string(); + if let Some(module) = self.modules.get(&full_namespace) { + let namespace_suffix = full_namespace.split('.').last().unwrap_or(&full_namespace); + if namespace_suffix == type_name && module.symbols.types.contains_key(type_name) { + return Some(ResolvedRef::ImplicitMain { + namespace: full_namespace, + def_name: type_name.clone(), + }); } + } - None + None + } + + /// Return only the resolved lexicon namespace for a reference path. + /// Thin shim over [`Self::resolve_ref`]; prefer that for new call + /// sites where the variant (local vs external vs implicit-main) + /// matters. + pub fn resolve_reference_namespace(&self, path: &Path, current_namespace: &str) -> Option { + match self.resolve_ref(path, current_namespace)? { + ResolvedRef::Local { .. } => Some(current_namespace.to_string()), + ResolvedRef::External { namespace, .. } + | ResolvedRef::ImplicitMain { namespace, .. } => Some(namespace), } } diff --git a/tests/real_world/roundtrip.rs b/tests/real_world/roundtrip.rs index 5401aca..6437bc5 100644 --- a/tests/real_world/roundtrip.rs +++ b/tests/real_world/roundtrip.rs @@ -285,8 +285,22 @@ fn compare_lexicon_json( ) -> ComparisonResult { let mut acceptable_diffs = Vec::new(); - let original_stripped = strip_dollar_type(original); - let generated_stripped = strip_dollar_type(generated); + // 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. + let lexicon_id = original + .get("id") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + let mut original = original.clone(); + let mut generated = generated.clone(); + canonicalize_refs(&mut original, &lexicon_id); + canonicalize_refs(&mut generated, &lexicon_id); + + let original_stripped = strip_dollar_type(&original); + let generated_stripped = strip_dollar_type(&generated); if original_stripped == generated_stripped { return ComparisonResult::Perfect; @@ -302,6 +316,61 @@ 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: +/// +/// * `#foo` → `#foo` +/// * `ns.path#foo` → `ns.path#foo` (unchanged) +/// * `ns.path` (bare NSID) → `ns.path#main` (ATProto spec shorthand) +/// +/// 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) { + 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); + } + } + } + } + _ => {} + } + for (_, v) in obj.iter_mut() { + canonicalize_refs(v, lexicon_id); + } + } + serde_json::Value::Array(arr) => { + for v in arr.iter_mut() { + canonicalize_refs(v, lexicon_id); + } + } + _ => {} + } +} + +fn canonicalize_ref_string(ref_str: &str, lexicon_id: &str) -> String { + if let Some(fragment) = ref_str.strip_prefix('#') { + format!("{}#{}", lexicon_id, fragment) + } else if ref_str.contains('#') { + ref_str.to_string() + } else { + format!("{}#main", ref_str) + } +} + fn strip_dollar_type(value: &serde_json::Value) -> serde_json::Value { match value { serde_json::Value::Object(map) => { -- 2.51.2