ruff icon indicating copy to clipboard operation
ruff copied to clipboard

`ruff server`: Support `noqa` comment code action

Open snowsignal opened this issue 9 months ago • 5 comments

Summary

Fixes https://github.com/astral-sh/ruff/issues/10594.

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

https://github.com/astral-sh/ruff/assets/19577865/6d3bcf11-a9d9-499b-8c7f-a10cd39cfbba

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 avatar May 04 '24 01:05 snowsignal

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

github-actions[bot] avatar May 04 '24 01:05 github-actions[bot]

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.

T-256 avatar May 04 '24 11:05 T-256

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.

https://github.com/astral-sh/ruff/assets/67177269/c75bb4dd-73d2-4c92-90fe-c3ec8268e138

dhruvmanila avatar May 07 '24 08:05 dhruvmanila

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

snowsignal avatar May 07 '24 19:05 snowsignal

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

snowsignal avatar May 09 '24 19:05 snowsignal

@dhruvmanila I've confirmed that https://github.com/astral-sh/ruff/pull/11276/commits/b321a3fdbb3ef0c57d8029a80f461c282b10e1b0 fixes the issue with multiline diagnostics 👍

snowsignal avatar May 11 '24 21:05 snowsignal

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

snowsignal avatar May 12 '24 21:05 snowsignal