From e0304ef73abf76400347aa21dabd50d125791cd5 Mon Sep 17 00:00:00 2001 From: bitloi Date: Thu, 26 Mar 2026 09:10:09 +0100 Subject: [PATCH] [`refurb`] Parenthesize generator arguments in FURB142 fixer (#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. --- .../resources/test/fixtures/refurb/FURB142.py | 8 ++++ .../refurb/rules/for_loop_set_mutations.rs | 22 ++++++---- ...es__refurb__tests__FURB142_FURB142.py.snap | 42 +++++++++++++++++++ 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB142.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB142.py index cd39b2f9831c0..45acc2345239c 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB142.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB142.py @@ -99,3 +99,11 @@ def g(): for x in (1,) if True else (2,): s.add(x) + +# https://github.com/astral-sh/ruff/issues/21098 +for x in ("abc", "def"): + s.add(c for c in x) + +# don't add extra parens for already parenthesized generators +for x in ("abc", "def"): + s.add((c for c in x)) diff --git a/crates/ruff_linter/src/rules/refurb/rules/for_loop_set_mutations.rs b/crates/ruff_linter/src/rules/refurb/rules/for_loop_set_mutations.rs index 202f5095578e7..e55aca2d5e248 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/for_loop_set_mutations.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/for_loop_set_mutations.rs @@ -111,13 +111,21 @@ pub(crate) fn for_loop_set_mutations(checker: &Checker, for_stmt: &StmtFor) { parenthesize_loop_iter_if_necessary(for_stmt, checker, IterLocation::Call), ) } - (for_target, arg) => format!( - "{}.{batch_method_name}({} for {} in {})", - set.id, - locator.slice(arg), - locator.slice(for_target), - parenthesize_loop_iter_if_necessary(for_stmt, checker, IterLocation::Comprehension), - ), + (for_target, arg) => { + let arg_content = match arg { + Expr::Generator(generator) if !generator.parenthesized => { + format!("({})", locator.slice(arg)) + } + _ => locator.slice(arg).to_string(), + }; + format!( + "{}.{batch_method_name}({} for {} in {})", + set.id, + arg_content, + locator.slice(for_target), + parenthesize_loop_iter_if_necessary(for_stmt, checker, IterLocation::Comprehension), + ) + } }; let applicability = if checker.comment_ranges().intersects(for_stmt.range) { diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB142_FURB142.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB142_FURB142.py.snap index 4133f63c729a3..820a3a3d1a8e8 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB142_FURB142.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB142_FURB142.py.snap @@ -386,6 +386,8 @@ FURB142 [*] Use of `set.add()` in a for loop 100 | / for x in (1,) if True else (2,): 101 | | s.add(x) | |____________^ +102 | +103 | # https://github.com/astral-sh/ruff/issues/21098 | help: Replace with `.update()` 97 | for x in lambda: 0: @@ -394,3 +396,43 @@ help: Replace with `.update()` - for x in (1,) if True else (2,): - s.add(x) 100 + s.update((1,) if True else (2,)) +101 | +102 | # https://github.com/astral-sh/ruff/issues/21098 +103 | for x in ("abc", "def"): + +FURB142 [*] Use of `set.add()` in a for loop + --> FURB142.py:104:1 + | +103 | # https://github.com/astral-sh/ruff/issues/21098 +104 | / for x in ("abc", "def"): +105 | | s.add(c for c in x) + | |_______________________^ +106 | +107 | # don't add extra parens for already parenthesized generators + | +help: Replace with `.update()` +101 | s.add(x) +102 | +103 | # https://github.com/astral-sh/ruff/issues/21098 + - for x in ("abc", "def"): + - s.add(c for c in x) +104 + s.update((c for c in x) for x in ("abc", "def")) +105 | +106 | # don't add extra parens for already parenthesized generators +107 | for x in ("abc", "def"): + +FURB142 [*] Use of `set.add()` in a for loop + --> FURB142.py:108:1 + | +107 | # don't add extra parens for already parenthesized generators +108 | / for x in ("abc", "def"): +109 | | s.add((c for c in x)) + | |_________________________^ + | +help: Replace with `.update()` +105 | s.add(c for c in x) +106 | +107 | # don't add extra parens for already parenthesized generators + - for x in ("abc", "def"): + - s.add((c for c in x)) +108 + s.update((c for c in x) for x in ("abc", "def"))