[ruff] Treat f-string interpolation as potential side effect in RUF019#24426
Conversation
|
e27a024 to
c2b5a83
Compare
MichaReiser
left a comment
There was a problem hiding this comment.
I think this is somewhat pedantic but it's probably fine for RUF019. I left a few smaller nit comments on how we can improve the code.
|
@MichaReiser Thank you so much for the review! Addressed all feedback, refactored to SideEffect::from_expr with a match, renamed variants to Absent/Possible/Present, added Named as a side effect, and updated the fixture with a class that has a side-effectful str. I really appreciate the suggestions. Ready for re-review whenever you have a moment. |
MichaReiser
left a comment
There was a problem hiding this comment.
This is great. Thank you. A few more smaller nits.
|
@MichaReiser thank you so much for the review. Addressed all three feedback, removed Option from from_expr, made both matches exhaustive, added const fn. Ready for another look whenever you get a chance. thank you |
#24426) <!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? - Does this PR follow our AI policy (https://github.com/astral-sh/.github/blob/main/AI_POLICY.md)? --> ## Summary Fixes #12953 F-string interpolation can call `__format__`/`__str__`/`__repr__`, which may have side effects. RUF019 was applying a safe auto-fix that collapsed two `__str__` calls into one, changing behavior. Added a tri-state `SideEffect` enum (`No`/`Maybe`/`Yes`) and a `side_effect()` function that reuses the existing `any_over_expr` traversal via a new `FnMut` variant (`any_over_expr_mut`), following the approach suggested by @ntBre in the other closed pr tagged in the issue In RUF019, `SideEffect::Maybe` (non-literal f-string interpolation) now produces an unsafe fix instead of a safe one. Literal interpolations like `f"{1}"` remain safe. ## Test Plan - Added f-string fixture cases to `RUF019.py` (non-literal → unsafe, literal → safe, no interpolation → safe). - `cargo nextest run -p ruff_linter` - Ecosystem check (stable + preview)
##### [\`v0.15.10\`](https://github.com/astral-sh/ruff/blob/HEAD/CHANGELOG.md#01510) Released on 2026-04-09. ##### Preview features - \[`flake8-logging`] Allow closures in except handlers (`LOG004`) ([#24464](astral-sh/ruff#24464)) - \[`flake8-self`] Make `SLF` diagnostics robust to non-self-named variables ([#24281](astral-sh/ruff#24281)) - \[`flake8-simplify`] Make the fix for `collapsible-if` safe in `preview` (`SIM102`) ([#24371](astral-sh/ruff#24371)) ##### Bug fixes - Avoid emitting multi-line f-string elements before Python 3.12 ([#24377](astral-sh/ruff#24377)) - Avoid syntax error from `E502` fixes in f-strings and t-strings ([#24410](astral-sh/ruff#24410)) - Strip form feeds from indent passed to `dedent_to` ([#24381](astral-sh/ruff#24381)) - \[`pyupgrade`] Fix panic caused by handling of octals (`UP012`) ([#24390](astral-sh/ruff#24390)) - Reject multi-line f-string elements before Python 3.12 ([#24355](astral-sh/ruff#24355)) ##### Rule changes - \[`ruff`] Treat f-string interpolation as potential side effect (`RUF019`) ([#24426](astral-sh/ruff#24426)) ##### Server - Add support for custom file extensions ([#24463](astral-sh/ruff#24463)) ##### Documentation - Document adding fixes in CONTRIBUTING.md ([#24393](astral-sh/ruff#24393)) - Fix JSON typo in settings example ([#24517](astral-sh/ruff#24517)) ##### Contributors - [@charliermarsh](https://github.com/charliermarsh) - [@dylwil3](https://github.com/dylwil3) - [@silverstein](https://github.com/silverstein) - [@anishgirianish](https://github.com/anishgirianish) - [@shizukushq](https://github.com/shizukushq) - [@zanieb](https://github.com/zanieb) - [@AlexWaygood](https://github.com/AlexWaygood) ##### [\`v0.15.9\`](https://github.com/astral-sh/ruff/blob/HEAD/CHANGELOG.md#0159) Released on 2026-04-02. ##### Preview features - \[`pyflakes`] Flag annotated variable redeclarations as `F811` in preview mode ([#24244](astral-sh/ruff#24244)) - \[`ruff`] Allow dunder-named assignments in non-strict mode for `RUF067` ([#24089](astral-sh/ruff#24089)) ##### Bug fixes - \[`flake8-errmsg`] Avoid shadowing existing `msg` in fix for `EM101` ([#24363](astral-sh/ruff#24363)) - \[`flake8-simplify`] Ignore pre-initialization references in `SIM113` ([#24235](astral-sh/ruff#24235)) - \[`pycodestyle`] Fix `W391` fixes for consecutive empty notebook cells ([#24236](astral-sh/ruff#24236)) - \[`pyupgrade`] Fix `UP008` nested class matching ([#24273](astral-sh/ruff#24273)) - \[`pyupgrade`] Ignore strings with string-only escapes (`UP012`) ([#16058](astral-sh/ruff#16058)) - \[`ruff`] `RUF072`: skip formfeeds on dedent ([#24308](astral-sh/ruff#24308)) - \[`ruff`] Avoid re-using symbol in `RUF024` fix ([#24316](astral-sh/ruff#24316)) - \[`ruff`] Parenthesize expression in `RUF050` fix ([#24234](astral-sh/ruff#24234)) - Disallow starred expressions as values of starred expressions ([#24280](astral-sh/ruff#24280)) ##### Rule changes - \[`flake8-simplify`] Suppress `SIM105` for `except*` before Python 3.12 ([#23869](astral-sh/ruff#23869)) - \[`pyflakes`] Extend `F507` to flag `%`-format strings with zero placeholders ([#24215](astral-sh/ruff#24215)) - \[`pyupgrade`] `UP018` should detect more unnecessarily wrapped literals (UP018) ([#24093](astral-sh/ruff#24093)) - \[`pyupgrade`] Fix `UP008` callable scope handling to support lambdas ([#24274](astral-sh/ruff#24274)) - \[`ruff`] `RUF010`: Mark fix as unsafe when it deletes a comment ([#24270](astral-sh/ruff#24270)) ##### Formatter - Add `nested-string-quote-style` formatting option ([#24312](astral-sh/ruff#24312)) ##### Documentation - \[`flake8-bugbear`] Clarify RUF071 fix safety for non-path string comparisons ([#24149](astral-sh/ruff#24149)) - \[`flake8-type-checking`] Clarify import cycle wording for `TC001`/`TC002`/`TC003` ([#24322](astral-sh/ruff#24322)) ##### Other changes - Avoid rendering fix lines with trailing whitespace after `|` ([#24343](astral-sh/ruff#24343)) ##### Contributors - [@charliermarsh](https://github.com/charliermarsh) - [@MichaReiser](https://github.com/MichaReiser) - [@tranhoangtu-it](https://github.com/tranhoangtu-it) - [@dylwil3](https://github.com/dylwil3) - [@zsol](https://github.com/zsol) - [@renovate](https://github.com/renovate) - [@bitloi](https://github.com/bitloi) - [@danparizher](https://github.com/danparizher) - [@chinar-amrutkar](https://github.com/chinar-amrutkar) - [@second-ed](https://github.com/second-ed) - [@getehen](https://github.com/getehen) - [@Redovo1](https://github.com/Redovo1) - [@matthewlloyd](https://github.com/matthewlloyd) - [@zanieb](https://github.com/zanieb) - [@InSyncWithFoo](https://github.com/InSyncWithFoo) - [@RenzoMXD](https://github.com/RenzoMXD) Renovate-Branch: renovate/2024.6-ruff-0.15.x Change-Id: Id4bd542d4f128b509284d9dcda312f2b39c29964 Priv-Id: 28ebcacdeffa50cec7a52eee59091a75ca5e9539
Summary
Fixes #12953
F-string interpolation can call
__format__/__str__/__repr__, which may have side effects. RUF019 was applying a safe auto-fix that collapsed two__str__calls into one, changing behavior.Added a tri-state
SideEffectenum (No/Maybe/Yes) and aside_effect()function that reuses the existingany_over_exprtraversal via a newFnMutvariant (any_over_expr_mut), following the approach suggested by @ntBre in the other closed pr tagged in the issueIn RUF019,
SideEffect::Maybe(non-literal f-string interpolation) now produces an unsafe fix instead of a safe one. Literal interpolations likef"{1}"remain safe.Test Plan
RUF019.py(non-literal → unsafe, literal → safe, no interpolation → safe).cargo nextest run -p ruff_linter