diff --git a/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py b/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py index bead4d5ce0afaf..553742fe26c0cf 100644 --- a/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py +++ b/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py @@ -74,7 +74,7 @@ def f(): result.append(i) # Ok -def f(): +async def f(): items = [1, 2, 3, 4] result = [] async for i in items: @@ -82,17 +82,24 @@ def f(): result.append(i) # PERF401 -def f(): +async def f(): items = [1, 2, 3, 4] result = [] async for i in items: result.append(i) # PERF401 +async def f(): + items = [1, 2, 3, 4] + result = [1, 2] + async for i in items: + result.append(i) # PERF401 + + def f(): - result, _ = [1,2,3,4], ... + result, _ = [1, 2, 3, 4], ... for i in range(10): - result.append(i*2) # PERF401 + result.append(i * 2) # PERF401 def f(): @@ -100,23 +107,24 @@ def f(): if True: for i in range(10): # single-line comment 1 should be protected # single-line comment 2 should be protected - if i % 2: # single-line comment 3 should be protected - result.append(i) # PERF401 + if i % 2: # single-line comment 3 should be protected + result.append(i) # PERF401 def f(): - result = [] # comment after assignment should be protected + result = [] # comment after assignment should be protected for i in range(10): # single-line comment 1 should be protected # single-line comment 2 should be protected - if i % 2: # single-line comment 3 should be protected - result.append(i) # PERF401 + if i % 2: # single-line comment 3 should be protected + result.append(i) # PERF401 def f(): result = [] for i in range(10): """block comment stops the fix""" - result.append(i*2) # Ok + result.append(i * 2) # Ok + def f(param): # PERF401 @@ -125,3 +133,107 @@ def f(param): new_layers = [] for value in param: new_layers.append(value * 3) + + +def f(): + result = [] + var = 1 + for _ in range(10): + result.append(var + 1) # PERF401 + + +def f(): + # make sure that `tmp` is not deleted + tmp = 1; result = [] # commment should be protected + for i in range(10): + result.append(i + 1) # PERF401 + + +def f(): + # make sure that `tmp` is not deleted + result = []; tmp = 1 # commment should be protected + for i in range(10): + result.append(i + 1) # PERF401 + + +def f(): + result = [] # comment should be protected + for i in range(10): + result.append(i * 2) # PERF401 + + +def f(): + result = [] + result.append(1) + for i in range(10): + result.append(i * 2) # PERF401 + + +def f(): + result = [] + result += [1] + for i in range(10): + result.append(i * 2) # PERF401 + + +def f(): + result = [] + for val in range(5): + result.append(val * 2) # Ok + print(val) + + +def f(): + result = [] + for val in range(5): + result.append(val * 2) # PERF401 + val = 1 + print(val) + + +def f(): + i = [1, 2, 3] + result = [] + for i in i: + result.append(i + 1) # PERF401 + + +def f(): + result = [] + for i in range( # Comment 1 should not be duplicated + ( + 2 # Comment 2 + + 1 + ) + ): # Comment 3 + if i % 2: # Comment 4 + result.append( + ( + i + 1, + # Comment 5 + 2, + ) + ) # PERF401 + + +def f(): + result: list[int] = [] + for i in range(10): + result.append(i * 2) # PERF401 + + +def f(): + a, b = [1, 2, 3], [4, 5, 6] + result = [] + for i in a, b: + result.append(i[0] + i[1]) # PERF401 + return result + + +def f(): + values = [1, 2, 3] + result = [] + for a in values: + print(a) + for a in values: + result.append(a + 1) # PERF401 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs index 35fe8168e0eaa1..7bc5cf097f9e37 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs @@ -14,7 +14,6 @@ pub(crate) fn deferred_for_loops(checker: &mut Checker) { let Stmt::For(stmt_for) = checker.semantic.current_statement() else { unreachable!("Expected Stmt::For"); }; - if checker.enabled(Rule::UnusedLoopControlVariable) { flake8_bugbear::rules::unused_loop_control_variable(checker, stmt_for); } @@ -36,6 +35,9 @@ pub(crate) fn deferred_for_loops(checker: &mut Checker) { if checker.enabled(Rule::DictIndexMissingItems) { pylint::rules::dict_index_missing_items(checker, stmt_for); } + if checker.enabled(Rule::ManualListComprehension) { + perflint::rules::manual_list_comprehension(checker, stmt_for); + } } } } diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 655aa3db9e3dd5..f88101292574c4 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1366,6 +1366,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { Rule::UnnecessaryEnumerate, Rule::UnusedLoopControlVariable, Rule::YieldInForLoop, + Rule::ManualListComprehension, ]) { checker.analyze.for_loops.push(checker.semantic.snapshot()); } @@ -1390,9 +1391,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::DictIterMissingItems) { pylint::rules::dict_iter_missing_items(checker, target, iter); } - if checker.enabled(Rule::ManualListComprehension) { - perflint::rules::manual_list_comprehension(checker, for_stmt); - } if checker.enabled(Rule::ManualListCopy) { perflint::rules::manual_list_copy(checker, for_stmt); } 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 ff3671f72c154c..415f7313d005aa 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 @@ -1,17 +1,16 @@ -use ruff_python_ast::{self as ast, Arguments, Expr, Stmt}; +use ruff_python_ast::{self as ast, Arguments, Expr}; +use crate::checkers::ast::Checker; use anyhow::{anyhow, Result}; use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; + use ruff_macros::{derive_message_formats, ViolationMetadata}; -use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::helpers::any_over_expr; use ruff_python_semantic::{analyze::typing::is_list, Binding}; -use ruff_python_trivia::PythonWhitespace; +use ruff_python_trivia::{BackwardsTokenizer, PythonWhitespace, SimpleTokenKind, SimpleTokenizer}; use ruff_source_file::LineRanges; use ruff_text_size::{Ranged, TextRange}; -use crate::checkers::ast::Checker; - /// ## What it does /// Checks for `for` loops that can be replaced by a list comprehension. /// @@ -93,7 +92,11 @@ impl Violation for ManualListComprehension { /// PERF401 pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::StmtFor) { - let Expr::Name(ast::ExprName { id, .. }) = &*for_stmt.target else { + let Expr::Name(ast::ExprName { + id: for_stmt_target_id, + .. + }) = &*for_stmt.target + else { return; }; @@ -103,7 +106,7 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::S // if z: // filtered.append(x) // ``` - [Stmt::If(ast::StmtIf { + [ast::Stmt::If(ast::StmtIf { body, elif_else_clauses, test, @@ -125,7 +128,7 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::S _ => return, }; - let Stmt::Expr(ast::StmtExpr { value, .. }) = stmt else { + let ast::Stmt::Expr(ast::StmtExpr { value, .. }) = stmt else { return; }; @@ -151,53 +154,53 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::S return; }; - let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func.as_ref() else { + let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = &**func else { return; }; if attr.as_str() != "append" { return; } + + // Avoid non-list values. + let Some(list_name) = value.as_name_expr() else { + 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) { + if arg + .as_name_expr() + .is_some_and(|arg| arg.id == *for_stmt_target_id) + { return; } } } - // Avoid, e.g., `for x in y: filtered[x].append(x * x)`. - if any_over_expr(value, &|expr| { - expr.as_name_expr().is_some_and(|expr| expr.id == *id) - }) { - return; - } - // Avoid, e.g., `for x in y: filtered.append(filtered[-1] * 2)`. if any_over_expr(arg, &|expr| { - ComparableExpr::from(expr) == ComparableExpr::from(value) + expr.as_name_expr() + .is_some_and(|expr| expr.id == list_name.id) }) { return; } - // Avoid non-list values. - let Some(name) = value.as_name_expr() else { - return; - }; - let Some(binding) = checker + let Some(list_binding) = checker .semantic() - .only_binding(name) + .only_binding(list_name) .map(|id| checker.semantic().binding(id)) else { return; }; - if !is_list(binding, checker.semantic()) { + + if !is_list(list_binding, checker.semantic()) { return; } - // Avoid if the value is used in the conditional test, e.g., + // Avoid if the list is used in the conditional test, e.g., // // ```python // for x in y: @@ -213,28 +216,80 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::S // ``` if if_test.is_some_and(|test| { any_over_expr(test, &|expr| { - expr.as_name_expr().is_some_and(|expr| expr.id == name.id) + expr.as_name_expr() + .is_some_and(|expr| expr.id == list_name.id) }) }) { return; } - let binding_stmt = binding - .statement(checker.semantic()) - .and_then(|stmt| stmt.as_assign_stmt()); + // Avoid if the for-loop target is used outside the for loop, e.g., + // + // ```python + // for x in y: + // filtered.append(x) + // print(x) + // ``` + // + // If this were a comprehension, x would no longer have the correct scope: + // + // ```python + // filtered = [x for x in y] + // print(x) + // ``` + let last_target_binding = checker + .semantic() + .lookup_symbol(for_stmt_target_id) + .expect("for loop target must exist"); + + let target_binding = { + let mut bindings = [last_target_binding].into_iter().chain( + checker + .semantic() + .shadowed_bindings(checker.semantic().scope_id, last_target_binding) + .filter_map(|shadowed| shadowed.same_scope().then_some(shadowed.shadowed_id())), + ); + + bindings + .find_map(|binding_id| { + let binding = checker.semantic().binding(binding_id); + binding + .statement(checker.semantic()) + .and_then(ast::Stmt::as_for_stmt) + .is_some_and(|stmt| stmt.range == for_stmt.range) + .then_some(binding) + }) + .expect("for target binding must exist") + }; + // If any references to the loop target variable are after the loop, + // then converting it into a comprehension would cause a NameError + if target_binding + .references() + .map(|reference| checker.semantic().reference(reference)) + .any(|other_reference| for_stmt.end() < other_reference.start()) + { + return; + } + + let list_binding_stmt = list_binding.statement(checker.semantic()); + let list_binding_value = list_binding_stmt.and_then(|binding_stmt| match binding_stmt { + ast::Stmt::AnnAssign(assign) => assign.value.as_deref(), + ast::Stmt::Assign(assign) => Some(&assign.value), + _ => None, + }); // If the variable is an empty list literal, then we might be able to replace it with a full list comprehension // otherwise, it has to be replaced with a `list.extend` let binding_is_empty_list = - binding_stmt.is_some_and(|binding_stmt| match binding_stmt.value.as_list_expr() { + list_binding_value.is_some_and(|binding_value| match binding_value.as_list_expr() { Some(list_expr) => list_expr.elts.is_empty(), None => false, }); // If the for loop does not have the same parent element as the binding, then it cannot always be - // deleted and replaced with a list comprehension. This does not apply when using an extend. + // deleted and replaced with a list comprehension. This does not apply when using `extend`. let assignment_in_same_statement = { - binding.source.is_some_and(|binding_source| { + list_binding.source.is_some_and(|binding_source| { let for_loop_parent = checker.semantic().current_statement_parent_id(); let binding_parent = checker.semantic().parent_statement_id(binding_source); for_loop_parent == binding_parent @@ -243,20 +298,33 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::S // If the binding is not a single name expression, it could be replaced with a list comprehension, // but not necessarily, so this needs to be manually fixed. This does not apply when using an extend. - let binding_has_one_target = { - match binding_stmt.map(|binding_stmt| binding_stmt.targets.as_slice()) { - Some([only_target]) => only_target.is_name_expr(), - _ => false, - } - }; + let binding_has_one_target = list_binding_stmt.is_some_and(|binding_stmt| match binding_stmt { + ast::Stmt::AnnAssign(_) => true, + ast::Stmt::Assign(assign) => assign.targets.len() == 1, + _ => false, + }); + + // If the binding gets used in between the assignment and the for loop, a list comprehension is no longer safe + let binding_unused_between = list_binding_stmt.is_some_and(|binding_stmt| { + let from_assign_to_loop = TextRange::new(binding_stmt.end(), for_stmt.start()); + // Test if there's any reference to the list symbol between its definition and the for loop. + // if there's at least one, then it's been accessed in the middle somewhere, so it's not safe to change into a list comprehension + !list_binding + .references() + .map(|ref_id| checker.semantic().reference(ref_id).range()) + .any(|text_range| from_assign_to_loop.contains_range(text_range)) + }); // A list extend works in every context, while a list comprehension only works when all the criteria are true - let comprehension_type = - if binding_is_empty_list && assignment_in_same_statement && binding_has_one_target { - ComprehensionType::ListComprehension - } else { - ComprehensionType::Extend - }; + let comprehension_type = if binding_is_empty_list + && assignment_in_same_statement + && binding_has_one_target + && binding_unused_between + { + ComprehensionType::ListComprehension + } else { + ComprehensionType::Extend + }; let mut diagnostic = Diagnostic::new( ManualListComprehension { @@ -271,7 +339,7 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, for_stmt: &ast::S diagnostic.try_set_fix(|| { convert_to_list_extend( comprehension_type, - binding, + list_binding, for_stmt, if_test.map(std::convert::AsRef::as_ref), arg, @@ -291,13 +359,31 @@ fn convert_to_list_extend( to_append: &Expr, checker: &Checker, ) -> Result { + let semantic = checker.semantic(); let locator = checker.locator(); let if_str = match if_test { Some(test) => format!(" if {}", locator.slice(test.range())), None => String::new(), }; - let for_iter_str = locator.slice(for_stmt.iter.range()); + // if the loop target was an implicit tuple, add parentheses around it + // ```python + // for i in a, b: + // ... + // ``` + // becomes + // [... for i in (a, b)] + let for_iter_str = if for_stmt + .iter + .as_ref() + .as_tuple_expr() + .is_some_and(|expr| !expr.parenthesized) + { + format!("({})", locator.slice(for_stmt.iter.range())) + } else { + locator.slice(for_stmt.iter.range()).to_string() + }; + let for_type = if for_stmt.is_async { "async for" } else { @@ -312,29 +398,40 @@ fn convert_to_list_extend( .comment_ranges() .comments_in_range(range) .iter() + // Ignore comments inside of the append or iterator, since these are preserved + .filter(|comment| { + !to_append.range().contains_range(**comment) + && !for_stmt.iter.range().contains_range(**comment) + }) .map(|range| locator.slice(range).trim_whitespace_start()) .collect() }; - let for_stmt_end = for_stmt.range.end(); + + let variable_name = locator.slice(binding); let for_loop_inline_comments: Vec<&str> = comment_strings_in_range(for_stmt.range); - let for_loop_trailing_comment = - comment_strings_in_range(TextRange::new(for_stmt_end, locator.line_end(for_stmt_end))); + let newline = checker.stylist().line_ending().as_str(); + let indent = locator.slice(TextRange::new( + locator.line_start(for_stmt.range.start()), + for_stmt.range.start(), + )); + match fix_type { ComprehensionType::Extend => { - let variable_name = checker.locator().slice(binding.range); + let generator_str = if for_stmt.is_async { + // generators do not implement __iter__, so `async for` requires the generator to be a list + format!("[{generator_str}]") + } else { + generator_str + }; let comprehension_body = format!("{variable_name}.extend({generator_str})"); - let indent_range = TextRange::new( - locator.line_start(for_stmt.range.start()), - for_stmt.range.start(), - ); let indentation = if for_loop_inline_comments.is_empty() { String::new() } else { - format!("{newline}{}", locator.slice(indent_range)) + format!("{newline}{indent}") }; let text_to_replace = format!( "{}{indentation}{comprehension_body}", @@ -346,46 +443,98 @@ fn convert_to_list_extend( ))) } ComprehensionType::ListComprehension => { - let binding_stmt = binding - .statement(checker.semantic()) - .and_then(|stmt| stmt.as_assign_stmt()) + let binding_stmt = binding.statement(semantic); + let binding_stmt_range = binding_stmt + .and_then(|stmt| match stmt { + ast::Stmt::AnnAssign(assign) => Some(assign.range), + ast::Stmt::Assign(assign) => Some(assign.range), + _ => None, + }) .ok_or(anyhow!( "Binding must have a statement to convert into a list comprehension" ))?; - let empty_list_to_replace = binding_stmt.value.as_list_expr().ok_or(anyhow!( - "Assignment value must be an empty list literal in order to replace with a list comprehension" - ))?; - - let comprehension_body = format!("[{generator_str}]"); - - let indent_range = TextRange::new( - locator.line_start(binding_stmt.range.start()), - binding_stmt.range.start(), - ); + // If the binding has multiple statements on its line, the fix would be substantially more complicated + let (semicolon_before, after_semicolon) = { + // determine whether there's a semicolon either before or after the binding statement. + // Since it's a binding statement, we can just check whether there's a semicolon immediately + // after the whitespace in front of or behind it + let mut after_tokenizer = + SimpleTokenizer::starts_at(binding_stmt_range.end(), locator.contents()) + .skip_trivia(); + + let after_semicolon = if after_tokenizer + .next() + .is_some_and(|token| token.kind() == SimpleTokenKind::Semi) + { + after_tokenizer.next() + } else { + None + }; + + let semicolon_before = BackwardsTokenizer::up_to( + binding_stmt_range.start(), + locator.contents(), + checker.comment_ranges(), + ) + .skip_trivia() + .next() + .filter(|token| token.kind() == SimpleTokenKind::Semi); + + (semicolon_before, after_semicolon) + }; + // If there are multiple binding statements in one line, we don't want to accidentally delete them + // Instead, we just delete the binding statement and leave any comments where they are + let (binding_stmt_deletion_range, binding_is_multiple_stmts) = + match (semicolon_before, after_semicolon) { + // ```python + // a = [] + // ``` + (None, None) => (locator.full_lines_range(binding_stmt_range), false), + + // ```python + // a = 1; b = [] + // ^^^^^^^^ + // a = 1; b = []; c = 3 + // ^^^^^^^^ + // ``` + (Some(semicolon_before), Some(_) | None) => ( + TextRange::new(semicolon_before.start(), binding_stmt_range.end()), + true, + ), + + // ```python + // a = []; b = 3 + // ^^^^^^^ + // ``` + (None, Some(after_semicolon)) => ( + TextRange::new(binding_stmt_range.start(), after_semicolon.start()), + true, + ), + }; + + let annotations = match binding_stmt.and_then(|stmt| stmt.as_ann_assign_stmt()) { + Some(assign) => format!(": {}", locator.slice(assign.annotation.range())), + None => String::new(), + }; - let mut for_loop_comments = for_loop_inline_comments; - for_loop_comments.extend(for_loop_trailing_comment); + let mut comments_to_move = for_loop_inline_comments; + if !binding_is_multiple_stmts { + comments_to_move.extend(comment_strings_in_range(binding_stmt_deletion_range)); + } - let indentation = if for_loop_comments.is_empty() { + let indentation = if comments_to_move.is_empty() { String::new() } else { - format!("{newline}{}", locator.slice(indent_range)) + format!("{newline}{indent}") }; - let leading_comments = format!("{}{indentation}", for_loop_comments.join(&indentation)); - - let mut additional_fixes = vec![Edit::range_deletion( - locator.full_lines_range(for_stmt.range), - )]; - // if comments are empty, trying to insert them panics - if !leading_comments.is_empty() { - additional_fixes.push(Edit::insertion( - leading_comments, - binding_stmt.range.start(), - )); - } + let leading_comments = format!("{}{indentation}", comments_to_move.join(&indentation)); + + let comprehension_body = + format!("{leading_comments}{variable_name}{annotations} = [{generator_str}]"); + Ok(Fix::unsafe_edits( - Edit::range_replacement(comprehension_body, empty_list_to_replace.range), - additional_fixes, + Edit::range_deletion(binding_stmt_deletion_range), + [Edit::range_replacement(comprehension_body, for_stmt.range)], )) } } 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 ecb553565b8824..e27274aa29e5be 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 @@ -38,38 +38,156 @@ PERF401.py:89:9: PERF401 Use an async list comprehension to create a transformed | = help: Replace for loop with list comprehension -PERF401.py:95:9: PERF401 Use `list.extend` to create a transformed list +PERF401.py:96:9: PERF401 Use `list.extend` with an async comprehension to create a transformed list | -93 | result, _ = [1,2,3,4], ... -94 | for i in range(10): -95 | result.append(i*2) # PERF401 - | ^^^^^^^^^^^^^^^^^^ PERF401 +94 | result = [1, 2] +95 | async for i in items: +96 | result.append(i) # PERF401 + | ^^^^^^^^^^^^^^^^ PERF401 | = help: Replace for loop with list.extend -PERF401.py:104:17: PERF401 Use `list.extend` to create a transformed list +PERF401.py:102:9: PERF401 Use `list.extend` to create a transformed list + | +100 | result, _ = [1, 2, 3, 4], ... +101 | for i in range(10): +102 | result.append(i * 2) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 | -102 | # single-line comment 2 should be protected -103 | if i % 2: # single-line comment 3 should be protected -104 | result.append(i) # PERF401 + = help: Replace for loop with list.extend + +PERF401.py:111:17: PERF401 Use `list.extend` to create a transformed list + | +109 | # single-line comment 2 should be protected +110 | if i % 2: # single-line comment 3 should be protected +111 | result.append(i) # PERF401 | ^^^^^^^^^^^^^^^^ PERF401 | = help: Replace for loop with list.extend -PERF401.py:112:13: PERF401 Use a list comprehension to create a transformed list +PERF401.py:119:13: PERF401 Use a list comprehension to create a transformed list | -110 | # single-line comment 2 should be protected -111 | if i % 2: # single-line comment 3 should be protected -112 | result.append(i) # PERF401 +117 | # single-line comment 2 should be protected +118 | if i % 2: # single-line comment 3 should be protected +119 | result.append(i) # PERF401 | ^^^^^^^^^^^^^^^^ PERF401 | = help: Replace for loop with list comprehension -PERF401.py:127:13: PERF401 Use a list comprehension to create a transformed list +PERF401.py:135:13: PERF401 Use a list comprehension to create a transformed list | -125 | new_layers = [] -126 | for value in param: -127 | new_layers.append(value * 3) +133 | new_layers = [] +134 | for value in param: +135 | new_layers.append(value * 3) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PERF401 | = help: Replace for loop with list comprehension + +PERF401.py:142:9: PERF401 Use a list comprehension to create a transformed list + | +140 | var = 1 +141 | for _ in range(10): +142 | result.append(var + 1) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +PERF401.py:149:9: PERF401 Use a list comprehension to create a transformed list + | +147 | tmp = 1; result = [] # commment should be protected +148 | for i in range(10): +149 | result.append(i + 1) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +PERF401.py:156:9: PERF401 Use a list comprehension to create a transformed list + | +154 | result = []; tmp = 1 # commment should be protected +155 | for i in range(10): +156 | result.append(i + 1) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +PERF401.py:162:9: PERF401 Use a list comprehension to create a transformed list + | +160 | result = [] # comment should be protected +161 | for i in range(10): +162 | result.append(i * 2) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +PERF401.py:169:9: PERF401 Use `list.extend` to create a transformed list + | +167 | result.append(1) +168 | for i in range(10): +169 | result.append(i * 2) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list.extend + +PERF401.py:189:9: PERF401 Use a list comprehension to create a transformed list + | +187 | result = [] +188 | for val in range(5): +189 | result.append(val * 2) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^^^ PERF401 +190 | val = 1 +191 | print(val) + | + = help: Replace for loop with list comprehension + +PERF401.py:198:9: PERF401 Use a list comprehension to create a transformed list + | +196 | result = [] +197 | for i in i: +198 | result.append(i + 1) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +PERF401.py:210:13: PERF401 Use a list comprehension to create a transformed list + | +208 | ): # Comment 3 +209 | if i % 2: # Comment 4 +210 | result.append( + | _____________^ +211 | | ( +212 | | i + 1, +213 | | # Comment 5 +214 | | 2, +215 | | ) +216 | | ) # PERF401 + | |_____________^ PERF401 + | + = help: Replace for loop with list comprehension + +PERF401.py:222:9: PERF401 Use a list comprehension to create a transformed list + | +220 | result: list[int] = [] +221 | for i in range(10): +222 | result.append(i * 2) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +PERF401.py:229:9: PERF401 Use a list comprehension to create a transformed list + | +227 | result = [] +228 | for i in a, b: +229 | result.append(i[0] + i[1]) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ PERF401 +230 | return result + | + = help: Replace for loop with list comprehension + +PERF401.py:239:9: PERF401 Use a list comprehension to create a transformed list + | +237 | print(a) +238 | for a in values: +239 | result.append(a + 1) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap index a981285c8b4a5a..a8c6adde82002f 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap +++ b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap @@ -18,11 +18,10 @@ PERF401.py:6:13: PERF401 [*] Use a list comprehension to create a transformed li 4 |- for i in items: 5 |- if i % 2: 6 |- result.append(i) # PERF401 - 3 |+ # PERF401 - 4 |+ result = [i for i in items if i % 2] -7 5 | -8 6 | -9 7 | def f(): + 3 |+ result = [i for i in items if i % 2] # PERF401 +7 4 | +8 5 | +9 6 | def f(): PERF401.py:13:9: PERF401 [*] Use a list comprehension to create a transformed list | @@ -40,11 +39,10 @@ PERF401.py:13:9: PERF401 [*] Use a list comprehension to create a transformed li 11 |- result = [] 12 |- for i in items: 13 |- result.append(i * i) # PERF401 - 11 |+ # PERF401 - 12 |+ result = [i * i for i in items] -14 13 | -15 14 | -16 15 | def f(): + 11 |+ result = [i * i for i in items] # PERF401 +14 12 | +15 13 | +16 14 | def f(): PERF401.py:82:13: PERF401 [*] Use an async list comprehension to create a transformed list | @@ -57,17 +55,16 @@ PERF401.py:82:13: PERF401 [*] Use an async list comprehension to create a transf ℹ Unsafe fix 76 76 | -77 77 | def f(): +77 77 | async def f(): 78 78 | items = [1, 2, 3, 4] 79 |- result = [] 80 |- async for i in items: 81 |- if i % 2: 82 |- result.append(i) # PERF401 - 79 |+ # PERF401 - 80 |+ result = [i async for i in items if i % 2] -83 81 | -84 82 | -85 83 | def f(): + 79 |+ result = [i async for i in items if i % 2] # PERF401 +83 80 | +84 81 | +85 82 | async def f(): PERF401.py:89:9: PERF401 [*] Use an async list comprehension to create a transformed list | @@ -80,103 +77,387 @@ PERF401.py:89:9: PERF401 [*] Use an async list comprehension to create a transfo ℹ Unsafe fix 84 84 | -85 85 | def f(): +85 85 | async def f(): 86 86 | items = [1, 2, 3, 4] 87 |- result = [] 88 |- async for i in items: 89 |- result.append(i) # PERF401 - 87 |+ # PERF401 - 88 |+ result = [i async for i in items] -90 89 | -91 90 | -92 91 | def f(): + 87 |+ result = [i async for i in items] # PERF401 +90 88 | +91 89 | +92 90 | async def f(): -PERF401.py:95:9: PERF401 [*] Use `list.extend` to create a transformed list +PERF401.py:96:9: PERF401 [*] Use `list.extend` with an async comprehension to create a transformed list | -93 | result, _ = [1,2,3,4], ... -94 | for i in range(10): -95 | result.append(i*2) # PERF401 - | ^^^^^^^^^^^^^^^^^^ PERF401 +94 | result = [1, 2] +95 | async for i in items: +96 | result.append(i) # PERF401 + | ^^^^^^^^^^^^^^^^ PERF401 | = help: Replace for loop with list.extend ℹ Unsafe fix -91 91 | -92 92 | def f(): -93 93 | result, _ = [1,2,3,4], ... -94 |- for i in range(10): -95 |- result.append(i*2) # PERF401 - 94 |+ result.extend(i*2 for i in range(10)) # PERF401 -96 95 | +92 92 | async def f(): +93 93 | items = [1, 2, 3, 4] +94 94 | result = [1, 2] +95 |- async for i in items: +96 |- result.append(i) # PERF401 + 95 |+ result.extend([i async for i in items]) # PERF401 97 96 | -98 97 | def f(): +98 97 | +99 98 | def f(): + +PERF401.py:102:9: PERF401 [*] Use `list.extend` to create a transformed list + | +100 | result, _ = [1, 2, 3, 4], ... +101 | for i in range(10): +102 | result.append(i * 2) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list.extend -PERF401.py:104:17: PERF401 [*] Use `list.extend` to create a transformed list +ℹ Unsafe fix +98 98 | +99 99 | def f(): +100 100 | result, _ = [1, 2, 3, 4], ... +101 |- for i in range(10): +102 |- result.append(i * 2) # PERF401 + 101 |+ result.extend(i * 2 for i in range(10)) # PERF401 +103 102 | +104 103 | +105 104 | def f(): + +PERF401.py:111:17: PERF401 [*] Use `list.extend` to create a transformed list | -102 | # single-line comment 2 should be protected -103 | if i % 2: # single-line comment 3 should be protected -104 | result.append(i) # PERF401 +109 | # single-line comment 2 should be protected +110 | if i % 2: # single-line comment 3 should be protected +111 | result.append(i) # PERF401 | ^^^^^^^^^^^^^^^^ PERF401 | = help: Replace for loop with list.extend ℹ Unsafe fix -98 98 | def f(): -99 99 | result = [] -100 100 | if True: -101 |- for i in range(10): # single-line comment 1 should be protected -102 |- # single-line comment 2 should be protected -103 |- if i % 2: # single-line comment 3 should be protected -104 |- result.append(i) # PERF401 - 101 |+ # single-line comment 1 should be protected - 102 |+ # single-line comment 2 should be protected - 103 |+ # single-line comment 3 should be protected - 104 |+ result.extend(i for i in range(10) if i % 2) # PERF401 -105 105 | -106 106 | -107 107 | def f(): - -PERF401.py:112:13: PERF401 [*] Use a list comprehension to create a transformed list - | -110 | # single-line comment 2 should be protected -111 | if i % 2: # single-line comment 3 should be protected -112 | result.append(i) # PERF401 +105 105 | def f(): +106 106 | result = [] +107 107 | if True: +108 |- for i in range(10): # single-line comment 1 should be protected +109 |- # single-line comment 2 should be protected +110 |- if i % 2: # single-line comment 3 should be protected +111 |- result.append(i) # PERF401 + 108 |+ # single-line comment 1 should be protected + 109 |+ # single-line comment 2 should be protected + 110 |+ # single-line comment 3 should be protected + 111 |+ result.extend(i for i in range(10) if i % 2) # PERF401 +112 112 | +113 113 | +114 114 | def f(): + +PERF401.py:119:13: PERF401 [*] Use a list comprehension to create a transformed list + | +117 | # single-line comment 2 should be protected +118 | if i % 2: # single-line comment 3 should be protected +119 | result.append(i) # PERF401 | ^^^^^^^^^^^^^^^^ PERF401 | = help: Replace for loop with list comprehension ℹ Unsafe fix -105 105 | -106 106 | -107 107 | def f(): -108 |- result = [] # comment after assignment should be protected -109 |- for i in range(10): # single-line comment 1 should be protected -110 |- # single-line comment 2 should be protected -111 |- if i % 2: # single-line comment 3 should be protected -112 |- result.append(i) # PERF401 - 108 |+ # single-line comment 1 should be protected - 109 |+ # single-line comment 2 should be protected - 110 |+ # single-line comment 3 should be protected - 111 |+ # PERF401 - 112 |+ result = [i for i in range(10) if i % 2] # comment after assignment should be protected +112 112 | 113 113 | -114 114 | -115 115 | def f(): +114 114 | def f(): +115 |- result = [] # comment after assignment should be protected +116 |- for i in range(10): # single-line comment 1 should be protected +117 |- # single-line comment 2 should be protected +118 |- if i % 2: # single-line comment 3 should be protected +119 |- result.append(i) # PERF401 + 115 |+ # single-line comment 1 should be protected + 116 |+ # single-line comment 2 should be protected + 117 |+ # single-line comment 3 should be protected + 118 |+ # comment after assignment should be protected + 119 |+ result = [i for i in range(10) if i % 2] # PERF401 +120 120 | +121 121 | +122 122 | def f(): -PERF401.py:127:13: PERF401 [*] Use a list comprehension to create a transformed list +PERF401.py:135:13: PERF401 [*] Use a list comprehension to create a transformed list | -125 | new_layers = [] -126 | for value in param: -127 | new_layers.append(value * 3) +133 | new_layers = [] +134 | for value in param: +135 | new_layers.append(value * 3) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PERF401 | = help: Replace for loop with list comprehension ℹ Unsafe fix -122 122 | # PERF401 -123 123 | # make sure the fix does not panic if there is no comments -124 124 | if param: -125 |- new_layers = [] -126 |- for value in param: -127 |- new_layers.append(value * 3) - 125 |+ new_layers = [value * 3 for value in param] +130 130 | # PERF401 +131 131 | # make sure the fix does not panic if there is no comments +132 132 | if param: +133 |- new_layers = [] +134 |- for value in param: +135 |- new_layers.append(value * 3) + 133 |+ new_layers = [value * 3 for value in param] +136 134 | +137 135 | +138 136 | def f(): + +PERF401.py:142:9: PERF401 [*] Use a list comprehension to create a transformed list + | +140 | var = 1 +141 | for _ in range(10): +142 | result.append(var + 1) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +136 136 | +137 137 | +138 138 | def f(): +139 |- result = [] +140 139 | var = 1 +141 |- for _ in range(10): +142 |- result.append(var + 1) # PERF401 + 140 |+ result = [var + 1 for _ in range(10)] # PERF401 +143 141 | +144 142 | +145 143 | def f(): + +PERF401.py:149:9: PERF401 [*] Use a list comprehension to create a transformed list + | +147 | tmp = 1; result = [] # commment should be protected +148 | for i in range(10): +149 | result.append(i + 1) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +144 144 | +145 145 | def f(): +146 146 | # make sure that `tmp` is not deleted +147 |- tmp = 1; result = [] # commment should be protected +148 |- for i in range(10): +149 |- result.append(i + 1) # PERF401 + 147 |+ tmp = 1 # commment should be protected + 148 |+ result = [i + 1 for i in range(10)] # PERF401 +150 149 | +151 150 | +152 151 | def f(): + +PERF401.py:156:9: PERF401 [*] Use a list comprehension to create a transformed list + | +154 | result = []; tmp = 1 # commment should be protected +155 | for i in range(10): +156 | result.append(i + 1) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +151 151 | +152 152 | def f(): +153 153 | # make sure that `tmp` is not deleted +154 |- result = []; tmp = 1 # commment should be protected +155 |- for i in range(10): +156 |- result.append(i + 1) # PERF401 + 154 |+ tmp = 1 # commment should be protected + 155 |+ result = [i + 1 for i in range(10)] # PERF401 +157 156 | +158 157 | +159 158 | def f(): + +PERF401.py:162:9: PERF401 [*] Use a list comprehension to create a transformed list + | +160 | result = [] # comment should be protected +161 | for i in range(10): +162 | result.append(i * 2) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +157 157 | +158 158 | +159 159 | def f(): +160 |- result = [] # comment should be protected +161 |- for i in range(10): +162 |- result.append(i * 2) # PERF401 + 160 |+ # comment should be protected + 161 |+ result = [i * 2 for i in range(10)] # PERF401 +163 162 | +164 163 | +165 164 | def f(): + +PERF401.py:169:9: PERF401 [*] Use `list.extend` to create a transformed list + | +167 | result.append(1) +168 | for i in range(10): +169 | result.append(i * 2) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list.extend + +ℹ Unsafe fix +165 165 | def f(): +166 166 | result = [] +167 167 | result.append(1) +168 |- for i in range(10): +169 |- result.append(i * 2) # PERF401 + 168 |+ result.extend(i * 2 for i in range(10)) # PERF401 +170 169 | +171 170 | +172 171 | def f(): + +PERF401.py:189:9: PERF401 [*] Use a list comprehension to create a transformed list + | +187 | result = [] +188 | for val in range(5): +189 | result.append(val * 2) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^^^ PERF401 +190 | val = 1 +191 | print(val) + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +184 184 | +185 185 | +186 186 | def f(): +187 |- result = [] +188 |- for val in range(5): +189 |- result.append(val * 2) # PERF401 + 187 |+ result = [val * 2 for val in range(5)] # PERF401 +190 188 | val = 1 +191 189 | print(val) +192 190 | + +PERF401.py:198:9: PERF401 [*] Use a list comprehension to create a transformed list + | +196 | result = [] +197 | for i in i: +198 | result.append(i + 1) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +193 193 | +194 194 | def f(): +195 195 | i = [1, 2, 3] +196 |- result = [] +197 |- for i in i: +198 |- result.append(i + 1) # PERF401 + 196 |+ result = [i + 1 for i in i] # PERF401 +199 197 | +200 198 | +201 199 | def f(): + +PERF401.py:210:13: PERF401 [*] Use a list comprehension to create a transformed list + | +208 | ): # Comment 3 +209 | if i % 2: # Comment 4 +210 | result.append( + | _____________^ +211 | | ( +212 | | i + 1, +213 | | # Comment 5 +214 | | 2, +215 | | ) +216 | | ) # PERF401 + | |_____________^ PERF401 + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +199 199 | +200 200 | +201 201 | def f(): +202 |- result = [] +203 |- for i in range( # Comment 1 should not be duplicated + 202 |+ # Comment 3 + 203 |+ # Comment 4 + 204 |+ result = [( + 205 |+ i + 1, + 206 |+ # Comment 5 + 207 |+ 2, + 208 |+ ) for i in range( # Comment 1 should not be duplicated +204 209 | ( +205 210 | 2 # Comment 2 +206 211 | + 1 +207 212 | ) +208 |- ): # Comment 3 +209 |- if i % 2: # Comment 4 +210 |- result.append( +211 |- ( +212 |- i + 1, +213 |- # Comment 5 +214 |- 2, +215 |- ) +216 |- ) # PERF401 + 213 |+ ) if i % 2] # PERF401 +217 214 | +218 215 | +219 216 | def f(): + +PERF401.py:222:9: PERF401 [*] Use a list comprehension to create a transformed list + | +220 | result: list[int] = [] +221 | for i in range(10): +222 | result.append(i * 2) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +217 217 | +218 218 | +219 219 | def f(): +220 |- result: list[int] = [] +221 |- for i in range(10): +222 |- result.append(i * 2) # PERF401 + 220 |+ result: list[int] = [i * 2 for i in range(10)] # PERF401 +223 221 | +224 222 | +225 223 | def f(): + +PERF401.py:229:9: PERF401 [*] Use a list comprehension to create a transformed list + | +227 | result = [] +228 | for i in a, b: +229 | result.append(i[0] + i[1]) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ PERF401 +230 | return result + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +224 224 | +225 225 | def f(): +226 226 | a, b = [1, 2, 3], [4, 5, 6] +227 |- result = [] +228 |- for i in a, b: +229 |- result.append(i[0] + i[1]) # PERF401 + 227 |+ result = [i[0] + i[1] for i in (a, b)] # PERF401 +230 228 | return result +231 229 | +232 230 | + +PERF401.py:239:9: PERF401 [*] Use a list comprehension to create a transformed list + | +237 | print(a) +238 | for a in values: +239 | result.append(a + 1) # PERF401 + | ^^^^^^^^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +232 232 | +233 233 | def f(): +234 234 | values = [1, 2, 3] +235 |- result = [] +236 235 | for a in values: +237 236 | print(a) +238 |- for a in values: +239 |- result.append(a + 1) # PERF401 + 237 |+ result = [a + 1 for a in values] # PERF401