False positive invalid-overridden-method for async generators overriding AsyncIterable

[couling]

Bug description

For classes

class Foo(Protocol):
    def bar() -> AsyncIterable[str]:
        pass

class Foo2(Foo):
    async def bar() -> AsyncIterable[str]:
        yield "hello"

Pylint will give a false positive:

W0236: Method 'bar' was expected to be 'non-async', found it instead as 'async' (invalid-overridden-method)

That's incorrect because the yield has a subtly different effect for async methods. An async generator method (such as Foo2.bar()) are not coroutines. If you call them without await you get an AsyncIterable not a Coroutine.

That is, to use an async generator you call:

foo = Foo2()
async for b in foo.bar():
    # ...

You do not call:

foo = Foo2()
async for b in await foo.bar():
    # ...

Configuration

No response

Command used

pylint source

Pylint output

W0236: Method 'bar' was expected to be 'non-async', found it instead as 'async' (invalid-overridden-method)

Expected behavior

(If at all possible) pylint should detect the yield in an async method and interpret it as a a non-async method.

Pylint version

pylint 2.12.2
astroid 2.9.3
Python 3.9.6 (v3.9.6:db3ff76da1, Jun 28 2021, 11:49:53)

OS / Environment

No response

Additional dependencies

No response

[cdce8p]

That's because the yield has a subtly different effect for async methods. An async generator method (such as Foo2.bar()) are not coroutines. If you call them without await you get an AsyncIterable not a Coroutine.

Interesting 🤔 Tbh I wasn't aware of that. However, I still think the error make sense here. Foo.bar isn't a Generator, let alone an async one. The return type should probably be Iterable[str] instead. With that, both mypy and pyright also report that Foo2.bar overrides Foo.bar in an incompatible manner.

[couling]

Iterator vs Generator

@cdce8p
Foo.bar isn't a Generator , let alone an async one.

That's putting the relationship the wrong way around. Generators are Iterators, not all Iterators are Generators.

This issue is about the async equivalent. If you've got an alternative to make an async equivalent of the code below then I'm open to suggestions.

class Foo(Protocol):
    # Note baz correctly interacts with bar by calling to obtain an Iterable
    def baz(self) -> Iterable[str]:
        return self.bar()

    def bar(self) -> Iterable[str]:
        pass

class Foo2(Foo):
    # It's legitimate to implement an Iterator using a Generator
    # This doesn't break Foo.baz.
    # This is incredibly useful and not something a linter should block you from doing.
    def bar(self) -> Iterable[str]:
        yield "hello"

Python's behaviour

There's three legitimate ways to use async combined with a for loop. BUT there's no overlap, only one of them will work for a given method. Whichever the parent class supported, must also be supported by the child class:

1. async for b in foo.bar(): calls (not awaits) foo.bar() and then awaits __aiter__() and __anext__() on the result
2. async for b in await foo.bar(): awaits foo.bar() and then awaits __aiter__() and __anext__() on the result
3. for b in await foo.bar(): awaits foo.bar() but does not await anything else, it calls __iter__ and __next__.

If foo.bar is an async generator:

1. Will work correctly
2. Will try to await an AsyncIterator which is not a Coroutine so will raise an error.
3. Will try to call __iter__() instead of await __aiter__() and will raise an error.

Making a protocol support an async generator

For a protocol to support implementation by async generator it MUST be possible to use the method with (1) async for b in foo.bar():.

To implement that same protocol without a generator requires a non-async method to return an AsyncIterator.

Otherwise an async method returning an AsyncIterator MUST be called with (2) async for b in await foo.bar():. This would be incomparable with the protocol and incompatible with any implementation using an async generator.

Other Linters

With that, both mypy and pyright also report that Foo2.bar overrides Foo.bar in an incompatible manner.

Pycharm alerted me to the issue when working with a protocol. Though I now see Pycharm only raises it for protocols. I've edited the initial bug report to match. Pycharm will raise a warning from its Invalid protocol definitions and usages checker. So in the case of protocols there's no way to get Pylint and Pycharm to agree (AFAIK).

This issue has caught out plenty of people (one discussion on StackOverflow here). If you accept my logic then maybe mypy and pyright will need issues raising against them too.

The problem comes down to what's practically compatible. Python's behaviour breaks the Principle of Least Astonishment IMHO and I have no doubt that the authors of other linters were not aware of this behaviour either.

---

Apologies for the lengthy follow up. I live in hope of being proved wrong.

[benwah]

I think the same issue is occuring with Abstract Base Classes; the code below makes mypy happy; but pylint fails with:

************* Module publishing.ben
publishing/ben.py:14:4: W0236: Method 'my_method' was expected to be 'non-async', found it instead as 'async' (invalid-overridden-method)

from abc import ABC, abstractmethod
from contextlib import _AsyncGeneratorContextManager, asynccontextmanager
from typing import AsyncIterator

class BaseOperation(ABC):
    @abstractmethod
    def my_method(self) -> _AsyncGeneratorContextManager:
        ...

class ConcreteOperation(BaseOperation):
    @asynccontextmanager
    async def my_method(self) -> AsyncIterator[str]:
        yield "Hello"

[mickare]

@benwah If you haven't found a nice workaround yet, there is one now: https://github.com/pylint-dev/pylint/issues/8939#issuecomment-1810894993 😉