add async119: yield in contextmanager in async generator

CHANGELOG.md

@@ -1,6 +1,8 @@
 # Changelog
 *[CalVer, YY.month.patch](https://calver.org/)*
 
+## 24.4.23
+- Add ASYNC119: yield in contextmanager in async generator.
 
 ## 24.4.1
 - ASYNC91X fix internal error caused by multiple `try/except` incorrectly sharing state.

README.md

@@ -41,6 +41,7 @@ pip install flake8-async
 - **ASYNC115**: Replace `[trio/anyio].sleep(0)` with the more suggestive `[trio/anyio].lowlevel.checkpoint()`.
 - **ASYNC116**: `[trio/anyio].sleep()` with >24 hour interval should usually be `[trio/anyio].sleep_forever()`.
 - **ASYNC118**: Don't assign the value of `anyio.get_cancelled_exc_class()` to a variable, since that breaks linter checks and multi-backend programs.
+- **ASYNC119**: `yield` in context manager in async generator is unsafe, the cleanup may be delayed until `await` is no longer allowed. We strongly encourage you to read PEP-533 and use `async with aclosing(...)`, or better yet avoid async generators entirely (see ASYNC900) in favor of context managers which return an iterable channel/queue.
 
 ### Warnings for blocking sync calls in async functions
 Note: 22X, 23X and 24X has not had asyncio-specific suggestions written.

docs/rules.rst

@@ -21,6 +21,9 @@ General rules
 - **ASYNC115**: Replace ``[trio/anyio].sleep(0)`` with the more suggestive ``[trio/anyio].lowlevel.checkpoint()``.
 - **ASYNC116**: ``[trio/anyio].sleep()`` with >24 hour interval should usually be ``[trio/anyio].sleep_forever()``.
 - **ASYNC118**: Don't assign the value of ``anyio.get_cancelled_exc_class()`` to a variable, since that breaks linter checks and multi-backend programs.
+- **ASYNC119**: ``yield`` in context manager in async generator is unsafe, the cleanup may be delayed until ``await`` is no longer allowed. We strongly encourage you to read `PEP 533 <https://peps.python.org/pep-0533/>`_ and use `async with aclosing(...) <https://docs.python.org/3/library/contextlib.html#contextlib.aclosing>`_, or better yet avoid async generators entirely (see :ref:`ASYNC900 <async900>` ) in favor of context managers which return an iterable `channel (trio) <https://trio.readthedocs.io/en/stable/reference-core.html#channels>`_, `stream (anyio) <https://anyio.readthedocs.io/en/stable/streams.html#streams>`_, or `queue (asyncio) <https://docs.python.org/3/library/asyncio-queue.html>`_.
+
+  .. TODO: use intersphinx(?) instead of having to specify full URL
 
 Blocking sync calls in async functions
 ======================================
@@ -42,6 +45,8 @@ Note: 22X, 23X and 24X has not had asyncio-specific suggestions written.
 - **ASYNC250**: Builtin ``input()`` should not be called from async function. Wrap in ``[trio/anyio].to_thread.run_sync()`` or ``asyncio.loop.run_in_executor()``.
 - **ASYNC251**: ``time.sleep(...)`` should not be called from async function. Use ``[trio/anyio/asyncio].sleep(...)``.
 
+.. _async900:
+
 Optional rules disabled by default
 ==================================
 

flake8_async/visitors/visitors.py

@@ -281,6 +281,47 @@ def visit_Call(self, node: ast.Call):
                 self.error(node, m[2])
 
 
+@error_class
+class Visitor119(Flake8AsyncVisitor):
+    error_codes: Mapping[str, str] = {
+        "ASYNC119": "Yield in contextmanager in async generator might not trigger"
+        " cleanup. Use `@asynccontextmanager` or refactor."
+    }
+
+    def __init__(self, *args: Any, **kwargs: Any):
+        super().__init__(*args, **kwargs)
+        self.unsafe_function: ast.AsyncFunctionDef | None = None
+        self.contextmanager: ast.With | ast.AsyncWith | None = None
+
+    def visit_AsyncFunctionDef(
+        self, node: ast.AsyncFunctionDef | ast.FunctionDef | ast.Lambda
+    ):
+        self.save_state(node, "unsafe_function", "contextmanager")
+        self.contextmanager = None
+        if isinstance(node, ast.AsyncFunctionDef) and not has_decorator(
+            node, "asynccontextmanager"
+        ):
+            self.unsafe_function = node
+        else:
+            self.unsafe_function = None
+
+    def visit_With(self, node: ast.With | ast.AsyncWith):
+        self.save_state(node, "contextmanager")
+        self.contextmanager = node
+
+    def visit_Yield(self, node: ast.Yield):
+        if self.unsafe_function is not None and self.contextmanager is not None:
+            # Decision point: the error could point to the method, or context manager,
+            # or the yield.
+            self.error(node)
+            # only warn once per method (?)
+            self.unsafe_function = None
+
+    visit_AsyncWith = visit_With
+    visit_FunctionDef = visit_AsyncFunctionDef
+    visit_Lambda = visit_AsyncFunctionDef
+
+
 @error_class
 @disabled_by_default
 class Visitor900(Flake8AsyncVisitor):

tests/eval_files/async119.py

@@ -0,0 +1,55 @@
+import contextlib
+
+from contextlib import asynccontextmanager
+
+
+async def unsafe_yield():
+    with open(""):
+        yield  # error: 8
+
+
+async def async_with():
+    async with unsafe_yield():
+        yield  # error: 8
+
+
+async def yield_not_in_contextmanager():
+    yield
+    with open(""):
+        ...
+    yield
+
+
+async def yield_in_nested_function():
+    with open(""):
+
+        def foo():
+            yield
+
+
+async def yield_in_nested_async_function():
+    with open(""):
+
+        async def foo():
+            yield
+
+
+async def yield_after_nested_async_function():
+    with open(""):
+
+        async def foo():
+            yield
+
+        yield  # error: 8
+
+
+@asynccontextmanager
+async def safe_in_contextmanager():
+    with open(""):
+        yield
+
+
+@contextlib.asynccontextmanager
+async def safe_in_contextmanager2():
+    with open(""):
+        yield