Skip to content

Commit

Permalink
Avoid reading shadowed bindings of loop variables when considering if…
Browse files Browse the repository at this point in the history
… UP028 is safe

Closes #13266
  • Loading branch information
zanieb committed Sep 24, 2024
1 parent 03503f7 commit f4e68c4
Show file tree
Hide file tree
Showing 3 changed files with 185 additions and 3 deletions.
73 changes: 73 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_0.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,76 @@ def _serve_method(fn):
.markup(highlight=args.region)
):
yield h


def f():
for x in (1, 2, 3):
yield x
# Shadowing with another loop
for x in (1, 2, 3):
yield x


def f():
for x in (1, 2, 3):
yield x
# Shadowing with an `except`
try:
pass
except Exception as x:
pass


def f():
for x in (1, 2, 3):
yield x
# Shadowing with `with`
with contextlib.nullcontext() as x:
pass


def f():
for x in (1, 2, 3):
yield x
# Shadowing with a type annotation
x: int


def f():
for x in (1, 2, 3):
yield x
# Shadowing with `del`
del x


def f():
for x in (1, 2, 3):
yield x
# Shadowing with multiple `except` blocks
try:
pass
except Exception as x:
pass
try:
pass
except Exception as x:
pass


def f():
for x in (1, 2, 3):
yield x
# Shadowing with multiple `del` statements
del x
del x


def f():
for x in (1, 2, 3):
yield x
# Shadowing with a reference and non-reference binding
print(x)
try:
pass
except Exception as x:
pass
22 changes: 19 additions & 3 deletions crates/ruff_linter/src/rules/pyupgrade/rules/yield_in_for_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,29 @@ pub(crate) fn yield_in_for_loop(checker: &mut Checker, stmt_for: &ast::StmtFor)
checker
.semantic()
.current_scope()
.get_all(name.id.as_str())
.any(|binding_id| {
let binding = checker.semantic().binding(binding_id);
.get(name.id.as_str())
.map(|mut binding_id| {
let mut binding = checker.semantic().binding(binding_id);

// If the binding is an unbound kind shadowing another binding, resolve it to the
// shadowed binding.
while binding.is_unbound() {
let Some(shadowed_id) = checker
.semantic()
.current_scope()
.shadowed_binding(binding_id)
else {
break;
};
binding_id = shadowed_id;
binding = checker.semantic().binding(binding_id);
}

binding.references.iter().any(|reference_id| {
checker.semantic().reference(*reference_id).range() != name.range()
})
})
.unwrap_or(false)
}) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,5 +298,98 @@ UP028_0.py:79:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
82 |- ):
83 |- yield h
82 |+ )
84 83 |
85 84 |
86 85 | def f():

UP028_0.py:95:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
|
94 | def f():
95 | for x in (1, 2, 3):
| _____^
96 | | yield x
| |_______________^ UP028
97 | # Shadowing with an `except`
98 | try:
|
= help: Replace with `yield from`

Unsafe fix
92 92 |
93 93 |
94 94 | def f():
95 |- for x in (1, 2, 3):
96 |- yield x
95 |+ yield from (1, 2, 3)
97 96 | # Shadowing with an `except`
98 97 | try:
99 98 | pass

UP028_0.py:105:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
|
104 | def f():
105 | for x in (1, 2, 3):
| _____^
106 | | yield x
| |_______________^ UP028
107 | # Shadowing with `with`
108 | with contextlib.nullcontext() as x:
|
= help: Replace with `yield from`

Unsafe fix
102 102 |
103 103 |
104 104 | def f():
105 |- for x in (1, 2, 3):
106 |- yield x
105 |+ yield from (1, 2, 3)
107 106 | # Shadowing with `with`
108 107 | with contextlib.nullcontext() as x:
109 108 | pass

UP028_0.py:113:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
|
112 | def f():
113 | for x in (1, 2, 3):
| _____^
114 | | yield x
| |_______________^ UP028
115 | # Shadowing with a type annotation
116 | x: int
|
= help: Replace with `yield from`

Unsafe fix
110 110 |
111 111 |
112 112 | def f():
113 |- for x in (1, 2, 3):
114 |- yield x
113 |+ yield from (1, 2, 3)
115 114 | # Shadowing with a type annotation
116 115 | x: int
117 116 |

UP028_0.py:127:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
|
126 | def f():
127 | for x in (1, 2, 3):
| _____^
128 | | yield x
| |_______________^ UP028
129 | # Shadowing with multiple `except` blocks
130 | try:
|
= help: Replace with `yield from`

Unsafe fix
124 124 |
125 125 |
126 126 | def f():
127 |- for x in (1, 2, 3):
128 |- yield x
127 |+ yield from (1, 2, 3)
129 128 | # Shadowing with multiple `except` blocks
130 129 | try:
131 130 | pass

0 comments on commit f4e68c4

Please sign in to comment.