feat: static semantic analysis layer, and one-pass registry-driven lint - #20
Merged
Conversation
Read-only side tables beside the syntax tree, so lint rules can reason about what code *means* rather than how it is spelled. The tree is never rewritten: formatting survives a refactor because edits are byte-span replacements over untouched source, and that only holds while the tree stays authoritative. Four contexts, each a model/policy/service split: - `binding` inverts `lexical_scope`'s per-symbol query into a per-file table. The query answers "where is `x` referenced, accounting for shadowing?"; the value layer needs the inverse, "what does the name at *this* position mean?", which no per-symbol query can answer. The shadowing rules are not restated — `patterns.rs` now yields each bound name with its span, and both readings are derived from that one traversal. A differential test asserts the full partition `live = free ⊎ resolved ⊎ definitions` over the existing `lexical_scope` fixtures, deriving the live-atom set independently from the reader rules so it cannot agree with a bug by construction. - `value` reads literals, folds pure operations, and propagates constants through bindings nothing can have changed. `PropagatableValue` omits strings and floats *by construction* rather than by a check the propagation pass could forget: a Common Lisp string is mutable in place, so substituting its contents would lie about identity. - `typing` is a coarse CLHS lattice. The relation is a DAG, not a tree — `null` is both a symbol and a list — so `join` returns `t` where the least common supertype is not unique. Widening loses a deduction; guessing would license a false one. - `project` gives a symbol its home package, which is what makes `app:run` and `test:run` two values instead of one. Everything the layers cannot prove is absent rather than guessed, and each links to the one below by id rather than by borrow — a `ValueTable` holding `&BindingTable` would make them one self-referential struct. `NodeKey` keys the tables on `(span, kind)` because `ExpressionView` carries no `NodeId`; a property test pins that every non-root node's pair is unique.
The suite was four hand-maintained parallel arrays (`RULES`, `RULE_DOCS`, `FIXABLE_RULES`, `WARNING_RULES`) plus a 1,650-line function calling 134 collectors in sequence, each walking the whole document on its own, with the matching automatic fixes stranded in the CLI layer. Adding a rule meant ~500 lines across 11 edit sites. A rule now declares which nodes it wants and what to say about one; it never walks the document. `registry` is the single array the four public constants are derived from at compile time, with `const` assertions pinning each derived length, so the catalogue can no longer disagree with itself. `engine` walks the document once, reaching head-specific rules through an operator index. Adding a rule is one module plus one registry line. Reproducing the report's historical order needed care. Sorting by `(rule, span)` looks sufficient but is not: the duplicate-key rules emit several findings on one span and the tie has no defined resolution. Counting instead — which rule, which node of the walk, which emission within that node — is a total order by construction and never consults a span. Fixes moved into the rules that own them. `LintRule::fix(ctx, finding)` was not expressible: a `LintFinding` is a projection that has dropped the data a fix needs (the inner span to splice, the complement operator to substitute), so findings and fixes are produced by the same visit instead. 131 of 134 rules are reached through the operator index. The three that are not — `duplicate-parameters`, `duplicate-lambda-list-keyword`, `lambda-list-keyword-order` — correlate separate definitions and have no per-node form. Three rules now resolve constants before reporting, through lazily-built tables on `RuleContext`: `zero-divisor` sees `(let ((z 0)) (/ x z))`, `(/ x (- 1 1))` and `#x0`; `parse-integer-default-radix` sees `#xA`; `constant-if-test` sees any test the value layer settles. Each stays silent on anything merely *possible* at run time. Behaviour is unchanged everywhere else, pinned by a golden test comparing `inspect lint` text/JSON/SARIF/fix-plan/fix output byte for byte against the previous implementation. On `benches/lint_report.rs`, 1024 forms of ordinary code went from 124.0 ms to 3.60 ms.
`let` chains stabilized after Rust 1.85, which `rust-version` pins. A newer local toolchain compiles them happily, so only `nix flake check` — which builds against the pinned version — catches it.
…elongs A reviewer opening `src/domain/semantics` had no way to find out what the four contexts are for, why they link by id rather than by borrow, or why the layer deliberately refuses to answer. The change-routing table now covers lint rules and static facts too.
The type and project contexts were implemented and tested but nothing read them, which is what the `#![allow(dead_code)]` in `semantics/mod.rs` covered. Both now have the consumers the design named. `eq-number-comparison` and `eq-char-comparison` matched a *literal* argument to `eq`. But `eq` on a number or character is undefined behaviour however the value got there, so `(eq (length xs) n)` is the same bug as `(eq n 5)` — the type context knows `length` returns an integer. The report modules take the test as a parameter, exactly as `zero-divisor` does, so the standalone `inspect` commands keep reading only literals and their output is unchanged. Evidence is an enum rather than an empty `literal` string: "recognized without a literal" is a different fact from "recognized with an empty one", and a sentinel would have let the two blur. `impact_report` compared bare symbol names, so `app:run` and `test:run` looked like one definition and a report about changing one claimed it affected callers of the other. It now narrows that comparison with each side's package — and only narrows it. When either side does not resolve, which is every file with no `in-package`, the comparison falls back to exactly what it did before. That fallback is why no existing test changed: guessing `CL-USER` for an unqualified symbol would manufacture the confusion this removes. `RuleContext::type_table()` joins the binding and value tables, built on first use like they are.
With the type and package consumers wired up, the residue is just the binding table's structural surface — the scope tree and a binding's provenance — which only a scope-aware rule would ask for. The old note claimed two whole contexts were unread, which is no longer true.
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.
Implements the five phases of
semantic-analysis.md: a read-only semanticlayer beside the syntax tree, the lint-engine rework that lets rules consume
it, and the consumers each phase names.
The motivating example:
zero-divisorused to compareatom_text(view) == Some("0"), so it saw(/ x 0)and nothing else. It now sees(let ((z 0)) (/ x z)),(/ x (- 1 1)),(/ x #x0), and a divisor reachedthrough a same-file
defconstant— while staying silent on anything merelypossible at run time.
What changed
src/domain/lint— one trait, one registry, one pass.The suite was four hand-maintained parallel arrays (
RULES,RULE_DOCS,FIXABLE_RULES,WARNING_RULES) plus a 1,650-line function calling 134collectors in sequence, each walking the whole document on its own, with the
matching automatic fixes stranded in the CLI layer. Adding a rule meant ~500
lines across 11 edit sites; it is now one module plus one registry line. The
four constants are derived from that single array at compile time, with
constassertions pinning each derived length.
131 of the 134 rules are reached through an operator index. The three that are
not —
duplicate-parameters,duplicate-lambda-list-keyword,lambda-list-keyword-order— scan onlytree.root_children()to reachtop-level definitions, and a per-node check cannot tell that it is looking at a
direct root child. Converting them would silently start matching nested
defunsinside
flet/let, which the current behaviour never examines.src/domain/lint_report.rssrc/presentation/cli/lint_report/workflow.rssrc/domain/semantics— four stacked contexts.bindinginvertslexical_scope's per-symbol query into a per-file table;valuereads literals, folds pure operations, and propagates constants;typingis a coarse CLHS lattice;projectgives a symbol its home package.Each splits into
model/policy/service, and links downward by idrather than by borrow — a
ValueTableholding a&BindingTablewould makethem one self-referential struct.
RuleContextbuilds each on first use, so arun whose rules ask for none pays for none.
Consumers.
zero-divisor,parse-integer-default-radixandconstant-if-testread the value table;eq-number-comparisonandeq-char-comparisonread the type table, so(eq (length xs) n)is caughtalongside
(eq n 5);impact_reportreads the project table, soapp:runandtest:runstop being one symbol. Every one of them stays silent when theanswer is
Unknown.Verification
nix flake check— 8/8 (package, clippy, treefmt, actionlint, documentation,lint-format-integration, nextest, msrv)
inspect lintoutput is byte-identical to the previous implementationacross text / JSON / SARIF /
--fix-plan/--fix, pinned by a golden testgenerated from the old code before any of this landed. The upgraded rules
only ever add findings; none were lost.
lexical_scopeby a differential testasserting the full partition
live = free ⊎ resolved ⊎ definitionsover theexisting fixtures, deriving the live-atom set independently from the reader
rules so it cannot agree with a bug by construction.
impact_reporttest changed: a file with noin-packagecannotresolve a package, and then the comparison is exactly what it was.
Each of the six commits builds on its own.
Three deliberate departures from the specification
LintRule::fix(ctx, finding)was dropped. ALintFindingis aprojection that has already discarded what a fix needs — the inner span to
splice, the complement operator to substitute — so findings and fixes are
produced by the same visit instead.
(rule, span)looks sufficient but is not: the duplicate-key rules emitseveral findings on one span and the tie has no defined resolution.
head is opaque" makes
(/ x z)opaque —CommonLispOperatorregisters onlybinding forms — which would make the specification's own acceptance
criterion unreachable. What actually threatens a propagated value is a macro
expanding into an invisible assignment. A function cannot reach the caller's
lexical environment at all; a control form evaluates its subforms where they
are written, so any assignment inside is already collected. Only unknown
heads stay opaque. The reasoning is in
semantics/binding/policy/standard_{functions,control_forms}.rs.Measured, and where to go next
cargo run --example semantic_coverage -- <corpus>reports 15.5% of variablebindings resolving to a constant. 44% of the misses are bindings with no
initial form — mostly lambda-list parameters, a structural ceiling no loosening
would lift. The actionable 26.6% is opaque scopes, caused by ordinary macros
like
handler-casehaving no registered semantics: onehandler-casecallmarks its whole enclosing
letopaque. Registering a handful of common CLmacros looks like the highest-leverage next step.
The remaining
#![allow(dead_code)]insemantics/mod.rsnow covers only thebinding table's structural surface — the scope tree and a binding's provenance
— which a scope-aware rule would use and none of today's consumers do.