Add "Disable for this line" quickfix #76
Conversation
bluetech
force-pushed
the
disable-rule-for-line-quickfix
branch
from
167d486
to
b3cf81d
Compare
In ruff-lsp (astral-sh/ruff-lsp#76) we want to add a "Disable \<rule\> for this line" quickfix. However, finding the correct line into which the `noqa` comment should be inserted is non-trivial (multi-line strings for example). Ruff already has this info, so expose it in the JSON output for use by ruff-lsp.
bluetech
force-pushed
the
disable-rule-for-line-quickfix
branch
from
b3cf81d
to
dad628a
Compare
bluetech
force-pushed
the
disable-rule-for-line-quickfix
branch
from
dad628a
to
dc603a0
Compare
| if not params.context.only or CodeActionKind.QuickFix in params.context.only: | ||
| for diagnostic in params.context.diagnostics: | ||
| if diagnostic.source == "Ruff": | ||
| noqa_row = cast(DiagnosticData, diagnostic.data)["noqa_row"] |
There was a problem hiding this comment.
I think this needs to be more defensive, since this key could be absent for older versions of Ruff (as opposed to being None).
There was a problem hiding this comment.
The DiagnosticData is created by the LSP, when I create it I use noqa_row=check.get("noqa_row") so I think it's OK?
ruff_lsp/server.py
Outdated
| match = NOQA_REGEX.search(line) | ||
| # `foo # noqa: OLD` -> `foo # noqa: OLD,NEW` | ||
| if match and match.group("codes") is not None: | ||
| codes = match.group("codes") + f",{diagnostic.code}" |
There was a problem hiding this comment.
Nit: lets add a space after the comma, to be consistent with Ruff's own RUF100 behavior.
There was a problem hiding this comment.
Added a space after the comma
| codes = match.group("codes") + f",{diagnostic.code}" | ||
| start, end = match.start("codes"), match.end("codes") | ||
| new_line = line[:start] + codes + line[end:] | ||
| # `foo # noqa` -> `foo # noqa: NEW` |
There was a problem hiding this comment.
I think this case might be impossible... since if a line has foo # noqa, the error will by definition be suppressed, right? And so this action wouldn't appear?
There was a problem hiding this comment.
I managed it using the "useless noqa" lint :)
|
Thx! |
|
@bluetech - Out of curiosity, what editor do you use with the LSP? |
|
I use vscode |
|
BTW, since this was merged I was thinking that it might have been better if the entire autofix was generated in ruff, and not in the LSP, since ruff has a much better idea how to do it best. Of course it can't be a regular rule/autofix, but maybe the LSP could ask ruff to generate them using some undocumented side-channel flag. OTOH, if the LSP can do it maybe it's better... |
|
Yeah interesting. It's close to Ruff's |
Fix #50.
This likely has some unhandled edge cases, but hopefully better than nothing :)