Add more

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

@@ -8,3 +8,11 @@
 t5 = tuple(
     (1, 2)
 )
+
+tuple(  # comment
+    [1, 2]
+)
+
+tuple([  # comment
+    1, 2
+])

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

@@ -657,9 +657,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 );
             }
             if checker.enabled(Rule::UnnecessaryLiteralSet) {
-                flake8_comprehensions::rules::unnecessary_literal_set(
-                    checker, expr, func, args, keywords,
-                );
+                flake8_comprehensions::rules::unnecessary_literal_set(checker, call);
             }
             if checker.enabled(Rule::UnnecessaryLiteralDict) {
                 flake8_comprehensions::rules::unnecessary_literal_dict(
@@ -677,9 +675,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 );
             }
             if checker.enabled(Rule::UnnecessaryLiteralWithinTupleCall) {
-                flake8_comprehensions::rules::unnecessary_literal_within_tuple_call(
-                    checker, expr, func, args, keywords,
-                );
+                flake8_comprehensions::rules::unnecessary_literal_within_tuple_call(checker, call);
             }
             if checker.enabled(Rule::UnnecessaryLiteralWithinListCall) {
                 flake8_comprehensions::rules::unnecessary_literal_within_list_call(checker, call);

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

@@ -6,7 +6,7 @@ use libcst_native::{
     Arg, AssignEqual, AssignTargetExpression, Call, Comment, CompFor, Dict, DictComp, DictElement,
     Element, EmptyLine, Expression, GeneratorExp, LeftCurlyBrace, LeftParen, LeftSquareBracket,
     List, ListComp, Name, ParenthesizableWhitespace, ParenthesizedNode, ParenthesizedWhitespace,
-    RightCurlyBrace, RightParen, RightSquareBracket, Set, SetComp, SimpleString, SimpleWhitespace,
+    RightCurlyBrace, RightParen, RightSquareBracket, SetComp, SimpleString, SimpleWhitespace,
     TrailingWhitespace, Tuple,
 };
 
@@ -145,95 +145,6 @@ pub(crate) fn fix_unnecessary_list_comprehension_dict(
     ))
 }
 
-/// Drop a trailing comma from a list of tuple elements.
-fn drop_trailing_comma<'a>(
-    tuple: &Tuple<'a>,
-) -> Result<(
-    Vec<Element<'a>>,
-    ParenthesizableWhitespace<'a>,
-    ParenthesizableWhitespace<'a>,
-)> {
-    let whitespace_after = tuple
-        .lpar
-        .first()
-        .ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
-        .whitespace_after
-        .clone();
-    let whitespace_before = tuple
-        .rpar
-        .first()
-        .ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
-        .whitespace_before
-        .clone();
-
-    let mut elements = tuple.elements.clone();
-    if elements.len() == 1 {
-        if let Some(Element::Simple {
-            value,
-            comma: Some(..),
-            ..
-        }) = elements.last()
-        {
-            if whitespace_before == ParenthesizableWhitespace::default()
-                && whitespace_after == ParenthesizableWhitespace::default()
-            {
-                elements[0] = Element::Simple {
-                    value: value.clone(),
-                    comma: None,
-                };
-            }
-        }
-    }
-
-    Ok((elements, whitespace_after, whitespace_before))
-}
-
-/// (C405) Convert `set((1, 2))` to `{1, 2}`.
-pub(crate) fn fix_unnecessary_literal_set(expr: &Expr, checker: &Checker) -> Result<Edit> {
-    let locator = checker.locator();
-    let stylist = checker.stylist();
-
-    // Expr(Call(List|Tuple)))) -> Expr(Set)))
-    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) => drop_trailing_comma(inner)?,
-        Expression::List(inner) => (
-            inner.elements.clone(),
-            inner.lbracket.whitespace_after.clone(),
-            inner.rbracket.whitespace_before.clone(),
-        ),
-        _ => {
-            bail!("Expected Expression::Tuple | Expression::List");
-        }
-    };
-
-    if elements.is_empty() {
-        call.args = vec![];
-    } else {
-        tree = Expression::Set(Box::new(Set {
-            elements,
-            lbrace: LeftCurlyBrace { whitespace_after },
-            rbrace: RightCurlyBrace { whitespace_before },
-            lpar: vec![],
-            rpar: vec![],
-        }));
-    }
-
-    Ok(Edit::range_replacement(
-        pad_expression(
-            tree.codegen_stylist(stylist),
-            expr.range(),
-            checker.locator(),
-            checker.semantic(),
-        ),
-        expr.range(),
-    ))
-}
-
 /// (C406) Convert `dict([(1, 2)])` to `{1: 2}`.
 pub(crate) fn fix_unnecessary_literal_dict(expr: &Expr, checker: &Checker) -> Result<Edit> {
     let locator = checker.locator();
@@ -511,56 +422,6 @@ pub(crate) fn pad_end(
     }
 }
 
-/// (C409) Convert `tuple([1, 2])` to `tuple(1, 2)`
-pub(crate) fn fix_unnecessary_literal_within_tuple_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::Tuple(Box::new(Tuple {
-        elements: elements.clone(),
-        lpar: vec![LeftParen {
-            whitespace_after: whitespace_after.clone(),
-        }],
-        rpar: vec![RightParen {
-            whitespace_before: whitespace_before.clone(),
-        }],
-    }));
-
-    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,

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_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::{self as ast, Expr};
+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;
 
@@ -53,16 +51,13 @@ impl AlwaysFixableViolation for UnnecessaryLiteralSet {
 }
 
 /// C405 (`set([1, 2])`)
-pub(crate) fn unnecessary_literal_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_literal_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") {
@@ -73,13 +68,69 @@ pub(crate) fn unnecessary_literal_set(
         Expr::Tuple(_) => "tuple",
         _ => return,
     };
+
     let mut diagnostic = Diagnostic::new(
         UnnecessaryLiteralSet {
             obj_type: kind.to_string(),
         },
-        expr.range(),
+        call.range(),
     );
-    diagnostic
-        .try_set_fix(|| fixes::fix_unnecessary_literal_set(expr, checker).map(Fix::unsafe_edit));
+
+    // Convert `set((1, 2))` to `{1, 2}`.
+    diagnostic.set_fix({
+        let elts = match &argument {
+            Expr::List(ast::ExprList { elts, .. }) => elts,
+            Expr::Tuple(ast::ExprTuple { elts, .. }) => elts,
+            _ => unreachable!(),
+        };
+
+        match elts.as_slice() {
+            // If the list or tuple is empty, replace the entire call with `set()`.
+            [] => Fix::unsafe_edit(Edit::range_replacement("set()".to_string(), call.range())),
+            // If it's a single-element tuple (with no whitespace around it), remove the trailing
+            // comma.
+            [elt]
+                if argument.is_tuple_expr()
+                    // The element must start right after the `(`.
+                    && elt.start() == argument.start() + TextSize::new(1)
+                    // The element must be followed by exactly one comma and a closing `)`.
+                    && elt.end() + TextSize::new(2) == argument.end() =>
+            {
+                // Replace from the start of the call to the start of the inner element.
+                let call_start = Edit::replacement(
+                    pad_start("{", call.range(), checker.locator(), checker.semantic()),
+                    call.start(),
+                    elt.start(),
+                );
+
+                // Replace from the end of the inner element to the end of the call with `}`.
+                let call_end = Edit::replacement(
+                    pad_end("}", call.range(), checker.locator(), checker.semantic()),
+                    elt.end(),
+                    call.end(),
+                );
+
+                Fix::unsafe_edits(call_start, [call_end])
+            }
+            _ => {
+                // Replace from the start of the call to the start of the inner list or tuple with `{`.
+                let call_start = Edit::replacement(
+                    pad_start("{", call.range(), checker.locator(), checker.semantic()),
+                    call.start(),
+                    argument.start() + TextSize::from(1),
+                );
+
+                // Replace from the end of the inner list or tuple to the end of the call with `}`.
+                let call_end = Edit::replacement(
+                    pad_end("}", call.range(), checker.locator(), checker.semantic()),
+                    argument.end() - TextSize::from(1),
+                    call.end(),
+                );
+
+                Fix::unsafe_edits(call_start, [call_end])
+            }
+        }
+    });
+
     checker.diagnostics.push(diagnostic);
 }