diff --git a/crates/ruff/resources/test/fixtures/perflint/PERF401.py b/crates/ruff/resources/test/fixtures/perflint/PERF401.py new file mode 100644 index 0000000000..ac19d19876 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/perflint/PERF401.py @@ -0,0 +1,18 @@ +def foo(): + items = [1, 2, 3, 4] + result = [] + for i in items: + if i % 2: + result.append(i) # PERF401 + + +def foo(): + items = [1,2,3,4] + result = [] + for i in items: + if i % 2: + result.append(i) # PERF401 + elif i % 2: + result.append(i) # PERF401 + else: + result.append(i) # PERF401 diff --git a/crates/ruff/resources/test/fixtures/perflint/PERF402.py b/crates/ruff/resources/test/fixtures/perflint/PERF402.py new file mode 100644 index 0000000000..0d6842dce7 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/perflint/PERF402.py @@ -0,0 +1,12 @@ +def foo(): + items = [1, 2, 3, 4] + result = [] + for i in items: + result.append(i) # PERF402 + + +def foo(): + items = [1, 2, 3, 4] + result = [] + for i in items: + result.insert(0, i) # PERF402 diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 0c9f892cce..d20c1271d6 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -1501,6 +1501,12 @@ where if self.enabled(Rule::IncorrectDictIterator) { perflint::rules::incorrect_dict_iterator(self, target, iter); } + if self.enabled(Rule::ManualListComprehension) { + perflint::rules::manual_list_comprehension(self, body); + } + if self.enabled(Rule::ManualListCopy) { + perflint::rules::manual_list_copy(self, body); + } if self.enabled(Rule::UnnecessaryListCast) { perflint::rules::unnecessary_list_cast(self, iter); } diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 06adeba95b..79a7982dc9 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -793,6 +793,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Perflint, "101") => (RuleGroup::Unspecified, rules::perflint::rules::UnnecessaryListCast), (Perflint, "102") => (RuleGroup::Unspecified, rules::perflint::rules::IncorrectDictIterator), (Perflint, "203") => (RuleGroup::Unspecified, rules::perflint::rules::TryExceptInLoop), + (Perflint, "401") => (RuleGroup::Unspecified, rules::perflint::rules::ManualListComprehension), + (Perflint, "402") => (RuleGroup::Unspecified, rules::perflint::rules::ManualListCopy), // flake8-fixme (Flake8Fixme, "001") => (RuleGroup::Unspecified, rules::flake8_fixme::rules::LineContainsFixme), diff --git a/crates/ruff/src/rules/perflint/mod.rs b/crates/ruff/src/rules/perflint/mod.rs index 33c9691206..291bfcd207 100644 --- a/crates/ruff/src/rules/perflint/mod.rs +++ b/crates/ruff/src/rules/perflint/mod.rs @@ -16,6 +16,8 @@ mod tests { #[test_case(Rule::UnnecessaryListCast, Path::new("PERF101.py"))] #[test_case(Rule::IncorrectDictIterator, Path::new("PERF102.py"))] #[test_case(Rule::TryExceptInLoop, Path::new("PERF203.py"))] + #[test_case(Rule::ManualListComprehension, Path::new("PERF401.py"))] + #[test_case(Rule::ManualListCopy, Path::new("PERF402.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff/src/rules/perflint/rules/incorrect_dict_iterator.rs b/crates/ruff/src/rules/perflint/rules/incorrect_dict_iterator.rs index 17e2d2f93b..af2677f2f1 100644 --- a/crates/ruff/src/rules/perflint/rules/incorrect_dict_iterator.rs +++ b/crates/ruff/src/rules/perflint/rules/incorrect_dict_iterator.rs @@ -23,6 +23,10 @@ use crate::registry::AsRule; /// avoid allocating tuples for every item in the dictionary. They also /// communicate the intent of the code more clearly. /// +/// Note that, as with all `perflint` rules, this is only intended as a +/// micro-optimization, and will have a negligible impact on performance in +/// most cases. +/// /// ## Example /// ```python /// some_dict = {"a": 1, "b": 2} diff --git a/crates/ruff/src/rules/perflint/rules/manual_list_comprehension.rs b/crates/ruff/src/rules/perflint/rules/manual_list_comprehension.rs new file mode 100644 index 0000000000..d7143bb080 --- /dev/null +++ b/crates/ruff/src/rules/perflint/rules/manual_list_comprehension.rs @@ -0,0 +1,76 @@ +use rustpython_parser::ast::{self, Expr, Stmt}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for `for` loops that can be replaced by a list comprehension. +/// +/// ## Why is this bad? +/// When creating a filtered list from an existing list using a for-loop, +/// prefer a list comprehension. List comprehensions are more readable and +/// more performant. +/// +/// Using the below as an example, the list comprehension is ~10% faster on +/// Python 3.11, and ~25% faster on Python 3.10. +/// +/// Note that, as with all `perflint` rules, this is only intended as a +/// micro-optimization, and will have a negligible impact on performance in +/// most cases. +/// +/// ## Example +/// ```python +/// original = list(range(10000)) +/// filtered = [] +/// for i in original: +/// if i % 2: +/// filtered.append(i) +/// ``` +/// +/// Use instead: +/// ```python +/// original = list(range(10000)) +/// filtered = [x for x in original if x % 2] +/// ``` +#[violation] +pub struct ManualListComprehension; + +impl Violation for ManualListComprehension { + #[derive_message_formats] + fn message(&self) -> String { + format!("Use a list comprehension to create a new filtered list") + } +} + +/// PERF401 +pub(crate) fn manual_list_comprehension(checker: &mut Checker, body: &[Stmt]) { + let [stmt] = body else { + return; + }; + + let Stmt::If(ast::StmtIf { body, .. }) = stmt else { + return; + }; + + for stmt in body { + let Stmt::Expr(ast::StmtExpr { value, .. }) = stmt else { + continue; + }; + + let Expr::Call(ast::ExprCall { func, range, .. }) = value.as_ref() else { + continue; + }; + + let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func.as_ref() else { + continue; + }; + + if attr.as_str() == "append" { + 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 new file mode 100644 index 0000000000..dd7545129d --- /dev/null +++ b/crates/ruff/src/rules/perflint/rules/manual_list_copy.rs @@ -0,0 +1,69 @@ +use rustpython_parser::ast::{self, Expr, Stmt}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for `for` loops that can be replaced by a making a copy of a list. +/// +/// ## Why is this bad? +/// When creating a copy of an existing list using a for-loop, prefer +/// `list` or `list.copy` instead. Making a direct copy is more readable and +/// more performant. +/// +/// Using the below as an example, the `list`-based copy is ~2x faster on +/// Python 3.11. +/// +/// Note that, as with all `perflint` rules, this is only intended as a +/// micro-optimization, and will have a negligible impact on performance in +/// most cases. +/// +/// ## Example +/// ```python +/// original = list(range(10000)) +/// filtered = [] +/// for i in original: +/// filtered.append(i) +/// ``` +/// +/// Use instead: +/// ```python +/// original = list(range(10000)) +/// filtered = list(original) +/// ``` +#[violation] +pub struct ManualListCopy; + +impl Violation for ManualListCopy { + #[derive_message_formats] + fn message(&self) -> String { + format!("Use `list` or `list.copy` to create a copy of a list") + } +} + +/// PERF402 +pub(crate) fn manual_list_copy(checker: &mut Checker, body: &[Stmt]) { + let [stmt] = body else { + return; + }; + + let Stmt::Expr(ast::StmtExpr { value, .. }) = stmt else { + return; + }; + + let Expr::Call(ast::ExprCall { func, range, .. }) = value.as_ref() else { + 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)); + } +} diff --git a/crates/ruff/src/rules/perflint/rules/mod.rs b/crates/ruff/src/rules/perflint/rules/mod.rs index 4af80c1432..690b0fc1fe 100644 --- a/crates/ruff/src/rules/perflint/rules/mod.rs +++ b/crates/ruff/src/rules/perflint/rules/mod.rs @@ -1,7 +1,11 @@ pub(crate) use incorrect_dict_iterator::*; +pub(crate) use manual_list_comprehension::*; +pub(crate) use manual_list_copy::*; pub(crate) use try_except_in_loop::*; pub(crate) use unnecessary_list_cast::*; mod incorrect_dict_iterator; +mod manual_list_comprehension; +mod manual_list_copy; mod try_except_in_loop; 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 index 900b22cc52..670256d10a 100644 --- a/crates/ruff/src/rules/perflint/rules/unnecessary_list_cast.rs +++ b/crates/ruff/src/rules/perflint/rules/unnecessary_list_cast.rs @@ -19,6 +19,10 @@ use crate::registry::AsRule; /// Removing the `list()` call will not change the behavior of the code, but /// may improve performance. /// +/// Note that, as with all `perflint` rules, this is only intended as a +/// micro-optimization, and will have a negligible impact on performance in +/// most cases. +/// /// ## Example /// ```python /// items = (1, 2, 3) diff --git a/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF401_PERF401.py.snap b/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF401_PERF401.py.snap new file mode 100644 index 0000000000..e59eae4adc --- /dev/null +++ b/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF401_PERF401.py.snap @@ -0,0 +1,22 @@ +--- +source: crates/ruff/src/rules/perflint/mod.rs +--- +PERF401.py:6:13: PERF401 Use a list comprehension to create a new filtered list + | +4 | for i in items: +5 | if i % 2: +6 | result.append(i) # PERF401 + | ^^^^^^^^^^^^^^^^ PERF401 + | + +PERF401.py:14:13: PERF401 Use a list comprehension to create a new filtered list + | +12 | for i in items: +13 | if i % 2: +14 | result.append(i) # PERF401 + | ^^^^^^^^^^^^^^^^ PERF401 +15 | elif i % 2: +16 | result.append(i) # PERF401 + | + + diff --git a/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF402_PERF402.py.snap b/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF402_PERF402.py.snap new file mode 100644 index 0000000000..55cd69db8b --- /dev/null +++ b/crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF402_PERF402.py.snap @@ -0,0 +1,20 @@ +--- +source: crates/ruff/src/rules/perflint/mod.rs +--- +PERF402.py:5:9: PERF402 Use `list` or `list.copy` to create a copy of a list + | +3 | result = [] +4 | for i in items: +5 | result.append(i) # PERF402 + | ^^^^^^^^^^^^^^^^ PERF402 + | + +PERF402.py:12:9: PERF402 Use `list` or `list.copy` to create a copy of a list + | +10 | result = [] +11 | for i in items: +12 | result.insert(0, i) # PERF402 + | ^^^^^^^^^^^^^^^^^^^ PERF402 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 8e7d37f7e5..785fac6520 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2093,6 +2093,10 @@ "PERF2", "PERF20", "PERF203", + "PERF4", + "PERF40", + "PERF401", + "PERF402", "PGH", "PGH0", "PGH00", diff --git a/scripts/check_docs_formatted.py b/scripts/check_docs_formatted.py index be2ef93a46..f10e38cb0e 100755 --- a/scripts/check_docs_formatted.py +++ b/scripts/check_docs_formatted.py @@ -186,10 +186,7 @@ def main(argv: Sequence[str] | None = None) -> int: generate_docs() # Get static docs - static_docs = [] - for file in os.listdir("docs"): - if file.endswith(".md"): - static_docs.append(Path("docs") / file) + static_docs = [Path("docs") / f for f in os.listdir("docs") if f.endswith(".md")] # Check rules generated if not Path("docs/rules").exists(): @@ -197,10 +194,9 @@ def main(argv: Sequence[str] | None = None) -> int: return 1 # Get generated rules - generated_docs = [] - for file in os.listdir("docs/rules"): - if file.endswith(".md"): - generated_docs.append(Path("docs/rules") / file) + generated_docs = [ + Path("docs/rules") / f for f in os.listdir("docs/rules") if f.endswith(".md") + ] if len(generated_docs) == 0: print("Please generate rules first.") diff --git a/scripts/update_ambiguous_characters.py b/scripts/update_ambiguous_characters.py index 27a06fd039..cf165af585 100644 --- a/scripts/update_ambiguous_characters.py +++ b/scripts/update_ambiguous_characters.py @@ -45,9 +45,7 @@ def format_confusables_rs(raw_data: dict[str, list[int]]) -> str: for i in range(0, len(items), 2): flattened_items.add((items[i], items[i + 1])) - tuples = [] - for left, right in sorted(flattened_items): - tuples.append(f" {left}u32 => {right},\n") + tuples = [f" {left}u32 => {right},\n" for left, right in sorted(flattened_items)] print(f"{len(tuples)} confusable tuples.")