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 e0935713d4..f570638af9 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_0.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_0.py @@ -163,3 +163,26 @@ def f(): pass except Exception as x: pass + + +# https://github.com/astral-sh/ruff/issues/15540 +def f(): + for a in 1,: + yield a + + +SOME_GLOBAL = None + +def f(iterable): + global SOME_GLOBAL + + for SOME_GLOBAL in iterable: + yield SOME_GLOBAL + + some_non_local = None + + def g(): + nonlocal some_non_local + + for some_non_local in iterable: + yield some_non_local diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_1.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_1.py index 2f602211f9..6bb6eed6c4 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_1.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP028_1.py @@ -123,7 +123,20 @@ def f(): yield x, y, x + y -# https://github.com/astral-sh/ruff/issues/15540 def f(): - for a in 1,: - yield a + global some_global + + for element in iterable: + some_global = element + yield some_global + + +def f(): + some_nonlocal = 1 + + def g(): + nonlocal some_nonlocal + + for element in iterable: + some_nonlocal = element + yield some_nonlocal 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 cfbbec09d5..b289f23c79 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 @@ -1,4 +1,4 @@ -use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::{self as ast, Expr, Stmt}; @@ -17,11 +17,19 @@ use crate::checkers::ast::Checker; /// ```python /// for x in foo: /// yield x +/// +/// global y +/// for y in foo: +/// yield y /// ``` /// /// Use instead: /// ```python /// yield from foo +/// +/// for _element in foo: +/// y = _element +/// yield y /// ``` /// /// ## Fix safety @@ -31,6 +39,9 @@ use crate::checkers::ast::Checker; /// to a `yield from` could lead to an attribute error if the underlying /// generator does not implement the `send` method. /// +/// Additionally, if at least one target is `global` or `nonlocal`, +/// no fix will be offered. +/// /// In most cases, however, the fix is safe, and such a modification should have /// no effect on the behavior of the program. /// @@ -40,14 +51,16 @@ use crate::checkers::ast::Checker; #[derive(ViolationMetadata)] pub(crate) struct YieldInForLoop; -impl AlwaysFixableViolation for YieldInForLoop { +impl Violation for YieldInForLoop { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + #[derive_message_formats] fn message(&self) -> String { "Replace `yield` over `for` loop with `yield from`".to_string() } - fn fix_title(&self) -> String { - "Replace with `yield from`".to_string() + fn fix_title(&self) -> Option { + Some("Replace with `yield from`".to_string()) } } @@ -130,10 +143,22 @@ pub(crate) fn yield_in_for_loop(checker: &Checker, stmt_for: &ast::StmtFor) { format!("yield from {contents}") }; - diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( - contents, - stmt_for.range(), - ))); + if !collect_names(value).any(|name| { + let semantic = checker.semantic(); + let mut bindings = semantic.current_scope().get_all(name.id.as_str()); + + bindings.any(|id| { + let binding = semantic.binding(id); + + binding.is_global() || binding.is_nonlocal() + }) + }) { + diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( + contents, + stmt_for.range(), + ))); + } + checker.report_diagnostic(diagnostic); } 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 cf0f292036..2f00cdcfda 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 @@ -380,3 +380,46 @@ UP028_0.py:134:5: UP028 [*] Replace `yield` over `for` loop with `yield from` 136 135 | # Shadowing with multiple `except` blocks 137 136 | try: 138 137 | pass + +UP028_0.py:170:5: UP028 [*] Replace `yield` over `for` loop with `yield from` + | +168 | # https://github.com/astral-sh/ruff/issues/15540 +169 | def f(): +170 | / for a in 1,: +171 | | yield a + | |_______________^ UP028 + | + = help: Replace with `yield from` + +ℹ Unsafe fix +167 167 | +168 168 | # https://github.com/astral-sh/ruff/issues/15540 +169 169 | def f(): +170 |- for a in 1,: +171 |- yield a + 170 |+ yield from (1,) +172 171 | +173 172 | +174 173 | SOME_GLOBAL = None + +UP028_0.py:179:5: UP028 Replace `yield` over `for` loop with `yield from` + | +177 | global SOME_GLOBAL +178 | +179 | / for SOME_GLOBAL in iterable: +180 | | yield SOME_GLOBAL + | |_________________________^ UP028 +181 | +182 | some_non_local = None + | + = help: Replace with `yield from` + +UP028_0.py:187:9: UP028 Replace `yield` over `for` loop with `yield from` + | +185 | nonlocal some_non_local +186 | +187 | / for some_non_local in iterable: +188 | | yield some_non_local + | |________________________________^ UP028 + | + = help: Replace with `yield from` diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP028_1.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP028_1.py.snap index c08796dfe7..2bacb5d540 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP028_1.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP028_1.py.snap @@ -1,20 +1,4 @@ --- source: crates/ruff_linter/src/rules/pyupgrade/mod.rs --- -UP028_1.py:128:5: UP028 [*] Replace `yield` over `for` loop with `yield from` - | -126 | # https://github.com/astral-sh/ruff/issues/15540 -127 | def f(): -128 | / for a in 1,: -129 | | yield a - | |_______________^ UP028 - | - = help: Replace with `yield from` -ℹ Unsafe fix -125 125 | -126 126 | # https://github.com/astral-sh/ruff/issues/15540 -127 127 | def f(): -128 |- for a in 1,: -129 |- yield a - 128 |+ yield from (1,)