Skip to content

Commit 148e5f7

Browse files
authored
Diagnose references through a use path that resolves nowhere (#92)
1 parent bf37961 commit 148e5f7

3 files changed

Lines changed: 199 additions & 13 deletions

File tree

crates/allium-parser/src/analysis.rs

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ pub fn analyze_with_cross_module(
119119
ambiguous_imports: &AmbiguousImports,
120120
reverse: &ReverseContributions,
121121
imported_referenced_triggers: &HashMap<String, HashSet<String>>,
122+
missing_use_paths: &HashSet<String>,
122123
) -> Vec<Diagnostic> {
123124
let mut ctx = Ctx::new(
124125
module,
@@ -130,6 +131,7 @@ pub fn analyze_with_cross_module(
130131
ctx.imported_entity_fields = Some(imported_entity_fields);
131132
ctx.reverse_contributions = Some(reverse);
132133
ctx.imported_referenced_triggers = Some(imported_referenced_triggers);
134+
ctx.missing_use_paths = Some(missing_use_paths);
133135
run_checks(ctx, source)
134136
}
135137

@@ -207,6 +209,7 @@ pub fn analyse_with_cross_module(
207209
reverse: &ReverseContributions,
208210
imported_referenced_triggers: &HashMap<String, HashSet<String>>,
209211
imported_entity_statuses: &HashMap<String, HashSet<String>>,
212+
missing_use_paths: &HashSet<String>,
210213
) -> crate::diagnostic::AnalyseResult {
211214
let diagnostics = analyze_with_cross_module(
212215
module,
@@ -218,6 +221,7 @@ pub fn analyse_with_cross_module(
218221
ambiguous_imports,
219222
reverse,
220223
imported_referenced_triggers,
224+
missing_use_paths,
221225
);
222226
let findings = find_process_issues(
223227
module,
@@ -421,6 +425,12 @@ struct Ctx<'a> {
421425
/// entities and triggers. `None` in single-file mode (and effectively empty
422426
/// when no importer references this module).
423427
reverse_contributions: Option<&'a ReverseContributions>,
428+
/// Multi-file mode only: `use` path strings that resolve neither to a file
429+
/// in the check set nor to a file on disk — a broken import, as opposed to
430+
/// an out-of-set one. References through an alias bound to such a path are
431+
/// diagnosed rather than left unknowable. `None` in single-file mode and
432+
/// for callers without filesystem access (they behave as before).
433+
missing_use_paths: Option<&'a HashSet<String>>,
424434
diagnostics: Vec<Diagnostic>,
425435
findings: Vec<crate::diagnostic::Finding>,
426436
}
@@ -442,6 +452,7 @@ impl<'a> Ctx<'a> {
442452
imported_entity_fields: None,
443453
imported_referenced_triggers: None,
444454
reverse_contributions: None,
455+
missing_use_paths: None,
445456
diagnostics: Vec::new(),
446457
findings: Vec::new(),
447458
}
@@ -5981,6 +5992,30 @@ impl Ctx<'_> {
59815992
})
59825993
.collect();
59835994

5995+
// Aliases whose use path resolves neither in the check set nor on
5996+
// disk: the import is broken, not merely out of set. References
5997+
// through such an alias resolve against nothing, so each one is
5998+
// diagnosed — independent of the use-line unresolvedPath warning,
5999+
// which a per-line allium-ignore can suppress.
6000+
let broken_alias_paths: HashMap<&str, &str> = match self.missing_use_paths {
6001+
Some(missing) => self
6002+
.module
6003+
.declarations
6004+
.iter()
6005+
.filter_map(|d| match d {
6006+
Decl::Use(u) => {
6007+
let path = u.path.text();
6008+
let alias = u.alias.as_ref()?;
6009+
missing
6010+
.get(&path)
6011+
.map(|p| (alias.name.as_str(), p.as_str()))
6012+
}
6013+
_ => None,
6014+
})
6015+
.collect(),
6016+
None => HashMap::new(),
6017+
};
6018+
59846019
let mut refs = collect_qref_nodes(self.module);
59856020
// A `default alias/Type` reference's qualifier sits on the declaration,
59866021
// not inside its value expression, so add it explicitly.
@@ -6009,6 +6044,17 @@ impl Ctx<'_> {
60096044
)
60106045
.with_code("allium.reference.undefinedImportedAlias"),
60116046
);
6047+
} else if let Some(path) = broken_alias_paths.get(r.qualifier) {
6048+
self.push(
6049+
Diagnostic::warning(
6050+
r.span,
6051+
format!(
6052+
"Reference '{}/{}' goes through use path \"{}\", which does not resolve to a file in the check set or on disk.",
6053+
r.qualifier, r.name, path
6054+
),
6055+
)
6056+
.with_code("allium.reference.unresolvedImport"),
6057+
);
60126058
} else if let Some(offered) = self
60136059
.imported_referenced_triggers
60146060
.and_then(|m| m.get(r.qualifier))
@@ -7597,6 +7643,7 @@ mod tests {
75977643
&AmbiguousImports::default(),
75987644
&ReverseContributions::default(),
75997645
&HashMap::new(),
7646+
&HashSet::new(),
76007647
)
76017648
}
76027649

@@ -7646,6 +7693,7 @@ mod tests {
76467693
&ambiguous,
76477694
&ReverseContributions::default(),
76487695
&HashMap::new(),
7696+
&HashSet::new(),
76497697
)
76507698
}
76517699

@@ -8228,7 +8276,7 @@ surface AccountManagement {
82288276
let input = format!("-- allium: 3\n{src}");
82298277
let result = parse(&input);
82308278
let resolved: HashSet<String> = ["./core.allium".to_string()].into_iter().collect();
8231-
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new());
8279+
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new(), &HashSet::new());
82328280
assert!(!has_code(&ds, "allium.use.unresolvedPath"));
82338281
}
82348282

@@ -8239,7 +8287,7 @@ surface AccountManagement {
82398287
let result = parse(&input);
82408288
// Only "./other.allium" is resolved — "./missing.allium" is not.
82418289
let resolved: HashSet<String> = ["./other.allium".to_string()].into_iter().collect();
8242-
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new());
8290+
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new(), &HashSet::new());
82438291
assert!(has_code(&ds, "allium.use.unresolvedPath"));
82448292
}
82458293

@@ -8260,7 +8308,7 @@ surface AccountManagement {
82608308
let src = "use \"./missing.allium\" as missing\n\nentity Handler {\n x: String\n}\n";
82618309
let input = format!("-- allium: 3\n{src}");
82628310
let result = parse(&input);
8263-
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &HashSet::new(), &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new());
8311+
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &HashSet::new(), &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new(), &HashSet::new());
82648312
assert!(has_code(&ds, "allium.use.unresolvedPath"));
82658313
}
82668314

@@ -8270,7 +8318,7 @@ surface AccountManagement {
82708318
let input = format!("-- allium: 3\n{src}");
82718319
let result = parse(&input);
82728320
let resolved: HashSet<String> = ["./other.allium".to_string()].into_iter().collect();
8273-
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new());
8321+
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new(), &HashSet::new());
82748322
let diag = ds.iter().find(|d| d.code == Some("allium.use.unresolvedPath")).unwrap();
82758323
assert!(diag.message.contains("nowhere.allium"), "message should name the path: {}", diag.message);
82768324
}
@@ -8281,7 +8329,7 @@ surface AccountManagement {
82818329
let input = format!("-- allium: 3\n{src}");
82828330
let result = parse(&input);
82838331
let resolved: HashSet<String> = ["./other.allium".to_string()].into_iter().collect();
8284-
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new());
8332+
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new(), &HashSet::new());
82858333
assert!(!has_code(&ds, "allium.use.unresolvedPath"));
82868334
}
82878335

@@ -8291,7 +8339,7 @@ surface AccountManagement {
82918339
let input = format!("-- allium: 3\n{src}");
82928340
let result = parse(&input);
82938341
let resolved: HashSet<String> = ["./found.allium".to_string()].into_iter().collect();
8294-
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new());
8342+
let ds = analyze_with_cross_module(&result.module, &input, &HashSet::new(), &resolved, &HashMap::new(), &HashMap::new(), &AmbiguousImports::default(), &ReverseContributions::default(), &HashMap::new(), &HashSet::new());
82958343
let unresolved: Vec<_> = ds.iter()
82968344
.filter(|d| d.code == Some("allium.use.unresolvedPath"))
82978345
.collect();

crates/allium/src/main.rs

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,10 @@ struct CrossModuleContext {
222222
/// across every `use` alias whose target is in the check set. Lets the
223223
/// importer's conflict pass attribute a rule to an imported entity.
224224
imported_entity_statuses: HashMap<PathBuf, HashMap<String, HashSet<String>>>,
225+
/// Per-file: use path strings that resolve neither to a file in the check
226+
/// set nor to a file on disk — broken imports, as opposed to out-of-set
227+
/// ones. References through their aliases are diagnosed per reference.
228+
missing_use_paths: HashMap<PathBuf, HashSet<String>>,
225229
}
226230

227231
/// Shared loop for commands that process multiple .allium files.
@@ -232,7 +236,7 @@ struct CrossModuleContext {
232236
fn run_multi_file(
233237
command: &str,
234238
args: &[String],
235-
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, &ReverseContributions, &HashMap<String, HashSet<String>>, &HashMap<String, HashSet<String>>) -> FileResult,
239+
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, &ReverseContributions, &HashMap<String, HashSet<String>>, &HashMap<String, HashSet<String>>, &HashSet<String>) -> FileResult,
236240
) -> ExitCode {
237241
let files = resolve_files(args);
238242
if files.is_empty() {
@@ -278,7 +282,8 @@ fn run_multi_file(
278282
let reverse = ctx.reverse_contributions.get(&key).unwrap_or(&no_reverse);
279283
let referenced = ctx.imported_referenced_triggers.get(&key).cloned().unwrap_or_default();
280284
let imported_statuses = ctx.imported_entity_statuses.get(&key).cloned().unwrap_or_default();
281-
let file_result = analyse_file(&pf.path, &pf.source, &pf.result, &source_map, &refs, &use_paths, &imports, &imported_fields, ambiguous, reverse, &referenced, &imported_statuses);
285+
let missing = ctx.missing_use_paths.get(&key).cloned().unwrap_or_default();
286+
let file_result = analyse_file(&pf.path, &pf.source, &pf.result, &source_map, &refs, &use_paths, &imports, &imported_fields, ambiguous, reverse, &referenced, &imported_statuses, &missing);
282287

283288
if file_result.has_issues {
284289
any_issues = true;
@@ -387,6 +392,7 @@ fn build_cross_module_context(parsed: &[ParsedFile]) -> CrossModuleContext {
387392
let mut imported_entity_statuses: HashMap<PathBuf, HashMap<String, HashSet<String>>> =
388393
HashMap::new();
389394
let mut ambiguous_imports: HashMap<PathBuf, AmbiguousImports> = HashMap::new();
395+
let mut missing_use_paths: HashMap<PathBuf, HashSet<String>> = HashMap::new();
390396

391397
for pf in parsed {
392398
// For bare filenames (no directory component), parent() returns "".
@@ -396,10 +402,14 @@ fn build_cross_module_context(parsed: &[ParsedFile]) -> CrossModuleContext {
396402
let file_key = canonical_key(&pf.path);
397403

398404
// Collect use declarations: alias → use-path string, and resolve each
399-
// against the check set. Also build alias → canonical target key.
405+
// against the check set. Also build alias → canonical target key, and
406+
// note paths that resolve neither in the check set nor on disk — a
407+
// broken import, distinct from an on-disk file merely outside a
408+
// narrower check set (a single-file check of one member of a pair).
400409
let mut aliases: HashMap<&str, String> = HashMap::new();
401410
let mut alias_targets: HashMap<&str, PathBuf> = HashMap::new();
402411
let mut resolved_for_file: HashSet<String> = HashSet::new();
412+
let mut missing_for_file: HashSet<String> = HashSet::new();
403413

404414
for d in &pf.result.module.declarations {
405415
if let allium_parser::ast::Decl::Use(u) = d {
@@ -409,6 +419,12 @@ fn build_cross_module_context(parsed: &[ParsedFile]) -> CrossModuleContext {
409419

410420
if check_set.contains(&target_key) {
411421
resolved_for_file.insert(path_text.clone());
422+
} else if path_text.ends_with(".allium") && !target.exists() {
423+
// Only local file references (ending .allium) can be
424+
// broken. Registry coordinates (`github.com/org/spec/sha`)
425+
// are immutable remote references that are not expected on
426+
// disk; they stay out-of-set, never broken.
427+
missing_for_file.insert(path_text.clone());
412428
}
413429

414430
if let Some(alias) = &u.alias {
@@ -417,6 +433,7 @@ fn build_cross_module_context(parsed: &[ParsedFile]) -> CrossModuleContext {
417433
}
418434
}
419435
}
436+
missing_use_paths.insert(file_key.clone(), missing_for_file);
420437

421438
// 0. Reverse contributions — for each alias resolving to a module in the
422439
// check set, the contributions this importer makes back to it
@@ -573,6 +590,7 @@ fn build_cross_module_context(parsed: &[ParsedFile]) -> CrossModuleContext {
573590
ambiguous_imports,
574591
reverse_contributions,
575592
imported_entity_statuses,
593+
missing_use_paths,
576594
}
577595
}
578596

@@ -586,10 +604,10 @@ fn canonical_key(path: &Path) -> PathBuf {
586604
}
587605

588606
fn cmd_check(args: &[String]) -> ExitCode {
589-
run_multi_file("check", args, |path, source, result, source_map, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports, reverse, referenced_triggers, _imported_entity_statuses| {
607+
run_multi_file("check", args, |path, source, result, source_map, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports, reverse, referenced_triggers, _imported_entity_statuses, missing_use_paths| {
590608
// `check` emits diagnostics only, not findings, so it needs no imported
591609
// status vocabulary (conflicts are findings, surfaced by `analyse`).
592-
let analysis = allium_parser::analyze_with_cross_module(&result.module, source, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports, reverse, referenced_triggers);
610+
let analysis = allium_parser::analyze_with_cross_module(&result.module, source, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports, reverse, referenced_triggers, missing_use_paths);
593611
let diagnostics: Vec<serde_json::Value> = result
594612
.diagnostics
595613
.iter()
@@ -604,8 +622,8 @@ fn cmd_check(args: &[String]) -> ExitCode {
604622
}
605623

606624
fn cmd_analyse(args: &[String]) -> ExitCode {
607-
run_multi_file("analyse", args, |path, source, result, source_map, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports, reverse, referenced_triggers, imported_entity_statuses| {
608-
let analyse_result = allium_parser::analyse_with_cross_module(&result.module, source, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports, reverse, referenced_triggers, imported_entity_statuses);
625+
run_multi_file("analyse", args, |path, source, result, source_map, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports, reverse, referenced_triggers, imported_entity_statuses, missing_use_paths| {
626+
let analyse_result = allium_parser::analyse_with_cross_module(&result.module, source, external_refs, resolved_use_paths, imported_triggers, imported_entity_fields, ambiguous_imports, reverse, referenced_triggers, imported_entity_statuses, missing_use_paths);
609627
let diagnostics: Vec<serde_json::Value> = result
610628
.diagnostics
611629
.iter()

0 commit comments

Comments
 (0)