Preserve trailing content in --add-noqa and --add-ignore#26409
Draft
ntBre wants to merge 2 commits into
Draft
Conversation
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. 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.
|
ntBre
added a commit
that referenced
this pull request
Jul 7, 2026
Summary -- This PR adds the new `--add-ignore` flag. In preview, this adds `ruff:ignore` comments with human-readable names instead of `noqa` comments with codes. Without preview enabled, using the flag emits an error. `--add-noqa` itself is still available in both preview and stable mode. 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. Test Plan -- Many new 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. Ecosystem check -- I also ran some tests against ecosystem projects as described in my [comment](#26346 (comment)) below. These revealed some additional discrepancies between `ruff:ignore` and `noqa` (and some bugs in `noqa`/`--add-noqa` itself), but I think it makes sense to leave these as follow-ups. I've already opened (rough) drafts of them in: - #26373 - #26409 - #26412 We could land the shebang fixes separately, but the other two are stacked on this PR. With all three of these applied, the ecosystem check showed parity with `--add-noqa`. This also seems like an interesting check to add to our ecosystem analysis in general.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #26287, currently stacked on #26346
Test Plan
New CLI tests