Skip to content

Commit

Permalink
Ignore list-copy recommendations for async for loops
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed May 2, 2024
1 parent 1673bc4 commit 42d8bc5
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 17 deletions.
15 changes: 15 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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;
}
}
}

Expand Down Expand Up @@ -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,
));
}
10 changes: 7 additions & 3 deletions crates/ruff_linter/src/rules/perflint/rules/manual_list_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|

0 comments on commit 42d8bc5

Please sign in to comment.