Skip to content

Commit

Permalink
[perflint] Parenthesize walrus expressions in autofix for `manual-l…
Browse files Browse the repository at this point in the history
…ist-comprehension` (`PERF401`) (#15050)
  • Loading branch information
dylwil3 authored Dec 19, 2024
1 parent d8b9a36 commit 596d80c
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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)]

0 comments on commit 596d80c

Please sign in to comment.