Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid incorrect tuple transformation in single-element case (C409) #10491

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -16,3 +16,11 @@
tuple([ # comment
1, 2
])

tuple((
1,
))

t6 = tuple([1])
t7 = tuple((1,))
t8 = tuple([1,])
@@ -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;

Expand Down Expand Up @@ -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") {
Expand All @@ -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);
Expand Down
Expand Up @@ -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

Expand All @@ -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,)