Extend C416 to catch tuple unpacking (#7363)

Closes https://github.com/astral-sh/ruff/issues/7307.
This commit is contained in:
Charlie Marsh 2023-09-14 11:53:15 -04:00 committed by GitHub
parent 5d21b9c22e
commit d39eae2713
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 72 additions and 45 deletions

View File

@ -7,6 +7,8 @@ d = {"a": 1, "b": 2, "c": 3}
{i for i in x} {i for i in x}
{k: v for k, v in y} {k: v for k, v in y}
{k: v for k, v in d.items()} {k: v for k, v in d.items()}
[(k, v) for k, v in d.items()]
{k: (a, b) for k, (a, b) in d.items()}
[i for i, in z] [i for i, in z]
[i for i, j in y] [i for i, j in y]

View File

@ -1,5 +1,6 @@
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_ast::{self as ast, Comprehension, Expr}; use ruff_python_ast::{self as ast, Comprehension, Expr};
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
@ -86,28 +87,16 @@ pub(crate) fn unnecessary_dict_comprehension(
if !generator.ifs.is_empty() || generator.is_async { if !generator.ifs.is_empty() || generator.is_async {
return; return;
} }
let Some(key) = key.as_name_expr() else {
return;
};
let Some(value) = value.as_name_expr() else {
return;
};
let Expr::Tuple(ast::ExprTuple { elts, .. }) = &generator.target else { let Expr::Tuple(ast::ExprTuple { elts, .. }) = &generator.target else {
return; return;
}; };
let [target_key, target_value] = elts.as_slice() else { let [target_key, target_value] = elts.as_slice() else {
return; return;
}; };
let Some(target_key) = target_key.as_name_expr() else { if ComparableExpr::from(key) != ComparableExpr::from(target_key) {
return;
};
let Some(target_value) = target_value.as_name_expr() else {
return;
};
if target_key.id != key.id {
return; return;
} }
if target_value.id != value.id { if ComparableExpr::from(value) != ComparableExpr::from(target_value) {
return; return;
} }
add_diagnostic(checker, expr); add_diagnostic(checker, expr);
@ -126,13 +115,7 @@ pub(crate) fn unnecessary_list_set_comprehension(
if !generator.ifs.is_empty() || generator.is_async { if !generator.ifs.is_empty() || generator.is_async {
return; return;
} }
let Some(elt) = elt.as_name_expr() else { if ComparableExpr::from(elt) != ComparableExpr::from(&generator.target) {
return;
};
let Some(target) = generator.target.as_name_expr() else {
return;
};
if elt.id != target.id {
return; return;
} }
add_diagnostic(checker, expr); add_diagnostic(checker, expr);

View File

@ -40,7 +40,7 @@ C416.py:7:1: C416 [*] Unnecessary `set` comprehension (rewrite using `set()`)
7 |+set(x) 7 |+set(x)
8 8 | {k: v for k, v in y} 8 8 | {k: v for k, v in y}
9 9 | {k: v for k, v in d.items()} 9 9 | {k: v for k, v in d.items()}
10 10 | 10 10 | [(k, v) for k, v in d.items()]
C416.py:8:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`) C416.py:8:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`)
| |
@ -49,6 +49,7 @@ C416.py:8:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`)
8 | {k: v for k, v in y} 8 | {k: v for k, v in y}
| ^^^^^^^^^^^^^^^^^^^^ C416 | ^^^^^^^^^^^^^^^^^^^^ C416
9 | {k: v for k, v in d.items()} 9 | {k: v for k, v in d.items()}
10 | [(k, v) for k, v in d.items()]
| |
= help: Rewrite using `dict()` = help: Rewrite using `dict()`
@ -59,8 +60,8 @@ C416.py:8:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`)
8 |-{k: v for k, v in y} 8 |-{k: v for k, v in y}
8 |+dict(y) 8 |+dict(y)
9 9 | {k: v for k, v in d.items()} 9 9 | {k: v for k, v in d.items()}
10 10 | 10 10 | [(k, v) for k, v in d.items()]
11 11 | [i for i, in z] 11 11 | {k: (a, b) for k, (a, b) in d.items()}
C416.py:9:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`) C416.py:9:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`)
| |
@ -68,8 +69,8 @@ C416.py:9:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`)
8 | {k: v for k, v in y} 8 | {k: v for k, v in y}
9 | {k: v for k, v in d.items()} 9 | {k: v for k, v in d.items()}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416
10 | 10 | [(k, v) for k, v in d.items()]
11 | [i for i, in z] 11 | {k: (a, b) for k, (a, b) in d.items()}
| |
= help: Rewrite using `dict()` = help: Rewrite using `dict()`
@ -79,23 +80,64 @@ C416.py:9:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`)
8 8 | {k: v for k, v in y} 8 8 | {k: v for k, v in y}
9 |-{k: v for k, v in d.items()} 9 |-{k: v for k, v in d.items()}
9 |+dict(d.items()) 9 |+dict(d.items())
10 10 | 10 10 | [(k, v) for k, v in d.items()]
11 11 | [i for i, in z] 11 11 | {k: (a, b) for k, (a, b) in d.items()}
12 12 | [i for i, j in y] 12 12 |
C416.py:22:70: C416 [*] Unnecessary `list` comprehension (rewrite using `list()`) C416.py:10:1: C416 [*] Unnecessary `list` comprehension (rewrite using `list()`)
| |
21 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196 8 | {k: v for k, v in y}
22 | any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType]) 9 | {k: v for k, v in d.items()}
10 | [(k, v) for k, v in d.items()]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416
11 | {k: (a, b) for k, (a, b) in d.items()}
|
= help: Rewrite using `list()`
Suggested fix
7 7 | {i for i in x}
8 8 | {k: v for k, v in y}
9 9 | {k: v for k, v in d.items()}
10 |-[(k, v) for k, v in d.items()]
10 |+list(d.items())
11 11 | {k: (a, b) for k, (a, b) in d.items()}
12 12 |
13 13 | [i for i, in z]
C416.py:11:1: C416 [*] Unnecessary `dict` comprehension (rewrite using `dict()`)
|
9 | {k: v for k, v in d.items()}
10 | [(k, v) for k, v in d.items()]
11 | {k: (a, b) for k, (a, b) in d.items()}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416
12 |
13 | [i for i, in z]
|
= help: Rewrite using `dict()`
Suggested fix
8 8 | {k: v for k, v in y}
9 9 | {k: v for k, v in d.items()}
10 10 | [(k, v) for k, v in d.items()]
11 |-{k: (a, b) for k, (a, b) in d.items()}
11 |+dict(d.items())
12 12 |
13 13 | [i for i, in z]
14 14 | [i for i, j in y]
C416.py:24:70: C416 [*] Unnecessary `list` comprehension (rewrite using `list()`)
|
23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196
24 | any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType])
| ^^^^^^^^^^^^^^^^^^^^^^^ C416 | ^^^^^^^^^^^^^^^^^^^^^^^ C416
| |
= help: Rewrite using `list()` = help: Rewrite using `list()`
Suggested fix Suggested fix
19 19 | {k: v if v else None for k, v in y} 21 21 | {k: v if v else None for k, v in y}
20 20 | 22 22 |
21 21 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196 23 23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196
22 |-any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType]) 24 |-any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType])
22 |+any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in list(SymbolType)) 24 |+any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in list(SymbolType))