Avoid PERF101 if there's an append in loop body (#8809)
## Summary Avoid `PERF101` if there's an append in loop body ## Test Plan Add new test cases for this pattern. fixes: #8746
This commit is contained in:
@@ -50,3 +50,16 @@ import itertools
|
||||
|
||||
for i in itertools.product(foo_int): # Ok
|
||||
pass
|
||||
|
||||
for i in list(foo_list): # Ok
|
||||
foo_list.append(i + 1)
|
||||
|
||||
for i in list(foo_list): # PERF101
|
||||
# Make sure we match the correct list
|
||||
other_list.append(i + 1)
|
||||
|
||||
for i in list(foo_tuple): # Ok
|
||||
foo_tuple.append(i + 1)
|
||||
|
||||
for i in list(foo_set): # Ok
|
||||
foo_set.append(i + 1)
|
||||
|
||||
@@ -1269,7 +1269,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
||||
perflint::rules::manual_dict_comprehension(checker, target, body);
|
||||
}
|
||||
if checker.enabled(Rule::UnnecessaryListCast) {
|
||||
perflint::rules::unnecessary_list_cast(checker, iter);
|
||||
perflint::rules::unnecessary_list_cast(checker, iter, body);
|
||||
}
|
||||
if !is_async {
|
||||
if checker.enabled(Rule::ReimplementedBuiltin) {
|
||||
|
||||
@@ -49,7 +49,7 @@ impl AlwaysFixableViolation for UnnecessaryListCast {
|
||||
}
|
||||
|
||||
/// PERF101
|
||||
pub(crate) fn unnecessary_list_cast(checker: &mut Checker, iter: &Expr) {
|
||||
pub(crate) fn unnecessary_list_cast(checker: &mut Checker, iter: &Expr, body: &[Stmt]) {
|
||||
let Expr::Call(ast::ExprCall {
|
||||
func,
|
||||
arguments:
|
||||
@@ -98,6 +98,18 @@ pub(crate) fn unnecessary_list_cast(checker: &mut Checker, iter: &Expr) {
|
||||
range: iterable_range,
|
||||
..
|
||||
}) => {
|
||||
// If the variable is being appended to, don't suggest removing the cast:
|
||||
//
|
||||
// ```python
|
||||
// items = ["foo", "bar"]
|
||||
// for item in list(items):
|
||||
// items.append("baz")
|
||||
// ```
|
||||
//
|
||||
// Here, removing the `list()` cast would change the behavior of the code.
|
||||
if body.iter().any(|stmt| match_append(stmt, id)) {
|
||||
return;
|
||||
}
|
||||
let scope = checker.semantic().current_scope();
|
||||
if let Some(binding_id) = scope.get(id) {
|
||||
let binding = checker.semantic().binding(binding_id);
|
||||
@@ -128,6 +140,28 @@ pub(crate) fn unnecessary_list_cast(checker: &mut Checker, iter: &Expr) {
|
||||
}
|
||||
}
|
||||
|
||||
/// Check if a statement is an `append` call to a given identifier.
|
||||
///
|
||||
/// For example, `foo.append(bar)` would return `true` if `id` is `foo`.
|
||||
fn match_append(stmt: &Stmt, id: &str) -> bool {
|
||||
let Some(ast::StmtExpr { value, .. }) = stmt.as_expr_stmt() else {
|
||||
return false;
|
||||
};
|
||||
let Some(ast::ExprCall { func, .. }) = value.as_call_expr() else {
|
||||
return false;
|
||||
};
|
||||
let Some(ast::ExprAttribute { value, attr, .. }) = func.as_attribute_expr() else {
|
||||
return false;
|
||||
};
|
||||
if attr != "append" {
|
||||
return false;
|
||||
}
|
||||
let Some(ast::ExprName { id: target_id, .. }) = value.as_name_expr() else {
|
||||
return false;
|
||||
};
|
||||
target_id == id
|
||||
}
|
||||
|
||||
/// Generate a [`Fix`] to remove a `list` cast from an expression.
|
||||
fn remove_cast(list_range: TextRange, iterable_range: TextRange) -> Fix {
|
||||
Fix::safe_edits(
|
||||
|
||||
@@ -180,4 +180,25 @@ PERF101.py:34:10: PERF101 [*] Do not cast an iterable to `list` before iterating
|
||||
38 36 |
|
||||
39 37 | for i in list(foo_dict): # Ok
|
||||
|
||||
PERF101.py:57:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it
|
||||
|
|
||||
55 | foo_list.append(i + 1)
|
||||
56 |
|
||||
57 | for i in list(foo_list): # PERF101
|
||||
| ^^^^^^^^^^^^^^ PERF101
|
||||
58 | # Make sure we match the correct list
|
||||
59 | other_list.append(i + 1)
|
||||
|
|
||||
= help: Remove `list()` cast
|
||||
|
||||
ℹ Safe fix
|
||||
54 54 | for i in list(foo_list): # Ok
|
||||
55 55 | foo_list.append(i + 1)
|
||||
56 56 |
|
||||
57 |-for i in list(foo_list): # PERF101
|
||||
57 |+for i in foo_list: # PERF101
|
||||
58 58 | # Make sure we match the correct list
|
||||
59 59 | other_list.append(i + 1)
|
||||
60 60 |
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user