fix(lint): stop 38 more autofixes deleting a reader prefix - #127
Merged
Conversation
PR #122 fixed this in `redundant-progn` and `redundant-body-progn`. The same defect is in 38 further rules: `RuleFix::single(view.span, ...)` replaces a region that starts at the form's own reader prefix while the replacement text is built from the inner form, so a leading `'`, `` ` ``, `,`, `,@` or `#'` is deleted. The remedy is `view.content_span`, whose extent coincides with `span` on unprefixed code. A census over 5699 unique Common Lisp files, SHA-256 deduped from 16,115, found 42 rules dropping a prefix across 2650 of 17,751 replacements. Two are #122's and two -- `sharp-quoted-lambda` and `redundant-quote` -- exist to delete a prefix, so `content_span` would defeat them; they are excluded. Worst rates among the rest: `multiple-value-list-of-values` 100%, `redundant-prog1` 89.5%, `redundant-the` 80%, `if-not` 65.8%, `single-operand-boolean` 42.7%, `subseq-zero` 41.7%, `constant-if-test` 41.3%. Files SBCL could read before but not after go 29 to 2 with the two a strict subset of the 29, so nothing regressed, and prefix-dropping fixes among these rules go 706 to 0. Three rules did not take the plain swap. `empty-let` builds its fix as a sub-span inside its domain; `nested-progn` operates on a *child* node and took #122's `redundant_body_progn` treatment, a prefix requirement on the child; `constant-when-test` has two fix sites. `append-nil` showed zero drops in this corpus but carries the identical span and is named in the residual read-break list, so it is changed and the lack of corpus evidence is recorded here. The anti-over-suppression evidence is keyed on the *finding* span rather than the replacement offset, because the span change moves every replacement by construction and counting by it would report the migration as a behaviour change. On that measure the span change alone leaves fixes on unprefixed ordinary code at 14,080 before and after -- not one lost -- and the splice guard removes exactly 243 fixes, all of them `,@`, with no non-splice removed, no splice surviving, none added, and 1799 legitimate fixes retained. The oracle is SBCL's own reader with a custom readtable recording the byte offsets of each prefix, cross-validated against this parser over 80,321 regions in 4581 files with zero positional disagreements. Adding package-error recovery lifted readable coverage from 40% to 80%. Six apparent mismatches are all `,.`, where the positions agree but the parser labels unquote-dot as `Unquote` rather than `UnquoteSplicing` -- real, and separate. The two remaining read-breaks are a different bug and are reported rather than fixed: dotted-pair notation read as a call. `(append . nil)` becomes `(copy-list .)` and `(cons . (list sequence t))` becomes `(list . sequence t)`, because the parser exposes `.` as an ordinary child atom, so any rule counting operands is wrong there. It also reaches `redundant-body-progn`. Fixing two of an unknown number across 106 fixable rules would sprawl this change; it deserves its own audit. Mutation: 12 mutations, all killed, including widening the splice guard to any reader prefix and widening `nested-progn`'s guard to the enclosing form -- the two over-suppression mutations, both caught by the quasiquote-template controls. One mutation was initially invalid because `cargo fmt` had reflowed its anchor, which is why compile failure is matched on `error[E` and reported as invalid rather than as a kill.
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.
Completes #122. The same defect —
RuleFix::single(view.span, …)replaces a region starting at the form's own reader prefix while the replacement text is built from the inner form — is in 38 further rules. The remedy isview.content_span, whose extent coincides withspanon unprefixed code.Census
5,699 unique files (16,115 raw, SHA-256 deduped): 42 rules dropping a prefix across 2,650 of 17,751 replacements. Two are #122's; two —
sharp-quoted-lambda,redundant-quote— exist to delete a prefix, socontent_spanwould defeat them and they are excluded.Worst rates:
multiple-value-list-of-values100%,redundant-prog189.5%,redundant-the80%,if-not65.8%,single-operand-boolean42.7%,subseq-zero41.7%,constant-if-test41.3%.Result
The 2 are a strict subset of the 29 — zero regressions.
Three rules did not take the plain swap
empty-letbuilds its fix as a sub-span inside its domain.nested-prognoperates on a child node — took fix(lint): stop the progn fixes deleting a reader prefix #122'sredundant_body_progntreatment, a prefix requirement on the child.constant-when-testhas two fix sites.append-nilshowed zero drops in this corpus but carries the identical span and is named in the residual read-break list. Changed anyway; the absence of corpus evidence is recorded rather than glossed.Anti-over-suppression, keyed on the finding span
The naive check is useless here: the span change moves every replacement offset by construction, so counting by replacement would report the migration itself as a behaviour change. Keyed on the finding span instead:
nested-prognexemption.,@. 0 non-splice removed, 0 splices survived, 0 added, 1,799 legitimate fixes retained.Controls: every changed rule with tests has a "still fires inside a quasiquote template" case asserting the rewritten source — e.g.
`(when c (or ,x))→`(when c ,x)— plus symmetric negatives (a plain,xoperand is still one operand; an unprefixed child under a prefixed parent still fires), so neither guard can widen unnoticed.Oracle
SBCL's own reader with a custom readtable recording byte offsets of
'/`/,/,@/#', cross-validated against this parser over 80,321 regions in 4,581 files — up from 1,194 in #122's audit after adding package-error recovery that lifted readable coverage from 40% to 80%.0 positional disagreements, 0 byte-identity mismatches. Six apparent mismatches are all
,.(unquote-dot), where positions agree but the parser labels itUnquoterather thanUnquoteSplicing— real, and separate.The 2 remaining read-breaks are a different bug
Dotted-pair notation read as a call, reported not fixed:
The parser exposes
.as an ordinary child atom, so any rule counting operands is wrong there. It also reachesredundant-body-progn. Fixing 2 of an unknown number across 106 fixable rules would sprawl this PR — it deserves its own audit.Mutation
12 mutations, all killed — including M7 (widen the splice guard to any reader prefix) and M9 (widen
nested-progn's guard to the enclosing form), the two over-suppression mutations, both caught by the quasiquote-template controls. One mutation was initially reported INVALID becausecargo fmthad reflowed its anchor; re-anchored and killed. Compile failure is matched onerror[E/could not compileand reported INVALID, never as a kill.Scope
50 files, all under
packages/feature/lint-*.fix liststill reportsrule_count: 106; no golden and no pinned count moved — so no repo fixture contained one of these corruptions.The
lint-control-flow/src/support.rsconflict with #122 was two copies of the same helper with different doc comments; bodies verified byte-identical (42 lines) before keeping the merged one.cargo build --workspace/fmt --check/clippy --all-targets --all-features -D warnings/test --workspace/test --test cli(3,085 passed) all exit 0.