mirror of
https://github.com/astral-sh/ruff.git
synced 2025-11-03 05:13:00 +00:00
[flake8-comprehensions] Fix false positive for C420 with attribute, subscript, or slice assignment targets (#19513)
## Summary Fixes #19511
This commit is contained in:
parent
0095ff4c1a
commit
0ec4801b0d
4 changed files with 63 additions and 0 deletions
43
crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C420_3.py
vendored
Normal file
43
crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C420_3.py
vendored
Normal file
|
|
@ -0,0 +1,43 @@
|
|||
class C: a = None
|
||||
{C.a: None for C.a in "abc"}
|
||||
print(C.a)
|
||||
|
||||
x = [None]
|
||||
{x[0]: None for x[0] in "abc"}
|
||||
print(x)
|
||||
|
||||
class C(list):
|
||||
def __getitem__(self, index, /):
|
||||
item = super().__getitem__(index)
|
||||
if isinstance(index, slice): item = tuple(item)
|
||||
return item
|
||||
x = C()
|
||||
{x[:0]: None for x[:0] in "abc"}
|
||||
print(x)
|
||||
|
||||
|
||||
class C:
|
||||
a = None
|
||||
|
||||
def func():
|
||||
{(C.a,): None for (C.a,) in "abc"} # OK
|
||||
|
||||
|
||||
def func():
|
||||
obj = type('obj', (), {'attr': 1})()
|
||||
{(obj.attr,): None for (obj.attr,) in "abc"} # OK
|
||||
|
||||
|
||||
def func():
|
||||
lst = [1, 2, 3]
|
||||
{(lst[0],): None for (lst[0],) in "abc"} # OK
|
||||
|
||||
|
||||
def func():
|
||||
lst = [1, 2, 3, 4, 5]
|
||||
{(lst[1:3],): None for (lst[1:3],) in "abc"} # OK
|
||||
|
||||
|
||||
# C420: side-effecting assignment targets
|
||||
# These should NOT trigger C420 because they have side-effecting assignment targets
|
||||
# See https://github.com/astral-sh/ruff/issues/19511
|
||||
|
|
@ -25,6 +25,7 @@ mod tests {
|
|||
#[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("C420.py"))]
|
||||
#[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("C420_1.py"))]
|
||||
#[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("C420_2.py"))]
|
||||
#[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("C420_3.py"))]
|
||||
#[test_case(Rule::UnnecessaryDoubleCastOrProcess, Path::new("C414.py"))]
|
||||
#[test_case(Rule::UnnecessaryGeneratorDict, Path::new("C402.py"))]
|
||||
#[test_case(Rule::UnnecessaryGeneratorList, Path::new("C400.py"))]
|
||||
|
|
|
|||
|
|
@ -96,6 +96,12 @@ pub(crate) fn unnecessary_dict_comprehension_for_iterable(
|
|||
return;
|
||||
}
|
||||
|
||||
// Don't suggest `dict.fromkeys` if the target contains side-effecting expressions
|
||||
// (attributes, subscripts, or slices).
|
||||
if contains_side_effecting_sub_expression(&generator.target) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Don't suggest `dict.fromkeys` if the value is not a constant or constant-like.
|
||||
if !is_constant_like(dict_comp.value.as_ref()) {
|
||||
return;
|
||||
|
|
@ -217,3 +223,12 @@ fn fix_unnecessary_dict_comprehension(value: &Expr, generator: &Comprehension) -
|
|||
node_index: ruff_python_ast::AtomicNodeIndex::dummy(),
|
||||
})
|
||||
}
|
||||
|
||||
fn contains_side_effecting_sub_expression(target: &Expr) -> bool {
|
||||
any_over_expr(target, &|expr| {
|
||||
matches!(
|
||||
expr,
|
||||
Expr::Attribute(_) | Expr::Subscript(_) | Expr::Slice(_)
|
||||
)
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs
|
||||
---
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue