Skip to content

lsp: add quickfix code actions for autofixes and suppressions - #575

Open
ZayanKhan-12 wants to merge 3 commits into
Instagram:mainfrom
ZayanKhan-12:lsp-code-actions
Open

ZayanKhan-12 wants to merge 3 commits into
Instagram:mainfrom
ZayanKhan-12:lsp-code-actions

Conversation

@ZayanKhan-12

@ZayanKhan-12 ZayanKhan-12 commented Sep 15, 2026

Copy link
Copy Markdown

Implements both code action features tracked in #445.

What this adds

textDocument/codeAction is now advertised (quickfix kind only) and answered.
For every violation overlapping the requested range, the server offers:

Action When
Fix <RuleName> the violation has an autofix. Marked isPreferred, so it is what VSCode's ⌘. / Ctrl+. "auto fix" runs.
Silence <RuleName> with # lint-fixme always
Silence <RuleName> with # lint-ignore always

Both directives are offered because the docs leave the choice
between them to the user.

Quickfix: fixing one violation

The issue sketches rendering violation.replacement into a string and splicing
it over the diagnostic's range. I went through LintRunner instead:

updated = runner.apply_replacements([violation])
content = format_module(updated, path, config)

Three reasons the textual splice doesn't hold up:

  • a rule can report a position that doesn't cover the node it replaces —
    report(node, position=...) is part of the public API
  • replacement is NodeReplacement, so it can be a FlattenSentinel (one node
    becomes several statements) or a RemovalSentinel, neither of which is a
    single node with a single rendering
  • the configured formatter (black/ufmt) runs after fixes are applied and
    may 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 fix would.

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_lint never looks at it and the directive silently does
nothing.

value = os.path.join(
    "a",
    dict(),   # <- a directive here does nothing
)

So the anchor is computed by walking up from violation.node to the innermost
ancestor that owns leading_lines and isn't a Decorator — which is exactly
where LintRule.node_comments stops looking for directives. The comment is
inserted as a standalone line above that node, indented to match it.

That lands correctly for nested expressions, decorated functions, except
handlers and else clauses (which own their own leading_lines, so anchoring
to the enclosing try/if would not have worked). Each case has a test that
applies 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, so
clients keep cursors, folds and decorations elsewhere in the file.
textDocument/formatting now uses it too, instead of rewriting the whole
document on every format.

Tests

27 new tests in fixit/tests/lsp.py (the first tests for lsp.py), covering
the edit algebra, range intersection, only filtering, single-violation
fixing, suppression placement across seven source shapes, CRLF documents,
syntax errors, non-file: URIs, and the formatting handler. The suppression
tests 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 --stdio driven over JSON-RPC
(initializedidOpencodeAction) advertises
"codeActionProvider": {"codeActionKinds": ["quickfix"]} and returns the
expected actions and edits on the wire.

make test, make lint, make html and python -m build are clean locally.

CI

I ran the full matrix on my fork, and ran the identical matrix against an empty
commit on top of main as a baseline. The results are the same:

this branch main baseline
passing 20 20
failing 3 (test (3.13t, *)) 3 (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:

error: PyO3 does not support the free-threaded build of CPython versions
below 3.14, the selected Python version is 3.13

libcst publishes no free-threaded 3.13 wheel, so uv builds it from source and
pyo3 0.29 refuses. build and publish are skipped on both runs because they
need: test. That looks worth its own issue — happy to open one.

Notes for review

  • The CLAUDE.md commit is separable — drop the last commit if you'd rather not
    carry that file; nothing in the feature commit depends on it.
  • Code actions are computed eagerly rather than via codeAction/resolve, which
    means 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.
  • A source.fixAll action would be nearly free on top of this
    (format already applies every fix), but it's outside what lsp: tracking issue for code actions features #445 asks for, so
    I left it out.

Fixes #445

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>
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 15, 2026
Mohammad Ozair Zayan Khan and others added 2 commits September 15, 2026 00:18
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 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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lsp: tracking issue for code actions features

1 participant