Merge branch 'main' into use_docs

docs/rules.rst

@@ -6,7 +6,8 @@ General rules
 =============
 
 - **ASYNC100**: A ``with [trio/anyio].fail_after(...):`` or ``with [trio/anyio].move_on_after(...):`` context does not contain any ``await`` statements.  This makes it pointless, as the timeout can only be triggered by a checkpoint. This check also allows ``yield`` statements, since checkpoints can happen in the caller we yield to.
-- **ASYNC101**: ``yield`` inside a trio/anyio nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling.
+- **ASYNC101**: ``yield`` inside a trio nursery, anyio/asyncio TaskGroup, or in a timeout/cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling. See `this thread <https://discuss.python.org/t/preventing-yield-inside-certain-context-managers/1091/23>`_ for discussion of a future PEP.
+This has substantial overlap with :ref:`ASYNC119 <async119>`, which will warn on almost all instances of ASYNC101, but ASYNC101 is about a conceptually different problem that will not get resolved by `PEP 533 <https://peps.python.org/pep-0533/>`_.
 - **ASYNC102**: It's unsafe to await inside ``finally:`` or ``except BaseException/trio.Cancelled/anyio.get_cancelled_exc_class()/asyncio.exceptions.CancelledError`` unless you use a shielded cancel scope with a timeout. This is currently not able to detect asyncio shields.
 - **ASYNC103**: ``except BaseException/trio.Cancelled/anyio.get_cancelled_exc_class()/asyncio.exceptions.CancelledError``, or a bare ``except:`` with a code path that doesn't re-raise. If you don't want to re-raise ``BaseException``, add a separate handler for ``trio.Cancelled``/``anyio.get_cancelled_exc_class()``/``asyncio.exceptions.CancelledError`` before.
 - **ASYNC104**: ``trio.Cancelled``/``anyio.get_cancelled_exc_class()``/``asyncio.exceptions.CancelledError``/``BaseException`` must be re-raised. The same as ASYNC103, except specifically triggered on ``return`` or a different exception being raised.
@@ -21,6 +22,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:
+
 - **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

flake8_async/__init__.py

@@ -37,7 +37,7 @@
 
 
 # CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
-__version__ = "24.5.1"
+__version__ = "24.5.2"
 
 
 # taken from https://github.com/Zac-HD/shed

flake8_async/visitors/helpers.py

@@ -89,7 +89,8 @@ def _get_identifier(node: ast.expr) -> str:
 
 
 # ignores module and only checks the unqualified name of the decorator
-# used in 101, 113, 900 and 910/911
+# used in 113. cst version used in 101, 900 and 910/911
+# matches @name, @foo.name, @name(...), and @foo.name(...)
 def has_decorator(node: ast.FunctionDef | ast.AsyncFunctionDef, *names: str):
     return any(_get_identifier(dec) in names for dec in node.decorator_list)
 
@@ -343,7 +344,7 @@ def identifier_to_string(attr: cst.Name | cst.Attribute) -> str:
 
 
 def with_has_call(
-    node: cst.With, *names: str, base: Iterable[str] = ("trio", "anyio")
+    node: cst.With, *names: str, base: Iterable[str] | str = ("trio", "anyio")
 ) -> list[AttributeCall]:
     """Check if a with statement has a matching call, returning a list with matches.
 
@@ -395,8 +396,13 @@ def func_has_decorator(func: cst.FunctionDef, *names: str) -> bool:
             func.decorators,
             m.Decorator(
                 decorator=m.OneOf(
+                    # @name
                     oneof_names(*names),
+                    # @foo.name
                     m.Attribute(attr=oneof_names(*names)),
+                    # @name(...)
+                    m.Call(func=oneof_names(*names)),
+                    # @foo.name(...)
                     m.Call(func=m.Attribute(attr=oneof_names(*names))),
                 )
             ),

flake8_async/visitors/visitor101.py

@@ -40,12 +40,22 @@ def visit_With(self, node: cst.With):
         self.save_state(node, "_yield_is_error", copy=True)
         # if there's no safe decorator,
         # and it's not yet been determined that yield is error
-        # and this withitem opens a cancelscope:
+        # and this withitem opens a nursery/taskgroup/cancelscope:
         # then yielding is unsafe
         self._yield_is_error = (
             not self._safe_decorator
             and not self._yield_is_error
-            and bool(with_has_call(node, "open_nursery", *cancel_scope_names))
+            # It's not strictly necessary to specify the base, as raising errors on
+            # e.g. anyio.open_nursery isn't much of a problem.
+            and bool(
+                # nursery/taskgroup
+                with_has_call(node, "open_nursery", base="trio")
+                or with_has_call(node, "create_task_group", base="anyio")
+                or with_has_call(node, "TaskGroup", base="asyncio")
+                # cancel scopes
+                or with_has_call(node, "timeout", "timeout_at", base="asyncio")
+                or with_has_call(node, *cancel_scope_names, base=("trio", "anyio"))
+            )
         )
 
     def leave_With(

tests/eval_files/async101.py

@@ -1,74 +1,129 @@
-# ASYNCIO_NO_ERROR - nursery/cancelscope in asyncio is called taskgroup/timeout
-# Note: this one *shouldn't* be giving the same errors for anyio (but it currently does)
-# since it has TaskGroups instead of nurseries. #TODO #215
-# type: ignore
+# ASYNCIO_NO_ERROR
+
+# This file contains errors shared between trio and anyio, since they have some
+# overlap in naming.
+# See async101_xxx which has errors specific to trio/asyncio/anyio.
+
+
 import contextlib
 import contextlib as bla
 from contextlib import asynccontextmanager, contextmanager, contextmanager as blahabla
+import pytest
+import pytest as blo
+from pytest import fixture
 
 import trio
 
 
-def foo0():
-    with trio.open_nursery() as _:
+# cancel scope aliases
+async def foo_fail_after():
+    with trio.fail_after(10):
         yield 1  # error: 8
 
 
-async def foo1():
-    async with trio.open_nursery() as _:
+async def foo_fail_at():
+    with trio.fail_at(10):
         yield 1  # error: 8
 
 
-@contextmanager
-def foo2():
-    with trio.open_nursery() as _:
-        yield 1  # safe
+async def foo_move_on_aft():
+    with trio.move_on_after(10):
+        yield 1  # error: 8
 
 
-async def foo3():
-    async with trio.CancelScope() as _:
+# `as` makes no difference
+async def foo_move_on_at():
+    with trio.move_on_at(10) as _:
         yield 1  # error: 8
 
 
-@asynccontextmanager
-async def foo4():
-    async with trio.open_nursery() as _:
-        yield 1  # safe
+async def foo_CancelScope():
+    with trio.CancelScope() as _:
+        yield 1  # error: 8
 
 
+# the visitor does not care if the `with` is async
+async def foo_async_with():
+    async with trio.CancelScope() as _:  # type: ignore[attr-defined]
+        yield 1  # error: 8
+
+
+# raises error at each yield
+async def foo_multiple_yield():
+    with trio.CancelScope() as _:
+        yield 1  # error: 8
+        yield 1  # error: 8
+
+
+# nested method is safe
 async def foo5():
-    async with trio.open_nursery():
+    with trio.CancelScope():
         yield 1  # error: 8
 
         def foo6():
             yield 1  # safe
 
 
+# @[async]contextmanager suppresses the error
+@contextmanager
+def foo_contextmanager():
+    with trio.CancelScope() as _:
+        yield 1  # safe
+
+
+@asynccontextmanager
+async def foo4():
+    with trio.CancelScope() as _:
+        yield 1  # safe
+
+
 @contextlib.asynccontextmanager
 async def foo7():
-    async with trio.open_nursery() as _:
+    with trio.CancelScope() as _:
+        yield 1  # safe
+
+
+# pytest.fixture also silences async101, as they're morally context managers
+@fixture
+def foo_fixture():
+    with trio.CancelScope() as _:
+        yield 1  # safe
+
+
+@pytest.fixture
+def foo_pytest_fixture():
+    with trio.CancelScope() as _:
         yield 1  # safe
 
 
+# it does not care about what library that [async]contextmanager or fixture is in
 @bla.contextmanager
-def foo8():
-    with trio.open_nursery() as _:
+def foo_bla_contextmanager():
+    with trio.CancelScope() as _:
         yield 1  # safe
 
 
+@blo.fixture
+def foo_blo_fixture():
+    with trio.CancelScope() as _:
+        yield 1  # safe
+
+
+# but any other decorator does nothing
 @blahabla
-def foo9():
-    with trio.open_nursery() as _:
+def foo_blahabla():
+    with trio.CancelScope() as _:
         yield 1  # error: 8
 
 
-@pytest.fixture()
-def foo_false_alarm():
-    with trio.open_nursery() as _:
+# parentheses and parameters are also fine
+@fixture()
+def foo_pytest_fixture_paren():
+    with trio.CancelScope() as _:
         yield 1
 
 
-@pytest.fixture
-def foo_false_alarm_2():
-    with trio.open_nursery() as _:
+@pytest.fixture(autouse=True)
+def foo_pytest_fixture_params():
+    with trio.CancelScope() as _:
         yield 1

tests/eval_files/async101_anyio.py

@@ -0,0 +1,10 @@
+# BASE_LIBRARY anyio
+# TRIO_NO_ERROR
+# ASYNCIO_NO_ERROR
+
+import anyio
+
+
+async def foo():
+    async with anyio.create_task_group():
+        yield 1  # error: 8

tests/eval_files/async101_asyncio.py

@@ -0,0 +1,82 @@
+# BASE_LIBRARY asyncio
+# TRIO_NO_ERROR
+# ANYIO_NO_ERROR
+# TaskGroup and timeout[_at] was added in 3.11, we run type checking with 3.9
+# mypy: disable-error-code=attr-defined
+import contextlib
+import contextlib as bla
+from contextlib import asynccontextmanager, contextmanager, contextmanager as blahabla
+
+import asyncio
+import pytest
+
+
+async def test_async_with():
+    async with asyncio.TaskGroup() as _:
+        yield 1  # error: 8
+
+
+async def test_timeout():
+    async with asyncio.timeout() as _:
+        yield 1  # error: 8
+
+
+async def test_timeout_at():
+    async with asyncio.timeout_at() as _:
+        yield 1  # error: 8
+
+
+async def test_nested_method():
+    async with asyncio.TaskGroup():
+        yield 1  # error: 8
+
+        def foo6():
+            yield 1  # safe
+
+
+# TaskGroup is an async cm, but the visitor does not care about that
+def test_with():
+    with asyncio.TaskGroup() as _:
+        yield 1  # error: 8
+
+
+@contextmanager
+def safe_1():
+    with asyncio.TaskGroup() as _:
+        yield 1  # safe
+
+
+@asynccontextmanager
+async def safe_2():
+    async with asyncio.TaskGroup() as _:
+        yield 1  # safe
+
+
+@contextlib.asynccontextmanager
+async def safe_3():
+    async with asyncio.TaskGroup() as _:
+        yield 1  # safe
+
+
+@bla.contextmanager
+def safe_4():
+    with asyncio.TaskGroup() as _:
+        yield 1  # safe
+
+
+@blahabla
+def test_unrelated_decorator():
+    with asyncio.TaskGroup() as _:
+        yield 1  # error: 8
+
+
+@pytest.fixture()
+def foo_false_alarm():
+    with asyncio.TaskGroup() as _:
+        yield 1
+
+
+@pytest.fixture
+def foo_false_alarm_2():
+    with asyncio.TaskGroup() as _:
+        yield 1

tests/eval_files/async101_trio.py

@@ -0,0 +1,17 @@
+# ASYNCIO_NO_ERROR
+# ANYIO_NO_ERROR
+
+from contextlib import asynccontextmanager
+
+import trio
+
+
+async def foo_open_nursery():
+    async with trio.open_nursery() as _:
+        yield 1  # error: 8
+
+
+@asynccontextmanager
+async def foo_open_nursery_contextmanager():
+    async with trio.open_nursery() as _:
+        yield 1  # safe

tests/eval_files/async113_trio.py

@@ -129,3 +129,15 @@ async def contextlib_import_alias_acm():
 # code coverage for non-name, non-attribute decorator
 @None  # type: ignore
 async def foo4(): ...
+
+
+# while contextlib.asynccontextmanager does not allow parens or parameters,
+# the visitor will still raise an error for them.
+@asynccontextmanager()  # type: ignore[call-arg]
+async def foo_paren():
+    nursery.start_soon(trio.run_process)  # error: 4
+
+
+@asynccontextmanager(1, 2, 3)  # type: ignore[call-arg]
+async def foo_params():
+    nursery.start_soon(trio.run_process)  # error: 4