RET504 for yield

[opk12]

Hi, Python newbie here. test_return() warns about unnecessary assignment before return, so I was wondering, what about the same, but for yield?

$  cat a.py 
import math

def test_return() -> int:
    """Test `return`."""
    for size in range(1, 10):
        content = math.sqrt(size)
        return content

def test_yield() -> int:
    """Test `yield`."""
    for size in range(1, 10):
        content = math.sqrt(size)
        yield content

$    ruff check .  --select ALL --isolated
warning: `one-blank-line-before-class` (D203) and `no-blank-line-before-class` (D211) are incompatible. Ignoring `one-blank-line-before-class`.
warning: `multi-line-summary-first-line` (D212) and `multi-line-summary-second-line` (D213) are incompatible. Ignoring `multi-line-summary-second-line`.
a.py:1:1: D100 Missing docstring in public module
a.py:6:5: RET503 [*] Missing explicit `return` at the end of function able to return non-`None` value
a.py:8:16: RET504 [*] Unnecessary assignment to `content` before `return` statement
Found 3 errors.
[*] 2 potentially fixable with the --fix option.

$  # Debian sid
$  ruff --version
ruff 0.0.291

[JonathanPlasse]

A return statement will exit the function, so the variable of the assignment will never be used. A yield statement does not exit the function, so the variable of the assignment could be used elsewhere.

[opk12]

Good point. For context, I have a custom helper function for non-sequential iteration over Pandas pd.Series entries. This helper has no other usage of the variable and has the same core structure as test_yield() (which was actually trimmed down from the helper). OTOH this is not a big deal, so I understand if it's not found to be worth the effort.

[AlexWaygood]

We track the number of references to each binding that's created in our semantic model. We could plausibly emit this lint only if the variable being yielded is a local variable that's only ever used in the yield expression. I think this could be a reasonable rule -- though since it doesn't exist in the original flake8-return linter, it should probably be a RUF rule rather than a RET rule.

@JonathanPlasse would you still object to that version of the rule?

[JonathanPlasse]

I was not objecting to it. I am open to it.

[AlexWaygood]

Okay, great! Was just checking I hadn't missed something that you'd maybe spotted :D

I'll mark this as "accepted" then