Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[`flake8-comprehensions`] Skip `C416` if comprehension contains unpacking (#14909) 

<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

Fix #11482. Applies
https://github.com/adamchainz/flake8-comprehensions/pull/205 to ruff.
`C416` should be skipped if comprehension contains unpacking. Here's an
example:

```python
list_of_lists = [[1, 2], [3, 4]]

# ruff suggests `list(list_of_lists)` here, but that would change the result.
# `list(list_of_lists)` is not `[(1, 2), (3, 4)]`
a = [(x, y) for x, y in list_of_lists]

# This is equivalent to `list(list_of_lists)`
b = [x for x in list_of_lists]
```

## Test Plan

<!-- How was it tested? -->

Existing checks

---------

Signed-off-by: harupy <hkawamura0130@gmail.com>
This commit is contained in:
Harutaka Kawamura 2024-12-20 18:05:30 +09:00 committed by GitHub
parent c0b0491703
commit 6195c026ff
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 126 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C416.py vendored
View File

@ -8,6 +8,7 @@ d = {"a": 1, "b": 2, "c": 3}
{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]
@ -22,3 +23,7 @@ d = {"a": 1, "b": 2, "c": 3}
# Regression test for: https://github.com/astral-sh/ruff/issues/7196
any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType])
zz = [[1], [2], [3]]
[(i,) for (i,) in zz] # != list(zz)
{(i,) for (i,) in zz} # != set(zz)

97
crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs
View File

@ -1,7 +1,7 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_ast::{self as ast, Comprehension, Expr};
use ruff_python_semantic::analyze::typing;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -115,16 +115,22 @@ pub(crate) fn unnecessary_dict_comprehension(
let Expr::Tuple(ast::ExprTuple { elts, .. }) = &generator.target else {
return;
};
let [target_key, target_value] = elts.as_slice() else {
let [Expr::Name(ast::ExprName { id: target_key, .. }), Expr::Name(ast::ExprName {
id: target_value, ..
})] = elts.as_slice()
else {
return;
};
if ComparableExpr::from(key) != ComparableExpr::from(target_key) {
let Expr::Name(ast::ExprName { id: key, .. }) = &key else {
return;
}
if ComparableExpr::from(value) != ComparableExpr::from(target_value) {
};
let Expr::Name(ast::ExprName { id: value, .. }) = &value else {
return;
};
if target_key == key && target_value == value {
add_diagnostic(checker, expr);
}
add_diagnostic(checker, expr);
}
/// C416
@ -140,10 +146,83 @@ pub(crate) fn unnecessary_list_set_comprehension(
if !generator.ifs.is_empty() || generator.is_async {
return;
}
if ComparableExpr::from(elt) != ComparableExpr::from(&generator.target) {
return;
if is_dict_items(checker, &generator.iter) {
match (&generator.target, elt) {
// [(k, v) for k, v in dict.items()] or [(k, v) for [k, v] in dict.items()]
(
Expr::Tuple(ast::ExprTuple {
elts: target_elts, ..
})
| Expr::List(ast::ExprList {
elts: target_elts, ..
}),
Expr::Tuple(ast::ExprTuple { elts, .. }),
) => {
let [Expr::Name(ast::ExprName { id: target_key, .. }), Expr::Name(ast::ExprName {
id: target_value, ..
})] = target_elts.as_slice()
else {
return;
};
let [Expr::Name(ast::ExprName { id: key, .. }), Expr::Name(ast::ExprName { id: value, .. })] =
elts.as_slice()
else {
return;
};
if target_key == key && target_value == value {
add_diagnostic(checker, expr);
}
}
// [x for x in dict.items()]
(
Expr::Name(ast::ExprName {
id: target_name, ..
}),
Expr::Name(ast::ExprName { id: elt_name, .. }),
) if target_name == elt_name => {
add_diagnostic(checker, expr);
}
_ => {}
}
} else {
// [x for x in iterable]
let Expr::Name(ast::ExprName {
id: target_name, ..
}) = &generator.target
else {
return;
};
let Expr::Name(ast::ExprName { id: elt_name, .. }) = &elt else {
return;
};
if elt_name == target_name {
add_diagnostic(checker, expr);
}
}
add_diagnostic(checker, expr);
}
fn is_dict_items(checker: &Checker, expr: &Expr) -> bool {
let Expr::Call(ast::ExprCall { func, .. }) = expr else {
return false;
};
let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func.as_ref() else {
return false;
};
if attr.as_str() != "items" {
return false;
}
let Expr::Name(name) = value.as_ref() else {
return false;
};
let Some(id) = checker.semantic().resolve_name(name) else {
return false;
};
typing::is_dict(checker.semantic().binding(id), checker.semantic())
}
#[derive(Debug, Copy, Clone, PartialEq, Eq)]

60
crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C416_C416.py.snap
View File

@ -1,5 +1,6 @@
---
source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs
snapshot_kind: text
---
C416.py:6:1: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
|
@ -61,7 +62,7 @@ C416.py:8:1: C416 [*] Unnecessary dict comprehension (rewrite using `dict()`)
8 |+dict(y)
9 9 | {k: v for k, v in d.items()}
10 10 | [(k, v) for k, v in d.items()]
11 11 | {k: (a, b) for k, (a, b) in d.items()}
11 11 | [(k, v) for [k, v] in d.items()]
C416.py:9:1: C416 [*] Unnecessary dict comprehension (rewrite using `dict()`)
|
@ -70,7 +71,7 @@ C416.py:9:1: C416 [*] Unnecessary dict comprehension (rewrite using `dict()`)
9 | {k: v for k, v in d.items()}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416
10 | [(k, v) for k, v in d.items()]
11 | {k: (a, b) for k, (a, b) in d.items()}
11 | [(k, v) for [k, v] in d.items()]
|
= help: Rewrite using `dict()`
@ -81,8 +82,8 @@ C416.py:9:1: C416 [*] Unnecessary dict comprehension (rewrite using `dict()`)
9 |-{k: v for k, v in d.items()}
9 |+dict(d.items())
10 10 | [(k, v) for k, v in d.items()]
11 11 | {k: (a, b) for k, (a, b) in d.items()}
12 12 |
11 11 | [(k, v) for [k, v] in d.items()]
12 12 | {k: (a, b) for k, (a, b) in d.items()}
C416.py:10:1: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
|
@ -90,7 +91,8 @@ C416.py:10:1: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
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()}
11 | [(k, v) for [k, v] in d.items()]
12 | {k: (a, b) for k, (a, b) in d.items()}
|
= help: Rewrite using `list()`
@ -100,42 +102,46 @@ C416.py:10:1: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
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]
11 11 | [(k, v) for [k, v] in d.items()]
12 12 | {k: (a, b) for k, (a, b) in d.items()}
13 13 |
C416.py:11:1: C416 [*] Unnecessary dict comprehension (rewrite using `dict()`)
C416.py:11:1: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
|
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]
11 | [(k, v) for [k, v] in d.items()]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416
12 | {k: (a, b) for k, (a, b) in d.items()}
|
= help: Rewrite using `dict()`
= help: Rewrite using `list()`
ℹ Unsafe 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]
11 |-[(k, v) for [k, v] in d.items()]
11 |+list(d.items())
12 12 | {k: (a, b) for k, (a, b) in d.items()}
13 13 |
14 14 | [i for i, in z]
C416.py:24:70: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
C416.py:25: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])
24 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196
25 | any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType])
| ^^^^^^^^^^^^^^^^^^^^^^^ C416
26 |
27 | zz = [[1], [2], [3]]
|
= help: Rewrite using `list()`
ℹ Unsafe fix
21 21 | {k: v if v else None for k, v in y}
22 22 |
23 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])
24 |+any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in list(SymbolType))
22 22 | {k: v if v else None for k, v in y}
23 23 |
24 24 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196
25 |-any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType])
25 |+any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in list(SymbolType))
26 26 |
27 27 | zz = [[1], [2], [3]]
28 28 | [(i,) for (i,) in zz] # != list(zz)