From d38de22795875a940de6ad7a1981d415037fe95e Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Thu, 30 Jan 2025 15:21:33 -0600 Subject: [PATCH 1/4] add fixture --- .../test/fixtures/flake8_comprehensions/C420_1.py | 7 +++++++ crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs | 1 + 2 files changed, 8 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C420_1.py diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C420_1.py b/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C420_1.py new file mode 100644 index 0000000000000..550b6d9c2e6f5 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C420_1.py @@ -0,0 +1,7 @@ +{x: NotImplemented for x in "XY"} + + +# Builtin bindings are placed at top of file, but should not count as +# an "expression defined within the comprehension". So the above +# should trigger C420 +# See https://github.com/astral-sh/ruff/issues/15830 diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs index 440dbc183fb9f..2ef6e2746e912 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs @@ -22,6 +22,7 @@ mod tests { #[test_case(Rule::UnnecessaryComprehensionInCall, Path::new("C419.py"))] #[test_case(Rule::UnnecessaryComprehensionInCall, Path::new("C419_2.py"))] #[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("C420.py"))] + #[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("C420_1.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"))] From 73f67c3aa65b198e6e82b75e0565bc9bb1346ed2 Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Thu, 30 Jan 2025 15:23:17 -0600 Subject: [PATCH 2/4] never count builtins as being defined within the comprehension --- .../rules/unnecessary_dict_comprehension_for_iterable.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_dict_comprehension_for_iterable.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_dict_comprehension_for_iterable.rs index 3d2efb2f79ca7..cbd6fc8acc0d6 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_dict_comprehension_for_iterable.rs +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_dict_comprehension_for_iterable.rs @@ -99,6 +99,10 @@ pub(crate) fn unnecessary_dict_comprehension_for_iterable( let binding = checker.semantic().binding(id); + if binding.kind.is_builtin() { + return false; + } + dict_comp.range().contains_range(binding.range()) }); if self_referential { From 61755b041ae84699bc4efaf6c27ab7a06b0022ec Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Thu, 30 Jan 2025 15:23:28 -0600 Subject: [PATCH 3/4] snapshot --- ...e8_comprehensions__tests__C420_C420_1.py.snap | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C420_C420_1.py.snap diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C420_C420_1.py.snap b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C420_C420_1.py.snap new file mode 100644 index 0000000000000..9551c10fdebb9 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C420_C420_1.py.snap @@ -0,0 +1,16 @@ +--- +source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs +--- +C420_1.py:1:1: C420 [*] Unnecessary dict comprehension for iterable; use `dict.fromkeys` instead + | +1 | {x: NotImplemented for x in "XY"} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C420 + | + = help: Replace with `dict.fromkeys(iterable)`) + +ℹ Safe fix +1 |-{x: NotImplemented for x in "XY"} + 1 |+dict.fromkeys("XY", NotImplemented) +2 2 | +3 3 | +4 4 | # Builtin bindings are placed at top of file, but should not count as From ac480eb3b9b29a1af16a6036e4840c04d746e1bc Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Thu, 30 Jan 2025 15:41:39 -0600 Subject: [PATCH 4/4] clarifying comment --- .../rules/unnecessary_dict_comprehension_for_iterable.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_dict_comprehension_for_iterable.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_dict_comprehension_for_iterable.rs index cbd6fc8acc0d6..c6533ebec6459 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_dict_comprehension_for_iterable.rs +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_dict_comprehension_for_iterable.rs @@ -99,6 +99,10 @@ pub(crate) fn unnecessary_dict_comprehension_for_iterable( let binding = checker.semantic().binding(id); + // Builtin bindings have a range of 0..0, and are never + // defined within the comprehension, so we abort before + // checking the range overlap below. Note this only matters + // if the comprehension appears at the top of the file! if binding.kind.is_builtin() { return false; }