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

Improve yield from inference for unions of generators #16717

Merged
merged 7 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -992,8 +992,9 @@ def get_generator_return_type(self, return_type: Type, is_coroutine: bool) -> Ty
# AwaitableGenerator, Generator: tr is args[2].
return return_type.args[2]
else:
# Supertype of Generator (Iterator, Iterable, object): tr is any.
return AnyType(TypeOfAny.special_form)
# We have a supertype of Generator (Iterator, Iterable, object)
# Treat `Iterator[X]` as a shorthand for `Generator[X, Any, None]`.
Copy link
Member

Choose a reason for hiding this comment

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

Why? Intuitively Any makes more sense to me here.

This affects code like this, right?

x: Iterator[T]
y = yield from x

Copy link
Collaborator Author

@hauntsaninja hauntsaninja Mar 7, 2024

Choose a reason for hiding this comment

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

This allows us to produce errors for cases like python/typeshed#11222 / testReturnInIterator

return NoneType()

def visit_func_def(self, defn: FuncDef) -> None:
if not self.recurse_into_functions:
Expand Down
12 changes: 1 addition & 11 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -5943,17 +5943,7 @@ def visit_yield_from_expr(self, e: YieldFromExpr, allow_none_return: bool = Fals

# Determine the type of the entire yield from expression.
iter_type = get_proper_type(iter_type)
if isinstance(iter_type, Instance) and iter_type.type.fullname == "typing.Generator":
expr_type = self.chk.get_generator_return_type(iter_type, False)
else:
# Non-Generators don't return anything from `yield from` expressions.
# However special-case Any (which might be produced by an error).
actual_item_type = get_proper_type(actual_item_type)
if isinstance(actual_item_type, AnyType):
expr_type = AnyType(TypeOfAny.from_another_any, source_any=actual_item_type)
else:
# Treat `Iterator[X]` as a shorthand for `Generator[X, None, Any]`.
expr_type = NoneType()
expr_type = self.chk.get_generator_return_type(iter_type, is_coroutine=False)

if not allow_none_return and isinstance(get_proper_type(expr_type), NoneType):
self.chk.msg.does_not_return_value(None, e)
Expand Down
9 changes: 5 additions & 4 deletions mypyc/test-data/run-generators.test
Original file line number Diff line number Diff line change
Expand Up @@ -246,12 +246,12 @@ assert run_generator(another_triple()()) == ((1,), None)
assert run_generator(outer()) == ((0, 1, 2, 3, 4), None)

[case testYieldThrow]
from typing import Generator, Iterable, Any
from typing import Generator, Iterable, Any, Union
from traceback import print_tb
from contextlib import contextmanager
import wrapsys

def generator() -> Iterable[int]:
def generator() -> Generator[int, None, Union[int, None]]:
try:
yield 1
yield 2
Expand All @@ -264,6 +264,7 @@ def generator() -> Iterable[int]:
else:
print('caught exception without value')
return 0
return None

def no_except() -> Iterable[int]:
yield 1
Expand Down Expand Up @@ -355,11 +356,11 @@ with ctx_manager() as c:
raise Exception
File "native.py", line 10, in generator
yield 3
File "native.py", line 30, in wrapper
File "native.py", line 31, in wrapper
return (yield from x)
File "native.py", line 9, in generator
yield 2
File "native.py", line 30, in wrapper
File "native.py", line 31, in wrapper
return (yield from x)
caught exception without value
caught exception with value some string
Expand Down
28 changes: 27 additions & 1 deletion test-data/unit/check-statements.test
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def f() -> Generator[int, None, None]:
from typing import Iterator
def f() -> Iterator[int]:
yield 1
return "foo"
return "foo" # E: No return value expected
[out]


Expand Down Expand Up @@ -2231,6 +2231,32 @@ class B: pass
def foo(x: int) -> Union[Generator[A, None, None], Generator[B, None, None]]:
yield x # E: Incompatible types in "yield" (actual type "int", expected type "Union[A, B]")

[case testYieldFromUnionOfGenerators]
from typing import Generator, Union

class T: pass

def foo(arg: Union[Generator[int, None, T], Generator[str, None, T]]) -> Generator[Union[int, str], None, T]:
return (yield from arg)

[case testYieldFromInvalidUnionReturn]
from typing import Generator, Union

class T: pass
class R: pass

def foo(arg: Union[T, R]) -> Generator[Union[int, str], None, T]:
return (yield from arg) # E: "yield from" can't be applied to "Union[T, R]"

[case testYieldFromUnionOfGeneratorWithIterableStr]
from typing import Generator, Union, Iterable, Optional

class T: pass

def foo(arg: Union[Generator[int, None, T], Iterable[str]]) -> Generator[Union[int, str], None, Optional[T]]:
Copy link
Member

Choose a reason for hiding this comment

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

This is unsound and should arguably error. For example, I could pass a Generator[str, None, str] and get a different type out.

Possibly we should allow it for pragmatic reasons, but then again I'm not sure how often people use yield from with values typed as just Iterable.

Copy link
Collaborator Author

@hauntsaninja hauntsaninja Mar 7, 2024

Choose a reason for hiding this comment

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

Oh this is a great point. I've added a test, but I think this is pre-existing, Iterable[str] acts like Generator[str, Any, Any] in argument positions. mypy handles generator returns kind of weirdly, like it thinks the type of the yield from expr is the return type

mypy's existing behaviour is to error with Function does not return a value (it only ever returns None) [func-returns-value] on the return statement, which doesn't make sense to me

return (yield from arg)
[builtins fixtures/tuple.pyi]

[case testNoCrashOnStarRightHandSide]
x = *(1, 2, 3) # E: can't use starred expression here
[builtins fixtures/tuple.pyi]
Expand Down