Skip to content

[refurb] Parenthesize generator arguments in FURB142 fixer (#21098) - #24200

Merged
MichaReiser merged 1 commit into
astral-sh:mainfrom
bitloi:fix/issue-21098-furb142-generator-parens
Mar 26, 2026
Merged

[refurb] Parenthesize generator arguments in FURB142 fixer (#21098)#24200
MichaReiser merged 1 commit into
astral-sh:mainfrom
bitloi:fix/issue-21098-furb142-generator-parens

Conversation

@bitloi

@bitloi bitloi commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Parenthesize unparenthesized generator arguments in the FURB142 fixer when rewriting set.add(...) loops to set.update(...), so the rewrite preserves scoping and behavior. Added fixture/snapshot regressions for both unparenthesized and already-parenthesized generator arguments.

Test Plan

  • cargo test -p ruff_linter rule_forloopsetmutations_path_new_furb142_py_expects
  • cargo test -p ruff_linter refurb::tests::rules -- FURB142
  • cargo fmt --all --check
  • cargo clippy -p ruff_linter --all-targets --all-features -- -D warnings

Closes #21098

…sh#21098)

Preserve semantics when rewriting `set.add(...)` loops by parenthesizing unparenthesized generator arguments and add snapshot-backed regressions for both parenthesized and unparenthesized generator cases.
@astral-sh-bot
astral-sh-bot Bot requested a review from ntBre March 26, 2026 08:15
@astral-sh-bot

astral-sh-bot Bot commented Mar 26, 2026

Copy link
Copy Markdown

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser MichaReiser added the fixes Related to suggested fixes for violations label Mar 26, 2026

@MichaReiser MichaReiser left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

@MichaReiser
MichaReiser merged commit fa07cda into astral-sh:main Mar 26, 2026
42 checks passed
@bitloi

bitloi commented Mar 26, 2026

Copy link
Copy Markdown
Contributor Author

Appreciate your review and support.

nicopauss pushed a commit to Intersec/lib-common that referenced this pull request Apr 1, 2026
##### [\`v0.15.8\`](https://github.com/astral-sh/ruff/blob/HEAD/CHANGELOG.md#0158)

Released on 2026-03-26.

##### Preview features

- \[`ruff`] New rule `unnecessary-if` (`RUF050`) ([#24114](astral-sh/ruff#24114))
- \[`ruff`] New rule `useless-finally` (`RUF072`) ([#24165](astral-sh/ruff#24165))
- \[`ruff`] New rule `f-string-percent-format` (`RUF073`): warn when using `%` operator on an f-string ([#24162](astral-sh/ruff#24162))
- \[`pyflakes`] Recognize `frozendict` as a builtin for Python 3.15+ ([#24100](astral-sh/ruff#24100))

##### Bug fixes

- \[`flake8-async`] Use fully-qualified `anyio.lowlevel` import in autofix (`ASYNC115`) ([#24166](astral-sh/ruff#24166))
- \[`flake8-bandit`] Check tuple arguments for partial paths in `S607` ([#24080](astral-sh/ruff#24080))
- \[`pyflakes`] Skip `undefined-name` (`F821`) for conditionally deleted variables ([#24088](astral-sh/ruff#24088))
- `E501`/`W505`/formatter: Exclude nested pragma comments from line width calculation ([#24071](astral-sh/ruff#24071))
- Fix `%foo?` parsing in IPython assignment expressions ([#24152](astral-sh/ruff#24152))
- `analyze graph`: resolve string imports that reference attributes, not just modules ([#24058](astral-sh/ruff#24058))

##### Rule changes

- \[`eradicate`] ignore `ty: ignore` comments in `ERA001` ([#24192](astral-sh/ruff#24192))
- \[`flake8-bandit`] Treat `sys.executable` as trusted input in `S603` ([#24106](astral-sh/ruff#24106))
- \[`flake8-self`] Recognize `Self` annotation and `self` assignment in `SLF001` ([#24144](astral-sh/ruff#24144))
- \[`pyflakes`] `F507`: Fix false negative for non-tuple RHS in `%`-formatting ([#24142](astral-sh/ruff#24142))
- \[`refurb`] Parenthesize generator arguments in `FURB142` fixer ([#24200](astral-sh/ruff#24200))

##### Performance

- Speed up diagnostic rendering ([#24146](astral-sh/ruff#24146))

##### Server

- Warn when Markdown files are skipped due to preview being disabled ([#24150](astral-sh/ruff#24150))

##### Documentation

- Clarify `extend-ignore` and `extend-select` settings documentation ([#24064](astral-sh/ruff#24064))
- Mention AI policy in PR template ([#24198](astral-sh/ruff#24198))

##### Other changes

- Use trusted publishing for NPM packages ([#24171](astral-sh/ruff#24171))

##### Contributors

- [@bitloi](https://github.com/bitloi)
- [@Sim-hu](https://github.com/Sim-hu)
- [@mvanhorn](https://github.com/mvanhorn)
- [@chinar-amrutkar](https://github.com/chinar-amrutkar)
- [@markjm](https://github.com/markjm)
- [@RenzoMXD](https://github.com/RenzoMXD)
- [@vivekkhimani](https://github.com/vivekkhimani)
- [@seroperson](https://github.com/seroperson)
- [@moktamd](https://github.com/moktamd)
- [@charliermarsh](https://github.com/charliermarsh)
- [@ntBre](https://github.com/ntBre)
- [@zanieb](https://github.com/zanieb)
- [@dylwil3](https://github.com/dylwil3)
- [@MichaReiser](https://github.com/MichaReiser)

Renovate-Branch: renovate/2024.6-ruff-0.15.x
Change-Id: Ifd4216a963962ffb24a4df69802bc60fcc29628d
Priv-Id: 46d2f61be3a5e65a9fdd2fef998ba41ea3388f12
ntBre pushed a commit that referenced this pull request Jul 27, 2026
)

## Summary

The `FURB192` fix rebuilds the call by slicing the source text of the
`sorted()` argument node. That range doesn't include surrounding
parentheses, so a parenthesized `yield` loses them and the result is
invalid syntax:

```python
def f():
    x = sorted((yield))[0]
```

```
$ ruff check --isolated --select FURB192 --fix --unsafe-fixes t.py
error: Fix introduced a syntax error. Reverting all changes.
This indicates a bug in Ruff.
```

The replacement it tries to write is `x = min(yield)`. A `yield`
expression is only valid as a call argument when it's parenthesized, so
the fix gets discarded and the diagnostic is left unfixable.

All four forms are affected on `main` (a5cdc6d):

| source | fix produced today |
| --- | --- |
| `sorted((yield))[0]` | `min(yield)` |
| `sorted((yield x))[-1]` | `max(yield x)` |
| `sorted((yield), key=k)[0]` | `min(yield, key=k)` |
| `sorted((yield from g()))[0]` | `min(yield from g())` |

This slices `parenthesized_range()` for the argument and falls back to
the node range, which is what `quadratic-list-summation` and a few other
rules already do. #24200 fixed the same kind of thing in the `FURB142`
fixer.

One deliberate side effect worth flagging: parentheses are now kept for
*any* parenthesized argument, so `sorted((a := b))[0]` becomes `min((a
:= b))` rather than `min(a := b)`. Both are valid; keeping them seemed
more consistent than special-casing `yield`. Happy to narrow it to just
`Expr::Yield`/`Expr::YieldFrom` if you'd rather not change the other
cases.

## Test Plan

Added `FURB192_1.py`. It has to be a separate fixture: `FURB192.py`
shadows `sorted` with a module-level `def sorted()` at the bottom, and
because function bodies are resolved against the final module scope, the
rule doesn't fire inside *any* function in that file — so `yield` cases
can't go there.

- `cargo test -p ruff_linter` — 2807 passed, 0 failed
- `cargo clippy -p ruff_linter --all-targets --all-features -- -D
warnings` — clean
- `prek run --files <changed files>` — clean

I also ran `--select FURB192 --fix --unsafe-fixes` over all 1599 files
under `crates/ruff_linter/resources/test/fixtures`, once with 0.16.0 and
once with this branch, and diffed the output. The only file whose result
changes is the new fixture, and neither build produces unparseable
output anywhere else.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fixes Related to suggested fixes for violations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FURB142 fix should parenthesize generator expressions

3 participants