[ruff] Add unnecessary-assign-before-yield (RUF070)#23300
[ruff] Add unnecessary-assign-before-yield (RUF070)#23300ntBre merged 6 commits intoastral-sh:mainfrom
ruff] Add unnecessary-assign-before-yield (RUF070)#23300Conversation
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| RUF070 | 82 | 82 | 0 | 0 | 0 |
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Outdated
Show resolved
Hide resolved
ntBre
left a comment
There was a problem hiding this comment.
Thank you! This looks good to me overall, just a few minor simplification and test suggestions.
...ter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF070_RUF070.py.snap
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Show resolved
Hide resolved
ntBre
left a comment
There was a problem hiding this comment.
Nice, thank you! This looks good overall, just a couple remaining questions about the weird yield yield case and the special with handling I saw in RET504.
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/unnecessary_assign_before_yield.rs
Show resolved
Hide resolved
|
Thanks for the detailed review, @ntBre and @amyreese! I've pushed updates addressing all the feedback:
Happy to adjust anything further! |
ntBre
left a comment
There was a problem hiding this comment.
Thank you! I pushed one commit reusing the has_conditional_body implementation from flake8_return instead of copying it over. It's slightly unusual to share code across rule modules, but I think it's preferable to duplicating the helper function.
…23300) ## Summary Closes astral-sh#13141 Adds a new rule `unnecessary-assign-before-yield` (`RUF070`) that detects variable assignments immediately followed by a `yield` (or `yield from`) of that variable, where the variable is not referenced anywhere else. This is the `yield` equivalent of `RET504` (`unnecessary-assign`). ```python # Before def gen(): x = 1 yield x # After def gen(): yield 1 ``` Unlike return, yield does not exit the function, so the rule only triggers when the binding has exactly one reference (the yielditself). The fix is marked as unsafe for the same reason. ## Test Plan cargo nextest run -p ruff_linter -- RUF070 --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
Summary
Closes #13141
Adds a new rule
unnecessary-assign-before-yield(RUF070) that detects variable assignments immediately followed by ayield(oryield from) of that variable, where the variable is not referenced anywhere else. This is theyieldequivalent ofRET504(unnecessary-assign).Unlike return, yield does not exit the function, so the rule only triggers when the binding has exactly one reference (the yielditself). The fix is marked as unsafe for the same reason.
Test Plan
cargo nextest run -p ruff_linter -- RUF070