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 25, 2024
1 parent 03503f7 commit 9b5c922
Show file tree
Hide file tree
Showing 3 changed files with 183 additions and 1 deletion.
82 changes: 82 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,85 @@ def _serve_method(fn):
.markup(highlight=args.region)
):
yield h


# UP028: The later loop variable is not a reference to the earlier loop variable
def f():
for x in (1, 2, 3):
yield x
# Shadowing with another loop
for x in (1, 2, 3):
yield x


# UP028: The exception binding is not a reference to the loop variable
def f():
for x in (1, 2, 3):
yield x
# Shadowing with an `except`
try:
pass
except Exception as x:
pass


# UP028: The context binding is not a reference to the loop variable
def f():
for x in (1, 2, 3):
yield x
# Shadowing with `with`
with contextlib.nullcontext() as x:
pass



# UP028: The type annotation binding is not a reference to the loop variable
def f():
for x in (1, 2, 3):
yield x
# Shadowing with a type annotation
x: int


# OK: The `del` statement requires the loop variable to exist
def f():
for x in (1, 2, 3):
yield x
# Shadowing with `del`
del x


# UP028: The exception bindings are not a reference to the loop variable
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


# OK: The `del` statement requires the loop variable to exist
def f():
for x in (1, 2, 3):
yield x
# Shadowing with multiple `del` statements
del x
del x


# OK: The `print` call requires the loop variable to exist
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
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,15 @@ pub(crate) fn yield_in_for_loop(checker: &mut Checker, stmt_for: &ast::StmtFor)
.semantic()
.current_scope()
.get_all(name.id.as_str())
.any(|binding_id| {
// Skip unbound bindings like `del x`
.find(|&id| !checker.semantic().binding(id).is_unbound())
.map(|binding_id| {
let binding = checker.semantic().binding(binding_id);
binding.references.iter().any(|reference_id| {
checker.semantic().reference(*reference_id).range() != name.range()
})
})
.unwrap_or_default()
}) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,5 +298,102 @@ 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 | # UP028: The later loop variable is not a reference to the earlier loop variable

UP028_0.py:97:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
|
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
| |_______________^ UP028
99 | # Shadowing with an `except`
100 | try:
|
= help: Replace with `yield from`

Unsafe fix
94 94 |
95 95 | # UP028: The exception binding is not a reference to the loop variable
96 96 | def f():
97 |- for x in (1, 2, 3):
98 |- yield x
97 |+ yield from (1, 2, 3)
99 98 | # Shadowing with an `except`
100 99 | try:
101 100 | pass

UP028_0.py:108:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
|
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
| |_______________^ UP028
110 | # Shadowing with `with`
111 | with contextlib.nullcontext() as x:
|
= help: Replace with `yield from`

Unsafe fix
105 105 |
106 106 | # UP028: The context binding is not a reference to the loop variable
107 107 | def f():
108 |- for x in (1, 2, 3):
109 |- yield x
108 |+ yield from (1, 2, 3)
110 109 | # Shadowing with `with`
111 110 | with contextlib.nullcontext() as x:
112 111 | pass

UP028_0.py:118:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
|
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
| |_______________^ UP028
120 | # Shadowing with a type annotation
121 | x: int
|
= help: Replace with `yield from`

Unsafe fix
115 115 |
116 116 | # UP028: The type annotation binding is not a reference to the loop variable
117 117 | def f():
118 |- for x in (1, 2, 3):
119 |- yield x
118 |+ yield from (1, 2, 3)
120 119 | # Shadowing with a type annotation
121 120 | x: int
122 121 |

UP028_0.py:134:5: UP028 [*] Replace `yield` over `for` loop with `yield from`
|
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
| |_______________^ UP028
136 | # Shadowing with multiple `except` blocks
137 | try:
|
= help: Replace with `yield from`

Unsafe fix
131 131 |
132 132 | # UP028: The exception bindings are not a reference to the loop variable
133 133 | def f():
134 |- for x in (1, 2, 3):
135 |- yield x
134 |+ yield from (1, 2, 3)
136 135 | # Shadowing with multiple `except` blocks
137 136 | try:
138 137 | pass

0 comments on commit 9b5c922

Please sign in to comment.