From 42d8bc5388a452eb3ee0bb3e846b24447c192cb3 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 2 May 2024 11:13:23 -0700 Subject: [PATCH] Ignore list-copy recommendations for async for loops --- .../test/fixtures/perflint/PERF401.py | 15 ++++++++ .../test/fixtures/perflint/PERF402.py | 7 ++++ .../src/checkers/ast/analyze/statement.rs | 4 +-- .../rules/manual_list_comprehension.rs | 36 ++++++++++++------- .../rules/perflint/rules/manual_list_copy.rs | 10 ++++-- ...__perflint__tests__PERF401_PERF401.py.snap | 14 ++++++++ 6 files changed, 69 insertions(+), 17 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py b/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py index 86cd660d394cb..f8b8014745017 100644 --- a/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py +++ b/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py @@ -72,3 +72,18 @@ def f(): result = Foo() for i in items: result.append(i) # Ok + + +def f(): + items = [1, 2, 3, 4] + result = [] + async for i in items: + if i % 2: + result.append(i) # PERF401 + + +def f(): + items = [1, 2, 3, 4] + result = [] + async for i in items: + result.append(i) # PERF401 diff --git a/crates/ruff_linter/resources/test/fixtures/perflint/PERF402.py b/crates/ruff_linter/resources/test/fixtures/perflint/PERF402.py index b45a8c80c9edc..bdb16de3304df 100644 --- a/crates/ruff_linter/resources/test/fixtures/perflint/PERF402.py +++ b/crates/ruff_linter/resources/test/fixtures/perflint/PERF402.py @@ -43,3 +43,10 @@ def f(): for path in ("foo", "bar"): sys.path.append(path) # OK + + +def f(): + items = [1, 2, 3, 4] + result = [] + async for i in items: + result.append(i) # PERF402 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 0190de443528c..7b0a9bf2d627d 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1323,10 +1323,10 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { pylint::rules::dict_iter_missing_items(checker, target, iter); } if checker.enabled(Rule::ManualListComprehension) { - perflint::rules::manual_list_comprehension(checker, target, body); + perflint::rules::manual_list_comprehension(checker, for_stmt); } if checker.enabled(Rule::ManualListCopy) { - perflint::rules::manual_list_copy(checker, target, body); + perflint::rules::manual_list_copy(checker, for_stmt); } if checker.enabled(Rule::ManualDictComprehension) { perflint::rules::manual_dict_comprehension(checker, target, body); diff --git a/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs b/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs index 5377003849640..cc3ced50830db 100644 --- a/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs +++ b/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs @@ -44,22 +44,28 @@ use crate::checkers::ast::Checker; /// filtered.extend(x for x in original if x % 2) /// ``` #[violation] -pub struct ManualListComprehension; +pub struct ManualListComprehension { + is_async: bool, +} impl Violation for ManualListComprehension { #[derive_message_formats] fn message(&self) -> String { - format!("Use a list comprehension to create a transformed list") + let ManualListComprehension { is_async } = self; + match is_async { + false => format!("Use a list comprehension to create a transformed list"), + true => format!("Use an async list comprehension to create a transformed list"), + } } } /// PERF401 -pub(crate) fn manual_list_comprehension(checker: &mut Checker, target: &Expr, body: &[Stmt]) { - let Expr::Name(ast::ExprName { id, .. }) = target else { +pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::StmtFor) { + let Expr::Name(ast::ExprName { id, .. }) = &*for_stmt.target else { return; }; - let (stmt, if_test) = match body { + let (stmt, if_test) = match &*for_stmt.body { // ```python // for x in y: // if z: @@ -121,10 +127,13 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, target: &Expr, bo return; } - // Ignore direct list copies (e.g., `for x in y: filtered.append(x)`). - if if_test.is_none() { - if arg.as_name_expr().is_some_and(|arg| arg.id == *id) { - return; + // Ignore direct list copies (e.g., `for x in y: filtered.append(x)`), unless it's async, which + // `manual-list-copy` doesn't cover. + if !for_stmt.is_async { + if if_test.is_none() { + if arg.as_name_expr().is_some_and(|arg| arg.id == *id) { + return; + } } } @@ -179,7 +188,10 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, target: &Expr, bo return; } - checker - .diagnostics - .push(Diagnostic::new(ManualListComprehension, *range)); + checker.diagnostics.push(Diagnostic::new( + ManualListComprehension { + is_async: for_stmt.is_async, + }, + *range, + )); } diff --git a/crates/ruff_linter/src/rules/perflint/rules/manual_list_copy.rs b/crates/ruff_linter/src/rules/perflint/rules/manual_list_copy.rs index f3d1c25a07cfb..5b5843a9a9cc0 100644 --- a/crates/ruff_linter/src/rules/perflint/rules/manual_list_copy.rs +++ b/crates/ruff_linter/src/rules/perflint/rules/manual_list_copy.rs @@ -45,12 +45,16 @@ 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 { +pub(crate) fn manual_list_copy(checker: &mut Checker, for_stmt: &ast::StmtFor) { + if for_stmt.is_async { + return; + } + + let Expr::Name(ast::ExprName { id, .. }) = &*for_stmt.target else { return; }; - let [stmt] = body else { + let [stmt] = &*for_stmt.body else { return; }; diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap index 2010bb74a6b4e..ae0dcb8e5a10f 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap +++ b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap @@ -17,4 +17,18 @@ PERF401.py:13:9: PERF401 Use a list comprehension to create a transformed list | ^^^^^^^^^^^^^^^^^^^^ PERF401 | +PERF401.py:82:13: PERF401 Use an async list comprehension to create a transformed list + | +80 | async for i in items: +81 | if i % 2: +82 | result.append(i) # PERF401 + | ^^^^^^^^^^^^^^^^ PERF401 + | +PERF401.py:89:9: PERF401 Use an async list comprehension to create a transformed list + | +87 | result = [] +88 | async for i in items: +89 | result.append(i) # PERF401 + | ^^^^^^^^^^^^^^^^ PERF401 + |