Skip to content

Commit 596d80c

Browse files
authored
[perflint] Parenthesize walrus expressions in autofix for manual-list-comprehension (PERF401) (#15050)
1 parent d8b9a36 commit 596d80c

File tree

4 files changed

+58
-1
lines changed

4 files changed

+58
-1
lines changed

crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,3 +251,12 @@ def f():
251251
for i in values:
252252
result.append(i + 1) # Ok
253253
del i
254+
255+
# The fix here must parenthesize the walrus operator
256+
# https://github.com/astral-sh/ruff/issues/15047
257+
def f():
258+
items = []
259+
260+
for i in range(5):
261+
if j := i:
262+
items.append(j)

crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ use ruff_text_size::{Ranged, TextRange};
4646
/// original = list(range(10000))
4747
/// filtered.extend(x for x in original if x % 2)
4848
/// ```
49+
///
50+
/// Take care that if the original for-loop uses an assignment expression
51+
/// as a conditional, such as `if match:=re.match("\d+","123")`, then
52+
/// the corresponding comprehension must wrap the assignment
53+
/// expression in parentheses to avoid a syntax error.
4954
#[derive(ViolationMetadata)]
5055
pub(crate) struct ManualListComprehension {
5156
is_async: bool,
@@ -347,7 +352,21 @@ fn convert_to_list_extend(
347352
let semantic = checker.semantic();
348353
let locator = checker.locator();
349354
let if_str = match if_test {
350-
Some(test) => format!(" if {}", locator.slice(test.range())),
355+
Some(test) => {
356+
// If the test is an assignment expression,
357+
// we must parenthesize it when it appears
358+
// inside the comprehension to avoid a syntax error.
359+
//
360+
// Notice that we do not need `any_over_expr` here,
361+
// since if the assignment expression appears
362+
// internally (e.g. as an operand in a boolean
363+
// operation) then it will already be parenthesized.
364+
if test.is_named_expr() {
365+
format!(" if ({})", locator.slice(test.range()))
366+
} else {
367+
format!(" if {}", locator.slice(test.range()))
368+
}
369+
}
351370
None => String::new(),
352371
};
353372

crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,3 +202,12 @@ PERF401.py:245:13: PERF401 Use `list.extend` to create a transformed list
202202
246 | result = []
203203
|
204204
= help: Replace for loop with list.extend
205+
206+
PERF401.py:262:13: PERF401 Use a list comprehension to create a transformed list
207+
|
208+
260 | for i in range(5):
209+
261 | if j := i:
210+
262 | items.append(j)
211+
| ^^^^^^^^^^^^^^^ PERF401
212+
|
213+
= help: Replace for loop with list comprehension

crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,3 +486,23 @@ PERF401.py:245:13: PERF401 [*] Use `list.extend` to create a transformed list
486486
246 245 | result = []
487487
247 246 |
488488
248 247 | def f():
489+
490+
PERF401.py:262:13: PERF401 [*] Use a list comprehension to create a transformed list
491+
|
492+
260 | for i in range(5):
493+
261 | if j := i:
494+
262 | items.append(j)
495+
| ^^^^^^^^^^^^^^^ PERF401
496+
|
497+
= help: Replace for loop with list comprehension
498+
499+
Unsafe fix
500+
255 255 | # The fix here must parenthesize the walrus operator
501+
256 256 | # https://github.com/astral-sh/ruff/issues/15047
502+
257 257 | def f():
503+
258 |- items = []
504+
259 258 |
505+
260 |- for i in range(5):
506+
261 |- if j := i:
507+
262 |- items.append(j)
508+
259 |+ items = [j for i in range(5) if (j := i)]

0 commit comments

Comments
 (0)