Skip to content

Commit

Permalink
[perflint] Simplify finding the loop target in PERF401 (#15025)
Browse files Browse the repository at this point in the history
Fixes #15012.

```python
def f():
    # panics when the code can't find the loop variable
    values = [1, 2, 3]
    result = []
    for i in values:
        result.append(i + 1)
    del i
```

I'm not sure exactly why this test case panics, but I suspect the `del
i` removes the binding from the semantic model's symbols.

I changed the code to search for the correct binding by directly
iterating through the bindings. Since we know exactly which binding we
want, this should find the loop variable without any complications.
  • Loading branch information
w0nder1ng authored Dec 17, 2024
1 parent dcdc6e7 commit e22718f
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -244,3 +244,10 @@ def g():
for a in values:
result.append(a + 1) # PERF401
result = []

def f():
values = [1, 2, 3]
result = []
for i in values:
result.append(i + 1) # Ok
del i
Original file line number Diff line number Diff line change
Expand Up @@ -237,31 +237,12 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::S
// filtered = [x for x in y]
// print(x)
// ```
let last_target_binding = checker
let target_binding = checker
.semantic()
.lookup_symbol(for_stmt_target_id)
.expect("for loop target must exist");

let target_binding = {
let mut bindings = [last_target_binding].into_iter().chain(
checker
.semantic()
.shadowed_bindings(checker.semantic().scope_id, last_target_binding)
.filter_map(|shadowed| shadowed.same_scope().then_some(shadowed.shadowed_id())),
);

bindings
.find_map(|binding_id| {
let binding = checker.semantic().binding(binding_id);
binding
.statement(checker.semantic())
.and_then(ast::Stmt::as_for_stmt)
.is_some_and(|stmt| stmt.range == for_stmt.range)
.then_some(binding)
})
.expect("for target binding must exist")
};

.bindings
.iter()
.find(|binding| for_stmt.target.range() == binding.range)
.unwrap();
// If any references to the loop target variable are after the loop,
// then converting it into a comprehension would cause a NameError
if target_binding
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -484,3 +484,5 @@ PERF401.py:245:13: PERF401 [*] Use `list.extend` to create a transformed list
245 |- result.append(a + 1) # PERF401
244 |+ result.extend(a + 1 for a in values) # PERF401
246 245 | result = []
247 246 |
248 247 | def f():

0 comments on commit e22718f

Please sign in to comment.