lsp: add quickfix code actions for autofixes and suppressions - #575
Open
ZayanKhan-12 wants to merge 3 commits into
Open
ZayanKhan-12 wants to merge 3 commits into
ZayanKhan-12 wants to merge 3 commits into
Conversation
Implements the two code action features tracked in Instagram#445. Placing the cursor on a Fixit diagnostic now offers, per violation under the cursor or selection: - "Fix <RuleName>", applying that one violation's autofix and leaving every other violation in the file alone - "Silence <RuleName> with # lint-fixme" / "# lint-ignore", inserting a suppression comment above the offending statement Applying a single fix goes through `LintRunner.apply_replacements([violation])` and `format_module`, rather than rendering `violation.replacement` into the diagnostic's range. A rule can report a position that doesn't cover the node it replaces (`report(..., position=...)`), a replacement can be a `FlattenSentinel` or `RemovalSentinel` rather than a single node, and the configured formatter may reflow the result — so round-tripping through the module is the only way to get the same output `fixit fix` would produce. The suppression comment is anchored to the innermost enclosing node that owns leading comment lines, mirroring where `LintRule.node_comments` stops looking for directives. Inserting above the reported line instead would land inside the brackets of any multi-line expression, where the directive is silently ignored: value = os.path.join( "a", dict(), # <- a directive here does nothing ) Edits are now computed as a minimal line-span replacement instead of a whole-document rewrite, so clients keep cursors, folds and decorations outside the changed lines. `textDocument/formatting` uses the same helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both are states an editor is routinely in — a half-typed document, or a scratch buffer opened under a scheme with no filesystem path. Neither should raise; both should simply offer nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers the things that aren't discoverable from the code: which checks CI actually gates, which files are generated and must not be hand-edited, that new test modules have to be registered in `fixit/tests/__init__.py`, that rule tests come from VALID/INVALID rather than test modules, and the lint/fix generator protocol. Complements CONTRIBUTING.md rather than restating it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ZayanKhan-12
force-pushed
the
lsp-code-actions
branch
from
September 15, 2026 04:18
ead0688 to
dc0a71a
Compare
ZayanKhan-12
added a commit
to ZayanKhan-12/Fixit
that referenced
this pull request
Sep 15, 2026
Implements Instagram#445. Upstream PR: Instagram#575 CI: 20 pass / 3 fail, identical to the unmodified-main baseline in #2 (the 3 failures are 3.13t jobs that die building libcst, before any Fixit code runs).
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 both code action features tracked in #445.
What this adds
textDocument/codeActionis now advertised (quickfixkind only) and answered.For every violation overlapping the requested range, the server offers:
<RuleName>isPreferred, so it is what VSCode's ⌘. / Ctrl+. "auto fix" runs.<RuleName>with# lint-fixme<RuleName>with# lint-ignoreBoth directives are offered because the docs leave the choice
between them to the user.
Quickfix: fixing one violation
The issue sketches rendering
violation.replacementinto a string and splicingit over the diagnostic's range. I went through
LintRunnerinstead:Three reasons the textual splice doesn't hold up:
report(node, position=...)is part of the public APIreplacementisNodeReplacement, so it can be aFlattenSentinel(one nodebecomes several statements) or a
RemovalSentinel, neither of which is asingle node with a single rendering
formatter(black/ufmt) runs after fixes are applied andmay reflow lines the replacement didn't touch
Going through the module is also the only way to guarantee the LSP produces
byte-for-byte what
fixit fixwould.Because the module is already parsed by then, applying one violation's fix is a
cheap transformer pass over the existing tree — no re-lint per action.
Suppressions: where to put the comment
This was the part the issue called out as hard. Anchoring to the reported line
is wrong: for any multi-line expression the comment lands inside the brackets,
where
LintRule.ignore_lintnever looks at it and the directive silently doesnothing.
So the anchor is computed by walking up from
violation.nodeto the innermostancestor that owns
leading_linesand isn't aDecorator— which is exactlywhere
LintRule.node_commentsstops looking for directives. The comment isinserted as a standalone line above that node, indented to match it.
That lands correctly for nested expressions, decorated functions,
excepthandlers and
elseclauses (which own their ownleading_lines, so anchoringto the enclosing
try/ifwould not have worked). Each case has a test thatapplies the edit and asserts the violation is actually gone afterwards, rather
than just asserting on the edit's text.
Minimal edits
text_edits(before, after)replaces only the span of lines that changed, soclients keep cursors, folds and decorations elsewhere in the file.
textDocument/formattingnow uses it too, instead of rewriting the wholedocument on every format.
Tests
27 new tests in
fixit/tests/lsp.py(the first tests forlsp.py), coveringthe edit algebra, range intersection,
onlyfiltering, single-violationfixing, suppression placement across seven source shapes, CRLF documents,
syntax errors, non-
file:URIs, and the formatting handler. The suppressiontests apply the edit and assert the violation is gone afterwards, rather than
only asserting on the edit's text.
Also exercised against a real client:
fixit lsp --stdiodriven over JSON-RPC(
initialize→didOpen→codeAction) advertises"codeActionProvider": {"codeActionKinds": ["quickfix"]}and returns theexpected actions and edits on the wire.
make test,make lint,make htmlandpython -m buildare clean locally.CI
I ran the full matrix on my fork, and ran the identical matrix against an empty
commit on top of
mainas a baseline. The results are the same:mainbaselinetest (3.13t, *))test (3.13t, *))branch run ·
baseline PR
So this change introduces no CI regressions. The 3.13t jobs fail at
make install, before any Fixit code runs:libcstpublishes no free-threaded 3.13 wheel, so uv builds it from source andpyo30.29 refuses.buildandpublishare skipped on both runs because theyneed: test. That looks worth its own issue — happy to open one.Notes for review
CLAUDE.mdcommit is separable — drop the last commit if you'd rather notcarry that file; nothing in the feature commit depends on it.
codeAction/resolve, whichmeans one formatter run per offered fix. That is fine for a cursor sitting on
one or two diagnostics, but a selection covering a whole file of autofixable
errors would do real work. Happy to move the edit computation into a resolve
handler if you'd prefer.
source.fixAllaction would be nearly free on top of this(
formatalready applies every fix), but it's outside what lsp: tracking issue for code actions features #445 asks for, soI left it out.
Fixes #445