Skip to content

Cross-module drift detection for qualified default types (closes #47) #48

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.

Sign up for GitHub

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

Merged
yavorpanayotov merged 1 commit into from
Jun 16, 2026
Merged

Cross-module drift detection for qualified default types (closes #47) #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 76 additions & 20 deletions crates/allium-parser/src/analysis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,18 @@ pub fn analyze_with_cross_module(
external_refs: &HashSet<String>,
resolved_use_paths: &HashSet<String>,
imported_triggers: &HashMap<String, HashSet<String>>,
imported_entity_fields: &HashMap<String, HashMap<String, HashSet<String>>>,
ambiguous_imports: &AmbiguousImports,
) -> Vec<Diagnostic> {
run_checks(
Ctx::new(
module,
external_refs,
Some(resolved_use_paths),
Some(imported_triggers),
Some(ambiguous_imports),
),
source,
)
let mut ctx = Ctx::new(
module,
external_refs,
Some(resolved_use_paths),
Some(imported_triggers),
Some(ambiguous_imports),
);
ctx.imported_entity_fields = Some(imported_entity_fields);
run_checks(ctx, source)
}

fn run_checks(mut ctx: Ctx<'_>, source: &str) -> Vec<Diagnostic> {
Expand Down Expand Up @@ -131,6 +131,7 @@ pub fn analyse_with_cross_module(
external_refs: &HashSet<String>,
resolved_use_paths: &HashSet<String>,
imported_triggers: &HashMap<String, HashSet<String>>,
imported_entity_fields: &HashMap<String, HashMap<String, HashSet<String>>>,
ambiguous_imports: &AmbiguousImports,
) -> crate::diagnostic::AnalyseResult {
let diagnostics = analyze_with_cross_module(
Expand All @@ -139,6 +140,7 @@ pub fn analyse_with_cross_module(
external_refs,
resolved_use_paths,
imported_triggers,
imported_entity_fields,
ambiguous_imports,
);
let findings = find_process_issues(module, Some(imported_triggers));
Expand Down Expand Up @@ -309,6 +311,11 @@ struct Ctx<'a> {
/// `Some(map)` = multi-file mode; names and triggers that more than one
/// imported module could resolve.
ambiguous_imports: Option<&'a AmbiguousImports>,
/// Multi-file mode only: per `use` alias, the imported module's entity/value
/// type → declared field names. Lets a qualified `default alias/Type` literal
/// be validated against the imported schema. Aliases whose targets fall
/// outside the check set are absent. `None` in single-file mode.
imported_entity_fields: Option<&'a HashMap<String, HashMap<String, HashSet<String>>>>,
diagnostics: Vec<Diagnostic>,
findings: Vec<crate::diagnostic::Finding>,
}
Expand All @@ -327,6 +334,7 @@ impl<'a> Ctx<'a> {
resolved_use_paths,
imported_triggers,
ambiguous_imports,
imported_entity_fields: None,
diagnostics: Vec::new(),
findings: Vec::new(),
}
Expand Down Expand Up @@ -4157,6 +4165,22 @@ pub fn collect_trigger_outputs(module: &Module) -> HashSet<String> {
names.into_iter().map(str::to_string).collect()
}

/// Collect each entity/value type's declared field names, keyed by type name.
///
/// Used by multi-file checking to build the per-alias schema map that lets a
/// qualified `default alias/Type = { ... }` literal be validated against the
/// imported type's fields (drift detection across `use` imports).
pub fn collect_entity_field_schemas(module: &Module) -> HashMap<String, HashSet<String>> {
let mut out: HashMap<String, HashSet<String>> = HashMap::new();
for (name, fields) in collect_local_type_schemas(module) {
out.insert(
name.to_string(),
fields.keys().map(|f| f.to_string()).collect(),
);
}
out
}

fn collect_qrefs_from_item(kind: &BlockItemKind, out: &mut Vec<(String, String)>) {
match kind {
BlockItemKind::Clause { value, .. }
Expand Down Expand Up @@ -4613,15 +4637,45 @@ impl Ctx<'_> {
let mut diagnostics = Vec::new();
for d in &self.module.declarations {
let Decl::Default(def) = d else { continue };
if def.type_alias.is_some() {
continue; // imported type — schema not visible here
}
let (Some(type_name), Expr::ObjectLiteral { fields, .. }) =
(&def.type_name, &def.value)
else {
continue;
};
validate_object_literal(fields, &type_name.name, &schemas, &mut diagnostics);
match &def.type_alias {
None => {
validate_object_literal(fields, &type_name.name, &schemas, &mut diagnostics);
}
Some(alias) => {
// Qualified `default alias/Type`: validate the top-level
// field set against the imported module's schema, when that
// module is in the check set (multi-file mode). Aliases or
// types outside the check set are left unvalidated rather
// than flagged. Nested validation and rule 14c need the
// imported field *types*, which aren't carried cross-module,
// so only unknown-field drift is checked here.
if let Some(imported) = self
.imported_entity_fields
.and_then(|m| m.get(alias.name.as_str()))
.and_then(|types| types.get(type_name.name.as_str()))
{
for field in fields {
if !imported.contains(field.name.name.as_str()) {
diagnostics.push(
Diagnostic::error(
field.name.span,
format!(
"Default sets field '{}' which is not declared on '{}/{}'.",
field.name.name, alias.name, type_name.name
),
)
.with_code("allium.default.unknownField"),
);
}
}
}
}
}
}
for diag in diagnostics {
self.push(diag);
Expand Down Expand Up @@ -6087,6 +6141,7 @@ mod tests {
&HashSet::new(),
&HashSet::new(),
&imported,
&HashMap::new(),
&AmbiguousImports::default(),
)
}
Expand Down Expand Up @@ -6133,6 +6188,7 @@ mod tests {
&HashSet::new(),
&HashSet::new(),
&imported,
&HashMap::new(),
&ambiguous,
)
}
Expand Down Expand Up @@ -6567,7 +6623,7 @@ mod tests {
let input = format!("-- allium: 3\n{src}");
let result = parse(&input);
let resolved: HashSet<String> = ["./core.allium".to_string()].into_iter().collect();
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &AmbiguousImports::default());
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default());
assert!(!has_code(&ds, "allium.use.unresolvedPath"));
}

Expand All @@ -6578,7 +6634,7 @@ mod tests {
let result = parse(&input);
// Only "./other.allium" is resolved — "./missing.allium" is not.
let resolved: HashSet<String> = ["./other.allium".to_string()].into_iter().collect();
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &AmbiguousImports::default());
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default());
assert!(has_code(&ds, "allium.use.unresolvedPath"));
}

Expand All @@ -6599,7 +6655,7 @@ mod tests {
let src = "use \"./missing.allium\" as missing\n\nentity Handler {\n x: String\n}\n";
let input = format!("-- allium: 3\n{src}");
let result = parse(&input);
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &HashSet::new(), &HashMap::new(), &AmbiguousImports::default());
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &HashSet::new(), &HashMap::new(), &HashMap::new(), &AmbiguousImports::default());
assert!(has_code(&ds, "allium.use.unresolvedPath"));
}

Expand All @@ -6609,7 +6665,7 @@ mod tests {
let input = format!("-- allium: 3\n{src}");
let result = parse(&input);
let resolved: HashSet<String> = ["./other.allium".to_string()].into_iter().collect();
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &AmbiguousImports::default());
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default());
let diag = ds.iter().find(|d| d.code == Some("allium.use.unresolvedPath")).unwrap();
assert!(diag.message.contains("nowhere.allium"), "message should name the path: {}", diag.message);
}
Expand All @@ -6620,7 +6676,7 @@ mod tests {
let input = format!("-- allium: 3\n{src}");
let result = parse(&input);
let resolved: HashSet<String> = ["./other.allium".to_string()].into_iter().collect();
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &AmbiguousImports::default());
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default());
assert!(!has_code(&ds, "allium.use.unresolvedPath"));
}

Expand All @@ -6630,7 +6686,7 @@ mod tests {
let input = format!("-- allium: 3\n{src}");
let result = parse(&input);
let resolved: HashSet<String> = ["./found.allium".to_string()].into_iter().collect();
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &AmbiguousImports::default());
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default());
let unresolved: Vec<_> = ds.iter()
.filter(|d| d.code == Some("allium.use.unresolvedPath"))
.collect();
Expand Down
4 changes: 2 additions & 2 deletions crates/allium-parser/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ pub mod span;
pub use analysis::{
analyze, analyze_with_cross_module, analyze_with_external_refs, analyse,
analyse_with_cross_module, analyse_with_external_refs, collect_all_referenced_idents,
collect_declared_names, collect_qualified_references, collect_trigger_outputs,
AmbiguousImports,
collect_declared_names, collect_entity_field_schemas, collect_qualified_references,
collect_trigger_outputs, AmbiguousImports,
};
pub use ast::Module;
pub use diagnostic::{AnalyseResult, Diagnostic, Finding};
Expand Down
47 changes: 41 additions & 6 deletions crates/allium/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ struct CrossModuleContext {
/// Per-file: `use` alias → trigger names the aliased module provides or
/// emits. Aliases whose targets are outside the check set are absent.
imported_triggers: HashMap<PathBuf, HashMap<String, HashSet<String>>>,
/// Per-file: `use` alias → (imported entity/value type → declared field
/// names). Lets a qualified `default alias/Type` literal be validated
/// against the imported schema. Aliases whose targets are outside the check
/// set are absent.
imported_entity_fields: HashMap<PathBuf, HashMap<String, HashMap<String, HashSet<String>>>>,
/// Per-file: names and triggers that more than one imported module could
/// resolve, so unqualified references to them are ambiguous (issue #15).
ambiguous_imports: HashMap<PathBuf, AmbiguousImports>,
Expand All @@ -207,7 +212,7 @@ struct CrossModuleContext {
fn run_multi_file(
command: &str,
args: &[String],
analyse_file: impl Fn(&Path, &str, &allium_parser::ParseResult, &SourceMap, &HashSet<String>, &HashSet<String>, &HashMap<String, HashSet<String>>, &AmbiguousImports) -> FileResult,
analyse_file: impl Fn(&Path, &str, &allium_parser::ParseResult, &SourceMap, &HashSet<String>, &HashSet<String>, &HashMap<String, HashSet<String>>, &HashMap<String, HashMap<String, HashSet<String>>>, &AmbiguousImports) -> FileResult,
) -> ExitCode {
let files = resolve_files(args);
if files.is_empty() {
Expand Down Expand Up @@ -247,8 +252,9 @@ fn run_multi_file(
let refs = ctx.external_refs.get(&key).cloned().unwrap_or_default();
let use_paths = ctx.resolved_use_paths.get(&key).cloned().unwrap_or_default();
let imports = ctx.imported_triggers.get(&key).cloned().unwrap_or_default();
let imported_fields = ctx.imported_entity_fields.get(&key).cloned().unwrap_or_default();
let ambiguous = ctx.ambiguous_imports.get(&key).unwrap_or(&no_ambiguity);
let file_result = analyse_file(&pf.path, &pf.source, &pf.result, &source_map, &refs, &use_paths, &imports, ambiguous);
let file_result = analyse_file(&pf.path, &pf.source, &pf.result, &source_map, &refs, &use_paths, &imports, &imported_fields, ambiguous);

if file_result.has_issues {
any_issues = true;
Expand Down Expand Up @@ -299,9 +305,26 @@ fn build_cross_module_context(parsed: &[ParsedFile]) -> CrossModuleContext {
})
.collect();

// Pre-compute each file's entity/value type → field-name schema so importing
// files can validate `default alias/Type = { ... }` literals against the
// imported type's declared fields.
let entity_field_outputs: HashMap<PathBuf, HashMap<String, HashSet<String>>> = parsed
.iter()
.map(|pf| {
(
canonical_key(&pf.path),
allium_parser::collect_entity_field_schemas(&pf.result.module),
)
})
.collect();

let mut external_refs: HashMap<PathBuf, HashSet<String>> = HashMap::new();
let mut resolved_use_paths: HashMap<PathBuf, HashSet<String>> = HashMap::new();
let mut imported_triggers: HashMap<PathBuf, HashMap<String, HashSet<String>>> = HashMap::new();
let mut imported_entity_fields: HashMap<
PathBuf,
HashMap<String, HashMap<String, HashSet<String>>>,
> = HashMap::new();
let mut ambiguous_imports: HashMap<PathBuf, AmbiguousImports> = HashMap::new();

for pf in parsed {
Expand Down Expand Up @@ -373,6 +396,17 @@ fn build_cross_module_context(parsed: &[ParsedFile]) -> CrossModuleContext {
}
imported_triggers.insert(file_key.clone(), imported_for_file);

// 3b. Imported entity field schemas — for each alias whose target is in
// the check set, that module's entity/value type → field names.
let mut imported_fields_for_file: HashMap<String, HashMap<String, HashSet<String>>> =
HashMap::new();
for (alias, target_key) in &alias_targets {
if let Some(fields) = entity_field_outputs.get(target_key) {
imported_fields_for_file.insert((*alias).to_string(), fields.clone());
}
}
imported_entity_fields.insert(file_key.clone(), imported_fields_for_file);

// 4. Ambiguous imports — names declared, and triggers provided or
// emitted, by more than one distinct imported file. Keyed by
// distinct target so that two aliases for the same file are not
Expand Down Expand Up @@ -426,6 +460,7 @@ fn build_cross_module_context(parsed: &[ParsedFile]) -> CrossModuleContext {
external_refs,
resolved_use_paths,
imported_triggers,
imported_entity_fields,
ambiguous_imports,
}
}
Expand All @@ -440,8 +475,8 @@ fn canonical_key(path: &Path) -> PathBuf {
}

fn cmd_check(args: &[String]) -> ExitCode {
run_multi_file("check", args, |path, source, result, source_map, external_refs, resolved_use_paths, imported_triggers, ambiguous_imports| {
let analysis = allium_parser::analyze_with_cross_module(&result.module, source, external_refs, resolved_use_paths, imported_triggers, ambiguous_imports);
run_multi_file("check", args, |path, source, result, source_map, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports| {
let analysis = allium_parser::analyze_with_cross_module(&result.module, source, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports);
let diagnostics: Vec<serde_json::Value> = result
.diagnostics
.iter()
Expand All @@ -456,8 +491,8 @@ fn cmd_check(args: &[String]) -> ExitCode {
}

fn cmd_analyse(args: &[String]) -> ExitCode {
run_multi_file("analyse", args, |path, source, result, source_map, external_refs, resolved_use_paths, imported_triggers, ambiguous_imports| {
let analyse_result = allium_parser::analyse_with_cross_module(&result.module, source, external_refs, resolved_use_paths, imported_triggers, ambiguous_imports);
run_multi_file("analyse", args, |path, source, result, source_map, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports| {
let analyse_result = allium_parser::analyse_with_cross_module(&result.module, source, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports);
let diagnostics: Vec<serde_json::Value> = result
.diagnostics
.iter()
Expand Down
59 changes: 59 additions & 0 deletions crates/allium/tests/cross_module_unused.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,3 +903,62 @@ fn qualified_trigger_with_alias_outside_check_set_not_flagged() {
diags.iter().map(|d| (&d.code, &d.message)).collect::<Vec<_>>()
);
}

#[test]
fn qualified_default_drift_flagged_across_modules() {
// A `default alias/Type` literal is validated against the imported type's
// fields when the target module is in the check set (allium-tools#47).
let dir = TempDir::new("xmod-default-drift");
dir.write(
"parent.allium",
"-- allium: 3\nentity Policy {\n id: String\n name: String\n}\n",
);
dir.write(
"fragment.allium",
"-- allium: 3\nuse \"./parent.allium\" as gp\n\ndefault gp/Policy good = { id: \"p1\", name: \"ok\" }\ndefault gp/Policy drift = { id: \"p2\", naem: \"typo\" }\n",
);

let output = allium()
.args(["check", dir.path().to_str().unwrap()])
.output()
.expect("spawn allium");
let diags = parse_diagnostics(&String::from_utf8_lossy(&output.stdout));

let unknown: Vec<_> = diags
.iter()
.filter(|d| d.code == "allium.default.unknownField")
.collect();
assert_eq!(
unknown.len(),
1,
"exactly one drift field (naem): {:?}",
unknown.iter().map(|d| &d.message).collect::<Vec<_>>()
);
assert!(unknown[0].message.contains("naem"));
assert!(unknown[0].message.contains("gp/Policy"));
}

#[test]
fn qualified_default_not_flagged_when_target_outside_check_set() {
// Only fragment.allium is checked; the imported module's schema is not
// visible, so the qualified default must not be flagged.
let dir = TempDir::new("xmod-default-noflag");
dir.write(
"parent.allium",
"-- allium: 3\nentity Policy {\n id: String\n}\n",
);
dir.write(
"fragment.allium",
"-- allium: 3\nuse \"./parent.allium\" as gp\n\ndefault gp/Policy d = { id: \"x\", whatever: 1 }\n",
);

let output = allium()
.args(["check", dir.path().join("fragment.allium").to_str().unwrap()])
.output()
.expect("spawn allium");
let codes = diagnostic_codes(&String::from_utf8_lossy(&output.stdout));
assert!(
!codes.iter().any(|c| c == "allium.default.unknownField"),
"qualified default with target outside check set must not be flagged: {codes:?}"
);
}
Loading