Skip to content

feat(lint): add 3 compile-time phase lint rules - #92

Merged
takeokunn merged 1 commit into
mainfrom
feat/lint-batch-r10
Aug 3, 2026
Merged

feat(lint): add 3 compile-time phase lint rules#92
takeokunn merged 1 commit into
mainfrom
feat/lint-batch-r10

Conversation

@takeokunn

Copy link
Copy Markdown
Collaborator

Stacked on #90. RULE_COUNT 313 → 316, three standalone commands. All three are Severity::Error and report-only.

lint-compile-time: eval-when-execute-only, eval-when-body-never-runs, defconstant-non-eql-value.

Why the two-phase testing mattered

These are the "works in the REPL, breaks from a compiled file" class, so every premise was run under both load of source and compile-file + load of the fasl. That distinction did most of the work:

expression load src compile-file + load fasl
(eval-when (:execute) (defmacro m (x) \(* ,x 10)))` 40 style-warning only, failure-p NIL, then UNDEFINED-FUNCTION
(eval-when (:load-toplevel :execute) …) 40 40
(defconstant +x+ #("Mon" "Tue")) ok DEFCONSTANT-UNEQL

The middle row killed the obvious predicate. "Missing :compile-toplevel" would have fired on a correct and common idiom, because CLHS 3.2.3.1 keeps an eval-when body top level so it runs anyway. The real predicate is "reaches the compiler at all".

defconstant is stronger than expected: it fires on the first build in a fresh image, and asdf:load-system on an ordinary project containing (defconstant +days+ #("Mon" "Tue")) aborts. The probe that established this initially reported every initform including 42 and nil as UNEQL — because it reused one constant name across cases. The controls caught it.

The proposed third rule split in two, because one predicate cannot cover both positions: a top-level eval-when that never reaches the compiler, and a nested one whose body never runs in any phase with zero compiler diagnostic. The nested case wasn't in the original proposal.

Five proposals dropped against SBCL 2.6.0

  • macro-used-before-defined — premise refuted. It does not work under load; both phases fail identically with the same message.
  • defpackage-not-first-form — premise refuted. (defun early () 1) before the defpackage still yields R6PKG:EARLY :EXTERNAL in both phases; the earlier defun interned into a different package.
  • defmacro-without-eval-when — soundly decidable (the quote model separates template from evaluated position), but dropped on value: SBCL raises a hard ERROR that fails compilation saying "It is defined earlier in the file but is not available at compile-time".
  • load-time-value-in-macro and read-time-eval — both fail identically in both phases. No asymmetry, nothing actionable.

Three rules now share the head eval-when

A first for this registry. lint-control-flow's existing eval-when-situation fires on invalid spellings (:executee, category Malformed). Disjointness was proved through the engine, not by reading: :executee trips only that rule, a nested ignored situation trips only eval-when-body-never-runs, and neither trips eval-when-execute-only. The rule was renamed from eval-when-situation-ignored for exactly this reason — two same-head rules one character-class apart is a trap.

One visible seam from that choice: a top-level (eval-when (:executee) (defmacro m () 1)) has its body discarded just like the eval-when-execute-only shape, but that rule requires :execute to be literally named, so it declines. eval-when-situation catches the form, so nothing goes unreported — it's reported as a spelling error rather than a phase error. Flagging it so the trade is visible.

Corpus audit

1588 files (SBCL 2.6.0 sources + ~/quicklisp/dists/), 347 eval-when and 1079 defconstant forms reached as code. 9 findings, 0 false positives — each adjudicated by extracting the exact form standalone and running compile-file + load, and every one reproduces DEFCONSTANT-UNEQL.

The two zero-finding rules are a true clean, not a dead predicate: the corpus contains no execute-only eval-when at all and exactly one nested one, which correctly names :execute. Critically, 133 eval-when nodes are quoted data, of which 30 would have been false positives without the two-counter quote model — e.g. closer-mop-shared.lisp:509, a template emitting (eval-when (:compile-toplevel) …) for the caller's file. That shape is now a permanent test.

The audit harness was self-tested first (4 findings on a known-dirty file, 0 on a known-clean one), because a sibling batch had a sweep silently report zero findings when an invalid --emit json made every batch error out.

Honest limitation

The repo's own corpus is near-empty for this package — 1 eval-when form and 3 defconstant forms. Its silence shows no regression rather than good scoping. The 1588-file audit is what carries the claim.

Cost

Zero invocations of all three rules on a clean/forms/*-shaped file at both sizes — structural, since Heads filters before check, so that gate cannot be affected. Per-invocation 195/518/547 ns against a shipped self-recursive-tail-call at 244 ns in the same run; doubling ratios 1.94–1.96. Every rule answers a node-local question before touching the tree, and the top-level binary search uses the allocation-free root_child_span rather than Path::root_child.

Mutation testing: 17 guards, 2 survivors, both chased

One was dead code (a reader-conditional bail already handled by the fallthrough arm) and is removed with the property still pinned. One was a missing test — the macrolet cases never reached the guard because a bindings list's head is itself a list, so the descent stopped a level early. All 16 remaining mutations killed.

Verification

cargo build --workspace, cargo test --workspace, cargo test --test cli (3083 passed), cargo fmt --all --check, cargo clippy --all-targets --all-features -- -D warnings — all exit 0.

Wiring turned up a fifth pinned site the earlier batches had missed: dialect_contract.rs's per-category BTreeMap breakdown (introspection 337→340), which only surfaced on the second test run. determinism_contract.rs's doc comment was also already stale by 10 from a previous batch.

@takeokunn
takeokunn force-pushed the feat/fix-ignore-declaration-conflict branch from f41e9e7 to 743a652 Compare August 3, 2026 08:07
Base automatically changed from feat/fix-ignore-declaration-conflict to main August 3, 2026 08:33
RULE_COUNT 313 -> 316, three standalone commands. All three are
Severity::Error and report-only.

lint-compile-time: eval-when-execute-only, eval-when-body-never-runs,
defconstant-non-eql-value.

These are the "works in the REPL, breaks from a compiled file" class,
so every premise was run under **both** `load` of source and
`compile-file` + load of the fasl. That distinction did most of the
work:

- `(eval-when (:execute) (defmacro m ...))` returns 40 under `load` and
  raises UNDEFINED-FUNCTION from the fasl, with only a style-warning and
  `failure-p NIL` at compile time.
- `(eval-when (:load-toplevel :execute) ...)` works in **both**. That
  row killed the obvious predicate: "missing :compile-toplevel" would
  have fired on a correct, common idiom, because CLHS 3.2.3.1 keeps an
  eval-when body top level so it runs anyway. The real predicate is
  "reaches the compiler at all".
- `defconstant` with an allocating initform is stronger than expected --
  it fires on the *first* build in a fresh image, and `asdf:load-system`
  on an ordinary project with `(defconstant +days+ #("Mon" "Tue"))`
  aborts outright.

The proposed rule 3 split in two, because one predicate cannot cover
both positions: a top-level eval-when that never reaches the compiler,
and a nested one whose body never runs in any phase with **zero**
compiler diagnostic.

Five proposals were dropped against SBCL 2.6.0. `macro-used-before-
defined` does *not* work under `load` -- both phases fail identically.
`defpackage-not-first-form` is refuted outright: a `defun` before the
`defpackage` still yields the symbol in the right package, because the
earlier form interned into a different one. `defmacro-without-eval-when`
is soundly decidable but SBCL already raises a hard ERROR naming the
cause. `load-time-value-in-macro` and `read-time-eval` fail identically
in both phases, so there is no asymmetry to report.

`eval-when` now has three registered rules on one head, a first for this
registry. Disjointness from lint-control-flow's `eval-when-situation`
(which fires on invalid *spellings*) was proved through the engine:
`:executee` trips only that one, a nested ignored situation trips only
`eval-when-body-never-runs`, and neither trips the third.

The corpus audit swept 1588 files with 347 eval-when and 1079
defconstant forms as code, producing 9 findings and 0 false positives,
each reproduced standalone under compile-file. 133 eval-when nodes were
quoted data, of which 30 would have been false positives without the
two-counter quote model -- that shape is now a permanent test.

Note the repo's own corpus is near-empty for this package (1 eval-when,
3 defconstant), so its silence shows no regression rather than good
scoping. The 1588-file audit is what carries the claim.
@takeokunn
takeokunn force-pushed the feat/lint-batch-r10 branch from 70b8245 to bd84c62 Compare August 3, 2026 08:35
@takeokunn
takeokunn merged commit 199f425 into main Aug 3, 2026
10 checks passed
@takeokunn
takeokunn deleted the feat/lint-batch-r10 branch August 3, 2026 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant