Avoid PERF401 if conditional depends on list var (#5603)

## Summary

Avoid `PERF401` if conditional depends on list var

## Test Plan

`cargo test`

fixes: #5581
This commit is contained in:
Dhruv Manilawala 2023-07-10 01:23:27 +05:30 committed by GitHub
parent 9dd05424c4
commit 6a4b216362
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 40 additions and 5 deletions

View file

@ -37,3 +37,11 @@ def f():
result = {}
for i in items:
result[i].append(i) # OK
def f():
items = [1, 2, 3, 4]
result = []
for i in items:
if i not in result:
result.append(i) # OK

View file

@ -57,26 +57,28 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, target: &Expr, bo
return;
};
let (stmt, conditional) = match body {
let (stmt, if_test) = match body {
// ```python
// for x in y:
// if z:
// filtered.append(x)
// ```
[Stmt::If(ast::StmtIf { body, orelse, .. })] => {
[Stmt::If(ast::StmtIf {
body, orelse, test, ..
})] => {
if !orelse.is_empty() {
return;
}
let [stmt] = body.as_slice() else {
return;
};
(stmt, true)
(stmt, Some(test))
}
// ```python
// for x in y:
// filtered.append(f(x))
// ```
[stmt] => (stmt, false),
[stmt] => (stmt, None),
_ => return,
};
@ -103,7 +105,7 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, target: &Expr, bo
};
// Ignore direct list copies (e.g., `for x in y: filtered.append(x)`).
if !conditional {
if if_test.is_none() {
if arg.as_name_expr().map_or(false, |arg| arg.id == *id) {
return;
}
@ -124,6 +126,31 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, target: &Expr, bo
return;
}
// Avoid if the value is used in the conditional test, e.g.,
//
// ```python
// for x in y:
// if x in filtered:
// filtered.append(x)
// ```
//
// Converting this to a list comprehension would raise a `NameError` as
// `filtered` is not defined yet:
//
// ```python
// filtered = [x for x in y if x in filtered]
// ```
if let Some(value_name) = value.as_name_expr() {
if if_test.map_or(false, |test| {
any_over_expr(test, &|expr| {
expr.as_name_expr()
.map_or(false, |expr| expr.id == value_name.id)
})
}) {
return;
}
}
checker
.diagnostics
.push(Diagnostic::new(ManualListComprehension, *range));