Skip to content

Commit

Permalink
#10323: Fixed incorrect rule transformation rule C409 (unnecessary-li…
Browse files Browse the repository at this point in the history
…teral-within-tuple-call)
  • Loading branch information
WindowGenerator committed Mar 20, 2024
1 parent ad84eed commit 41107a8
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 18 deletions.
Expand Up @@ -16,3 +16,13 @@
tuple([ # comment
1, 2
])

tuple(
(
1,
)
)

t6 = tuple([1])
t7 = tuple((1,))

@@ -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;

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

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

0 comments on commit 41107a8

Please sign in to comment.