[flake8-pie] Unnecessary list comprehension, with autofix (PIE802) (#3149)

crates/ruff/resources/test/fixtures/flake8_pie/PIE802.py

@@ -0,0 +1,10 @@
+# no error
+all((x.id for x in bar))
+all(x.id for x in bar)
+all(x.id for x in bar)
+any(x.id for x in bar)
+any({x.id for x in bar})
+
+# PIE 802
+any([x.id for x in bar])
+all([x.id for x in bar])

crates/ruff/src/checkers/ast.rs

@@ -2490,6 +2490,13 @@ where
                 if self.settings.rules.enabled(&Rule::UnnecessaryDictKwargs) {
                     flake8_pie::rules::no_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) {

crates/ruff/src/codes.rs

@@ -516,6 +516,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Flake8Pie, "794") => Rule::DupeClassFieldDefinitions,
         (Flake8Pie, "796") => Rule::PreferUniqueEnums,
         (Flake8Pie, "800") => Rule::UnnecessarySpread,
+        (Flake8Pie, "802") => Rule::UnnecessaryComprehensionAnyAll,
         (Flake8Pie, "804") => Rule::UnnecessaryDictKwargs,
         (Flake8Pie, "807") => Rule::PreferListBuiltin,
         (Flake8Pie, "810") => Rule::SingleStartsEndsWith,

crates/ruff/src/registry.rs

@@ -491,6 +491,7 @@ ruff_macros::register_rules!(
     rules::flake8_pie::rules::UnnecessaryDictKwargs,
     rules::flake8_pie::rules::PreferListBuiltin,
     rules::flake8_pie::rules::SingleStartsEndsWith,
+    rules::flake8_pie::rules::UnnecessaryComprehensionAnyAll,
     // flake8-commas
     rules::flake8_commas::rules::TrailingCommaMissing,
     rules::flake8_commas::rules::TrailingCommaOnBareTupleProhibited,

crates/ruff/src/rules/flake8_pie/fixes.rs

@@ -0,0 +1,45 @@
+use anyhow::{bail, Result};
+use libcst_native::{Codegen, CodegenState, Expression, GeneratorExp};
+
+use crate::ast::types::Range;
+use crate::cst::matchers::{match_expr, match_module};
+use crate::fix::Fix;
+use crate::source_code::{Locator, Stylist};
+
+/// (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<Fix> {
+    // Expr(ListComp) -> Expr(GeneratorExp)
+    let module_text = locator.slice(&Range::from_located(expr));
+    let mut tree = match_module(module_text)?;
+    let mut body = match_expr(&mut tree)?;
+
+    let Expression::ListComp(list_comp) = &body.value else {
+        bail!(
+            "Expected Expression::ListComp"
+        );
+    };
+
+    body.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(),
+    }));
+
+    let mut state = CodegenState {
+        default_newline: stylist.line_ending(),
+        default_indent: stylist.indentation(),
+        ..CodegenState::default()
+    };
+    tree.codegen(&mut state);
+
+    Ok(Fix::replacement(
+        state.to_string(),
+        expr.location,
+        expr.end_location.unwrap(),
+    ))
+}

crates/ruff/src/rules/flake8_pie/mod.rs

@@ -1,4 +1,5 @@
 //! Rules from [flake8-pie](https://pypi.org/project/flake8-pie/).
+mod fixes;
 pub(crate) mod rules;
 
 #[cfg(test)]
@@ -20,6 +21,7 @@ mod tests {
     #[test_case(Rule::UnnecessarySpread, Path::new("PIE800.py"); "PIE800")]
     #[test_case(Rule::PreferListBuiltin, Path::new("PIE807.py"); "PIE807")]
     #[test_case(Rule::PreferUniqueEnums, 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(

crates/ruff/src/rules/flake8_pie/rules.rs

@@ -15,6 +15,8 @@ use crate::message::Location;
 use crate::registry::Diagnostic;
 use crate::violation::{AlwaysAutofixableViolation, Violation};
 
+use super::fixes;
+
 define_violation!(
     pub struct UnnecessaryPass;
 );
@@ -58,6 +60,50 @@ impl Violation for PreferUniqueEnums {
     }
 }
 
+define_violation!(
+    /// ## 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):
+    ///
+    /// ```python
+    /// 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)
+    /// ```
+    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()
+    }
+}
+
 define_violation!(
     pub struct UnnecessarySpread;
 );
@@ -282,6 +328,39 @@ pub fn no_unnecessary_spread(checker: &mut Checker, keys: &[Option<Expr>], value
     }
 }
 
+/// PIE802
+pub fn unnecessary_comprehension_any_all(
+    checker: &mut Checker,
+    expr: &Expr,
+    func: &Expr,
+    args: &[Expr],
+) {
+    if let ExprKind::Name { id, .. } = &func.node {
+        if (id == "all" || id == "any") && args.len() == 1 {
+            if !checker.is_builtin(id) {
+                return;
+            }
+            if let ExprKind::ListComp { .. } = args[0].node {
+                let mut diagnostic =
+                    Diagnostic::new(UnnecessaryComprehensionAnyAll, Range::from_located(expr));
+                if checker.patch(diagnostic.kind.rule()) {
+                    match fixes::fix_unnecessary_comprehension_any_all(
+                        checker.locator,
+                        checker.stylist,
+                        &args[0],
+                    ) {
+                        Ok(fix) => {
+                            diagnostic.amend(fix);
+                        }
+                        Err(e) => error!("Failed to generate fix: {e}"),
+                    }
+                }
+                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 {

crates/ruff/src/rules/flake8_pie/snapshots/ruff__rules__flake8_pie__tests__PIE802_PIE802.py.snap

@@ -0,0 +1,39 @@
+---
+source: crates/ruff/src/rules/flake8_pie/mod.rs
+expression: diagnostics
+---
+- kind:
+    UnnecessaryComprehensionAnyAll: ~
+  location:
+    row: 9
+    column: 0
+  end_location:
+    row: 9
+    column: 24
+  fix:
+    content: x.id for x in bar
+    location:
+      row: 9
+      column: 4
+    end_location:
+      row: 9
+      column: 23
+  parent: ~
+- kind:
+    UnnecessaryComprehensionAnyAll: ~
+  location:
+    row: 10
+    column: 0
+  end_location:
+    row: 10
+    column: 24
+  fix:
+    content: x.id for x in bar
+    location:
+      row: 10
+      column: 4
+    end_location:
+      row: 10
+      column: 23
+  parent: ~
+

ruff.schema.json

@@ -1762,6 +1762,7 @@
         "PIE8",
         "PIE80",
         "PIE800",
+        "PIE802",
         "PIE804",
         "PIE807",
         "PIE81",