Skip to content

Commit 0a0b4f5

Browse files
committed
Avoid reading shadowed bindings of loop variables when considering if UP028 is safe
Closes #13266
1 parent 03503f7 commit 0a0b4f5

File tree

3 files changed

+198
-3
lines changed

3 files changed

+198
-3
lines changed

crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_0.py

+82
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,85 @@ def _serve_method(fn):
8181
.markup(highlight=args.region)
8282
):
8383
yield h
84+
85+
86+
# UP028: The later loop variable is not a reference to the earlier loop variable
87+
def f():
88+
for x in (1, 2, 3):
89+
yield x
90+
# Shadowing with another loop
91+
for x in (1, 2, 3):
92+
yield x
93+
94+
95+
# UP028: The exception binding is not a reference to the loop variable
96+
def f():
97+
for x in (1, 2, 3):
98+
yield x
99+
# Shadowing with an `except`
100+
try:
101+
pass
102+
except Exception as x:
103+
pass
104+
105+
106+
# UP028: The context binding is not a reference to the loop variable
107+
def f():
108+
for x in (1, 2, 3):
109+
yield x
110+
# Shadowing with `with`
111+
with contextlib.nullcontext() as x:
112+
pass
113+
114+
115+
116+
# UP028: The type annotation binding is not a reference to the loop variable
117+
def f():
118+
for x in (1, 2, 3):
119+
yield x
120+
# Shadowing with a type annotation
121+
x: int
122+
123+
124+
# OK: The `del` statement requires the loop variable to exist
125+
def f():
126+
for x in (1, 2, 3):
127+
yield x
128+
# Shadowing with `del`
129+
del x
130+
131+
132+
# UP028: The exception bindings are not a reference to the loop variable
133+
def f():
134+
for x in (1, 2, 3):
135+
yield x
136+
# Shadowing with multiple `except` blocks
137+
try:
138+
pass
139+
except Exception as x:
140+
pass
141+
try:
142+
pass
143+
except Exception as x:
144+
pass
145+
146+
147+
# OK: The `del` statement requires the loop variable to exist
148+
def f():
149+
for x in (1, 2, 3):
150+
yield x
151+
# Shadowing with multiple `del` statements
152+
del x
153+
del x
154+
155+
156+
# OK: The `print` call requires the loop variable to exist
157+
def f():
158+
for x in (1, 2, 3):
159+
yield x
160+
# Shadowing with a reference and non-reference binding
161+
print(x)
162+
try:
163+
pass
164+
except Exception as x:
165+
pass

crates/ruff_linter/src/rules/pyupgrade/rules/yield_in_for_loop.rs

+19-3
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,29 @@ pub(crate) fn yield_in_for_loop(checker: &mut Checker, stmt_for: &ast::StmtFor)
100100
checker
101101
.semantic()
102102
.current_scope()
103-
.get_all(name.id.as_str())
104-
.any(|binding_id| {
105-
let binding = checker.semantic().binding(binding_id);
103+
.get(name.id.as_str())
104+
.map(|mut binding_id| {
105+
let mut binding = checker.semantic().binding(binding_id);
106+
107+
// If the binding is an unbound kind shadowing another binding, resolve it to the
108+
// shadowed binding.
109+
while binding.is_unbound() {
110+
let Some(shadowed_id) = checker
111+
.semantic()
112+
.current_scope()
113+
.shadowed_binding(binding_id)
114+
else {
115+
break;
116+
};
117+
binding_id = shadowed_id;
118+
binding = checker.semantic().binding(binding_id);
119+
}
120+
106121
binding.references.iter().any(|reference_id| {
107122
checker.semantic().reference(*reference_id).range() != name.range()
108123
})
109124
})
125+
.unwrap_or(false)
110126
}) {
111127
return;
112128
}

crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP028_0.py.snap

+97
Original file line numberDiff line numberDiff line change
@@ -298,5 +298,102 @@ UP028_0.py:79:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
298298
82 |- ):
299299
83 |- yield h
300300
82 |+ )
301+
84 83 |
302+
85 84 |
303+
86 85 | # UP028: The later loop variable is not a reference to the earlier loop variable
301304

305+
UP028_0.py:97:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
306+
|
307+
95 | # UP028: The exception binding is not a reference to the loop variable
308+
96 | def f():
309+
97 | for x in (1, 2, 3):
310+
| _____^
311+
98 | | yield x
312+
| |_______________^ UP028
313+
99 | # Shadowing with an `except`
314+
100 | try:
315+
|
316+
= help: Replace with `yield from`
302317

318+
Unsafe fix
319+
94 94 |
320+
95 95 | # UP028: The exception binding is not a reference to the loop variable
321+
96 96 | def f():
322+
97 |- for x in (1, 2, 3):
323+
98 |- yield x
324+
97 |+ yield from (1, 2, 3)
325+
99 98 | # Shadowing with an `except`
326+
100 99 | try:
327+
101 100 | pass
328+
329+
UP028_0.py:108:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
330+
|
331+
106 | # UP028: The context binding is not a reference to the loop variable
332+
107 | def f():
333+
108 | for x in (1, 2, 3):
334+
| _____^
335+
109 | | yield x
336+
| |_______________^ UP028
337+
110 | # Shadowing with `with`
338+
111 | with contextlib.nullcontext() as x:
339+
|
340+
= help: Replace with `yield from`
341+
342+
Unsafe fix
343+
105 105 |
344+
106 106 | # UP028: The context binding is not a reference to the loop variable
345+
107 107 | def f():
346+
108 |- for x in (1, 2, 3):
347+
109 |- yield x
348+
108 |+ yield from (1, 2, 3)
349+
110 109 | # Shadowing with `with`
350+
111 110 | with contextlib.nullcontext() as x:
351+
112 111 | pass
352+
353+
UP028_0.py:118:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
354+
|
355+
116 | # UP028: The type annotation binding is not a reference to the loop variable
356+
117 | def f():
357+
118 | for x in (1, 2, 3):
358+
| _____^
359+
119 | | yield x
360+
| |_______________^ UP028
361+
120 | # Shadowing with a type annotation
362+
121 | x: int
363+
|
364+
= help: Replace with `yield from`
365+
366+
Unsafe fix
367+
115 115 |
368+
116 116 | # UP028: The type annotation binding is not a reference to the loop variable
369+
117 117 | def f():
370+
118 |- for x in (1, 2, 3):
371+
119 |- yield x
372+
118 |+ yield from (1, 2, 3)
373+
120 119 | # Shadowing with a type annotation
374+
121 120 | x: int
375+
122 121 |
376+
377+
UP028_0.py:134:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
378+
|
379+
132 | # UP028: The exception bindings are not a reference to the loop variable
380+
133 | def f():
381+
134 | for x in (1, 2, 3):
382+
| _____^
383+
135 | | yield x
384+
| |_______________^ UP028
385+
136 | # Shadowing with multiple `except` blocks
386+
137 | try:
387+
|
388+
= help: Replace with `yield from`
389+
390+
Unsafe fix
391+
131 131 |
392+
132 132 | # UP028: The exception bindings are not a reference to the loop variable
393+
133 133 | def f():
394+
134 |- for x in (1, 2, 3):
395+
135 |- yield x
396+
134 |+ yield from (1, 2, 3)
397+
136 135 | # Shadowing with multiple `except` blocks
398+
137 136 | try:
399+
138 137 | pass

0 commit comments

Comments
 (0)