Treat empty yield as no-op for reachability

[ikonst]

Fixes #15345.

We're getting slightly more than what we bargained for, i.e. not only we don't flag any statement as 'unreachable' in this canonical "empty generator":

def f():
  return
  yield

but also here:

def f():
  return 42
  yield

or here:

def f():
  return
  yield
  yield

This is because it's simpler to add empty yields to is_noop_for_reachability, than to track state ("up to 1 empty yield").

If there's anything more meaningful a function does, a different statement would be flagged as the first unreachable one, e.g.

def f():
  return
  yield
  a = 42  # E: Statement is unreachable

Alternative

Another alternative is to pattern-match the entire function body as being return; yield. This would be slightly more code, and perhaps necessitate to mark suppress_unreachable_warnings on the frame.

I'm not entirely convinced return; yield should be special-cased on its own. Since the presence of yield is the only way to promote a function into a generator, we could be more lenient to empty yields (other statements would still get flagged so it's unlikely that unreachable code would evade attention).

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

[ikonst]

👋 @asottile

[asottile]

while this fixes the issue I'm not sure it is quite correct to ignore all empty yields -- (though I can't think of a case I'd use one that would be un-coverable beyond the return; yield example)

[ikonst]

I reflect on this in the PR description. Since we're trying to flag unintentionally unreachable code, and we flag the first unreachable statement (except a few ones we ignore), what are the chances there wouldn't have one to flag that's not an assert, raise or yield?

[asottile]

would it still flag something like this (a ~somewhat common pattern)?

@contextmanager
def f() -> Generator[None, None, None]:
    if some_impossible():
        yield
    else:
        with some_other_context():
            yield

[ikonst]

Ah right, I forget empty yields are a fixture of context managers.

Then I guess "a cutout for return; yield as the total body" is the way to go.