Do thing

Experimenting with rewriting one of the comprehension fixes _without_
LibCST.

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

@@ -2,3 +2,12 @@
 l2 = list((1, 2))
 l3 = list([])
 l4 = list(())
+
+
+list(  # comment
+    [1, 2]
+)
+
+list([  # comment
+    1, 2
+])

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

@@ -638,14 +638,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 flake8_bandit::rules::tarfile_unsafe_members(checker, call);
             }
             if checker.enabled(Rule::UnnecessaryGeneratorList) {
-                flake8_comprehensions::rules::unnecessary_generator_list(
-                    checker, expr, func, args, keywords,
-                );
+                flake8_comprehensions::rules::unnecessary_generator_list(checker, call);
             }
             if checker.enabled(Rule::UnnecessaryGeneratorSet) {
-                flake8_comprehensions::rules::unnecessary_generator_set(
-                    checker, expr, func, args, keywords,
-                );
+                flake8_comprehensions::rules::unnecessary_generator_set(checker, call);
             }
             if checker.enabled(Rule::UnnecessaryGeneratorDict) {
                 flake8_comprehensions::rules::unnecessary_generator_dict(
@@ -686,14 +682,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 );
             }
             if checker.enabled(Rule::UnnecessaryLiteralWithinListCall) {
-                flake8_comprehensions::rules::unnecessary_literal_within_list_call(
-                    checker, expr, func, args, keywords,
-                );
+                flake8_comprehensions::rules::unnecessary_literal_within_list_call(checker, call);
             }
             if checker.enabled(Rule::UnnecessaryLiteralWithinDictCall) {
-                flake8_comprehensions::rules::unnecessary_literal_within_dict_call(
-                    checker, expr, func, args, keywords,
-                );
+                flake8_comprehensions::rules::unnecessary_literal_within_dict_call(checker, call);
             }
             if checker.enabled(Rule::UnnecessaryListCall) {
                 flake8_comprehensions::rules::unnecessary_list_call(checker, expr, func, args);

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

@@ -29,73 +29,6 @@ use crate::{
     },
 };
 
-/// (C400) Convert `list(x for x in y)` to `[x for x in y]`.
-pub(crate) fn fix_unnecessary_generator_list(
-    expr: &Expr,
-    locator: &Locator,
-    stylist: &Stylist,
-) -> Result<Edit> {
-    // Expr(Call(GeneratorExp)))) -> Expr(ListComp)))
-    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 generator_exp = match_generator_exp(&arg.value)?;
-
-    tree = Expression::ListComp(Box::new(ListComp {
-        elt: generator_exp.elt.clone(),
-        for_in: generator_exp.for_in.clone(),
-        lbracket: LeftSquareBracket {
-            whitespace_after: call.whitespace_before_args.clone(),
-        },
-        rbracket: RightSquareBracket {
-            whitespace_before: arg.whitespace_after_arg.clone(),
-        },
-        lpar: generator_exp.lpar.clone(),
-        rpar: generator_exp.rpar.clone(),
-    }));
-
-    Ok(Edit::range_replacement(
-        tree.codegen_stylist(stylist),
-        expr.range(),
-    ))
-}
-
-/// (C401) Convert `set(x for x in y)` to `{x for x in y}`.
-pub(crate) fn fix_unnecessary_generator_set(expr: &Expr, checker: &Checker) -> Result<Edit> {
-    let locator = checker.locator();
-    let stylist = checker.stylist();
-
-    // Expr(Call(GeneratorExp)))) -> 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 generator_exp = match_generator_exp(&arg.value)?;
-
-    tree = Expression::SetComp(Box::new(SetComp {
-        elt: generator_exp.elt.clone(),
-        for_in: generator_exp.for_in.clone(),
-        lbrace: LeftCurlyBrace {
-            whitespace_after: call.whitespace_before_args.clone(),
-        },
-        rbrace: RightCurlyBrace {
-            whitespace_before: arg.whitespace_after_arg.clone(),
-        },
-        lpar: generator_exp.lpar.clone(),
-        rpar: generator_exp.rpar.clone(),
-    }));
-
-    let content = tree.codegen_stylist(stylist);
-
-    Ok(Edit::range_replacement(
-        pad_expression(content, expr.range(), checker.locator(), checker.semantic()),
-        expr.range(),
-    ))
-}
-
 /// (C402) Convert `dict((x, x) for x in range(3))` to `{x: x for x in
 /// range(3)}`.
 pub(crate) fn fix_unnecessary_generator_dict(expr: &Expr, checker: &Checker) -> Result<Edit> {
@@ -628,58 +561,6 @@ pub(crate) fn fix_unnecessary_literal_within_tuple_call(
     ))
 }
 
-/// (C410) Convert `list([1, 2])` to `[1, 2]`
-pub(crate) fn fix_unnecessary_literal_within_list_call(
-    expr: &Expr,
-    locator: &Locator,
-    stylist: &Stylist,
-) -> Result<Edit> {
-    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 (elements, whitespace_after, whitespace_before) = match &arg.value {
-        Expression::Tuple(inner) => (
-            &inner.elements,
-            &inner
-                .lpar
-                .first()
-                .ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
-                .whitespace_after,
-            &inner
-                .rpar
-                .first()
-                .ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
-                .whitespace_before,
-        ),
-        Expression::List(inner) => (
-            &inner.elements,
-            &inner.lbracket.whitespace_after,
-            &inner.rbracket.whitespace_before,
-        ),
-        _ => {
-            bail!("Expected Expression::Tuple | Expression::List");
-        }
-    };
-
-    tree = Expression::List(Box::new(List {
-        elements: elements.clone(),
-        lbracket: LeftSquareBracket {
-            whitespace_after: whitespace_after.clone(),
-        },
-        rbracket: RightSquareBracket {
-            whitespace_before: whitespace_before.clone(),
-        },
-        lpar: vec![],
-        rpar: vec![],
-    }));
-
-    Ok(Edit::range_replacement(
-        tree.codegen_stylist(stylist),
-        expr.range(),
-    ))
-}
-
 /// (C411) Convert `list([i * i for i in x])` to `[i * i for i in x]`.
 pub(crate) fn fix_unnecessary_list_call(
     expr: &Expr,
@@ -1094,25 +975,6 @@ pub(crate) fn fix_unnecessary_map(
     Ok(Edit::range_replacement(content, expr.range()))
 }
 
-/// (C418) Convert `dict({"a": 1})` to `{"a": 1}`
-pub(crate) fn fix_unnecessary_literal_within_dict_call(
-    expr: &Expr,
-    locator: &Locator,
-    stylist: &Stylist,
-) -> Result<Edit> {
-    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)?;
-
-    tree = arg.value.clone();
-
-    Ok(Edit::range_replacement(
-        tree.codegen_stylist(stylist),
-        expr.range(),
-    ))
-}
-
 /// (C419) Convert `[i for i in a]` into `i for i in a`
 pub(crate) fn fix_unnecessary_comprehension_any_all(
     expr: &Expr,

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

@@ -1,13 +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 super::helpers;
 
 /// ## What it does
@@ -47,27 +44,40 @@ impl AlwaysFixableViolation for UnnecessaryGeneratorList {
 }
 
 /// C400 (`list(generator)`)
-pub(crate) fn unnecessary_generator_list(
-    checker: &mut Checker,
-    expr: &Expr,
-    func: &Expr,
-    args: &[Expr],
-    keywords: &[Keyword],
-) {
-    let Some(argument) =
-        helpers::exactly_one_argument_with_matching_function("list", func, args, keywords)
-    else {
+pub(crate) fn unnecessary_generator_list(checker: &mut Checker, call: &ast::ExprCall) {
+    let Some(argument) = helpers::exactly_one_argument_with_matching_function(
+        "list",
+        &call.func,
+        &call.arguments.args,
+        &call.arguments.keywords,
+    ) else {
         return;
     };
     if !checker.semantic().is_builtin("list") {
         return;
     }
-    if let Expr::GeneratorExp(_) = argument {
-        let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorList, expr.range());
-        diagnostic.try_set_fix(|| {
-            fixes::fix_unnecessary_generator_list(expr, checker.locator(), checker.stylist())
-                .map(Fix::unsafe_edit)
+    if argument.is_generator_exp_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),
+            );
+
+            // Replace `)` with `]`.
+            let call_end = Edit::replacement(
+                "]".to_string(),
+                call.arguments.end() - TextSize::from(1),
+                call.end(),
+            );
+
+            Fix::unsafe_edits(call_start, [call_end])
         });
+
         checker.diagnostics.push(diagnostic);
     }
 }

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_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;
 
@@ -47,26 +45,40 @@ impl AlwaysFixableViolation for UnnecessaryGeneratorSet {
 }
 
 /// C401 (`set(generator)`)
-pub(crate) fn unnecessary_generator_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_generator_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 let Expr::GeneratorExp(_) = argument {
-        let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorSet, expr.range());
-        diagnostic.try_set_fix(|| {
-            fixes::fix_unnecessary_generator_set(expr, checker).map(Fix::unsafe_edit)
+    if argument.is_generator_exp_expr() {
+        let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorSet, call.range());
+
+        // Convert `set(x for x in y)` to `{x for x in y}`.
+        diagnostic.set_fix({
+            // 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(),
+            );
+
+            Fix::unsafe_edits(call_start, [call_end])
         });
+
         checker.diagnostics.push(diagnostic);
     }
 }

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

@@ -57,7 +57,7 @@ pub(crate) fn unnecessary_list_comprehension_set(checker: &mut Checker, call: &a
     }
     if argument.is_list_comp_expr() {
         let mut diagnostic = Diagnostic::new(UnnecessaryListComprehensionSet, call.range());
-        diagnostic.try_set_fix(|| {
+        diagnostic.set_fix({
             // Replace `set(` with `{`.
             let call_start = Edit::replacement(
                 pad_start("{", call.range(), checker.locator(), checker.semantic()),
@@ -79,10 +79,7 @@ pub(crate) fn unnecessary_list_comprehension_set(checker: &mut Checker, call: &a
             // 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],
-            ))
+            Fix::unsafe_edits(call_start, [argument_start, argument_end, call_end])
         });
         checker.diagnostics.push(diagnostic);
     }