Diagnose references through a use path that resolves nowhere - #92
Merged
Conversation
When a use path did not resolve, references through its alias produced no diagnostic of any kind: the single unresolvedPath warning on the use line was the only signal, and one line-scoped allium-ignore silenced it — after which a spec referencing completely made-up names through the dead alias checked clean, exit 0. Deleting the use line entirely was louder (an error per reference) than breaking it. The multi-file layer now distinguishes a BROKEN import (the path resolves neither in the check set nor on disk) from an out-of-set one (the file exists on disk but is not part of a narrower check set, e.g. a single-file check of one member of a pair). References through a broken alias each draw a new allium.reference.unresolvedImport warning, independent of the use-line suppression; references through an out-of-set alias stay unknowable and silent, exactly as before, so single-file workflows are unaffected. Callers without filesystem context (the wasm crate, legacy single-file analyze) pass nothing and behave as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Soaking the repo's own parser fixtures surfaced the gap: a registry coordinate (use "github.com/specs/auth/abc123") is an immutable remote reference that is not expected on local disk, so the exists() probe classified it as broken and drew false unresolvedImport warnings. Only local file references (paths ending .allium) participate in the broken classification; coordinates stay out-of-set and their references unknowable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #87.
Bug
When a
usepath does not resolve, references through the alias produce no diagnostic of any kind — the singleallium.use.unresolvedPathwarning on theuseline is the only signal. That warning is suppressible with one line-scopedallium-ignore, after which a spec referencing completely made-up names through the dead alias checks clean with exit 0:Three asymmetries make this a soundness hole rather than a UX choice:
usestatement errors on every reference (undefinedImportedAlias); an alias whoseuseexists but resolves nowhere produces nothing per reference.Users add the suppression for a legitimate-looking reason: single-file checks of an importing spec always warn
unresolvedPath(the sibling is not in the 1-file check set), so the directive keeps that output clean — while silently blinding the multi-file gate.Fix
The multi-file layer now distinguishes a broken import from an out-of-set one:
allium.reference.unresolvedImport("Reference 'p/Name' goes through use path "./does-not-exist.allium", which does not resolve to a file in the check set or on disk."), independent of the use-line suppression (which is line-scoped and cannot reach the reference lines).unresolvedPathwarning is unchanged in both cases.So the only behaviour change is for imports that are genuinely dead — precisely the bug — and single-file workflows are untouched. Callers without filesystem context (the wasm crate, legacy single-file
analyze) pass nothing and behave as before.analyze_with_cross_module/analyse_with_cross_modulegain a trailingmissing_use_paths: &HashSet<String>parameter (workspace-internal API).Tests
unresolvedImportnaming both the reference and the path (was exit 0 with zero diagnostics); an unsuppressed broken import reports both codes; an on-disk-but-out-of-set target drawsunresolvedPathonly — references stay silent (pins the single-file workflow).use "github.com/specs/auth/abc123") draws nounresolvedImport— coordinates are immutable remote references not expected on disk, so only local file references (paths ending.allium) participate in the broken classification. Found by soaking this repo's own parser fixtures, which use exactly that form.Real-world impact
A ~180-spec production corpus carries 8 of these suppressions (added for single-file ergonomics, annotated "Resolves in a multi-file check set"). ~37 cross-module references flow through the blinded aliases, and for two of the imported modules every importer suppresses — verified there: renaming one of those files leaves the whole-tree gate green today. With this fix the rename goes loud regardless of the directives.