[flake8_comprehensions] add sum/min/max to unnecessary comprehension check (C419)

crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C419.py

@@ -7,12 +7,18 @@
     [x.id for x in bar],  # second comment
 )  # third comment
 any({x.id for x in bar})
+sum([x.val for x in bar])
+min([x.val for x in bar])
+max([x.val for x in bar])
 
 # OK
 all(x.id for x in bar)
 all(x.id for x in bar)
 any(x.id for x in bar)
 all((x.id for x in bar))
+sum(x.val for x in bar)
+min(x.val for x in bar)
+max(x.val for x in bar)
 
 
 async def f() -> bool:

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -705,8 +705,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                     args,
                 );
             }
-            if checker.enabled(Rule::UnnecessaryComprehensionAnyAll) {
-                flake8_comprehensions::rules::unnecessary_comprehension_any_all(
+            if checker.enabled(Rule::UnnecessaryComprehensionInCall) {
+                flake8_comprehensions::rules::unnecessary_comprehension_in_call(
                     checker, expr, func, args, keywords,
                 );
             }

crates/ruff_linter/src/codes.rs

@@ -398,7 +398,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Comprehensions, "16") => (RuleGroup::Stable, rules::flake8_comprehensions::rules::UnnecessaryComprehension),
         (Flake8Comprehensions, "17") => (RuleGroup::Stable, rules::flake8_comprehensions::rules::UnnecessaryMap),
         (Flake8Comprehensions, "18") => (RuleGroup::Stable, rules::flake8_comprehensions::rules::UnnecessaryLiteralWithinDictCall),
-        (Flake8Comprehensions, "19") => (RuleGroup::Stable, rules::flake8_comprehensions::rules::UnnecessaryComprehensionAnyAll),
+        (Flake8Comprehensions, "19") => (RuleGroup::Stable, rules::flake8_comprehensions::rules::UnnecessaryComprehensionInCall),
 
         // flake8-debugger
         (Flake8Debugger, "0") => (RuleGroup::Stable, rules::flake8_debugger::rules::Debugger),

crates/ruff_linter/src/rules/flake8_comprehensions/fixes.rs

@@ -793,7 +793,7 @@ pub(crate) fn fix_unnecessary_map(
 }
 
 /// (C419) Convert `[i for i in a]` into `i for i in a`
-pub(crate) fn fix_unnecessary_comprehension_any_all(
+pub(crate) fn fix_unnecessary_comprehension_in_call(
     expr: &Expr,
     locator: &Locator,
     stylist: &Stylist,

crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs

@@ -18,7 +18,7 @@ mod tests {
     #[test_case(Rule::UnnecessaryCallAroundSorted, Path::new("C413.py"))]
     #[test_case(Rule::UnnecessaryCollectionCall, Path::new("C408.py"))]
     #[test_case(Rule::UnnecessaryComprehension, Path::new("C416.py"))]
-    #[test_case(Rule::UnnecessaryComprehensionAnyAll, Path::new("C419.py"))]
+    #[test_case(Rule::UnnecessaryComprehensionInCall, Path::new("C419.py"))]
     #[test_case(Rule::UnnecessaryDoubleCastOrProcess, Path::new("C414.py"))]
     #[test_case(Rule::UnnecessaryGeneratorDict, Path::new("C402.py"))]
     #[test_case(Rule::UnnecessaryGeneratorList, Path::new("C400.py"))]

crates/ruff_linter/src/rules/flake8_comprehensions/rules/mod.rs

@@ -1,7 +1,7 @@
 pub(crate) use unnecessary_call_around_sorted::*;
 pub(crate) use unnecessary_collection_call::*;
 pub(crate) use unnecessary_comprehension::*;
-pub(crate) use unnecessary_comprehension_any_all::*;
+pub(crate) use unnecessary_comprehension_in_call::*;
 pub(crate) use unnecessary_double_cast_or_process::*;
 pub(crate) use unnecessary_generator_dict::*;
 pub(crate) use unnecessary_generator_list::*;
@@ -21,7 +21,7 @@ mod helpers;
 mod unnecessary_call_around_sorted;
 mod unnecessary_collection_call;
 mod unnecessary_comprehension;
-mod unnecessary_comprehension_any_all;
+mod unnecessary_comprehension_in_call;
 mod unnecessary_double_cast_or_process;
 mod unnecessary_generator_dict;
 mod unnecessary_generator_list;

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_comprehension_any_all.rs → crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_comprehension_in_call.rs

@@ -11,15 +11,19 @@ use crate::checkers::ast::Checker;
 use crate::rules::flake8_comprehensions::fixes;
 
 /// ## What it does
-/// Checks for unnecessary list comprehensions passed to `any` and `all`.
+/// Checks for unnecessary list comprehensions passed to builtin functions that take an iterable.
 ///
 /// ## 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 requires iterating all values, even if `any`
-/// or `all` could short-circuit early.
+/// Many builtin functions (this rule currently covers `any`, `all`, `min`, `max`, and `sum`) take
+/// any iterable, including a generator. Converting a generator to a list by way of a list
+/// comprehension is unnecessary and wastes memory for large iterables by fully materializing a
+/// list rather than handling values one at a time.
 ///
-/// For example, compare the performance of `all` with a list comprehension against that
-/// of a generator in a case where an early short-circuit is possible (almost 40x faster):
+/// `any` and `all` can also short-circuit iteration, saving a lot of time. The unnecessary
+/// comprehension forces a full iteration of the input iterable, giving up the benefits of
+/// short-circuiting. For example, compare the performance of `all` with a list comprehension
+/// against that of a generator in a case where an early short-circuit is possible (almost 40x
+/// faster):
 ///
 /// ```console
 /// In [1]: %timeit all([i for i in range(1000)])
@@ -29,22 +33,28 @@ use crate::rules::flake8_comprehensions::fixes;
 /// 212 ns ± 0.892 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
 /// ```
 ///
-/// This performance difference is due to short-circuiting; if the entire iterable has to be
+/// This performance difference is due to short-circuiting. If the entire iterable has to be
 /// traversed, the comprehension version may even be a bit faster (list allocation overhead is not
-/// necessarily greater than generator overhead).
-///
-/// The generator version is more memory-efficient.
+/// necessarily greater than generator overhead). So applying this rule simplifies the code and
+/// will usually save memory, but in the absence of short-circuiting it may not improve (and may
+/// even slightly regress, though the difference will be small) performance.
 ///
 /// ## Examples
 /// ```python
 /// any([x.id for x in bar])
 /// all([x.id for x in bar])
+/// sum([x.val for x in bar])
+/// min([x.val for x in bar])
+/// max([x.val for x in bar])
 /// ```
 ///
 /// Use instead:
 /// ```python
 /// any(x.id for x in bar)
 /// all(x.id for x in bar)
+/// sum(x.val for x in bar)
+/// min(x.val for x in bar])
+/// max(x.val for x in bar)
 /// ```
 ///
 /// ## Fix safety
@@ -53,9 +63,9 @@ use crate::rules::flake8_comprehensions::fixes;
 /// rewriting some comprehensions.
 ///
 #[violation]
-pub struct UnnecessaryComprehensionAnyAll;
+pub struct UnnecessaryComprehensionInCall;
 
-impl Violation for UnnecessaryComprehensionAnyAll {
+impl Violation for UnnecessaryComprehensionInCall {
     const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
 
     #[derive_message_formats]
@@ -69,7 +79,7 @@ impl Violation for UnnecessaryComprehensionAnyAll {
 }
 
 /// C419
-pub(crate) fn unnecessary_comprehension_any_all(
+pub(crate) fn unnecessary_comprehension_in_call(
     checker: &mut Checker,
     expr: &Expr,
     func: &Expr,
@@ -82,7 +92,7 @@ pub(crate) fn unnecessary_comprehension_any_all(
     let Expr::Name(ast::ExprName { id, .. }) = func else {
         return;
     };
-    if !matches!(id.as_str(), "all" | "any") {
+    if !matches!(id.as_str(), "all" | "any" | "sum" | "min" | "max") {
         return;
     }
     let [arg] = args else {
@@ -100,9 +110,9 @@ pub(crate) fn unnecessary_comprehension_any_all(
         return;
     }
 
-    let mut diagnostic = Diagnostic::new(UnnecessaryComprehensionAnyAll, arg.range());
+    let mut diagnostic = Diagnostic::new(UnnecessaryComprehensionInCall, arg.range());
     diagnostic.try_set_fix(|| {
-        fixes::fix_unnecessary_comprehension_any_all(expr, checker.locator(), checker.stylist())
+        fixes::fix_unnecessary_comprehension_in_call(expr, checker.locator(), checker.stylist())
     });
     checker.diagnostics.push(diagnostic);
 }

crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C419_C419.py.snap

@@ -75,16 +75,16 @@ C419.py:7:5: C419 [*] Unnecessary list comprehension
   7 |+    x.id for x in bar  # second comment
 8 8 | )  # third comment
 9 9 | any({x.id for x in bar})
-10 10 | 
+10 10 | sum([x.val for x in bar])
 
 C419.py:9:5: C419 [*] Unnecessary list comprehension
    |
  7 |     [x.id for x in bar],  # second comment
  8 | )  # third comment
  9 | any({x.id for x in bar})
    |     ^^^^^^^^^^^^^^^^^^^ C419
-10 | 
-11 | # OK
+10 | sum([x.val for x in bar])
+11 | min([x.val for x in bar])
    |
    = help: Remove unnecessary list comprehension
 
@@ -94,71 +94,131 @@ C419.py:9:5: C419 [*] Unnecessary list comprehension
 8  8  | )  # third comment
 9     |-any({x.id for x in bar})
    9  |+any(x.id for x in bar)
-10 10 | 
-11 11 | # OK
-12 12 | all(x.id for x in bar)
+10 10 | sum([x.val for x in bar])
+11 11 | min([x.val for x in bar])
+12 12 | max([x.val for x in bar])
 
-C419.py:24:5: C419 [*] Unnecessary list comprehension
+C419.py:10:5: C419 [*] Unnecessary list comprehension
    |
-22 |   # Special comment handling
-23 |   any(
-24 |       [  # lbracket comment
-   |  _____^
-25 | |         # second line comment
-26 | |         i.bit_count()
-27 | |         # random middle comment
-28 | |         for i in range(5)  # rbracket comment
-29 | |     ]  # rpar comment
-   | |_____^ C419
-30 |       # trailing comment
-31 |   )
+ 8 | )  # third comment
+ 9 | any({x.id for x in bar})
+10 | sum([x.val for x in bar])
+   |     ^^^^^^^^^^^^^^^^^^^^ C419
+11 | min([x.val for x in bar])
+12 | max([x.val for x in bar])
    |
    = help: Remove unnecessary list comprehension
 
 ℹ Unsafe fix
-21 21 | 
-22 22 | # Special comment handling
-23 23 | any(
-24    |-    [  # lbracket comment
-25    |-        # second line comment
-26    |-        i.bit_count()
-   24 |+    # lbracket comment
-   25 |+    # second line comment
-   26 |+    i.bit_count()
-27 27 |         # random middle comment
-28    |-        for i in range(5)  # rbracket comment
-29    |-    ]  # rpar comment
-   28 |+        for i in range(5)  # rbracket comment  # rpar comment
-30 29 |     # trailing comment
-31 30 | )
-32 31 | 
-
-C419.py:35:5: C419 [*] Unnecessary list comprehension
+7  7  |     [x.id for x in bar],  # second comment
+8  8  | )  # third comment
+9  9  | any({x.id for x in bar})
+10    |-sum([x.val for x in bar])
+   10 |+sum(x.val for x in bar)
+11 11 | min([x.val for x in bar])
+12 12 | max([x.val for x in bar])
+13 13 | 
+
+C419.py:11:5: C419 [*] Unnecessary list comprehension
    |
-33 |   # Weird case where the function call, opening bracket, and comment are all
-34 |   # on the same line.
-35 |   any([  # lbracket comment
+ 9 | any({x.id for x in bar})
+10 | sum([x.val for x in bar])
+11 | min([x.val for x in bar])
+   |     ^^^^^^^^^^^^^^^^^^^^ C419
+12 | max([x.val for x in bar])
+   |
+   = help: Remove unnecessary list comprehension
+
+ℹ Unsafe fix
+8  8  | )  # third comment
+9  9  | any({x.id for x in bar})
+10 10 | sum([x.val for x in bar])
+11    |-min([x.val for x in bar])
+   11 |+min(x.val for x in bar)
+12 12 | max([x.val for x in bar])
+13 13 | 
+14 14 | # OK
+
+C419.py:12:5: C419 [*] Unnecessary list comprehension
+   |
+10 | sum([x.val for x in bar])
+11 | min([x.val for x in bar])
+12 | max([x.val for x in bar])
+   |     ^^^^^^^^^^^^^^^^^^^^ C419
+13 | 
+14 | # OK
+   |
+   = help: Remove unnecessary list comprehension
+
+ℹ Unsafe fix
+9  9  | any({x.id for x in bar})
+10 10 | sum([x.val for x in bar])
+11 11 | min([x.val for x in bar])
+12    |-max([x.val for x in bar])
+   12 |+max(x.val for x in bar)
+13 13 | 
+14 14 | # OK
+15 15 | all(x.id for x in bar)
+
+C419.py:30:5: C419 [*] Unnecessary list comprehension
+   |
+28 |   # Special comment handling
+29 |   any(
+30 |       [  # lbracket comment
    |  _____^
-36 | |         # second line comment
-37 | |         i.bit_count() for i in range(5)  # rbracket comment
-38 | |     ]  # rpar comment
+31 | |         # second line comment
+32 | |         i.bit_count()
+33 | |         # random middle comment
+34 | |         for i in range(5)  # rbracket comment
+35 | |     ]  # rpar comment
    | |_____^ C419
-39 |   )
+36 |       # trailing comment
+37 |   )
    |
    = help: Remove unnecessary list comprehension
 
 ℹ Unsafe fix
-32 32 | 
-33 33 | # Weird case where the function call, opening bracket, and comment are all
-34 34 | # on the same line.
-35    |-any([  # lbracket comment
-36    |-        # second line comment
-37    |-        i.bit_count() for i in range(5)  # rbracket comment
-38    |-    ]  # rpar comment
-   35 |+any(
-   36 |+# lbracket comment
-   37 |+# second line comment
-   38 |+i.bit_count() for i in range(5)  # rbracket comment  # rpar comment
-39 39 | )
+27 27 | 
+28 28 | # Special comment handling
+29 29 | any(
+30    |-    [  # lbracket comment
+31    |-        # second line comment
+32    |-        i.bit_count()
+   30 |+    # lbracket comment
+   31 |+    # second line comment
+   32 |+    i.bit_count()
+33 33 |         # random middle comment
+34    |-        for i in range(5)  # rbracket comment
+35    |-    ]  # rpar comment
+   34 |+        for i in range(5)  # rbracket comment  # rpar comment
+36 35 |     # trailing comment
+37 36 | )
+38 37 | 
 
+C419.py:41:5: C419 [*] Unnecessary list comprehension
+   |
+39 |   # Weird case where the function call, opening bracket, and comment are all
+40 |   # on the same line.
+41 |   any([  # lbracket comment
+   |  _____^
+42 | |         # second line comment
+43 | |         i.bit_count() for i in range(5)  # rbracket comment
+44 | |     ]  # rpar comment
+   | |_____^ C419
+45 |   )
+   |
+   = help: Remove unnecessary list comprehension
 
+ℹ Unsafe fix
+38 38 | 
+39 39 | # Weird case where the function call, opening bracket, and comment are all
+40 40 | # on the same line.
+41    |-any([  # lbracket comment
+42    |-        # second line comment
+43    |-        i.bit_count() for i in range(5)  # rbracket comment
+44    |-    ]  # rpar comment
+   41 |+any(
+   42 |+# lbracket comment
+   43 |+# second line comment
+   44 |+i.bit_count() for i in range(5)  # rbracket comment  # rpar comment
+45 45 | )