Remove LibCST-based fixer for C403

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

@@ -13,3 +13,11 @@ def f(x):
 
 s = f"{ set([x for x in 'ab']) | set([x for x in 'ab']) }"
 s = f"{set([x for x in 'ab']) | set([x for x in 'ab'])}"
+
+s = set(  # comment
+    [x for x in range(3)]
+)
+
+s = set([  # comment
+    x for x in range(3)
+])

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

@@ -653,9 +653,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 );
             }
             if checker.enabled(Rule::UnnecessaryListComprehensionSet) {
-                flake8_comprehensions::rules::unnecessary_list_comprehension_set(
-                    checker, expr, func, args, keywords,
-                );
+                flake8_comprehensions::rules::unnecessary_list_comprehension_set(checker, call);
             }
             if checker.enabled(Rule::UnnecessaryListComprehensionDict) {
                 flake8_comprehensions::rules::unnecessary_list_comprehension_dict(

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

@@ -1,3 +1,5 @@
+use std::iter;
+
 use anyhow::{bail, Result};
 use itertools::Itertools;
 use libcst_native::{
@@ -7,7 +9,6 @@ use libcst_native::{
     RightCurlyBrace, RightParen, RightSquareBracket, Set, SetComp, SimpleString, SimpleWhitespace,
     TrailingWhitespace, Tuple,
 };
-use std::iter;
 
 use ruff_diagnostics::{Edit, Fix};
 use ruff_python_ast::Expr;
@@ -151,46 +152,6 @@ pub(crate) fn fix_unnecessary_generator_dict(expr: &Expr, checker: &Checker) ->
     ))
 }
 
-/// (C403) Convert `set([x for x in y])` to `{x for x in y}`.
-pub(crate) fn fix_unnecessary_list_comprehension_set(
-    expr: &Expr,
-    checker: &Checker,
-) -> Result<Edit> {
-    let locator = checker.locator();
-    let stylist = checker.stylist();
-    // Expr(Call(ListComp)))) ->
-    // Expr(SetComp)))
-    let module_text = locator.slice(expr);
-    let mut tree = match_expression(module_text)?;
-    let call = match_call_mut(&mut tree)?;
-    let arg = match_arg(call)?;
-
-    let list_comp = match_list_comp(&arg.value)?;
-
-    tree = Expression::SetComp(Box::new(SetComp {
-        elt: list_comp.elt.clone(),
-        for_in: list_comp.for_in.clone(),
-        lbrace: LeftCurlyBrace {
-            whitespace_after: call.whitespace_before_args.clone(),
-        },
-        rbrace: RightCurlyBrace {
-            whitespace_before: arg.whitespace_after_arg.clone(),
-        },
-        lpar: list_comp.lpar.clone(),
-        rpar: list_comp.rpar.clone(),
-    }));
-
-    Ok(Edit::range_replacement(
-        pad_expression(
-            tree.codegen_stylist(stylist),
-            expr.range(),
-            checker.locator(),
-            checker.semantic(),
-        ),
-        expr.range(),
-    ))
-}
-
 /// (C404) Convert `dict([(i, i) for i in range(3)])` to `{i: i for i in
 /// range(3)}`.
 pub(crate) fn fix_unnecessary_list_comprehension_dict(
@@ -544,7 +505,7 @@ pub(crate) fn fix_unnecessary_collection_call(expr: &Expr, checker: &Checker) ->
 /// However, this is a syntax error under the f-string grammar. As such,
 /// this method will pad the start and end of an expression as needed to
 /// avoid producing invalid syntax.
-fn pad_expression(
+pub(crate) fn pad_expression(
     content: String,
     range: TextRange,
     locator: &Locator,
@@ -575,6 +536,48 @@ fn pad_expression(
     }
 }
 
+/// Like [`pad_expression`], but only pads the start of the expression.
+pub(crate) fn pad_start(
+    content: &str,
+    range: TextRange,
+    locator: &Locator,
+    semantic: &SemanticModel,
+) -> String {
+    if !semantic.in_f_string() {
+        return content.into();
+    }
+
+    // If the expression is immediately preceded by an opening brace, then
+    // we need to add a space before the expression.
+    let prefix = locator.up_to(range.start());
+    if matches!(prefix.chars().next_back(), Some('{')) {
+        format!(" {content}")
+    } else {
+        content.into()
+    }
+}
+
+/// Like [`pad_expression`], but only pads the end of the expression.
+pub(crate) fn pad_end(
+    content: &str,
+    range: TextRange,
+    locator: &Locator,
+    semantic: &SemanticModel,
+) -> String {
+    if !semantic.in_f_string() {
+        return content.into();
+    }
+
+    // If the expression is immediately preceded by an opening brace, then
+    // we need to add a space before the expression.
+    let suffix = locator.after(range.end());
+    if matches!(suffix.chars().next(), Some('}')) {
+        format!("{content} ")
+    } else {
+        content.into()
+    }
+}
+
 /// (C409) Convert `tuple([1, 2])` to `tuple(1, 2)`
 pub(crate) fn fix_unnecessary_literal_within_tuple_call(
     expr: &Expr,

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

@@ -76,7 +76,7 @@ pub(crate) fn unnecessary_collection_call(
         {
             // `dict()` or `dict(a=1)` (as opposed to `dict(**a)`)
         }
-        "list" | "tuple" => {
+        "list" | "tuple" if keywords.is_empty() => {
             // `list()` or `tuple()`
         }
         _ => return,

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

@@ -1,12 +1,10 @@
-use ruff_python_ast::{Expr, Keyword};
-
-use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
+use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
-use ruff_text_size::Ranged;
+use ruff_python_ast as ast;
+use ruff_text_size::{Ranged, TextSize};
 
 use crate::checkers::ast::Checker;
-
-use crate::rules::flake8_comprehensions::fixes;
+use crate::rules::flake8_comprehensions::fixes::{pad_end, pad_start};
 
 use super::helpers;
 
@@ -45,25 +43,46 @@ impl AlwaysFixableViolation for UnnecessaryListComprehensionSet {
 }
 
 /// C403 (`set([...])`)
-pub(crate) fn unnecessary_list_comprehension_set(
-    checker: &mut Checker,
-    expr: &Expr,
-    func: &Expr,
-    args: &[Expr],
-    keywords: &[Keyword],
-) {
-    let Some(argument) =
-        helpers::exactly_one_argument_with_matching_function("set", func, args, keywords)
-    else {
+pub(crate) fn unnecessary_list_comprehension_set(checker: &mut Checker, call: &ast::ExprCall) {
+    let Some(argument) = helpers::exactly_one_argument_with_matching_function(
+        "set",
+        &call.func,
+        &call.arguments.args,
+        &call.arguments.keywords,
+    ) else {
         return;
     };
     if !checker.semantic().is_builtin("set") {
         return;
     }
     if argument.is_list_comp_expr() {
-        let mut diagnostic = Diagnostic::new(UnnecessaryListComprehensionSet, expr.range());
+        let mut diagnostic = Diagnostic::new(UnnecessaryListComprehensionSet, call.range());
         diagnostic.try_set_fix(|| {
-            fixes::fix_unnecessary_list_comprehension_set(expr, checker).map(Fix::unsafe_edit)
+            // Replace `set(` with `{`.
+            let call_start = Edit::replacement(
+                pad_start("{", call.range(), checker.locator(), checker.semantic()),
+                call.start(),
+                call.arguments.start() + TextSize::from(1),
+            );
+
+            // Replace `)` with `}`.
+            let call_end = Edit::replacement(
+                pad_end("}", call.range(), checker.locator(), checker.semantic()),
+                call.arguments.end() - TextSize::from(1),
+                call.end(),
+            );
+
+            // Delete the open bracket (`[`).
+            let argument_start =
+                Edit::deletion(argument.start(), argument.start() + TextSize::from(1));
+
+            // Delete the close bracket (`]`).
+            let argument_end = Edit::deletion(argument.end() - TextSize::from(1), argument.end());
+
+            Ok(Fix::unsafe_edits(
+                call_start,
+                [argument_start, argument_end, call_end],
+            ))
         });
         checker.diagnostics.push(diagnostic);
     }

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

@@ -120,6 +120,8 @@ C403.py:14:9: C403 [*] Unnecessary `list` comprehension (rewrite as a `set` comp
 14    |-s = f"{ set([x for x in 'ab']) | set([x for x in 'ab']) }"
    14 |+s = f"{ {x for x in 'ab'} | set([x for x in 'ab']) }"
 15 15 | s = f"{set([x for x in 'ab']) | set([x for x in 'ab'])}"
+16 16 | 
+17 17 | s = set(  # comment
 
 C403.py:14:34: C403 [*] Unnecessary `list` comprehension (rewrite as a `set` comprehension)
    |
@@ -138,12 +140,16 @@ C403.py:14:34: C403 [*] Unnecessary `list` comprehension (rewrite as a `set` com
 14    |-s = f"{ set([x for x in 'ab']) | set([x for x in 'ab']) }"
    14 |+s = f"{ set([x for x in 'ab']) | {x for x in 'ab'} }"
 15 15 | s = f"{set([x for x in 'ab']) | set([x for x in 'ab'])}"
+16 16 | 
+17 17 | s = set(  # comment
 
 C403.py:15:8: C403 [*] Unnecessary `list` comprehension (rewrite as a `set` comprehension)
    |
 14 | s = f"{ set([x for x in 'ab']) | set([x for x in 'ab']) }"
 15 | s = f"{set([x for x in 'ab']) | set([x for x in 'ab'])}"
    |        ^^^^^^^^^^^^^^^^^^^^^^ C403
+16 | 
+17 | s = set(  # comment
    |
    = help: Rewrite as a `set` comprehension
 
@@ -153,12 +159,17 @@ C403.py:15:8: C403 [*] Unnecessary `list` comprehension (rewrite as a `set` comp
 14 14 | s = f"{ set([x for x in 'ab']) | set([x for x in 'ab']) }"
 15    |-s = f"{set([x for x in 'ab']) | set([x for x in 'ab'])}"
    15 |+s = f"{ {x for x in 'ab'} | set([x for x in 'ab'])}"
+16 16 | 
+17 17 | s = set(  # comment
+18 18 |     [x for x in range(3)]
 
 C403.py:15:33: C403 [*] Unnecessary `list` comprehension (rewrite as a `set` comprehension)
    |
 14 | s = f"{ set([x for x in 'ab']) | set([x for x in 'ab']) }"
 15 | s = f"{set([x for x in 'ab']) | set([x for x in 'ab'])}"
    |                                 ^^^^^^^^^^^^^^^^^^^^^^ C403
+16 | 
+17 | s = set(  # comment
    |
    = help: Rewrite as a `set` comprehension
 
@@ -168,5 +179,58 @@ C403.py:15:33: C403 [*] Unnecessary `list` comprehension (rewrite as a `set` com
 14 14 | s = f"{ set([x for x in 'ab']) | set([x for x in 'ab']) }"
 15    |-s = f"{set([x for x in 'ab']) | set([x for x in 'ab'])}"
    15 |+s = f"{set([x for x in 'ab']) | {x for x in 'ab'} }"
+16 16 | 
+17 17 | s = set(  # comment
+18 18 |     [x for x in range(3)]
+
+C403.py:17:5: C403 [*] Unnecessary `list` comprehension (rewrite as a `set` comprehension)
+   |
+15 |   s = f"{set([x for x in 'ab']) | set([x for x in 'ab'])}"
+16 |   
+17 |   s = set(  # comment
+   |  _____^
+18 | |     [x for x in range(3)]
+19 | | )
+   | |_^ C403
+20 |   
+21 |   s = set([  # comment
+   |
+   = help: Rewrite as a `set` comprehension
+
+ℹ Unsafe fix
+14 14 | s = f"{ set([x for x in 'ab']) | set([x for x in 'ab']) }"
+15 15 | s = f"{set([x for x in 'ab']) | set([x for x in 'ab'])}"
+16 16 | 
+17    |-s = set(  # comment
+18    |-    [x for x in range(3)]
+19    |-)
+   17 |+s = {  # comment
+   18 |+    x for x in range(3)
+   19 |+}
+20 20 | 
+21 21 | s = set([  # comment
+22 22 |     x for x in range(3)
+
+C403.py:21:5: C403 [*] Unnecessary `list` comprehension (rewrite as a `set` comprehension)
+   |
+19 |   )
+20 |   
+21 |   s = set([  # comment
+   |  _____^
+22 | |     x for x in range(3)
+23 | | ])
+   | |__^ C403
+   |
+   = help: Rewrite as a `set` comprehension
+
+ℹ Unsafe fix
+18 18 |     [x for x in range(3)]
+19 19 | )
+20 20 | 
+21    |-s = set([  # comment
+   21 |+s = {  # comment
+22 22 |     x for x in range(3)
+23    |-])
+   23 |+}