From 481065238b7f90fe756aa6bd989cf9d3973d4654 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 25 Sep 2024 10:03:09 -0500 Subject: [PATCH] Avoid UP028 false negatives with non-reference shadowed bindings of loop variables (#13504) Closes https://github.com/astral-sh/ruff/issues/13266 Avoids false negatives for shadowed bindings that aren't actually references to the loop variable. There are some shadowed bindings we need to support still, e.g., `del` requires the loop variable to exist. --- .../test/fixtures/pyupgrade/UP028_0.py | 82 ++++++++++++++++ .../pyupgrade/rules/yield_in_for_loop.rs | 4 +- ...__rules__pyupgrade__tests__UP028_0.py.snap | 97 +++++++++++++++++++ 3 files changed, 182 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_0.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_0.py index 08f382c956fd2..e0935713d45cb 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_0.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_0.py @@ -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 diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/yield_in_for_loop.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/yield_in_for_loop.rs index 8e0b9448812c0..5a66d937c3e97 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/yield_in_for_loop.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/yield_in_for_loop.rs @@ -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()) - .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() diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP028_0.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP028_0.py.snap index 8d5334bbd0da6..b3c29c378c90a 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP028_0.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP028_0.py.snap @@ -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