Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid UP028 false negatives with non-reference shadowed bindings of loop variables #13504

Merged
merged 1 commit into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,7 +101,9 @@ pub(crate) fn yield_in_for_loop(checker: &mut Checker, stmt_for: &ast::StmtFor)
.semantic()
.current_scope()
.get_all(name.id.as_str())
zanieb marked this conversation as resolved.
Show resolved Hide resolved
.any(|binding_id| {
// Skip unbound bindings like `del x`
.find(|&id| !checker.semantic().binding(id).is_unbound())
.is_some_and(|binding_id| {
let binding = checker.semantic().binding(binding_id);
binding.references.iter().any(|reference_id| {
checker.semantic().reference(*reference_id).range() != name.range()
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
Loading