Mypy: @overload breaks @coroutine

[aaugustin]

This seemingly simple example fails mypy --strict. Unless I'm missing something obvious, it should pass.

(The example looks artificial because it's reduced from a more complicated, real-world piece of code.)

This failure is new in mypy 0.701. It used to work a few months ago. (I didn't bisect the regression).

I'm using Python 3.7.3 from homebrew on an up-to-date macOS.

import asyncio

loop = asyncio.get_event_loop()

async def create_my_server() -> asyncio.AbstractServer:
    my_awaitable = loop.create_server(asyncio.Protocol, host='localhost', port=1234)

    reveal_type(my_awaitable)
    # typing.Generator[Any, None, asyncio.events.AbstractServer]

    my_server = await my_awaitable
    # Incompatible types in "await" (actual type "Generator[Any, None, AbstractServer]", expected type "Awaitable[Any]")

    return my_server
    # Returning Any from function declared to return "AbstractServer"

When mypy processes the code, if I understand the @asyncio.coroutine example in the docs correctly, the type of my_awaitable should be converted into Awaitable[AbstractServer]. For some reason this doesn't happen and it stays as Generator[Any, None, AbstractServer].

The type definition in typeshed matches the docs I just linked to, which is why I'm posting this on the mypy repository rather than typeshed.

This looks similar to #2907. The discussion there says that the order of functions matter. I have only one function so this can't be the whole explanation.

[aaugustin]

Upon further investigation, this seems to happen because the signature of create_server is overloaded in typeshed:

    @overload
    @abstractmethod
    @coroutine
    def create_server(self, protocol_factory: _ProtocolFactory, host: Optional[Union[str, Sequence[str]]] = ..., port: int = ..., *,
                      family: int = ..., flags: int = ...,
                      sock: None = ..., backlog: int = ..., ssl: _SSLContext = ...,
                      reuse_address: Optional[bool] = ...,
                      reuse_port: Optional[bool] = ...) -> Generator[Any, None, AbstractServer]: ...
    @overload
    @abstractmethod
    @coroutine
    def create_server(self, protocol_factory: _ProtocolFactory, host: None = ..., port: None = ..., *,
                      family: int = ..., flags: int = ...,
                      sock: socket, backlog: int = ..., ssl: _SSLContext = ...,
                      reuse_address: Optional[bool] = ...,
                      reuse_port: Optional[bool] = ...) -> Generator[Any, None, AbstractServer]: ...

If I remove the second definition, the problem goes away.

This suggests a bad interaction between the implementation of @overload and the feature that converts the return value of an @coroutine into an Awaitable.

[aaugustin]

I dived into the mypy source code and drowned :-/

I showed that the two create_server stubs are correctly converted to AwaitableGenerator in TypeChecker.check_func_def. The if defn.is_awaitable_coroutine condition matches.

Adding:

                if name == 'create_server':
                    print(typ)

at line 868 gives:

def (self: asyncio.events.AbstractEventLoop, protocol_factory: def () -> asyncio.protocols.BaseProtocol, host: Union[builtins.str, typing.Sequence[builtins.str], None] =, port: builtins.int =, *, family: builtins.int =, flags: builtins.int =, sock: None =, backlog: builtins.int =, ssl: Union[builtins.bool, None, ssl.SSLContext] =, reuse_address: Union[builtins.bool, None] =, reuse_port: Union[builtins.bool, None] =) -> typing.AwaitableGenerator[Any, None, asyncio.events.AbstractServer, typing.Generator[Any, None, asyncio.events.AbstractServer]]
def (self: asyncio.events.AbstractEventLoop, protocol_factory: def () -> asyncio.protocols.BaseProtocol, host: None =, port: None =, *, family: builtins.int =, flags: builtins.int =, sock: socket.socket, backlog: builtins.int =, ssl: Union[builtins.bool, None, ssl.SSLContext] =, reuse_address: Union[builtins.bool, None] =, reuse_port: Union[builtins.bool, None] =) -> typing.AwaitableGenerator[Any, None, asyncio.events.AbstractServer, typing.Generator[Any, None, asyncio.events.AbstractServer]]

I suspect that the issue occurs after these two definitions are grouped by @overload. Perhaps the resulting definition no longer has the correct return type. However, I'm failing to figure out where this happens, and it's too late here for me to find it tonight.

[aaugustin]

Actually this isn't a regression in mypy.

I had in mind a passing run of mypy v0.660. If I go back to this version of mypy, the change that triggers the regression is a patch I submitted to typeshed: https://github.com/python/typeshed/commit/503cbb97a313ff48eb47ba0c8d700f6e2edb89bb. I must have written that patch based on the docs and not tried it on my code...

I'm willing to investigate this issue further, however I could really use some pointers about how @overload works; I haven't been able to figure it out by reverse engineering the code.

[JelleZijlstra]

I'm not too familiar with the code either, but I know an overloaded function creates a different internal node in mypy, OverloadedFuncDef. One explanation for the bug could be that somewhere we do essentially if isinstance(..., FuncDef) and has_coroutine_decorator(...):, but don't take OverloadedFuncDef into account.

Also curious if the presence of @abstractmethod is also necessary to trigger the bug.

[aaugustin]

Removing @abstractmethod makes no difference.

Thanks for the pointer to OverloadedFuncDef.

tl;dr

I figured out what happens.

OverloadedFuncDef.type is calculated by SemanticAnalyzerPass2.find_overload_sigs_and_impl during semantic analysis.

The type of the create_server overload is set to a function returning a typing.Generator[Any, None, asyncio.events.AbstractServer] at this point. This is where the wrong type eventually comes from.

The conversion to AwaitableGenerator happens in TypeChecker.

The return types of the two coroutines underlying the overload are converted to typing.AwaitableGenerator[Any, None, asyncio.events.AbstractServer, typing.Generator[Any, None, asyncio.events.AbstractServer]] here but the overload isn't.

One possible solution is to apply something similar to this: https://github.com/python/mypy/blob/3c04293a7951c9f3c67bb00c11d1a2730c0dd320/mypy/checker.py#L852

to overloads that wrap coroutines in TypeChecker. This would go into visit_overloaded_func_def.

Propagating the is_coroutine flag to overloads from their items would likely make things easier.

---

gory details

For future reference and posterity, here are my debugging notes.

I managed to determine where the code paths diverge depending on whether there's an overload or not. It's in analyze_instance_member_access.

If you add this debug code:

diff --git a/mypy/checkmember.py b/mypy/checkmember.py
index e17d5f04..124f79fe 100644
--- a/mypy/checkmember.py
+++ b/mypy/checkmember.py
@@ -169,6 +169,10 @@ def analyze_instance_member_access(name: str,

     # Look up the member. First look up the method dictionary.
     method = info.get_method(name)
+
+    if name == 'create_server':
+        print(method)
+
     if method:
         if method.is_property:
             assert isinstance(method, OverloadedFuncDef)

and run python -m mypy --strict --no-incremental mypy_create_server.py on the example I provided, then you can see that method is an OverloadedFuncDef corresponding to the two definitions of create_server

If you comment out one of the two create_server definitions in mypy/typeshed/stdlib/3/asyncio/events.pyi, then method is None.

---

Execution then goes through: https://github.com/python/mypy/blob/3c04293a7951c9f3c67bb00c11d1a2730c0dd320/mypy/checkmember.py#L179 which returns the problematic type.

At this point, we have an OverloadedFuncDef which has the right items but the wrong type.

I followed the trail through:

• OverloadedFuncDef.accept
• SemanticAnalyzerPass2.visit_overloaded_func_def
• SemanticAnalyzerPass2.find_overload_sigs_and_impl

I ended up on this comment but I'm not sure it's the actual problem: https://github.com/python/mypy/blob/3c04293a7951c9f3c67bb00c11d1a2730c0dd320/mypy/semanal.py#L624

Digging through git blame, I discovered that this TODO has been there for over two years, so the trail is cold.

At this point, I realized that the types were "wrong" (i.e. not converted to AwaitableGenerator), which I had seen the "right" ones earlier. So they had to change later. With this debug code, the ordering issue becomes clear.

diff --git a/mypy/checker.py b/mypy/checker.py
index e25368a1..208a1931 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -860,6 +860,11 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
                         tr = self.get_coroutine_return_type(t)
                     else:
                         tr = self.get_generator_return_type(t, c)
+                    if defn.name() == "create_server":
+                        print()
+                        print()
+                        print()
+                        print("CONVERTING RETURN TYPE OF create_server()")
                     ret_type = self.named_generic_type('typing.AwaitableGenerator',
                                                        [ty, tc, tr, t])
                     typ = typ.copy_modified(ret_type=ret_type)
diff --git a/mypy/nodes.py b/mypy/nodes.py
index 25c38eb6..9780e9b6 100644
--- a/mypy/nodes.py
+++ b/mypy/nodes.py
@@ -494,7 +494,24 @@ class OverloadedFuncDef(FuncBase, SymbolNode, Statement):
             return self.impl.name()

     def accept(self, visitor: StatementVisitor[T]) -> T:
-        return visitor.visit_overloaded_func_def(self)
+        if self.name() == 'create_server':
+            print()
+            print()
+            print()
+            print(visitor)
+            print()
+            print("BEFORE")
+            print(self.type)
+            for item in self.items:
+                print(str(item))
+        res = visitor.visit_overloaded_func_def(self)
+        if self.name() == 'create_server':
+            print()
+            print("AFTER")
+            print(self.type)
+            for item in self.items:
+                print(str(item))
+        return res

     def serialize(self) -> JsonDict:
         return {'.class': 'OverloadedFuncDef',

[aaugustin]

Is there anything I can do to help move this forwards, besides the PR with tests that I submitted two weeks ago?

[ilevkivskyi]

The core team is unfortunately really busy last weeks, hopefully someone will look at your PR before the next release (planned soon), since technically this is a regression. cc @gvanrossum who is going to be the release manger.

[JelleZijlstra]

I just reviewed the PR, but somebody else on the PR should also take a look before it can be merged.

[aaugustin]

This bug no longer exists in mypy 0.730.

create_server is now typed as typing.Coroutine[Any, Any, asyncio.events.AbstractServer] rather than typing.Generator[Any, None, asyncio.events.AbstractServer]; I wouldn't expect that to make a difference.

Anyway, since it isn't reproducible anymore, I'll close it.

Thanks, whoever made improvements that had the side effect of fixing this for me :-)