diff --git a/crates/ruff/resources/test/fixtures/perflint/PERF401.py b/crates/ruff/resources/test/fixtures/perflint/PERF401.py index beb3d4546c..12b084e2f6 100644 --- a/crates/ruff/resources/test/fixtures/perflint/PERF401.py +++ b/crates/ruff/resources/test/fixtures/perflint/PERF401.py @@ -30,3 +30,10 @@ def f(): result = [] for i in items: result.append(i) # OK + + +def f(): + items = [1, 2, 3, 4] + result = {} + for i in items: + result[i].append(i) # OK diff --git a/crates/ruff/resources/test/fixtures/perflint/PERF402.py b/crates/ruff/resources/test/fixtures/perflint/PERF402.py index 4db9a3dc52..55f3e08cbc 100644 --- a/crates/ruff/resources/test/fixtures/perflint/PERF402.py +++ b/crates/ruff/resources/test/fixtures/perflint/PERF402.py @@ -17,3 +17,10 @@ def f(): result = [] for i in items: result.append(i * i) # OK + + +def f(): + items = [1, 2, 3, 4] + result = {} + for i in items: + result[i].append(i * i) # OK diff --git a/crates/ruff/src/rules/perflint/rules/manual_list_comprehension.rs b/crates/ruff/src/rules/perflint/rules/manual_list_comprehension.rs index eb6e56735b..7e9607d591 100644 --- a/crates/ruff/src/rules/perflint/rules/manual_list_comprehension.rs +++ b/crates/ruff/src/rules/perflint/rules/manual_list_comprehension.rs @@ -2,6 +2,7 @@ use rustpython_parser::ast::{self, Expr, Stmt}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::any_over_expr; use crate::checkers::ast::Checker; @@ -52,6 +53,10 @@ impl Violation for ManualListComprehension { /// PERF401 pub(crate) fn manual_list_comprehension(checker: &mut Checker, target: &Expr, body: &[Stmt]) { + let Expr::Name(ast::ExprName { id, .. }) = target else { + return; + }; + let (stmt, conditional) = match body { // ```python // for x in y: @@ -99,22 +104,27 @@ 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 arg.as_name_expr().map_or(false, |arg| { - target - .as_name_expr() - .map_or(false, |target| arg.id == target.id) - }) { + if arg.as_name_expr().map_or(false, |arg| arg.id == *id) { return; } } - let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func.as_ref() else { + let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func.as_ref() else { return; }; - if attr.as_str() == "append" { - checker - .diagnostics - .push(Diagnostic::new(ManualListComprehension, *range)); + if attr.as_str() != "append" { + return; } + + // Avoid, e.g., `for x in y: filtered[x].append(x * x)`. + if any_over_expr(value, &|expr| { + expr.as_name_expr().map_or(false, |expr| expr.id == *id) + }) { + return; + } + + checker + .diagnostics + .push(Diagnostic::new(ManualListComprehension, *range)); } diff --git a/crates/ruff/src/rules/perflint/rules/manual_list_copy.rs b/crates/ruff/src/rules/perflint/rules/manual_list_copy.rs index 13554488bd..3752c4eb1c 100644 --- a/crates/ruff/src/rules/perflint/rules/manual_list_copy.rs +++ b/crates/ruff/src/rules/perflint/rules/manual_list_copy.rs @@ -2,6 +2,7 @@ use rustpython_parser::ast::{self, Expr, Stmt}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::any_over_expr; use crate::checkers::ast::Checker; @@ -45,6 +46,10 @@ impl Violation for ManualListCopy { /// PERF402 pub(crate) fn manual_list_copy(checker: &mut Checker, target: &Expr, body: &[Stmt]) { + let Expr::Name(ast::ExprName { id, .. }) = target else { + return; + }; + let [stmt] = body else { return; }; @@ -72,21 +77,26 @@ pub(crate) fn manual_list_copy(checker: &mut Checker, target: &Expr, body: &[Stm }; // Only flag direct list copies (e.g., `for x in y: filtered.append(x)`). - if !arg.as_name_expr().map_or(false, |arg| { - target - .as_name_expr() - .map_or(false, |target| arg.id == target.id) + if !arg.as_name_expr().map_or(false, |arg| arg.id == *id) { + return; + } + + let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func.as_ref() else { + return; + }; + + if !matches!(attr.as_str(), "append" | "insert") { + return; + } + + // Avoid, e.g., `for x in y: filtered[x].append(x * x)`. + if any_over_expr(value, &|expr| { + expr.as_name_expr().map_or(false, |expr| expr.id == *id) }) { return; } - let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func.as_ref() else { - return; - }; - - if matches!(attr.as_str(), "append" | "insert") { - checker - .diagnostics - .push(Diagnostic::new(ManualListCopy, *range)); - } + checker + .diagnostics + .push(Diagnostic::new(ManualListCopy, *range)); }