[`perflint`] Fix panic in `perf401` (#14971)

Fixes #14969.

The issue was that this line:

```rust
let from_assign_to_loop = TextRange::new(binding_stmt.end(), for_stmt.start());
```

was not safe if the binding was after the target. The only way (at least
that I can think of) this can happen is if they are in different scopes,
so it now checks for that before checking if there are usages between
the two.
This commit is contained in:
w0nder1ng 2024-12-15 10:22:04 -05:00 committed by GitHub
parent 2d15d7d1af
commit 4a7536dc94
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 56 additions and 11 deletions

View File

@ -237,3 +237,10 @@ def f():
print(a)
for a in values:
result.append(a + 1) # PERF401
def f():
values = [1, 2, 3]
def g():
for a in values:
result.append(a + 1) # PERF401
result = []

View File

@ -305,15 +305,19 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::S
});
// If the binding gets used in between the assignment and the for loop, a list comprehension is no longer safe
let binding_unused_between = list_binding_stmt.is_some_and(|binding_stmt| {
let from_assign_to_loop = TextRange::new(binding_stmt.end(), for_stmt.start());
// Test if there's any reference to the list symbol between its definition and the for loop.
// if there's at least one, then it's been accessed in the middle somewhere, so it's not safe to change into a list comprehension
!list_binding
.references()
.map(|ref_id| checker.semantic().reference(ref_id).range())
.any(|text_range| from_assign_to_loop.contains_range(text_range))
});
// If the binding is after the for loop, then it can't be fixed, and this check would panic,
// so we check that they are in the same statement first
let binding_unused_between = assignment_in_same_statement
&& list_binding_stmt.is_some_and(|binding_stmt| {
let from_assign_to_loop = TextRange::new(binding_stmt.end(), for_stmt.start());
// Test if there's any reference to the list symbol between its definition and the for loop.
// if there's at least one, then it's been accessed in the middle somewhere, so it's not safe to change into a list comprehension
!list_binding
.references()
.map(|ref_id| checker.semantic().reference(ref_id).range())
.any(|text_range| from_assign_to_loop.contains_range(text_range))
});
// A list extend works in every context, while a list comprehension only works when all the criteria are true
let comprehension_type = if binding_is_empty_list

View File

@ -1,6 +1,5 @@
---
source: crates/ruff_linter/src/rules/perflint/mod.rs
snapshot_kind: text
---
PERF401.py:6:13: PERF401 Use a list comprehension to create a transformed list
|
@ -189,5 +188,17 @@ PERF401.py:239:9: PERF401 Use a list comprehension to create a transformed list
238 | for a in values:
239 | result.append(a + 1) # PERF401
| ^^^^^^^^^^^^^^^^^^^^ PERF401
240 |
241 | def f():
|
= help: Replace for loop with list comprehension
PERF401.py:245:13: PERF401 Use `list.extend` to create a transformed list
|
243 | def g():
244 | for a in values:
245 | result.append(a + 1) # PERF401
| ^^^^^^^^^^^^^^^^^^^^ PERF401
246 | result = []
|
= help: Replace for loop with list.extend

View File

@ -1,6 +1,5 @@
---
source: crates/ruff_linter/src/rules/perflint/mod.rs
snapshot_kind: text
---
PERF401.py:6:13: PERF401 [*] Use a list comprehension to create a transformed list
|
@ -448,6 +447,8 @@ PERF401.py:239:9: PERF401 [*] Use a list comprehension to create a transformed l
238 | for a in values:
239 | result.append(a + 1) # PERF401
| ^^^^^^^^^^^^^^^^^^^^ PERF401
240 |
241 | def f():
|
= help: Replace for loop with list comprehension
@ -461,3 +462,25 @@ PERF401.py:239:9: PERF401 [*] Use a list comprehension to create a transformed l
238 |- for a in values:
239 |- result.append(a + 1) # PERF401
237 |+ result = [a + 1 for a in values] # PERF401
240 238 |
241 239 | def f():
242 240 | values = [1, 2, 3]
PERF401.py:245:13: PERF401 [*] Use `list.extend` to create a transformed list
|
243 | def g():
244 | for a in values:
245 | result.append(a + 1) # PERF401
| ^^^^^^^^^^^^^^^^^^^^ PERF401
246 | result = []
|
= help: Replace for loop with list.extend
Unsafe fix
241 241 | def f():
242 242 | values = [1, 2, 3]
243 243 | def g():
244 |- for a in values:
245 |- result.append(a + 1) # PERF401
244 |+ result.extend(a + 1 for a in values) # PERF401
246 245 | result = []