Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't flag intentionally empty generators unreachable #15722

21 changes: 20 additions & 1 deletion mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@
Var,
WhileStmt,
WithStmt,
YieldExpr,
is_final_node,
)
from mypy.options import Options
Expand Down Expand Up @@ -1240,7 +1241,10 @@ def check_func_def(
# have no good way of doing this.
#
# TODO: Find a way of working around this limitation
if len(expanded) >= 2:
#
# We suppress reachability warnings for empty generators (return; yield), since there's
# no way to promote a function into a generator except by adding an "unreachable" yield.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# We suppress reachability warnings for empty generators (return; yield), since there's
# no way to promote a function into a generator except by adding an "unreachable" yield.
# We also suppress reachability warnings for empty generator functions
# (return; yield), since the most idiomatic way to promote a function into a
# generator function is often to add an "unreachable" yield.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had "also" at first, but I wanted to be able to remove the first comment w/o touching the second.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meh, I think I'd personally prioritise a readable comment over a tiny bit more churn in git blame :p

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, easy. I've just made the "TODO remove me" part second. :P

if len(expanded) >= 2 or _is_empty_generator(item):
self.binder.suppress_unreachable_warnings()
self.accept(item.body)
unreachable = self.binder.is_unreachable()
Expand Down Expand Up @@ -6961,6 +6965,21 @@ def is_literal_not_implemented(n: Expression) -> bool:
return isinstance(n, NameExpr) and n.fullname == "builtins.NotImplemented"


def _is_empty_generator(func: FuncItem) -> bool:
ikonst marked this conversation as resolved.
Show resolved Hide resolved
"""
Checks whether a function's body is 'return; yield' (the yield being added only
to promote the function into a generator).
ikonst marked this conversation as resolved.
Show resolved Hide resolved
"""
return (
len(body := func.body.body) == 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think the first walrus here is slightly gratuitous

Suggested change
return (
len(body := func.body.body) == 2
body = func.body.body
return (
len(body) == 2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and isinstance(ret_stmt := body[0], ReturnStmt)
and (ret_stmt.expr is None or is_literal_none(ret_stmt.expr))
and isinstance(expr_stmt := body[1], ExpressionStmt)
and isinstance(yield_expr := expr_stmt.expr, YieldExpr)
and (yield_expr.expr is None or is_literal_none(yield_expr.expr))
)


def builtin_item_type(tp: Type) -> Type | None:
"""Get the item type of a builtin container.

Expand Down
16 changes: 16 additions & 0 deletions test-data/unit/check-unreachable-code.test
Original file line number Diff line number Diff line change
Expand Up @@ -1446,3 +1446,19 @@ def f() -> None:
Foo()['a'] = 'a'
x = 0 # This should not be reported as unreachable
[builtins fixtures/exception.pyi]

[case testIntentionallyEmptyGenerator]
ikonst marked this conversation as resolved.
Show resolved Hide resolved
# flags: --warn-unreachable
from typing import Generator

def f() -> Generator[None, None, None]:
return
ikonst marked this conversation as resolved.
Show resolved Hide resolved
yield

[case testIntentionallyEmptyGenerator_None]
ikonst marked this conversation as resolved.
Show resolved Hide resolved
# flags: --warn-unreachable
from typing import Generator

def f() -> Generator[None, None, None]:
return None
yield None