diff --git a/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py b/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py index ad92de80ad094..24ff17e6c7e41 100644 --- a/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py +++ b/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py @@ -251,3 +251,12 @@ def f(): for i in values: result.append(i + 1) # Ok del i + +# The fix here must parenthesize the walrus operator +# https://github.com/astral-sh/ruff/issues/15047 +def f(): + items = [] + + for i in range(5): + if j := i: + items.append(j) diff --git a/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs b/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs index e7f57944fa7c9..e38e8efae64ae 100644 --- a/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs +++ b/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs @@ -46,6 +46,11 @@ use ruff_text_size::{Ranged, TextRange}; /// original = list(range(10000)) /// filtered.extend(x for x in original if x % 2) /// ``` +/// +/// Take care that if the original for-loop uses an assignment expression +/// as a conditional, such as `if match:=re.match("\d+","123")`, then +/// the corresponding comprehension must wrap the assignment +/// expression in parentheses to avoid a syntax error. #[derive(ViolationMetadata)] pub(crate) struct ManualListComprehension { is_async: bool, @@ -347,7 +352,21 @@ fn convert_to_list_extend( let semantic = checker.semantic(); let locator = checker.locator(); let if_str = match if_test { - Some(test) => format!(" if {}", locator.slice(test.range())), + Some(test) => { + // If the test is an assignment expression, + // we must parenthesize it when it appears + // inside the comprehension to avoid a syntax error. + // + // Notice that we do not need `any_over_expr` here, + // since if the assignment expression appears + // internally (e.g. as an operand in a boolean + // operation) then it will already be parenthesized. + if test.is_named_expr() { + format!(" if ({})", locator.slice(test.range())) + } else { + format!(" if {}", locator.slice(test.range())) + } + } None => String::new(), }; diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap index 2257cbe8b714a..8a660dd70938f 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap +++ b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap @@ -202,3 +202,12 @@ PERF401.py:245:13: PERF401 Use `list.extend` to create a transformed list 246 | result = [] | = help: Replace for loop with list.extend + +PERF401.py:262:13: PERF401 Use a list comprehension to create a transformed list + | +260 | for i in range(5): +261 | if j := i: +262 | items.append(j) + | ^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap index 83b2cf55f0468..dda14552ee4a4 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap +++ b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap @@ -486,3 +486,23 @@ PERF401.py:245:13: PERF401 [*] Use `list.extend` to create a transformed list 246 245 | result = [] 247 246 | 248 247 | def f(): + +PERF401.py:262:13: PERF401 [*] Use a list comprehension to create a transformed list + | +260 | for i in range(5): +261 | if j := i: +262 | items.append(j) + | ^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +255 255 | # The fix here must parenthesize the walrus operator +256 256 | # https://github.com/astral-sh/ruff/issues/15047 +257 257 | def f(): +258 |- items = [] +259 258 | +260 |- for i in range(5): +261 |- if j := i: +262 |- items.append(j) + 259 |+ items = [j for i in range(5) if (j := i)]