Skip to content

ruff server: Support noqa comment code action - #11276

Merged
snowsignal merged 4 commits into
mainfrom
jane/server/noqa-code-action
May 12, 2024
Merged

ruff server: Support noqa comment code action#11276
snowsignal merged 4 commits into
mainfrom
jane/server/noqa-code-action

Conversation

@snowsignal

@snowsignal snowsignal commented May 4, 2024

Copy link
Copy Markdown
Contributor

Summary

Fixes #10594.

Code actions to disable a diagnostic via noqa comment are now available.

noqa.mp4

DiagnosticFix has been changed so that noqa code actions appear even for diagnostics with no available quick fix. It can contain quick fix edits, noqa comment edits, or both.

Test Plan

The scenarios that need to be tested are as follows:

  • A code action to disable a diagnostic should be available for every diagnostic.
  • Using this code action should append to the appropriate line with the diagnostic, or modify an existing noqa comment.
  • Adding a noqa comment manually should make a diagnostic disappear
  • Fix all auto-fixable problems should not add noqa comments
  • Removing a code from a noqa comment should make the diagnostic re-appear

@snowsignal snowsignal added the server Related to the Ruff or ty LSP server label May 4, 2024
@snowsignal snowsignal mentioned this pull request May 4, 2024
Merged
@github-actions

github-actions Bot commented May 4, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@T-256

T-256 commented May 4, 2024

Copy link
Copy Markdown
Contributor

image

Could it possible instead of clearing noqa by hand, add code action when moving cursor on noqa code comments?
For example, moving cursor on # noqa:, show these code actions:

  • Ruff (ANN201): Re-enable for this line.
  • Ruff (D103): Re-enable for this line.
  • Re-enable all disabled rules for this line.

MichaReiser
MichaReiser approved these changes May 6, 2024
View reviewed changes
Comment thread crates/ruff_server/src/lint.rs Outdated
Comment thread crates/ruff_server/src/lint.rs
Comment thread crates/ruff_server/src/server/api/requests/code_action.rs Outdated
Comment thread crates/ruff_server/src/server/api/requests/code_action.rs Outdated
Comment thread crates/ruff_server/src/server/api/requests/code_action.rs
@snowsignal
snowsignal force-pushed the jane/linter/noqa-edits branch from cfb4f18 to cc45f8c Compare May 7, 2024 07:33
@dhruvmanila

dhruvmanila commented May 7, 2024

Copy link
Copy Markdown
Member

Is this PR up-to date with the base PR (jane/linter/noqa-edits)? I think there's an issue with multiline strings:

Considering the above code snippet, in the following screen recording (left is new server, right is ruff-lsp) you can see that the new server adds the NoQA comment inside the multiline string while ruff-lsp correctly adds it at the end.

Screen.Recording.2024-05-07.at.14.14.29.mov

@snowsignal

snowsignal commented May 7, 2024

Copy link
Copy Markdown
Contributor Author

@dhruvmanila Thanks for pointing this out, I'll investigate.

@snowsignal
snowsignal force-pushed the jane/server/noqa-code-action branch from b7cd925 to 06ddf50 Compare May 9, 2024 19:03
@snowsignal
snowsignal force-pushed the jane/linter/noqa-edits branch from cc45f8c to eebad65 Compare May 9, 2024 19:03
@snowsignal

snowsignal commented May 9, 2024

Copy link
Copy Markdown
Contributor Author

@AlexWaygood @dhruvmanila You can ignore the review request, that seemed to happen automatically when I rebased the branch.

@snowsignal
snowsignal force-pushed the jane/server/noqa-code-action branch from 06ddf50 to d48d094 Compare May 10, 2024 15:49
Base automatically changed from jane/linter/noqa-edits to main May 10, 2024 23:16
@snowsignal

snowsignal commented May 11, 2024

Copy link
Copy Markdown
Contributor Author

@dhruvmanila I've confirmed that b321a3f fixes the issue with multiline diagnostics 👍

@snowsignal
snowsignal force-pushed the jane/server/noqa-code-action branch from b321a3f to d3e4234 Compare May 11, 2024 21:43
@snowsignal
snowsignal merged commit d7f093e into main May 12, 2024
@snowsignal
snowsignal deleted the jane/server/noqa-code-action branch May 12, 2024 21:39
@snowsignal

snowsignal commented May 12, 2024

Copy link
Copy Markdown
Contributor Author

@T-256 Thanks for this suggestion - can you open an issue for this?

@T-256 T-256 mentioned this pull request May 12, 2024
Open
@BrewTestBot BrewTestBot mentioned this pull request May 23, 2024
Merged
@MichaReiser MichaReiser mentioned this pull request Aug 5, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees

No one assigned

Labels

server Related to the Ruff or ty LSP server

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Provide code action to ignore the diagnostic (via noqa)

4 participants

@snowsignal @T-256 @dhruvmanila @MichaReiser