fix(lint): stop two Emacs Lisp rules rewriting quoted symbol lists - #124
Merged
Conversation
`elisp-quoted-lambda` ships at `Severity::Error` with a destructive
autofix, and over 1751 GNU Emacs 31.0.91 and package files every one of
its fifteen findings was false. Its predicate was head-only -- a list,
carrying a `Quote` prefix, whose first child is `lambda` -- which matches
a *symbol list* just as readily as a quoted function. Demonstrated with
the shipped binary against `byte-opt.el`:
(memq head '(lambda internal-make-closure length cons))
-- after --fix -->
(memq head (lambda internal-make-closure length cons))
A membership test rewritten into a call, automatically, in GNU Emacs's
own source. The same shape appears in `bind-key.el`, `cus-start.el`,
`elint.el` and `calc-map.el`. Two further findings were `',(lambda ...)`,
`menu-bar.el`'s idiom where the unquote evaluates the lambda so the quote
applies to the resulting closure, and two were `''(...)` or `'#'(...)`.
It now requires a lambda list -- `(...)` or `nil` -- in the second
position, and requires the prefix to be exactly `[Quote]`. Fifteen
findings become six.
`elisp-obsolete-cl-alias` ships at `Severity::Error` with no context
check at all: 150 findings, and 25 of 25 sampled at random were false.
They are `(dolist (block blocks) ...)` and `(let (ll (do t)) ...)`
binding pairs, `(defun mail-comma-list-regexp (labels) ...)` lambda
lists, `(mapcar (lambda (case) ...))`, quoted data such as
`(memq word '(do doing))` and `(doctor-type '(do you know Stallman \?))`,
and `cl-indent.el`'s own indent-spec table. None was a call to a removed
macro, which is what you would expect: Emacs 31 would not compile if it
were.
It now requires at least two arguments, requires the first argument to
match each macro's real lambda list -- a list for `do` and `flet`, a
symbol for `block` -- skips a node carrying its own quote, and finally
asks `binding_table().resolve()` whether the head is a local binding.
That last check removed 21 of the remaining 35 on its own: contrary to
what the other dialects suggest, Emacs Lisp *does* have a modelled
binding table, in `semantics/binding/service/emacs_lisp.rs`, which knows
`named-let`. 150 findings become 14.
Of those 14, one is a genuine unprefixed `(case command ...)` in a
chibi-scheme company backend that was previously buried in noise, one is
`cl.el`'s own shim, and twelve are a single remaining class: a quote or
quasiquote on an *ancestor* rather than the node. `is_unevaluated_at` in
`lint-form-shape/src/support.rs` solves exactly that, and PRs #119 and
#120 established the pattern, but adopting it here means copying the
two-counter model into a fifth package. That is a separate decision and
is left alone.
Mutation testing earned its place twice over. The second harness first
reported five survivors, because the new arity guard masked every other
guard in the test cases I had written; rebuilding those controls around
the real GNU Emacs shapes took it to 7 of 7 killed. Without it this would
have shipped four guards with no coverage.
Reported and not fixed: `leftover-print-debug` is `Fixable` with 6560
findings and treats Scheme's `display` and Janet's `print` as debug
leftovers, when they are those languages' primary output primitives --
`--fix` deletes them. And `elisp-defcustom-missing-group` fires on 3668
of 8476 `defcustom` forms; it is correct, but it relies on documented
file-level `defgroup` inheritance and belongs in `RuleTag::Pedantic`.
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.
Survey of the non-Common-Lisp dialect rules — a dimension never audited before. Enumerated by a probe crate iterating the shipped
REGISTRY: 358 rules, 76 name a non-CL dialect (58 pure, 18 mixed) = 133 (rule, dialect) sweeps over 9,811 raw → 9,586 SHA-256-deduped files across nine dialects. 0 partial batches; every sweep assertedscanned == list length. 14,579,895 candidate nodes, 19,383 findings.Rank 1 —
elisp-quoted-lambda: Error + Fixable + Destructive, 15/15 falseThe predicate was head-only: a list, with a
Quoteprefix, whose first child islambda. That matches a symbol list as readily as a quoted function. Demonstrated with the shipped binary againstbyte-opt.el:A membership test rewritten into a call, automatically, in GNU Emacs's own source. Same shape in
bind-key.el,cus-start.el,elint.el,calc-map.el. Two more were',(lambda …)—menu-bar.el's idiom where the unquote evaluates the lambda so the quote applies to the closure; two were''(…)/'#'(…).Fix: require a lambda list (
(…)ornil) in second position, and require the prefix to be exactly[Quote]. 15 → 6.Rank 2 —
elisp-obsolete-cl-alias: Error, no context check at all, 25/25 sampled false150 findings. All binding pairs, lambda lists, or quoted data:
(dolist (block blocks) …),(let (ll (do t)) …)(defun mail-comma-list-regexp (labels) …)(mapcar (lambda (case) …))(memq word '(do doing)),(doctor-type '(do you know Stallman \?))cl-indent.el's own spec listNone was a call to a removed macro — unsurprising, since Emacs 31 wouldn't compile if it were.
Fix: require ≥2 arguments; require the first argument to match each macro's real lambda list (
Listfordo/flet,Symbolforblock); skip node-local quotes; then askbinding_table().resolve(). 150 → 35 → 14.A premise in the brief was wrong
I stated that non-CL dialects have an empty
binding_table(). Emacs Lisp does not —packages/core/semantics/src/semantics/binding/service/emacs_lisp.rsmodelsnamed-let, and that check alone removed 21 of 35 residual findings. It holds for LFE (whose rules say so in their own docs), not here.The residual 14
1 true positive — a genuine unprefixed
(case command …)in a chibi-scheme company backend, previously buried in noise. 1 arguable (cl.el's own shim). 12 false, all one class: quote/quasiquote on an ancestor.is_unevaluated_atinlint-form-shape/src/support.rssolves exactly that and #119/#120 established the pattern — but adopting it means copying the two-counter model into a fifth package, which is a separate decision.Mutation testing earned its place
The second harness first reported 5 survivors — because the new arity guard masked every other guard in the test cases. Rebuilding those controls around real GNU Emacs shapes took it to 7/7 killed (and 6/6 on the first rule). Without it, four guards would have shipped with no coverage.
Controls: 14 asserting every alias still fires in its real call shape; 4 asserting
elisp-quoted-lambdastill fires on'(lambda (n) n),'(lambda nil 1),'(lambda () 1),&optionalforms, and inside a backquote.All three briefed hypotheses were refuted
,-divergent dialectreader_policy.rs:748documents~as deliberately not a prefix, so the effect is suppression; Hy's coverage is understatedReported, not fixed
leftover-print-debug— Warning + Fixable, 6,560 findings. Treats Scheme'sdisplayand Janet'sprintas debug leftovers when they are those languages' primary output primitives:(display line out)in an echo server,(print (slurp path))in an amalgamation tool.--fixdeletes them.elisp-defcustom-missing-group— fires on 3,668 of 8,476defcustoms. Correct, but relies on documented file-leveldefgroupinheritance; belongs inRuleTag::Pedantic.Verification
Re-running all 133 sweeps against the patched build produced a diff of exactly two lines, both reductions, nothing added.
cargo build/test -p paredit-feature-emacs-lisp(43) /clippy --all-targets -- -D warnings/fmt --check/test -p paredit-cli --lib(334) /test --test cli(3,085 passed) — all exit 0, read unpiped. No golden moved, no pinned count moved.Known gap: two subagents adjudicating mid-tier rules (
elisp-interactive-arity-mismatch, the Scheme/RacketFixableset,duplicate-test-name,racket-match-unreachable-clause) never returned. Those are surveyed but not adjudicated.