fix(lint): demote equality-arity from Error to Warning - #125
Merged
Conversation
Four PRs took this rule from 1981 findings to 407 by closing three structural false-positive classes -- hard-quoted data (#119, #120), CLHS type positions (#121), and case keys and bound variables (#123). The 407 that remain are, on every sample drawn from them, still false: 120 of 120 in one adjudication and 14 of 14 in another. They cannot be closed the same way, and the investigation into a general mechanism is what settles the severity question. Suppressing findings inside any head the engine has never seen defined would silence 32,886 of 113,979 findings across 186 of the 214 rules that fire -- 28.9% of the catalogue's output. An adjudicated sample of thirty of the collateral was 23 genuine, 5 false, 2 ambiguous, which scales to roughly 25,000 real findings destroyed to remove 405 false ones, about 62 real per false. The premise behind that design is also simply untrue: `when`, `dolist`, `deftest`, `describe`, `macrolet` and `named-let` are all macros whose body arguments *are* evaluated, so knowing a head is a macro says nothing about which of its positions hold data. `OpacityCauseKind::UnknownHead`'s own documentation reaches the same conclusion -- "treating `(print x)` as opaque too would be sound as well, and would prove nothing about any real file" -- and Common Lisp has no registry of ordinary function heads to distinguish them. A configured list of unevaluated heads fares no better. `RuleSettings` carries `i64` values by explicit design, so the list needs `packages/core/lint-engine` and its config key needs `config_bridge`; a *name* list is the wrong granularity anyway, since every `deftransform` finding is at child index 2, its lambda list, while its body at index 3 and beyond is ordinary code. Even a perfect `(head, position)` table closes 312 of 407, leaving a build-blocking rule that still blocks builds on correct programs. Fourteen rules share this false-positive class, 624 findings, seven of them at `Severity::Error` for 459 build-blocking false positives, so this is not a problem peculiar to one rule. It is worth fixing properly later. Until then the honest severity for a rule measured at 407 findings and no true positives over 5556 files is `Warning`, not `Error`. Worth recording for whoever picks this up: the 407 live in three projects -- SBCL's compiler (561 of the 624), `trivia` (62) and `mgl-pax` (1) -- and the corpus holds five SBCL releases that content-hash dedup cannot collapse, so those 407 findings occupy 88 paths but only 36 canonical ones. The SBCL bucket is one codebase counted five times. `warning_count` 255 to 256 and the preset-filtered count 239 to 240; the rule is untagged, so both move together. `RULE_COUNT` and `fixable_count` are unchanged. The golden diff is seven lines across six files, all of them the severity token.
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.
Four PRs took this rule 1,981 → 407 by closing three structural false-positive classes: hard-quoted data (#119, #120), CLHS type positions (#121),
casekeys and bound variables (#123).The 407 that remain are still false on every sample drawn — 120/120 in one adjudication, 14/14 in another. They cannot be closed the same way, and the investigation into a general mechanism is what settles the severity question.
The "unknown head is opaque" design is dead by measurement
Suppressing inside any head the engine has never seen defined silences 32,886 of 113,979 findings — 28.9% of the catalogue's output, across 186 of the 214 rules that fire. The narrow "immediate parent unknown" variant still silences 18,086 (15.9%) across 150 rules. Thirteen rules with ≥20 findings lose ≥90%:
nested-function-parameter-shadows-enclosing-parameter190/190,when-unless-implicit-nil-misused61/61,thread-spawned-without-error-handler35/35.Adjudicated sample of 30 of the collateral: 23 genuine, 5 false, 2 ambiguous → ~25,000 real findings destroyed to remove 405 false ones, about 62 real per false.
And the premise is simply untrue. "A macro's arguments are not evaluated" fails for
when,dolist,deftest,describe/it,macrolet,named-let,define-inline— all macros whose body arguments are evaluated. Knowing a head is a macro says nothing about which of its positions hold data.OpacityCauseKind::UnknownHead's own doc had already reached this: "treating(print x)as opaque too would be sound as well, and would prove nothing about any real file." Common Lisp has no registry of ordinary function heads, by design.A configured head list can't be built in scope, and wouldn't suffice
RuleSettingsvalues arei64only, by explicit design ("When one is not, this grows a variant rather than aString"). The list needspackages/core/lint-engine; the config key needsconfig_bridge.deftransformfinding is at child index 2, its lambda list — its body at index 3+ is ordinary code and yields none.define-vopneeds indices 3–5, not its:generatorat 6. It needs(head, position)pairs.or/and/not/cons/ftypecombinators, plus a long tail of one-off DSL heads.A build-blocking rule that still blocks builds on correct programs after all that is not fixed.
This is not one rule's problem
14 rules share the class, 624 findings, 7 at
Severity::Error—equality-arity(312),setf-arity(77),single-arg-comparison(40),accessor-arity(21),eql-search-literal(4),eq-number-comparison(3),eql-list-comparison(2) = 459 build-blocking false positives. The shapes are identical:(setf symbol-function)is a setf function name indefknown;(>= optimizer)is adefoptimizername;(* word)is a type specifier.Worth fixing properly later. Until then, the honest severity for a rule measured at 407 findings and no true positives over 5,556 files is
Warning.Scale note for whoever picks this up
The 407 live in three projects — SBCL's compiler (561 of the 624),
trivia(62),mgl-pax(1) — and the corpus holds five SBCL releases that content-hash dedup cannot collapse. Those 407 findings occupy 88 paths but only 36 canonical ones. The SBCL bucket is one codebase counted five times.Counts
warning_count255 → 256, preset-filtered 239 → 240 (the rule is untagged, so both move together).RULE_COUNT(358) andfixable_count(106) unchanged. The golden diff is 7 lines across 6 files, all the severity token.cargo build --workspace/fmt --check/clippy --all-targets --all-features -D warnings/test --workspaceall exit 0.