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 acd98fd9303584..1e1b16f6264fa5 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,13 @@ tuple([ # comment 1, 2 ]) + +tuple( + ( + 1, + ) +) + +t6 = tuple([1]) +t7 = 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 94d8ecb21ac3d3..40b89499e8ce93 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,7 @@ 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_text_size::Ranged; use crate::checkers::ast::Checker; @@ -71,9 +71,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 +95,26 @@ 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(), - call.start(), - argument.start() + TextSize::from(1), - ); + let elts: Vec = match argument { + Expr::List(list_expr) => list_expr.elts.clone(), + Expr::Tuple(tuple_expr) => tuple_expr.elts.clone(), + _ => return, + }; - // Replace from the end of the inner list or tuple to the end of the call with `)`. - let call_end = Edit::replacement( - ")".to_string(), - argument.end() - TextSize::from(1), - call.end(), - ); + // Replace [1, 2] -> (1, 2) or [1] -> (1,) + let tuple: String = if let [elt] = elts.as_slice() { + let elt: &str = checker.locator().slice(elt); + format!("({},)", elt) + } else { + let elt_set: &str = checker.locator().slice(argument); + format!("({})", &elt_set[1..elt_set.len() - 1]) + }; + + let edit = Edit::replacement(tuple, call.start(), call.end()); - Fix::unsafe_edits(call_start, [call_end]) + Fix::unsafe_edit(edit) }); 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 45726f1f3aed23..10b1aaf9e6c966 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 @@ -1,5 +1,6 @@ --- source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs +assertion_line: 42 --- C409.py:1:6: C409 [*] Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal) | @@ -143,6 +144,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 +158,70 @@ 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 | ( +C409.py:20:1: C409 [*] Unnecessary `tuple` literal passed to `tuple()` (remove the outer call to `tuple()`) + | +18 | ]) +19 | +20 | / tuple( +21 | | ( +22 | | 1, +23 | | ) +24 | | ) + | |_^ C409 +25 | +26 | t6 = tuple([1]) + | + = help: Remove outer `tuple` call + +ℹ Unsafe fix +17 17 | 1, 2 +18 18 | ]) +19 19 | +20 |-tuple( +21 |- ( +22 |- 1, +23 |- ) +24 |-) + 20 |+(1,) +25 21 | +26 22 | t6 = tuple([1]) +27 23 | t7 = tuple((1,)) + +C409.py:26:6: C409 [*] Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal) + | +24 | ) +25 | +26 | t6 = tuple([1]) + | ^^^^^^^^^^ C409 +27 | t7 = tuple((1,)) + | + = help: Rewrite as a `tuple` literal +ℹ Unsafe fix +23 23 | ) +24 24 | ) +25 25 | +26 |-t6 = tuple([1]) + 26 |+t6 = (1,) +27 27 | t7 = tuple((1,)) +28 28 | + +C409.py:27:6: C409 [*] Unnecessary `tuple` literal passed to `tuple()` (remove the outer call to `tuple()`) + | +26 | t6 = tuple([1]) +27 | t7 = tuple((1,)) + | ^^^^^^^^^^^ C409 + | + = help: Remove outer `tuple` call + +ℹ Unsafe fix +24 24 | ) +25 25 | +26 26 | t6 = tuple([1]) +27 |-t7 = tuple((1,)) + 27 |+t7 = (1,) +28 28 |