RUF010: Mark fix as unsafe when it deletes a comment#24270
RUF010: Mark fix as unsafe when it deletes a comment#24270MichaReiser merged 6 commits intoastral-sh:mainfrom
RUF010: Mark fix as unsafe when it deletes a comment#24270Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates RUF010’s autofix logic so the fix is marked unsafe when applying it would delete comments inside the original call expression (addressing #19745).
Changes:
- Compute fix applicability for RUF010 by detecting comments that would be removed when rewriting
str/repr/asciicalls to f-string conversion flags. - Use
parenthesized_rangeto treat comments inside extra parentheses around the argument as preserved (keeping the fix safe in those cases). - Add fixture coverage for “comment deleted ⇒ unsafe” and “comment preserved ⇒ safe” scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
crates/ruff_linter/src/rules/ruff/rules/explicit_f_string_type_conversion.rs |
Marks the fix unsafe when it would delete comments outside the preserved (parenthesized) argument range. |
crates/ruff_linter/resources/test/fixtures/ruff/RUF010.py |
Adds fixture cases intended to validate unsafe vs safe fix applicability around comments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0646d9b6ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
MichaReiser
left a comment
There was a problem hiding this comment.
Thanks for working on this.
Can you take a look at the failing CI jobs and change the test as I suggested in the inline comment.
|
Wraps the `def ascii(arg): pass` shadow and its test case into a `def ascii_shadowing():` nested function so it doesn't shadow the builtin `ascii` for the new test cases below.
|
Done! Moved |
RUF010: Mark fix as unsafe when it deletes a comment
Fixes #19745