diff --git a/crates/ruff/resources/test/fixtures/perflint/PERF101.py b/crates/ruff/resources/test/fixtures/perflint/PERF101.py new file mode 100644 index 0000000000..6123e6d652 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/perflint/PERF101.py @@ -0,0 +1,52 @@ +foo_tuple = (1, 2, 3) +foo_list = [1, 2, 3] +foo_set = {1, 2, 3} +foo_dict = {1: 2, 3: 4} +foo_int = 123 + +for i in list(foo_tuple): # PERF101 + pass + +for i in list(foo_list): # PERF101 + pass + +for i in list(foo_set): # PERF101 + pass + +for i in list((1, 2, 3)): # PERF101 + pass + +for i in list([1, 2, 3]): # PERF101 + pass + +for i in list({1, 2, 3}): # PERF101 + pass + +for i in list( + { + 1, + 2, + 3, + } +): + pass + +for i in list( # Comment + {1, 2, 3} +): # PERF101 + pass + +for i in list(foo_dict): # Ok + pass + +for i in list(1): # Ok + pass + +for i in list(foo_int): # Ok + pass + + +import itertools + +for i in itertools.product(foo_int): # Ok + pass diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index ecd0b18a7a..11f73fa09f 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -1481,6 +1481,9 @@ where if self.enabled(Rule::IncorrectDictIterator) { perflint::rules::incorrect_dict_iterator(self, target, iter); } + if self.enabled(Rule::UnnecessaryListCast) { + perflint::rules::unnecessary_list_cast(self, iter); + } } Stmt::Try(ast::StmtTry { body, diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index f4f6211e43..1ea96ca00b 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -784,6 +784,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Airflow, "001") => (RuleGroup::Unspecified, rules::airflow::rules::AirflowVariableNameTaskIdMismatch), // perflint + (Perflint, "101") => (RuleGroup::Unspecified, rules::perflint::rules::UnnecessaryListCast), (Perflint, "102") => (RuleGroup::Unspecified, rules::perflint::rules::IncorrectDictIterator), // flake8-fixme diff --git a/crates/ruff/src/rules/perflint/mod.rs b/crates/ruff/src/rules/perflint/mod.rs index c0b0dd894d..b39aa84fc9 100644 --- a/crates/ruff/src/rules/perflint/mod.rs +++ b/crates/ruff/src/rules/perflint/mod.rs @@ -13,6 +13,7 @@ mod tests { use crate::settings::Settings; use crate::test::test_path; + #[test_case(Rule::UnnecessaryListCast, Path::new("PERF101.py"))] #[test_case(Rule::IncorrectDictIterator, Path::new("PERF102.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); diff --git a/crates/ruff/src/rules/perflint/rules/mod.rs b/crates/ruff/src/rules/perflint/rules/mod.rs index a092bb73f8..cc35c428b0 100644 --- a/crates/ruff/src/rules/perflint/rules/mod.rs +++ b/crates/ruff/src/rules/perflint/rules/mod.rs @@ -1,3 +1,5 @@ -pub(crate) use incorrect_dict_iterator::{incorrect_dict_iterator, IncorrectDictIterator}; +pub(crate) use incorrect_dict_iterator::*; +pub(crate) use unnecessary_list_cast::*; mod incorrect_dict_iterator; +mod unnecessary_list_cast; diff --git a/crates/ruff/src/rules/perflint/rules/unnecessary_list_cast.rs b/crates/ruff/src/rules/perflint/rules/unnecessary_list_cast.rs new file mode 100644 index 0000000000..57f2f49caa --- /dev/null +++ b/crates/ruff/src/rules/perflint/rules/unnecessary_list_cast.rs @@ -0,0 +1,129 @@ +use ruff_text_size::TextRange; +use rustpython_parser::ast::{self, Expr}; + +use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::prelude::Stmt; + +use crate::checkers::ast::Checker; +use crate::registry::AsRule; + +/// ## What it does +/// Checks for explicit casts to `list` on for-loop iterables. +/// +/// ## Why is this bad? +/// Using a `list()` call to eagerly iterate over an already-iterable type +/// (like a tuple, list, or set) is inefficient, as it forces Python to create +/// a new list unnecessarily. +/// +/// Removing the `list()` call will not change the behavior of the code, but +/// may improve performance. +/// +/// ## Example +/// ```python +/// items = (1, 2, 3) +/// for i in list(items): +/// print(i) +/// ``` +/// +/// Use instead: +/// ```python +/// items = (1, 2, 3) +/// for i in items: +/// print(i) +/// ``` +#[violation] +pub struct UnnecessaryListCast; + +impl AlwaysAutofixableViolation for UnnecessaryListCast { + #[derive_message_formats] + fn message(&self) -> String { + format!("Do not cast an iterable to `list` before iterating over it") + } + + fn autofix_title(&self) -> String { + format!("Remove `list()` cast") + } +} + +/// PERF101 +pub(crate) fn unnecessary_list_cast(checker: &mut Checker, iter: &Expr) { + let Expr::Call(ast::ExprCall{ func, args, range: list_range, ..}) = iter else { + return; + }; + + if args.len() != 1 { + return; + } + + let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else{ + return; + }; + + if !(id == "list" && checker.semantic().is_builtin("list")) { + return; + } + + match &args[0] { + Expr::Tuple(ast::ExprTuple { + range: iterable_range, + .. + }) + | Expr::List(ast::ExprList { + range: iterable_range, + .. + }) + | Expr::Set(ast::ExprSet { + range: iterable_range, + .. + }) => { + let mut diagnostic = Diagnostic::new(UnnecessaryListCast, *list_range); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.set_fix(remove_cast(*list_range, *iterable_range)); + } + checker.diagnostics.push(diagnostic); + } + Expr::Name(ast::ExprName { + id, + range: iterable_range, + .. + }) => { + let scope = checker.semantic().scope(); + if let Some(binding_id) = scope.get(id) { + let binding = checker.semantic().binding(binding_id); + if binding.kind.is_assignment() || binding.kind.is_named_expr_assignment() { + if let Some(parent_id) = binding.source { + let parent = checker.semantic().stmts[parent_id]; + if let Stmt::Assign(ast::StmtAssign { value, .. }) + | Stmt::AnnAssign(ast::StmtAnnAssign { + value: Some(value), .. + }) + | Stmt::AugAssign(ast::StmtAugAssign { value, .. }) = parent + { + if matches!( + value.as_ref(), + Expr::Tuple(_) | Expr::List(_) | Expr::Set(_) + ) { + let mut diagnostic = + Diagnostic::new(UnnecessaryListCast, *list_range); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.set_fix(remove_cast(*list_range, *iterable_range)); + } + checker.diagnostics.push(diagnostic); + } + } + } + } + } + } + _ => {} + } +} + +/// Generate a [`Fix`] to remove a `list` cast from an expression. +fn remove_cast(list_range: TextRange, iterable_range: TextRange) -> Fix { + Fix::automatic_edits( + Edit::deletion(list_range.start(), iterable_range.start()), + [Edit::deletion(iterable_range.end(), list_range.end())], + ) +} diff --git a/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF101_PERF101.py.snap b/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF101_PERF101.py.snap new file mode 100644 index 0000000000..54a17b4c4f --- /dev/null +++ b/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF101_PERF101.py.snap @@ -0,0 +1,183 @@ +--- +source: crates/ruff/src/rules/perflint/mod.rs +--- +PERF101.py:7:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it + | +5 | foo_int = 123 +6 | +7 | for i in list(foo_tuple): # PERF101 + | ^^^^^^^^^^^^^^^ PERF101 +8 | pass + | + = help: Remove `list()` cast + +ℹ Fix +4 4 | foo_dict = {1: 2, 3: 4} +5 5 | foo_int = 123 +6 6 | +7 |-for i in list(foo_tuple): # PERF101 + 7 |+for i in foo_tuple: # PERF101 +8 8 | pass +9 9 | +10 10 | for i in list(foo_list): # PERF101 + +PERF101.py:10:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it + | + 8 | pass + 9 | +10 | for i in list(foo_list): # PERF101 + | ^^^^^^^^^^^^^^ PERF101 +11 | pass + | + = help: Remove `list()` cast + +ℹ Fix +7 7 | for i in list(foo_tuple): # PERF101 +8 8 | pass +9 9 | +10 |-for i in list(foo_list): # PERF101 + 10 |+for i in foo_list: # PERF101 +11 11 | pass +12 12 | +13 13 | for i in list(foo_set): # PERF101 + +PERF101.py:13:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it + | +11 | pass +12 | +13 | for i in list(foo_set): # PERF101 + | ^^^^^^^^^^^^^ PERF101 +14 | pass + | + = help: Remove `list()` cast + +ℹ Fix +10 10 | for i in list(foo_list): # PERF101 +11 11 | pass +12 12 | +13 |-for i in list(foo_set): # PERF101 + 13 |+for i in foo_set: # PERF101 +14 14 | pass +15 15 | +16 16 | for i in list((1, 2, 3)): # PERF101 + +PERF101.py:16:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it + | +14 | pass +15 | +16 | for i in list((1, 2, 3)): # PERF101 + | ^^^^^^^^^^^^^^^ PERF101 +17 | pass + | + = help: Remove `list()` cast + +ℹ Fix +13 13 | for i in list(foo_set): # PERF101 +14 14 | pass +15 15 | +16 |-for i in list((1, 2, 3)): # PERF101 + 16 |+for i in (1, 2, 3): # PERF101 +17 17 | pass +18 18 | +19 19 | for i in list([1, 2, 3]): # PERF101 + +PERF101.py:19:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it + | +17 | pass +18 | +19 | for i in list([1, 2, 3]): # PERF101 + | ^^^^^^^^^^^^^^^ PERF101 +20 | pass + | + = help: Remove `list()` cast + +ℹ Fix +16 16 | for i in list((1, 2, 3)): # PERF101 +17 17 | pass +18 18 | +19 |-for i in list([1, 2, 3]): # PERF101 + 19 |+for i in [1, 2, 3]: # PERF101 +20 20 | pass +21 21 | +22 22 | for i in list({1, 2, 3}): # PERF101 + +PERF101.py:22:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it + | +20 | pass +21 | +22 | for i in list({1, 2, 3}): # PERF101 + | ^^^^^^^^^^^^^^^ PERF101 +23 | pass + | + = help: Remove `list()` cast + +ℹ Fix +19 19 | for i in list([1, 2, 3]): # PERF101 +20 20 | pass +21 21 | +22 |-for i in list({1, 2, 3}): # PERF101 + 22 |+for i in {1, 2, 3}: # PERF101 +23 23 | pass +24 24 | +25 25 | for i in list( + +PERF101.py:25:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it + | +23 | pass +24 | +25 | for i in list( + | __________^ +26 | | { +27 | | 1, +28 | | 2, +29 | | 3, +30 | | } +31 | | ): + | |_^ PERF101 +32 | pass + | + = help: Remove `list()` cast + +ℹ Fix +22 22 | for i in list({1, 2, 3}): # PERF101 +23 23 | pass +24 24 | +25 |-for i in list( +26 |- { + 25 |+for i in { +27 26 | 1, +28 27 | 2, +29 28 | 3, +30 |- } +31 |-): + 29 |+ }: +32 30 | pass +33 31 | +34 32 | for i in list( # Comment + +PERF101.py:34:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it + | +32 | pass +33 | +34 | for i in list( # Comment + | __________^ +35 | | {1, 2, 3} +36 | | ): # PERF101 + | |_^ PERF101 +37 | pass + | + = help: Remove `list()` cast + +ℹ Fix +31 31 | ): +32 32 | pass +33 33 | +34 |-for i in list( # Comment +35 |- {1, 2, 3} +36 |-): # PERF101 + 34 |+for i in {1, 2, 3}: # PERF101 +37 35 | pass +38 36 | +39 37 | for i in list(foo_dict): # Ok + + diff --git a/ruff.schema.json b/ruff.schema.json index a30f989acf..52ba8fea34 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2063,6 +2063,7 @@ "PERF", "PERF1", "PERF10", + "PERF101", "PERF102", "PGH", "PGH0",