Flag `return` inside generator function

[joaoe]

It has happened often with me, having some big function (more than a couple screens length) that I change from a standard return logic to yielding a generator; or editing some generator function and putting a return value by accident, instead of yield value.

Python allows return value inside generator functions, which is equivalent to raise StopIteration(value). However, I regard this an obscure feature of the language because StopIteration is not an error we should be catching but instead is transparently handled during for-loops. Hence, that value is never read.

So, in my opinion, a return value error in a generator is most likely a programming error which can be quite tricky to find unless you aim for 100% test coverage, to catch that pesky if-return check that handles some uncommon error.

I suggest that the following code is flagged as a warning:

anything = None # any value, really

def gen_function1():
  yield 42
  return anything

def gen_function2(weird_error):
  if weird_error:
    return anything
  yield 42

while the following is still allowed

def gen_function1():
  yield 42
  return

def gen_function2(weird_error):
  if weird_error:
    return
  yield 42

For the sake of backwards compatibility, and because I expect some people might disagree or have legitimate uses for return value inside the generator, this warning should be disableable, given there is no alternative way to do raise StopIterator(value) as the latter will trigger a RuntimeError.

Discuss :)

PS: I found this https://github.com/PyCQA/pyflakes/issues/166 so this is about enabling the warning always.

[asottile]

this puzzlingly seems to only be enabled for python2.x

https://github.com/PyCQA/pyflakes/blob/009f4e7c21890cefafe5858aa682a7e7f17d6a80/pyflakes/checker.py#L1364-L1373

digging a little more, the condition was added in b24d50b88e26990118ef17d67ce6714c5e2ea31f

ah yes, for the coroutine things:

def f(y):
    x = yield y
    return x * 9

g = f(3)
print(next(g))
try:
    g.send(2)
except StopIteration as e:
    print(e.args[0])

$ python3 t.py
3
18

In python2.x it's a syntax error, python3 not:

$ python2 t.py 
  File "t.py", line 3
    return x * 9
SyntaxError: 'return' with argument inside generator

[asottile]

pyflakes doesn't have a "disableable" aspect so I think this isn't actionable

[joaoe]

pyflakes doesn't have a "disableable" aspect so I think this isn't actionable

I actually implemented that in my own branch and it's very compact actually. I used the Message names and the command line arguments are passed directly to the Reporter. But I don't know if people have explicitly not allowed that nor want such feature in the code.

https://github.com/joaoe/pyflakes/commits/disable_warnings

[asottile]

pyflakes doesn't have a "disableable" aspect so I think this isn't actionable

I actually implemented that in my own branch and it's very compact actually. I used the Message names and the command line arguments are passed directly to the Reporter. But I don't know if people have explicitly not allowed that nor want such feature in the code.

https://github.com/joaoe/pyflakes/commits/disable_warnings

my hunch is no, that's flake8's job -- pyflakes currently has no options

[sigmavirus24]

pyflakes has no options and has no desire to grow them

[joaoe]

Ok, no options :)

Then back to the original problem: Since return value raises an exception StopIteration(value) the use case of raising some value works best by using a proper exception like raise ValueError(value) or something else. Or am I missing something ? What is the purpose of raise StopIteration(value) ?

[asottile]

it's how you implement coroutines, the return statement triggers StopIteration which is the non-error single exit point of a generator.

note also that it isn't correct to raise StopIteration yourself:

$ python3.7 -Werror
Python 3.7.2 (default, Dec 25 2018, 03:50:46) 
[GCC 7.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> def f():
...    yield 5
...    raise StopIteration(4)
... 
>>> g = f()
>>> next(g)
4
>>> next(g)
Traceback (most recent call last):
  File "<stdin>", line 3, in f
StopIteration: 4

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: generator raised StopIteration

See also PEP 479

I'd never use this construct in my code but it it is valid and pyflakes takes a pretty strong stance on avoiding false positives.

[joaoe]

I was looking for PEP 479, thanks. There's also PEP 342.

But none of those justify why return value is accepted.

Note: this issue is about return value, not about a simple return without an expression. You can re-read the first post.

I understand perfectly well the point about false positives. But if python was a perfect language, there would be no need for pyflakes :)

[asottile]

I believe you'r elooking for PEP 380

[joaoe]

Oh, I had seen that one before but did not see this sentence

Furthermore, when the iterator is another generator, the subgenerator is allowed to execute a return statement with a value, and that value becomes the value of the yield from expression.

def a():
    yield 1
    return 2

def b():
    x = yield from a()
    yield x

print(tuple(b()))

What a weird obscure feature :(

[bitglue]

It's not so obscure among python programs that do asynchronous IO like twisted, tornado, and asyncio. As this is a legitimate feature for some people, treating it as an error would conflict with the design principles.

flake8-bugbear or pylint might be a more appropriate place for such a check.