warn when the type hint of an async generator is not AsyncGenerator

[graingert]

Typing an async generator as AsyncIterator or AsyncIterable prevents wrapping it with contextlib.aclosing

[Zac-HD]

Sure, why not 🙂

Although I'll note that even if you correctly use with aclosing(...) as ait: in every single scope, you still have to avoid ever yielding across a cancel scope.

[jakkdl]

I see two different ways of implementing this:

1. Give an error any time AsyncIterator or AsyncIterable is used as return type.
2. Detect async generators by looking for async methods with a yield inside (except if that's a nested function), and give an error if there is a return type and it is not AsyncGenerator.

1 is significantly simpler to implement, avoids false positives, and sounds like it resolves the issue just as well? Whereas the title of the issue implies 2.

[Zac-HD]

(1) fails for functions which return a ReceiveChannel, though.

I'd treat this as a distinctly low priority issue since it's nontrivial to implement and I prefer to ban async generators anyway, but no objection if someone wants to contribute it.

[alicederyn]

This should probably not trigger for functions decorated with @pytest.fixture as AsyncIterator is just fine there.

[graingert]

Not if you're using pytest-trio - the yield can raise trio.Cancelled https://pytest-trio.readthedocs.io/en/stable/reference.html#an-important-note-about-yield-fixtures

Now, here’s the punchline: this means that in our examples above, the teardown code might not be executed at all! This is different from how pytest fixtures normally work. Normally, the yield in a pytest fixture never raises an exception, so you can be certain that any code you put after it will execute as normal. But if you have a fixture with background tasks, and they crash, then your yield might raise an exception, and Python will skip executing the code after the yield.

This means the fixture function needs to return something with .athrow()

[alicederyn]

That sounds like a severe bug in pytest-trio.

[Zac-HD]

No, that seems fine to me? The docs are a bit odd, but this is just like applying @asynccontextmanager.