Add --add-ignore for adding ruff:ignore comments#26346
Conversation
|
|
As part of this PR, I'm checking how well Then there are 923 new RUF100 diagnostics from a variety of rules that seem not to be suppressible. One example is RUF101: https://play.ruff.rs/45d0e92f-fe87-437e-af87-9a83b099ff5b. Another is PLR2044: https://play.ruff.rs/9bcf9da7-4a50-494b-b6d0-cda53188b876. Another is SIM109: https://play.ruff.rs/922b0cf5-b97e-4c9f-8bdc-e300283f35fe. We can also introduce syntax errors, mostly via UP031 from what I've seen so far: ❯ cat try.py
print("A long string \ # noqa: UP031
with a continuation on line %d" % 2)
❯ python try.py
File "try.py", line 1
print("A long string \ # noqa: UP031
^
SyntaxError: unterminated string literal (detected at line 1)There are only 10 of these, but we can also disrupt (our detection of?) script metadata and trigger INP001: # /// script # noqa: CPY001
# requires-python = ">=3.11"
# dependencies = [
# "uv==0.11.13",I'll plan to open a separate issue or issues for these, but for now I'll just try to make sure |
Summary -- This PR adds the `--add-ignore` flag as an alias to `--add-noqa`. In preview, this adds `ruff:ignore` comments with human-readable names instead of `noqa` comments with codes. I initially drafted this PR assuming that `--add-ignore` would be a separate flag that only functioned in preview mode, while `--add-noqa` would be available in both preview and stable, but after talking with Micha realized that we probably want to go ahead and push preview users toward `--add-ignore` entirely rather than giving them the choice. I think the one downside of the alias approach is that `--add-noqa` still exists in preview but will still add `ruff:ignore` comments. In cases where an existing `noqa` or `ruff:ignore` comment is present, I opted to preserve the existing comment and to append the new comment. So something like this: ```py import math # noqa: RUF100 ``` becomes: ```py import math # noqa: RUF100 # ruff:ignore[unused-import] ``` instead of also converting the existing `noqa` comment. I think we can defer to one of our planned lint rules for the conversion (and joining) instead. As also shown in this example, the flag adds human-readable names rather than rule codes and prefers the same placement as a `noqa` comment, at the end of the line. The one exception to this is an existing `ruff:ignore` comment on its own line. In that case, we will reuse and extend the existing comment instead of adding an additional trailing comment, which I think is a pretty nice balance. This PR also fixes #26287, which I discovered while working on this and turned out to be easy to fix while I was here. Test Plan -- Many new CLI tests exercising the various cases Codex and I could come up with. As the server test shows, this actually applies to the LSP as well, where the `Disable for this line` action now also inserts a `ruff:ignore` comment with a rule name instead of a `noqa` comment.
Summary
This PR adds the
--add-ignoreflag as an alias to--add-noqa. In preview, this addsruff:ignorecomments with human-readable names instead ofnoqacomments with codes.I initially drafted this PR assuming that
--add-ignorewould be a separate flag that onlyfunctioned in preview mode, while
--add-noqawould be available in both preview and stable, butafter talking with Micha realized that we probably want to go ahead and push preview users toward
--add-ignoreentirely rather than giving them the choice. I think the one downside of the aliasapproach is that
--add-noqastill exists in preview but will still addruff:ignorecomments.In cases where an existing
noqaorruff:ignorecomment is present, I opted to preserve theexisting comment and to append the new comment. So something like this:
becomes:
instead of also converting the existing
noqacomment. I think we can defer to one of our plannedlint rules for the conversion (and joining) instead.
As also shown in this example, the flag adds human-readable names rather than rule codes and prefers
the same placement as a
noqacomment, at the end of the line. The one exception to this is anexisting
ruff:ignorecomment on its own line. In that case, we will reuse and extend the existingcomment instead of adding an additional trailing comment, which I think is a pretty nice balance.
This PR also fixes #26287, which I discovered while working on this and turned out to be easy to fix
while I was here.
Test Plan
Many new CLI tests exercising the various cases Codex and I could come up with. As the server test
shows, this actually applies to the LSP as well, where the
Disable for this lineaction now alsoinserts a
ruff:ignorecomment with a rule name instead of anoqacomment.