[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,3 +1,4 @@
+use ast::{comparable::ComparableExpr, ExprGenerator};
 use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast as ast;
@@ -9,37 +10,49 @@ use super::helpers;
 
 /// ## What it does
 /// Checks for unnecessary generators that can be rewritten as `list`
-/// comprehensions.
+/// comprehensions or conversion to `list` using the builtin function.
 ///
 /// ## 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.
+/// comprehension/conversion to `list` is clearer and more idiomatic.
 ///
 /// ## 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 {
+    shortened_to_list_conversion: bool,
+}
 
 impl AlwaysFixableViolation for UnnecessaryGeneratorList {
     #[derive_message_formats]
     fn message(&self) -> String {
-        format!("Unnecessary generator (rewrite as a `list` comprehension)")
+        if self.shortened_to_list_conversion {
+            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.shortened_to_list_conversion {
+            "Rewrite using `list()`".to_string()
+        } else {
+            "Rewrite as a `list` comprehension".to_string()
+        }
     }
 }
 
@@ -56,28 +69,70 @@ 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),
+    if let Some(generator_expr) = argument.clone().generator_expr() {
+        // Short-circuit the case for list(x for x in y) -> list(y)
+        let mut shortcircuit_completed = false;
+        'shortcircuit: {
+            let mut diagnostic = Diagnostic::new(
+                UnnecessaryGeneratorList {
+                    shortened_to_list_conversion: true,
+                },
+                call.range(),
             );
 
-            // Replace `)` with `]`.
-            let call_end = Edit::replacement(
-                "]".to_string(),
-                call.arguments.end() - TextSize::from(1),
-                call.end(),
+            let ExprGenerator {
+                range: _,
+                elt,
+                generators,
+                parenthesized: _,
+            } = generator_expr;
+
+            let [generator] = &generators[..] else {
+                break 'shortcircuit;
+            };
+            if !generator.ifs.is_empty() || generator.is_async {
+                break 'shortcircuit;
+            };
+            if ComparableExpr::from(&elt) != ComparableExpr::from(&generator.target) {
+                break 'shortcircuit;
+            };
+
+            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);
+            shortcircuit_completed = true;
+        }
+
+        // Convert `list(f(x) for x in y)` to `[f(x) for x in y]`.
+        if !shortcircuit_completed {
+            let mut diagnostic = Diagnostic::new(
+                UnnecessaryGeneratorList {
+                    shortened_to_list_conversion: 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(),
+                );
 
-            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):