[flake8_comprehensions] Handled special case for C400 which also matches C416

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

@@ -1,11 +1,20 @@
+# Cannot combine with C416. Should use list comprehension here.
+even_nums = list(2 * x for x in range(3))
+odd_nums = list(
+    2 * x + 1 for x in range(3)
+)
+
+
+# Short-circuit case, combine with C416 and should produce x = list(range(3))
 x = list(x for x in range(3))
 x = list(
     x for x in range(3)
 )
 
-
+# Not built-in list.
 def list(*args, **kwargs):
     return None
 
 
+list(2 * x for x in range(3))
 list(x for x in range(3))

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

@@ -1,6 +1,8 @@
 use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast as ast;
+use ruff_python_ast::comparable::ComparableExpr;
+use ruff_python_ast::ExprGenerator;
 use ruff_text_size::{Ranged, TextSize};
 
 use crate::checkers::ast::Checker;
@@ -9,37 +11,53 @@ use super::helpers;
 
 /// ## What it does
 /// Checks for unnecessary generators that can be rewritten as `list`
-/// comprehensions.
+/// comprehensions (or with `list` directly).
 ///
 /// ## Why is this bad?
 /// It is unnecessary to use `list` around a generator expression, since
 /// there are equivalent comprehensions for these types. Using a
 /// comprehension is clearer and more idiomatic.
 ///
+/// Further, if the comprehension can be removed entirely, as in the case of
+/// `list(x for x in foo)`, it's better to use `list(foo)` directly, since it's
+/// even more direct.
+///
 /// ## Examples
 /// ```python
 /// list(f(x) for x in foo)
+/// list(x for x in foo)
 /// ```
 ///
 /// Use instead:
 /// ```python
 /// [f(x) for x in foo]
+/// list(foo)
 /// ```
 ///
 /// ## Fix safety
 /// This rule's fix is marked as unsafe, as it may occasionally drop comments
 /// when rewriting the call. In most cases, though, comments will be preserved.
 #[violation]
-pub struct UnnecessaryGeneratorList;
+pub struct UnnecessaryGeneratorList {
+    short_circuit: bool,
+}
 
 impl AlwaysFixableViolation for UnnecessaryGeneratorList {
     #[derive_message_formats]
     fn message(&self) -> String {
-        format!("Unnecessary generator (rewrite as a `list` comprehension)")
+        if self.short_circuit {
+            format!("Unnecessary generator (rewrite using `list()`")
+        } else {
+            format!("Unnecessary generator (rewrite as a `list` comprehension)")
+        }
     }
 
     fn fix_title(&self) -> String {
-        "Rewrite as a `list` comprehension".to_string()
+        if self.short_circuit {
+            "Rewrite using `list()`".to_string()
+        } else {
+            "Rewrite as a `list` comprehension".to_string()
+        }
     }
 }
 
@@ -56,28 +74,59 @@ pub(crate) fn unnecessary_generator_list(checker: &mut Checker, call: &ast::Expr
     if !checker.semantic().is_builtin("list") {
         return;
     }
-    if argument.is_generator_expr() {
-        let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorList, call.range());
 
-        // Convert `list(x for x in y)` to `[x for x in y]`.
-        diagnostic.set_fix({
-            // Replace `list(` with `[`.
-            let call_start = Edit::replacement(
-                "[".to_string(),
-                call.start(),
-                call.arguments.start() + TextSize::from(1),
-            );
+    let Some(ExprGenerator {
+        elt, generators, ..
+    }) = argument.as_generator_expr()
+    else {
+        return;
+    };
+
+    // Short-circuit: given `list(x for x in y)`, generate `list(y)` (in lieu of `[x for x in y]`).
+    if let [generator] = generators.as_slice() {
+        if generator.ifs.is_empty() && !generator.is_async {
+            if ComparableExpr::from(elt) == ComparableExpr::from(&generator.target) {
+                let mut diagnostic = Diagnostic::new(
+                    UnnecessaryGeneratorList {
+                        short_circuit: true,
+                    },
+                    call.range(),
+                );
+                let iterator = format!("list({})", checker.locator().slice(generator.iter.range()));
+                diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+                    iterator,
+                    call.range(),
+                )));
+                checker.diagnostics.push(diagnostic);
+                return;
+            }
+        }
+    }
+
+    // Convert `list(f(x) for x in y)` to `[f(x) for x in y]`.
+    let mut diagnostic = Diagnostic::new(
+        UnnecessaryGeneratorList {
+            short_circuit: false,
+        },
+        call.range(),
+    );
+    diagnostic.set_fix({
+        // Replace `list(` with `[`.
+        let call_start = Edit::replacement(
+            "[".to_string(),
+            call.start(),
+            call.arguments.start() + TextSize::from(1),
+        );
 
-            // Replace `)` with `]`.
-            let call_end = Edit::replacement(
-                "]".to_string(),
-                call.arguments.end() - TextSize::from(1),
-                call.end(),
-            );
+        // Replace `)` with `]`.
+        let call_end = Edit::replacement(
+            "]".to_string(),
+            call.arguments.end() - TextSize::from(1),
+            call.end(),
+        );
 
-            Fix::unsafe_edits(call_start, [call_end])
-        });
+        Fix::unsafe_edits(call_start, [call_end])
+    });
 
-        checker.diagnostics.push(diagnostic);
-    }
+    checker.diagnostics.push(diagnostic);
 }

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

@@ -1,42 +1,90 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs
 ---
-C400.py:1:5: C400 [*] Unnecessary generator (rewrite as a `list` comprehension)
+C400.py:2:13: C400 [*] Unnecessary generator (rewrite as a `list` comprehension)
   |
-1 | x = list(x for x in range(3))
-  |     ^^^^^^^^^^^^^^^^^^^^^^^^^ C400
-2 | x = list(
-3 |     x for x in range(3)
+1 | # Cannot combine with C416. Should use list comprehension here.
+2 | even_nums = list(2 * x for x in range(3))
+  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C400
+3 | odd_nums = list(
+4 |     2 * x + 1 for x in range(3)
   |
   = help: Rewrite as a `list` comprehension
 
 ℹ Unsafe fix
-1   |-x = list(x for x in range(3))
-  1 |+x = [x for x in range(3)]
-2 2 | x = list(
-3 3 |     x for x in range(3)
-4 4 | )
+1 1 | # Cannot combine with C416. Should use list comprehension here.
+2   |-even_nums = list(2 * x for x in range(3))
+  2 |+even_nums = [2 * x for x in range(3)]
+3 3 | odd_nums = list(
+4 4 |     2 * x + 1 for x in range(3)
+5 5 | )
 
-C400.py:2:5: C400 [*] Unnecessary generator (rewrite as a `list` comprehension)
+C400.py:3:12: C400 [*] Unnecessary generator (rewrite as a `list` comprehension)
   |
-1 |   x = list(x for x in range(3))
-2 |   x = list(
-  |  _____^
-3 | |     x for x in range(3)
-4 | | )
+1 |   # Cannot combine with C416. Should use list comprehension here.
+2 |   even_nums = list(2 * x for x in range(3))
+3 |   odd_nums = list(
+  |  ____________^
+4 | |     2 * x + 1 for x in range(3)
+5 | | )
   | |_^ C400
   |
   = help: Rewrite as a `list` comprehension
 
 ℹ Unsafe fix
-1 1 | x = list(x for x in range(3))
-2   |-x = list(
-  2 |+x = [
-3 3 |     x for x in range(3)
-4   |-)
-  4 |+]
-5 5 | 
+1 1 | # Cannot combine with C416. Should use list comprehension here.
+2 2 | even_nums = list(2 * x for x in range(3))
+3   |-odd_nums = list(
+  3 |+odd_nums = [
+4 4 |     2 * x + 1 for x in range(3)
+5   |-)
+  5 |+]
 6 6 | 
-7 7 | def list(*args, **kwargs):
+7 7 | 
+8 8 | # Short-circuit case, combine with C416 and should produce x = list(range(3))
 
+C400.py:9:5: C400 [*] Unnecessary generator (rewrite using `list()`
+   |
+ 8 | # Short-circuit case, combine with C416 and should produce x = list(range(3))
+ 9 | x = list(x for x in range(3))
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ C400
+10 | x = list(
+11 |     x for x in range(3)
+   |
+   = help: Rewrite using `list()`
 
+ℹ Unsafe fix
+6  6  | 
+7  7  | 
+8  8  | # Short-circuit case, combine with C416 and should produce x = list(range(3))
+9     |-x = list(x for x in range(3))
+   9  |+x = list(range(3))
+10 10 | x = list(
+11 11 |     x for x in range(3)
+12 12 | )
+
+C400.py:10:5: C400 [*] Unnecessary generator (rewrite using `list()`
+   |
+ 8 |   # Short-circuit case, combine with C416 and should produce x = list(range(3))
+ 9 |   x = list(x for x in range(3))
+10 |   x = list(
+   |  _____^
+11 | |     x for x in range(3)
+12 | | )
+   | |_^ C400
+13 |   
+14 |   # Not built-in list.
+   |
+   = help: Rewrite using `list()`
+
+ℹ Unsafe fix
+7  7  | 
+8  8  | # Short-circuit case, combine with C416 and should produce x = list(range(3))
+9  9  | x = list(x for x in range(3))
+10    |-x = list(
+11    |-    x for x in range(3)
+12    |-)
+   10 |+x = list(range(3))
+13 11 | 
+14 12 | # Not built-in list.
+15 13 | def list(*args, **kwargs):