From c457752f362871b01234dc4df52a8ea3c605d341 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 13 Apr 2023 22:12:32 -0400 Subject: [PATCH] Redirect `PIE802` to `C419` (#3971) --- .../C419.py} | 2 +- crates/ruff/src/checkers/ast/mod.rs | 16 ++-- crates/ruff/src/codes.rs | 2 +- crates/ruff/src/registry.rs | 28 +++--- crates/ruff/src/rule_redirects.rs | 1 + .../src/rules/flake8_comprehensions/fixes.rs | 44 +++++++++ .../src/rules/flake8_comprehensions/mod.rs | 27 +++--- .../rules/flake8_comprehensions/rules/mod.rs | 4 + .../unnecessary_comprehension_any_all.rs | 94 ++++++++++++++++++ ...8_comprehensions__tests__C419_C419.py.snap | 91 ++++++++++++++++++ crates/ruff/src/rules/flake8_pie/fixes.rs | 50 ---------- crates/ruff/src/rules/flake8_pie/mod.rs | 2 - crates/ruff/src/rules/flake8_pie/rules.rs | 83 +--------------- ...__flake8_pie__tests__PIE802_PIE802.py.snap | 95 ------------------- ruff.schema.json | 2 +- 15 files changed, 275 insertions(+), 266 deletions(-) rename crates/ruff/resources/test/fixtures/{flake8_pie/PIE802.py => flake8_comprehensions/C419.py} (98%) create mode 100644 crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension_any_all.rs create mode 100644 crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C419_C419.py.snap delete mode 100644 crates/ruff/src/rules/flake8_pie/fixes.rs delete mode 100644 crates/ruff/src/rules/flake8_pie/snapshots/ruff__rules__flake8_pie__tests__PIE802_PIE802.py.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_pie/PIE802.py b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C419.py similarity index 98% rename from crates/ruff/resources/test/fixtures/flake8_pie/PIE802.py rename to crates/ruff/resources/test/fixtures/flake8_comprehensions/C419.py index cf87437e25..74aea42a53 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pie/PIE802.py +++ b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C419.py @@ -1,4 +1,3 @@ -# PIE802 any([x.id for x in bar]) all([x.id for x in bar]) any( # first comment @@ -15,5 +14,6 @@ all(x.id for x in bar) any(x.id for x in bar) all((x.id for x in bar)) + async def f() -> bool: return all([await use_greeting(greeting) for greeting in await greetings()]) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 4e1427087b..d629300656 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2585,13 +2585,6 @@ where if self.settings.rules.enabled(Rule::UnnecessaryDictKwargs) { flake8_pie::rules::unnecessary_dict_kwargs(self, expr, keywords); } - if self - .settings - .rules - .enabled(Rule::UnnecessaryComprehensionAnyAll) - { - flake8_pie::rules::unnecessary_comprehension_any_all(self, expr, func, args); - } // flake8-bandit if self.settings.rules.enabled(Rule::ExecBuiltin) { @@ -2792,6 +2785,15 @@ where args, ); } + if self + .settings + .rules + .enabled(Rule::UnnecessaryComprehensionAnyAll) + { + flake8_comprehensions::rules::unnecessary_comprehension_any_all( + self, expr, func, args, keywords, + ); + } // flake8-boolean-trap if self diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 9296a468ee..c4072ceca2 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -264,6 +264,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Flake8Comprehensions, "16") => Rule::UnnecessaryComprehension, (Flake8Comprehensions, "17") => Rule::UnnecessaryMap, (Flake8Comprehensions, "18") => Rule::UnnecessaryLiteralWithinDictCall, + (Flake8Comprehensions, "19") => Rule::UnnecessaryComprehensionAnyAll, // flake8-debugger (Flake8Debugger, "0") => Rule::Debugger, @@ -617,7 +618,6 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Flake8Pie, "794") => Rule::DuplicateClassFieldDefinition, (Flake8Pie, "796") => Rule::NonUniqueEnums, (Flake8Pie, "800") => Rule::UnnecessarySpread, - (Flake8Pie, "802") => Rule::UnnecessaryComprehensionAnyAll, (Flake8Pie, "804") => Rule::UnnecessaryDictKwargs, (Flake8Pie, "807") => Rule::ReimplementedListBuiltin, (Flake8Pie, "810") => Rule::MultipleStartsEndsWith, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 8132097632..9fda460a5f 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -228,23 +228,24 @@ ruff_macros::register_rules!( // flake8-blind-except rules::flake8_blind_except::rules::BlindExcept, // flake8-comprehensions + rules::flake8_comprehensions::rules::UnnecessaryCallAroundSorted, + rules::flake8_comprehensions::rules::UnnecessaryCollectionCall, + rules::flake8_comprehensions::rules::UnnecessaryComprehension, + rules::flake8_comprehensions::rules::UnnecessaryComprehensionAnyAll, + rules::flake8_comprehensions::rules::UnnecessaryDoubleCastOrProcess, + rules::flake8_comprehensions::rules::UnnecessaryGeneratorDict, rules::flake8_comprehensions::rules::UnnecessaryGeneratorList, rules::flake8_comprehensions::rules::UnnecessaryGeneratorSet, - rules::flake8_comprehensions::rules::UnnecessaryGeneratorDict, - rules::flake8_comprehensions::rules::UnnecessaryListComprehensionSet, - rules::flake8_comprehensions::rules::UnnecessaryListComprehensionDict, - rules::flake8_comprehensions::rules::UnnecessaryLiteralSet, - rules::flake8_comprehensions::rules::UnnecessaryLiteralDict, - rules::flake8_comprehensions::rules::UnnecessaryCollectionCall, - rules::flake8_comprehensions::rules::UnnecessaryLiteralWithinTupleCall, - rules::flake8_comprehensions::rules::UnnecessaryLiteralWithinListCall, rules::flake8_comprehensions::rules::UnnecessaryListCall, - rules::flake8_comprehensions::rules::UnnecessaryCallAroundSorted, - rules::flake8_comprehensions::rules::UnnecessaryDoubleCastOrProcess, - rules::flake8_comprehensions::rules::UnnecessarySubscriptReversal, - rules::flake8_comprehensions::rules::UnnecessaryComprehension, - rules::flake8_comprehensions::rules::UnnecessaryMap, + rules::flake8_comprehensions::rules::UnnecessaryListComprehensionDict, + rules::flake8_comprehensions::rules::UnnecessaryListComprehensionSet, + rules::flake8_comprehensions::rules::UnnecessaryLiteralDict, + rules::flake8_comprehensions::rules::UnnecessaryLiteralSet, rules::flake8_comprehensions::rules::UnnecessaryLiteralWithinDictCall, + rules::flake8_comprehensions::rules::UnnecessaryLiteralWithinListCall, + rules::flake8_comprehensions::rules::UnnecessaryLiteralWithinTupleCall, + rules::flake8_comprehensions::rules::UnnecessaryMap, + rules::flake8_comprehensions::rules::UnnecessarySubscriptReversal, // flake8-debugger rules::flake8_debugger::rules::Debugger, // mccabe @@ -570,7 +571,6 @@ ruff_macros::register_rules!( rules::flake8_pie::rules::UnnecessaryDictKwargs, rules::flake8_pie::rules::ReimplementedListBuiltin, rules::flake8_pie::rules::MultipleStartsEndsWith, - rules::flake8_pie::rules::UnnecessaryComprehensionAnyAll, // flake8-commas rules::flake8_commas::rules::MissingTrailingComma, rules::flake8_commas::rules::TrailingCommaOnBareTuple, diff --git a/crates/ruff/src/rule_redirects.rs b/crates/ruff/src/rule_redirects.rs index a7fce22eec..4749b64cca 100644 --- a/crates/ruff/src/rule_redirects.rs +++ b/crates/ruff/src/rule_redirects.rs @@ -92,5 +92,6 @@ static REDIRECTS: Lazy> = Lazy::new(|| { ("TYP001", "TCH001"), // TODO(charlie): Remove by 2023-06-01. ("RUF004", "B026"), + ("PIE802", "C419"), ]) }); diff --git a/crates/ruff/src/rules/flake8_comprehensions/fixes.rs b/crates/ruff/src/rules/flake8_comprehensions/fixes.rs index 4cb0449fdb..ad99e3484b 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/fixes.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/fixes.rs @@ -1201,3 +1201,47 @@ pub fn fix_unnecessary_literal_within_dict_call( expr.end_location.unwrap(), )) } + +/// (C419) Convert `[i for i in a]` into `i for i in a` +pub fn fix_unnecessary_comprehension_any_all( + locator: &Locator, + stylist: &Stylist, + expr: &rustpython_parser::ast::Expr, +) -> Result { + // Expr(ListComp) -> Expr(GeneratorExp) + let module_text = locator.slice(expr); + let mut tree = match_module(module_text)?; + let body = match_expr(&mut tree)?; + let call = match_call(body)?; + + let Expression::ListComp(list_comp) = &call.args[0].value else { + bail!( + "Expected Expression::ListComp" + ); + }; + + call.args[0].value = Expression::GeneratorExp(Box::new(GeneratorExp { + elt: list_comp.elt.clone(), + for_in: list_comp.for_in.clone(), + lpar: list_comp.lpar.clone(), + rpar: list_comp.rpar.clone(), + })); + + if let Some(comma) = &call.args[0].comma { + call.args[0].whitespace_after_arg = comma.whitespace_after.clone(); + call.args[0].comma = None; + } + + let mut state = CodegenState { + default_newline: &stylist.line_ending(), + default_indent: stylist.indentation(), + ..CodegenState::default() + }; + tree.codegen(&mut state); + + Ok(Edit::replacement( + state.to_string(), + expr.location, + expr.end_location.unwrap(), + )) +} diff --git a/crates/ruff/src/rules/flake8_comprehensions/mod.rs b/crates/ruff/src/rules/flake8_comprehensions/mod.rs index a2463c2a7c..02bd63ddb2 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/mod.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/mod.rs @@ -16,23 +16,24 @@ mod tests { use crate::settings::Settings; use crate::test::test_path; + #[test_case(Rule::UnnecessaryCallAroundSorted, Path::new("C413.py"); "C413")] + #[test_case(Rule::UnnecessaryCollectionCall, Path::new("C408.py"); "C408")] + #[test_case(Rule::UnnecessaryComprehension, Path::new("C416.py"); "C416")] + #[test_case(Rule::UnnecessaryComprehensionAnyAll, Path::new("C419.py"); "C419")] + #[test_case(Rule::UnnecessaryDoubleCastOrProcess, Path::new("C414.py"); "C414")] + #[test_case(Rule::UnnecessaryGeneratorDict, Path::new("C402.py"); "C402")] #[test_case(Rule::UnnecessaryGeneratorList, Path::new("C400.py"); "C400")] #[test_case(Rule::UnnecessaryGeneratorSet, Path::new("C401.py"); "C401")] - #[test_case(Rule::UnnecessaryGeneratorDict, Path::new("C402.py"); "C402")] - #[test_case(Rule::UnnecessaryListComprehensionSet, Path::new("C403.py"); "C403")] - #[test_case(Rule::UnnecessaryListComprehensionDict, Path::new("C404.py"); "C404")] - #[test_case(Rule::UnnecessaryLiteralSet, Path::new("C405.py"); "C405")] - #[test_case(Rule::UnnecessaryLiteralDict, Path::new("C406.py"); "C406")] - #[test_case(Rule::UnnecessaryCollectionCall, Path::new("C408.py"); "C408")] - #[test_case(Rule::UnnecessaryLiteralWithinTupleCall, Path::new("C409.py"); "C409")] - #[test_case(Rule::UnnecessaryLiteralWithinListCall, Path::new("C410.py"); "C410")] #[test_case(Rule::UnnecessaryListCall, Path::new("C411.py"); "C411")] - #[test_case(Rule::UnnecessaryCallAroundSorted, Path::new("C413.py"); "C413")] - #[test_case(Rule::UnnecessaryDoubleCastOrProcess, Path::new("C414.py"); "C414")] - #[test_case(Rule::UnnecessarySubscriptReversal, Path::new("C415.py"); "C415")] - #[test_case(Rule::UnnecessaryComprehension, Path::new("C416.py"); "C416")] - #[test_case(Rule::UnnecessaryMap, Path::new("C417.py"); "C417")] + #[test_case(Rule::UnnecessaryListComprehensionDict, Path::new("C404.py"); "C404")] + #[test_case(Rule::UnnecessaryListComprehensionSet, Path::new("C403.py"); "C403")] + #[test_case(Rule::UnnecessaryLiteralDict, Path::new("C406.py"); "C406")] + #[test_case(Rule::UnnecessaryLiteralSet, Path::new("C405.py"); "C405")] #[test_case(Rule::UnnecessaryLiteralWithinDictCall, Path::new("C418.py"); "C418")] + #[test_case(Rule::UnnecessaryLiteralWithinListCall, Path::new("C410.py"); "C410")] + #[test_case(Rule::UnnecessaryLiteralWithinTupleCall, Path::new("C409.py"); "C409")] + #[test_case(Rule::UnnecessaryMap, Path::new("C417.py"); "C417")] + #[test_case(Rule::UnnecessarySubscriptReversal, Path::new("C415.py"); "C415")] 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/flake8_comprehensions/rules/mod.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/mod.rs index 54012fd85b..d7418e3a66 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/mod.rs @@ -5,6 +5,9 @@ pub use unnecessary_collection_call::{unnecessary_collection_call, UnnecessaryCo pub use unnecessary_comprehension::{ unnecessary_dict_comprehension, unnecessary_list_set_comprehension, UnnecessaryComprehension, }; +pub use unnecessary_comprehension_any_all::{ + unnecessary_comprehension_any_all, UnnecessaryComprehensionAnyAll, +}; pub use unnecessary_double_cast_or_process::{ unnecessary_double_cast_or_process, UnnecessaryDoubleCastOrProcess, }; @@ -38,6 +41,7 @@ mod helpers; mod unnecessary_call_around_sorted; mod unnecessary_collection_call; mod unnecessary_comprehension; +mod unnecessary_comprehension_any_all; mod unnecessary_double_cast_or_process; mod unnecessary_generator_dict; mod unnecessary_generator_list; diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension_any_all.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension_any_all.rs new file mode 100644 index 0000000000..92856ca2d6 --- /dev/null +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension_any_all.rs @@ -0,0 +1,94 @@ +use rustpython_parser::ast::{Expr, ExprKind, Keyword}; + +use ruff_diagnostics::AlwaysAutofixableViolation; +use ruff_diagnostics::Diagnostic; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::any_over_expr; +use ruff_python_ast::types::Range; + +use crate::checkers::ast::Checker; +use crate::registry::AsRule; +use crate::rules::flake8_comprehensions::fixes; + +/// ## What it does +/// Checks for unnecessary list comprehensions passed to `any` and `all`. +/// +/// ## Why is this bad? +/// `any` and `all` take any iterators, including generators. Converting a generator to a list +/// by way of a list comprehension is unnecessary and reduces performance due to the +/// overhead of creating the list. +/// +/// For example, compare the performance of `all` with a list comprehension against that +/// of a generator (~40x faster here): +/// +/// ```console +/// In [1]: %timeit all([i for i in range(1000)]) +/// 8.14 µs ± 25.4 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each) +/// +/// In [2]: %timeit all(i for i in range(1000)) +/// 212 ns ± 0.892 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each) +/// ``` +/// +/// ## Examples +/// ```python +/// any([x.id for x in bar]) +/// all([x.id for x in bar]) +/// ``` +/// +/// Use instead: +/// ```python +/// any(x.id for x in bar) +/// all(x.id for x in bar) +/// ``` +#[violation] +pub struct UnnecessaryComprehensionAnyAll; + +impl AlwaysAutofixableViolation for UnnecessaryComprehensionAnyAll { + #[derive_message_formats] + fn message(&self) -> String { + format!("Unnecessary list comprehension.") + } + + fn autofix_title(&self) -> String { + "Remove unnecessary list comprehension".to_string() + } +} + +/// C419 +pub fn unnecessary_comprehension_any_all( + checker: &mut Checker, + expr: &Expr, + func: &Expr, + args: &[Expr], + keywords: &[Keyword], +) { + if !keywords.is_empty() { + return; + } + let ExprKind::Name { id, .. } = &func.node else { + return; + }; + if (matches!(id.as_str(), "all" | "any")) && args.len() == 1 { + let (ExprKind::ListComp { elt, .. } | ExprKind::SetComp { elt, .. }) = &args[0].node else { + return; + }; + if is_async_generator(elt) { + return; + } + if !checker.ctx.is_builtin(id) { + return; + } + let mut diagnostic = Diagnostic::new(UnnecessaryComprehensionAnyAll, Range::from(&args[0])); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.try_set_fix(|| { + fixes::fix_unnecessary_comprehension_any_all(checker.locator, checker.stylist, expr) + }); + } + checker.diagnostics.push(diagnostic); + } +} + +/// Return `true` if the `Expr` contains an `await` expression. +fn is_async_generator(expr: &Expr) -> bool { + any_over_expr(expr, &|expr| matches!(expr.node, ExprKind::Await { .. })) +} diff --git a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C419_C419.py.snap b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C419_C419.py.snap new file mode 100644 index 0000000000..47df13091c --- /dev/null +++ b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C419_C419.py.snap @@ -0,0 +1,91 @@ +--- +source: crates/ruff/src/rules/flake8_comprehensions/mod.rs +--- +C419.py:1:5: C419 [*] Unnecessary list comprehension. + | +1 | any([x.id for x in bar]) + | ^^^^^^^^^^^^^^^^^^^ C419 +2 | all([x.id for x in bar]) +3 | any( # first comment + | + = help: Remove unnecessary list comprehension + +ℹ Suggested fix +1 |-any([x.id for x in bar]) + 1 |+any(x.id for x in bar) +2 2 | all([x.id for x in bar]) +3 3 | any( # first comment +4 4 | [x.id for x in bar], # second comment + +C419.py:2:5: C419 [*] Unnecessary list comprehension. + | +2 | any([x.id for x in bar]) +3 | all([x.id for x in bar]) + | ^^^^^^^^^^^^^^^^^^^ C419 +4 | any( # first comment +5 | [x.id for x in bar], # second comment + | + = help: Remove unnecessary list comprehension + +ℹ Suggested fix +1 1 | any([x.id for x in bar]) +2 |-all([x.id for x in bar]) + 2 |+all(x.id for x in bar) +3 3 | any( # first comment +4 4 | [x.id for x in bar], # second comment +5 5 | ) # third comment + +C419.py:4:5: C419 [*] Unnecessary list comprehension. + | +4 | all([x.id for x in bar]) +5 | any( # first comment +6 | [x.id for x in bar], # second comment + | ^^^^^^^^^^^^^^^^^^^ C419 +7 | ) # third comment +8 | all( # first comment + | + = help: Remove unnecessary list comprehension + +ℹ Suggested fix +1 1 | any([x.id for x in bar]) +2 2 | all([x.id for x in bar]) +3 3 | any( # first comment +4 |- [x.id for x in bar], # second comment + 4 |+ x.id for x in bar # second comment +5 5 | ) # third comment +6 6 | all( # first comment +7 7 | [x.id for x in bar], # second comment + +C419.py:7:5: C419 [*] Unnecessary list comprehension. + | + 7 | ) # third comment + 8 | all( # first comment + 9 | [x.id for x in bar], # second comment + | ^^^^^^^^^^^^^^^^^^^ C419 +10 | ) # third comment +11 | any({x.id for x in bar}) + | + = help: Remove unnecessary list comprehension + +ℹ Suggested fix +4 4 | [x.id for x in bar], # second comment +5 5 | ) # third comment +6 6 | all( # first comment +7 |- [x.id for x in bar], # second comment + 7 |+ x.id for x in bar # second comment +8 8 | ) # third comment +9 9 | any({x.id for x in bar}) +10 10 | + +C419.py:9:5: C419 [*] Unnecessary list comprehension. + | + 9 | [x.id for x in bar], # second comment +10 | ) # third comment +11 | any({x.id for x in bar}) + | ^^^^^^^^^^^^^^^^^^^ C419 +12 | +13 | # OK + | + = help: Remove unnecessary list comprehension + + diff --git a/crates/ruff/src/rules/flake8_pie/fixes.rs b/crates/ruff/src/rules/flake8_pie/fixes.rs deleted file mode 100644 index bc7615a53b..0000000000 --- a/crates/ruff/src/rules/flake8_pie/fixes.rs +++ /dev/null @@ -1,50 +0,0 @@ -use anyhow::{bail, Result}; -use libcst_native::{Codegen, CodegenState, Expression, GeneratorExp}; - -use ruff_diagnostics::Edit; -use ruff_python_ast::source_code::{Locator, Stylist}; - -use crate::cst::matchers::{match_call, match_expression}; - -/// (PIE802) Convert `[i for i in a]` into `i for i in a` -pub fn fix_unnecessary_comprehension_any_all( - locator: &Locator, - stylist: &Stylist, - expr: &rustpython_parser::ast::Expr, -) -> Result { - // Expr(ListComp) -> Expr(GeneratorExp) - let expression_text = locator.slice(expr); - let mut tree = match_expression(expression_text)?; - let call = match_call(&mut tree)?; - - let Expression::ListComp(list_comp) = &call.args[0].value else { - bail!( - "Expected Expression::ListComp" - ); - }; - - call.args[0].value = Expression::GeneratorExp(Box::new(GeneratorExp { - elt: list_comp.elt.clone(), - for_in: list_comp.for_in.clone(), - lpar: list_comp.lpar.clone(), - rpar: list_comp.rpar.clone(), - })); - - if let Some(comma) = &call.args[0].comma { - call.args[0].whitespace_after_arg = comma.whitespace_after.clone(); - call.args[0].comma = None; - } - - let mut state = CodegenState { - default_newline: &stylist.line_ending(), - default_indent: stylist.indentation(), - ..CodegenState::default() - }; - tree.codegen(&mut state); - - Ok(Edit::replacement( - state.to_string(), - expr.location, - expr.end_location.unwrap(), - )) -} diff --git a/crates/ruff/src/rules/flake8_pie/mod.rs b/crates/ruff/src/rules/flake8_pie/mod.rs index 291e7ba85b..67925fca2d 100644 --- a/crates/ruff/src/rules/flake8_pie/mod.rs +++ b/crates/ruff/src/rules/flake8_pie/mod.rs @@ -1,5 +1,4 @@ //! Rules from [flake8-pie](https://pypi.org/project/flake8-pie/). -mod fixes; pub(crate) mod rules; #[cfg(test)] @@ -21,7 +20,6 @@ mod tests { #[test_case(Rule::UnnecessarySpread, Path::new("PIE800.py"); "PIE800")] #[test_case(Rule::ReimplementedListBuiltin, Path::new("PIE807.py"); "PIE807")] #[test_case(Rule::NonUniqueEnums, Path::new("PIE796.py"); "PIE796")] - #[test_case(Rule::UnnecessaryComprehensionAnyAll, Path::new("PIE802.py"); "PIE802")] 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/flake8_pie/rules.rs b/crates/ruff/src/rules/flake8_pie/rules.rs index 513b9d8eb5..1eb603ab92 100644 --- a/crates/ruff/src/rules/flake8_pie/rules.rs +++ b/crates/ruff/src/rules/flake8_pie/rules.rs @@ -12,7 +12,7 @@ use ruff_diagnostics::{AlwaysAutofixableViolation, Violation}; use ruff_diagnostics::{Diagnostic, Edit}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::comparable::ComparableExpr; -use ruff_python_ast::helpers::{any_over_expr, create_expr, match_trailing_comment, unparse_expr}; +use ruff_python_ast::helpers::{create_expr, match_trailing_comment, unparse_expr}; use ruff_python_ast::types::{Range, RefEquality}; use ruff_python_stdlib::identifiers::is_identifier; @@ -21,8 +21,6 @@ use crate::checkers::ast::Checker; use crate::message::Location; use crate::registry::AsRule; -use super::fixes; - #[violation] pub struct UnnecessaryPass; @@ -66,50 +64,6 @@ impl Violation for NonUniqueEnums { } } -/// ## What it does -/// Checks for unnecessary list comprehensions passed to `any` and `all`. -/// -/// ## Why is this bad? -/// `any` and `all` take any iterators, including generators. Converting a generator to a list -/// by way of a list comprehension is unnecessary and reduces performance due to the -/// overhead of creating the list. -/// -/// For example, compare the performance of `all` with a list comprehension against that -/// of a generator (~40x faster here): -/// -/// ```console -/// In [1]: %timeit all([i for i in range(1000)]) -/// 8.14 µs ± 25.4 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each) -/// -/// In [2]: %timeit all(i for i in range(1000)) -/// 212 ns ± 0.892 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each) -/// ``` -/// -/// ## Examples -/// ```python -/// any([x.id for x in bar]) -/// all([x.id for x in bar]) -/// ``` -/// -/// Use instead: -/// ```python -/// any(x.id for x in bar) -/// all(x.id for x in bar) -/// ``` -#[violation] -pub struct UnnecessaryComprehensionAnyAll; - -impl AlwaysAutofixableViolation for UnnecessaryComprehensionAnyAll { - #[derive_message_formats] - fn message(&self) -> String { - format!("Unnecessary list comprehension.") - } - - fn autofix_title(&self) -> String { - "Remove unnecessary list comprehension".to_string() - } -} - #[violation] pub struct UnnecessarySpread; @@ -331,41 +285,6 @@ pub fn unnecessary_spread(checker: &mut Checker, keys: &[Option], values: } } -/// Return `true` if the `Expr` contains an `await` expression. -fn is_async_generator(expr: &Expr) -> bool { - any_over_expr(expr, &|expr| matches!(expr.node, ExprKind::Await { .. })) -} - -/// PIE802 -pub fn unnecessary_comprehension_any_all( - checker: &mut Checker, - expr: &Expr, - func: &Expr, - args: &[Expr], -) { - let ExprKind::Name { id, .. } = &func.node else { - return; - }; - if (matches!(id.as_str(), "all" | "any")) && args.len() == 1 { - let (ExprKind::ListComp { elt, .. } | ExprKind::SetComp { elt, .. }) = &args[0].node else { - return; - }; - if is_async_generator(elt) { - return; - } - if !checker.ctx.is_builtin(id) { - return; - } - let mut diagnostic = Diagnostic::new(UnnecessaryComprehensionAnyAll, Range::from(&args[0])); - if checker.patch(diagnostic.kind.rule()) { - diagnostic.try_set_fix(|| { - fixes::fix_unnecessary_comprehension_any_all(checker.locator, checker.stylist, expr) - }); - } - checker.diagnostics.push(diagnostic); - } -} - /// Return `true` if a key is a valid keyword argument name. fn is_valid_kwarg_name(key: &Expr) -> bool { if let ExprKind::Constant { diff --git a/crates/ruff/src/rules/flake8_pie/snapshots/ruff__rules__flake8_pie__tests__PIE802_PIE802.py.snap b/crates/ruff/src/rules/flake8_pie/snapshots/ruff__rules__flake8_pie__tests__PIE802_PIE802.py.snap deleted file mode 100644 index 47ad306ee2..0000000000 --- a/crates/ruff/src/rules/flake8_pie/snapshots/ruff__rules__flake8_pie__tests__PIE802_PIE802.py.snap +++ /dev/null @@ -1,95 +0,0 @@ ---- -source: crates/ruff/src/rules/flake8_pie/mod.rs ---- -PIE802.py:2:5: PIE802 [*] Unnecessary list comprehension. - | -2 | # PIE802 -3 | any([x.id for x in bar]) - | ^^^^^^^^^^^^^^^^^^^ PIE802 -4 | all([x.id for x in bar]) -5 | any( # first comment - | - = help: Remove unnecessary list comprehension - -ℹ Suggested fix -1 1 | # PIE802 -2 |-any([x.id for x in bar]) - 2 |+any(x.id for x in bar) -3 3 | all([x.id for x in bar]) -4 4 | any( # first comment -5 5 | [x.id for x in bar], # second comment - -PIE802.py:3:5: PIE802 [*] Unnecessary list comprehension. - | -3 | # PIE802 -4 | any([x.id for x in bar]) -5 | all([x.id for x in bar]) - | ^^^^^^^^^^^^^^^^^^^ PIE802 -6 | any( # first comment -7 | [x.id for x in bar], # second comment - | - = help: Remove unnecessary list comprehension - -ℹ Suggested fix -1 1 | # PIE802 -2 2 | any([x.id for x in bar]) -3 |-all([x.id for x in bar]) - 3 |+all(x.id for x in bar) -4 4 | any( # first comment -5 5 | [x.id for x in bar], # second comment -6 6 | ) # third comment - -PIE802.py:5:5: PIE802 [*] Unnecessary list comprehension. - | -5 | all([x.id for x in bar]) -6 | any( # first comment -7 | [x.id for x in bar], # second comment - | ^^^^^^^^^^^^^^^^^^^ PIE802 -8 | ) # third comment -9 | all( # first comment - | - = help: Remove unnecessary list comprehension - -ℹ Suggested fix -2 2 | any([x.id for x in bar]) -3 3 | all([x.id for x in bar]) -4 4 | any( # first comment -5 |- [x.id for x in bar], # second comment - 5 |+ x.id for x in bar # second comment -6 6 | ) # third comment -7 7 | all( # first comment -8 8 | [x.id for x in bar], # second comment - -PIE802.py:8:5: PIE802 [*] Unnecessary list comprehension. - | - 8 | ) # third comment - 9 | all( # first comment -10 | [x.id for x in bar], # second comment - | ^^^^^^^^^^^^^^^^^^^ PIE802 -11 | ) # third comment -12 | any({x.id for x in bar}) - | - = help: Remove unnecessary list comprehension - -ℹ Suggested fix -5 5 | [x.id for x in bar], # second comment -6 6 | ) # third comment -7 7 | all( # first comment -8 |- [x.id for x in bar], # second comment - 8 |+ x.id for x in bar # second comment -9 9 | ) # third comment -10 10 | any({x.id for x in bar}) -11 11 | - -PIE802.py:10:5: PIE802 [*] Unnecessary list comprehension. - | -10 | [x.id for x in bar], # second comment -11 | ) # third comment -12 | any({x.id for x in bar}) - | ^^^^^^^^^^^^^^^^^^^ PIE802 -13 | -14 | # OK - | - = help: Remove unnecessary list comprehension - - diff --git a/ruff.schema.json b/ruff.schema.json index cd28d1b33b..37320b5cbf 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1605,6 +1605,7 @@ "C416", "C417", "C418", + "C419", "C9", "C90", "C901", @@ -1920,7 +1921,6 @@ "PIE8", "PIE80", "PIE800", - "PIE802", "PIE804", "PIE807", "PIE81",