[flake8-simplify] Fix false negatives for shadowed bindings (SIM910, SIM911) (#18794)

<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
  requests.)
- Does this pull request include references to any relevant issues?
-->

## Summary
I also noticed that the tests for SIM911 were note being run, so I fixed
that.

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

## Test Plan
Add regression test
<!-- How was it tested? -->
This commit is contained in:
Victor Hugo Gomes 2025-06-20 14:25:36 -03:00 committed by GitHub
parent dc160c4a49
commit ffb09c84f2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 108 additions and 2 deletions

View file

@ -49,3 +49,9 @@ def foo(some_other: object):
# OK
def foo(some_other):
a = some_other.get('a', None)
# https://github.com/astral-sh/ruff/issues/18777
def foo():
dict = {"Tom": 23, "Maria": 23, "Dog": 11}
age = dict.get("Cat", None)

View file

@ -23,3 +23,9 @@ for k, v in zip(d2.keys(), d2.values()): # SIM911
items = zip(x.keys(), x.values()) # OK
items.bar = zip(x.keys(), x.values()) # OK
# https://github.com/astral-sh/ruff/issues/18777
def foo():
dict = {}
for country, stars in zip(dict.keys(), dict.values()):
...

View file

@ -48,6 +48,7 @@ mod tests {
#[test_case(Rule::IfElseBlockInsteadOfDictGet, Path::new("SIM401.py"))]
#[test_case(Rule::SplitStaticString, Path::new("SIM905.py"))]
#[test_case(Rule::DictGetWithNoneDefault, Path::new("SIM910.py"))]
#[test_case(Rule::ZipDictKeysAndValues, Path::new("SIM911.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(

View file

@ -280,7 +280,7 @@ pub(crate) fn dict_get_with_none_default(checker: &Checker, expr: &Expr) {
Expr::Name(name) => {
let Some(binding) = checker
.semantic()
.only_binding(name)
.resolve_name(name)
.map(|id| checker.semantic().binding(id))
else {
return;

View file

@ -92,7 +92,7 @@ pub(crate) fn zip_dict_keys_and_values(checker: &Checker, expr: &ast::ExprCall)
let Some(binding) = checker
.semantic()
.only_binding(var1)
.resolve_name(var1)
.map(|id| checker.semantic().binding(id))
else {
return;

View file

@ -160,3 +160,19 @@ SIM910.py:43:9: SIM910 [*] Use `some_dict.get('a')` instead of `some_dict.get('a
44 44 |
45 45 | # OK
46 46 | def foo(some_other: object):
SIM910.py:57:11: SIM910 [*] Use `dict.get("Cat")` instead of `dict.get("Cat", None)`
|
55 | def foo():
56 | dict = {"Tom": 23, "Maria": 23, "Dog": 11}
57 | age = dict.get("Cat", None)
| ^^^^^^^^^^^^^^^^^^^^^ SIM910
|
= help: Replace `dict.get("Cat", None)` with `dict.get("Cat")`
Safe fix
54 54 | # https://github.com/astral-sh/ruff/issues/18777
55 55 | def foo():
56 56 | dict = {"Tom": 23, "Maria": 23, "Dog": 11}
57 |- age = dict.get("Cat", None)
57 |+ age = dict.get("Cat")

View file

@ -0,0 +1,77 @@
---
source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs
---
SIM911.py:2:17: SIM911 [*] Use `d.items()` instead of `zip(d.keys(), d.values())`
|
1 | def foo(d: dict[str, str]) -> None:
2 | for k, v in zip(d.keys(), d.values()): # SIM911
| ^^^^^^^^^^^^^^^^^^^^^^^^^ SIM911
3 | ...
|
= help: Replace `zip(d.keys(), d.values())` with `d.items()`
Safe fix
1 1 | def foo(d: dict[str, str]) -> None:
2 |- for k, v in zip(d.keys(), d.values()): # SIM911
2 |+ for k, v in d.items(): # SIM911
3 3 | ...
4 4 |
5 5 | for k, v in zip(d.keys(), d.values(), strict=True): # SIM911
SIM911.py:5:17: SIM911 [*] Use `d.items()` instead of `zip(d.keys(), d.values(), strict=True)`
|
3 | ...
4 |
5 | for k, v in zip(d.keys(), d.values(), strict=True): # SIM911
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM911
6 | ...
|
= help: Replace `zip(d.keys(), d.values(), strict=True)` with `d.items()`
Safe fix
2 2 | for k, v in zip(d.keys(), d.values()): # SIM911
3 3 | ...
4 4 |
5 |- for k, v in zip(d.keys(), d.values(), strict=True): # SIM911
5 |+ for k, v in d.items(): # SIM911
6 6 | ...
7 7 |
8 8 | for k, v in zip(d.keys(), d.values(), struct=True): # OK
SIM911.py:20:13: SIM911 [*] Use `d2.items()` instead of `zip(d2.keys(), d2.values())`
|
18 | ...
19 |
20 | for k, v in zip(d2.keys(), d2.values()): # SIM911
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM911
21 | ...
|
= help: Replace `zip(d2.keys(), d2.values())` with `d2.items()`
Safe fix
17 17 | for k, v in zip(d1.items(), d2.values()): # OK
18 18 | ...
19 19 |
20 |-for k, v in zip(d2.keys(), d2.values()): # SIM911
20 |+for k, v in d2.items(): # SIM911
21 21 | ...
22 22 |
23 23 | items = zip(x.keys(), x.values()) # OK
SIM911.py:30:27: SIM911 [*] Use `dict.items()` instead of `zip(dict.keys(), dict.values())`
|
28 | def foo():
29 | dict = {}
30 | for country, stars in zip(dict.keys(), dict.values()):
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM911
31 | ...
|
= help: Replace `zip(dict.keys(), dict.values())` with `dict.items()`
Safe fix
27 27 | # https://github.com/astral-sh/ruff/issues/18777
28 28 | def foo():
29 29 | dict = {}
30 |- for country, stars in zip(dict.keys(), dict.values()):
30 |+ for country, stars in dict.items():
31 31 | ...