Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ruff server: Support noqa comment code action #11276

Merged
merged 4 commits into from
May 12, 2024

Conversation

snowsignal
Copy link
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

Copy link
Contributor

github-actions bot commented May 4, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@T-256
Copy link
Contributor

T-256 commented May 4, 2024

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.

crates/ruff_server/src/lint.rs Outdated Show resolved Hide resolved
crates/ruff_server/src/lint.rs Show resolved Hide resolved
crates/ruff_server/src/server/api/requests/code_action.rs Outdated Show resolved Hide resolved
crates/ruff_server/src/server/api/requests/code_action.rs Outdated Show resolved Hide resolved
crates/ruff_server/src/server/api/requests/code_action.rs Outdated Show resolved Hide resolved
@dhruvmanila
Copy link
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
Copy link
Contributor Author

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

@snowsignal
Copy link
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
Copy link
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
19 checks passed
@snowsignal snowsignal deleted the jane/server/noqa-code-action branch May 12, 2024 21:39
@snowsignal
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server Related to the LSP server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide code action to ignore the diagnostic (via noqa)
4 participants