diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C409.py b/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C409.py index acd98fd930358..c38feff8f5aea 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C409.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C409.py @@ -16,3 +16,11 @@ tuple([ # comment 1, 2 ]) + +tuple(( + 1, +)) + +t6 = tuple([1]) +t7 = tuple((1,)) +t8 = tuple([1,]) diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs index 94d8ecb21ac3d..5ad050bfa5a7e 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs @@ -1,7 +1,8 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, Expr}; -use ruff_text_size::{Ranged, TextSize}; +use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; +use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::checkers::ast::Checker; @@ -71,9 +72,12 @@ pub(crate) fn unnecessary_literal_within_tuple_call(checker: &mut Checker, call: if !call.arguments.keywords.is_empty() { return; } - let Some(argument) = - helpers::first_argument_with_matching_function("tuple", &call.func, &call.arguments.args) - else { + let Some(argument) = helpers::exactly_one_argument_with_matching_function( + "tuple", + &call.func, + &call.arguments.args, + &call.arguments.keywords, + ) else { return; }; if !checker.semantic().is_builtin("tuple") { @@ -92,23 +96,41 @@ pub(crate) fn unnecessary_literal_within_tuple_call(checker: &mut Checker, call: call.range(), ); - // Convert `tuple([1, 2])` to `tuple(1, 2)` + // Convert `tuple([1, 2])` to `(1, 2)` diagnostic.set_fix({ - // Replace from the start of the call to the start of the inner list or tuple with `(`. - let call_start = Edit::replacement( - "(".to_string(), + let elts = match argument { + Expr::List(ast::ExprList { elts, .. }) => elts.as_slice(), + Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.as_slice(), + _ => return, + }; + + let needs_trailing_comma = if let [item] = elts { + SimpleTokenizer::new( + checker.locator().contents(), + TextRange::new(item.end(), call.end()), + ) + .all(|token| token.kind != SimpleTokenKind::Comma) + } else { + false + }; + + // Replace `[` with `(`. + let elt_start = Edit::replacement( + "(".into(), 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( - ")".to_string(), + // Replace `]` with `)` or `,)`. + let elt_end = Edit::replacement( + if needs_trailing_comma { + ",)".into() + } else { + ")".into() + }, argument.end() - TextSize::from(1), call.end(), ); - - Fix::unsafe_edits(call_start, [call_end]) + Fix::unsafe_edits(elt_start, [elt_end]) }); checker.diagnostics.push(diagnostic); diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C409_C409.py.snap b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C409_C409.py.snap index 45726f1f3aed2..e7feb37b081cd 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C409_C409.py.snap +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C409_C409.py.snap @@ -143,6 +143,8 @@ C409.py:16:1: C409 [*] Unnecessary `list` literal passed to `tuple()` (rewrite a 17 | | 1, 2 18 | | ]) | |__^ C409 +19 | +20 | tuple(( | = help: Rewrite as a `tuple` literal @@ -155,5 +157,85 @@ C409.py:16:1: C409 [*] Unnecessary `list` literal passed to `tuple()` (rewrite a 17 17 | 1, 2 18 |-]) 18 |+) +19 19 | +20 20 | tuple(( +21 21 | 1, +C409.py:20:1: C409 [*] Unnecessary `tuple` literal passed to `tuple()` (remove the outer call to `tuple()`) + | +18 | ]) +19 | +20 | / tuple(( +21 | | 1, +22 | | )) + | |__^ C409 +23 | +24 | t6 = tuple([1]) + | + = help: Remove outer `tuple` call + +ℹ Unsafe fix +17 17 | 1, 2 +18 18 | ]) +19 19 | +20 |-tuple(( + 20 |+( +21 21 | 1, +22 |-)) + 22 |+) +23 23 | +24 24 | t6 = tuple([1]) +25 25 | t7 = tuple((1,)) + +C409.py:24:6: C409 [*] Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal) + | +22 | )) +23 | +24 | t6 = tuple([1]) + | ^^^^^^^^^^ C409 +25 | t7 = tuple((1,)) +26 | t8 = tuple([1,]) + | + = help: Rewrite as a `tuple` literal + +ℹ Unsafe fix +21 21 | 1, +22 22 | )) +23 23 | +24 |-t6 = tuple([1]) + 24 |+t6 = (1,) +25 25 | t7 = tuple((1,)) +26 26 | t8 = tuple([1,]) + +C409.py:25:6: C409 [*] Unnecessary `tuple` literal passed to `tuple()` (remove the outer call to `tuple()`) + | +24 | t6 = tuple([1]) +25 | t7 = tuple((1,)) + | ^^^^^^^^^^^ C409 +26 | t8 = tuple([1,]) + | + = help: Remove outer `tuple` call + +ℹ Unsafe fix +22 22 | )) +23 23 | +24 24 | t6 = tuple([1]) +25 |-t7 = tuple((1,)) + 25 |+t7 = (1,) +26 26 | t8 = tuple([1,]) + +C409.py:26:6: C409 [*] Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal) + | +24 | t6 = tuple([1]) +25 | t7 = tuple((1,)) +26 | t8 = tuple([1,]) + | ^^^^^^^^^^^ C409 + | + = help: Rewrite as a `tuple` literal +ℹ Unsafe fix +23 23 | +24 24 | t6 = tuple([1]) +25 25 | t7 = tuple((1,)) +26 |-t8 = tuple([1,]) + 26 |+t8 = (1,)