Skip to content

Commit 4f2aea8

Browse files
authored
[flake8-comprehensions] Handle builtins at top of file correctly for unnecessary-dict-comprehension-for-iterable (C420) (#15837)
Builtin bindings are given a range of `0..0`, which causes strange behavior when range checks are made at the top of the file. In this case, the logic of the rule demands that the value of the dict comprehension is not self-referential (i.e. it does not contain definitions for any of the variables used within it). This logic was confused by builtins which looked like they were defined "in the comprehension", if the comprehension appeared at the top of the file. Closes #15830
1 parent 5c77898 commit 4f2aea8

File tree

4 files changed

+32
-0
lines changed

4 files changed

+32
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{x: NotImplemented for x in "XY"}
2+
3+
4+
# Builtin bindings are placed at top of file, but should not count as
5+
# an "expression defined within the comprehension". So the above
6+
# should trigger C420
7+
# See https://github.com/astral-sh/ruff/issues/15830

crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ mod tests {
2222
#[test_case(Rule::UnnecessaryComprehensionInCall, Path::new("C419.py"))]
2323
#[test_case(Rule::UnnecessaryComprehensionInCall, Path::new("C419_2.py"))]
2424
#[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("C420.py"))]
25+
#[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("C420_1.py"))]
2526
#[test_case(Rule::UnnecessaryDoubleCastOrProcess, Path::new("C414.py"))]
2627
#[test_case(Rule::UnnecessaryGeneratorDict, Path::new("C402.py"))]
2728
#[test_case(Rule::UnnecessaryGeneratorList, Path::new("C400.py"))]

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_dict_comprehension_for_iterable.rs

+8
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,14 @@ pub(crate) fn unnecessary_dict_comprehension_for_iterable(
9999

100100
let binding = checker.semantic().binding(id);
101101

102+
// Builtin bindings have a range of 0..0, and are never
103+
// defined within the comprehension, so we abort before
104+
// checking the range overlap below. Note this only matters
105+
// if the comprehension appears at the top of the file!
106+
if binding.kind.is_builtin() {
107+
return false;
108+
}
109+
102110
dict_comp.range().contains_range(binding.range())
103111
});
104112
if self_referential {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs
3+
---
4+
C420_1.py:1:1: C420 [*] Unnecessary dict comprehension for iterable; use `dict.fromkeys` instead
5+
|
6+
1 | {x: NotImplemented for x in "XY"}
7+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C420
8+
|
9+
= help: Replace with `dict.fromkeys(iterable)`)
10+
11+
Safe fix
12+
1 |-{x: NotImplemented for x in "XY"}
13+
1 |+dict.fromkeys("XY", NotImplemented)
14+
2 2 |
15+
3 3 |
16+
4 4 | # Builtin bindings are placed at top of file, but should not count as

0 commit comments

Comments
 (0)