Skip to content

[refurb] Parenthesize yield arguments in the FURB192 fixer - #27192

Merged
ntBre merged 1 commit into
astral-sh:mainfrom
vidigoat:furb192-preserve-parens
Jul 27, 2026
Merged

ntBre merged 1 commit into
astral-sh:mainfrom
vidigoat:furb192-preserve-parens

Conversation

@vidigoat

Copy link
Copy Markdown
Contributor

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:

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.

`sorted((yield))[0]` was rewritten to `min(yield)`, which is invalid
syntax, so the fix was rejected with "Fix introduced a syntax error".

Slice the parenthesized range of the argument instead of the bare node,
the same way `quadratic-list-summation` does.
@astral-sh-bot
astral-sh-bot Bot requested a review from ntBre July 26, 2026 07:23

@ntBre ntBre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this looks good to me.

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.

I think this does make sense and matches how we use parenthesized_range elsewhere.

@ntBre ntBre added bug An issue describing something that isn't working, or a PR that fixes a bug fixes Related to suggested fixes for violations labels Jul 27, 2026
@astral-sh-bot

astral-sh-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre
ntBre merged commit f4e9979 into astral-sh:main Jul 27, 2026
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue describing something that isn't working, or a PR that fixes a bug fixes Related to suggested fixes for violations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants