various small fixes (#264)

* doc updates. don't trigger on open_nursery in 102 (it didn't work anyway). async112 error message now specifies if its nursery or taskgroup.

* update tests

* help repro coverage bug

* Revert "help repro coverage bug"

This reverts commit 2cf2519.

* updates after review. add test cases. type tracker can now handle attribute targets.

docs/rules.rst

@@ -56,6 +56,8 @@ ASYNC112 : useless-nursery
 _`ASYNC113` : start-soon-in-aenter
     Using :meth:`~trio.Nursery.start_soon`/:meth:`~anyio.abc.TaskGroup.start_soon` in ``__aenter__`` doesn't wait for the task to begin.
     Consider replacing with :meth:`~trio.Nursery.start`/:meth:`~anyio.abc.TaskGroup.start`.
+    This will only warn about functions listed in :ref:`ASYNC114 <async114>` or known from Trio.
+    If you're starting a function that does not define `task_status`, then neither will trigger.
 
 _`ASYNC114` : startable-not-in-config
     Startable function (i.e. has a ``task_status`` keyword parameter) not in :ref:`--startable-in-context-manager <--startable-in-context-manager>` parameter list, please add it so ASYNC113 can catch errors when using it.

docs/usage.rst

@@ -265,8 +265,11 @@ Example
 ``startable-in-context-manager``
 --------------------------------
 
-Comma-separated list of methods which should be used with :meth:`trio.Nursery.start`/:meth:`anyio.abc.TaskGroup.start` when opening a context manager,
-in addition to the default :func:`trio.run_process`, :func:`trio.serve_tcp`, :func:`trio.serve_ssl_over_tcp`, and :func:`trio.serve_listeners`.
+Comma-separated list of functions which should be used with :meth:`trio.Nursery.start`/:meth:`anyio.abc.TaskGroup.start` when opening a context manager.
+We then add known functions from Trio to this list, namely :func:`trio.run_process`, :func:`trio.serve_tcp`, :func:`trio.serve_ssl_over_tcp`, :func:`trio.serve_listeners`, and :meth:`trio.DTLSEndpoint.serve`.
+AnyIO does not have any functions in its API that defines ``task_status``.
+asyncio does not have an equivalent of :meth:`~trio.Nursery.start`, nor ``task_status``, but you could still add functions to this list that you want to be extra careful about when opening in an `asyncio.TaskGroup` in an ``__aenter__``
+
 Names must be valid identifiers as per :meth:`str.isidentifier`.
 Used by :ref:`ASYNC113 <async113>`, and :ref:`ASYNC114 <async114>` will warn when encountering methods not in the list.
 

flake8_async/visitors/visitor102.py

@@ -87,9 +87,7 @@ def visit_With(self, node: ast.With | ast.AsyncWith):
 
         # Check for a `with trio.<scope_creator>`
         for item in node.items:
-            call = get_matching_call(
-                item.context_expr, "open_nursery", *cancel_scope_names
-            )
+            call = get_matching_call(item.context_expr, *cancel_scope_names)
             if call is None:
                 continue
 

flake8_async/visitors/visitor_utility.py

@@ -64,9 +64,10 @@ def visit_ClassDef(self, node: ast.ClassDef):
         self.save_state(node, "variables", copy=True)
 
     def visit_AnnAssign(self, node: ast.AnnAssign):
-        if not isinstance(node.target, ast.Name):
-            return
-        target = node.target.id
+        if not isinstance(node.target, (ast.Name, ast.Attribute)):
+            # target can technically be a subscript
+            return  # pragma: no cover
+        target = ast.unparse(node.target)
         typename = ast.unparse(node.annotation)
         self.variables[target] = typename
 
@@ -87,6 +88,8 @@ def visit_Assign(self, node: ast.Assign):
             self.variables[node.targets[0].id] = value
 
     def visit_With(self, node: ast.With | ast.AsyncWith):
+        # TODO: it's actually the return type of
+        # `ast.unparse(item.context_expr.func).__[a]enter__()` that should be used
         if len(node.items) != 1:
             return
         item = node.items[0]

flake8_async/visitors/visitors.py

@@ -95,7 +95,7 @@ def visit_While(self, node: ast.While):
 class Visitor112(Flake8AsyncVisitor):
     error_codes: Mapping[str, str] = {
         "ASYNC112": (
-            "Redundant nursery {}, consider replacing with directly awaiting "
+            "Redundant {1} {0}, consider replacing with directly awaiting "
             "the function call."
         ),
     }
@@ -113,19 +113,22 @@ def visit_With(self, node: ast.With | ast.AsyncWith):
                 continue
             var_name = item.optional_vars.id
 
-            # check for trio.open_nursery and anyio.create_task_group
-            nursery = get_matching_call(
-                item.context_expr, "open_nursery", base="trio"
-            ) or get_matching_call(item.context_expr, "create_task_group", base="anyio")
             start_methods: tuple[str, ...] = ("start", "start_soon")
-            if nursery is None:
-                # check for asyncio.TaskGroup
-                nursery = get_matching_call(
-                    item.context_expr, "TaskGroup", base="asyncio"
-                )
-                if nursery is None:
-                    continue
+            # check for trio.open_nursery and anyio.create_task_group
+            if get_matching_call(item.context_expr, "open_nursery", base="trio"):
+                nursery_type = "nursery"
+
+            elif get_matching_call(
+                item.context_expr, "create_task_group", base="anyio"
+            ):
+                nursery_type = "taskgroup"
+            # check for asyncio.TaskGroup
+            elif get_matching_call(item.context_expr, "TaskGroup", base="asyncio"):
+                nursery_type = "taskgroup"
                 start_methods = ("create_task",)
+            else:
+                # incorrectly marked as not covered on py39
+                continue  # pragma: no cover  # https://github.com/nedbat/coveragepy/issues/198
 
             body_call = node.body[0].value
             if isinstance(body_call, ast.Await):
@@ -142,7 +145,7 @@ def visit_With(self, node: ast.With | ast.AsyncWith):
                     for n in self.walk(*body_call.args, *body_call.keywords)
                 )
             ):
-                self.error(item.context_expr, var_name)
+                self.error(item.context_expr, var_name, nursery_type)
 
     visit_AsyncWith = visit_With
 
@@ -168,8 +171,11 @@ class Visitor113(Flake8AsyncVisitor):
 
     def __init__(self, *args: Any, **kwargs: Any):
         super().__init__(*args, **kwargs)
+        # this is not entirely correct, it's trio.open_nursery.__aenter__()->trio.Nursery
+        # but VisitorTypeTracker currently does not work like that.
         self.typed_calls["trio.open_nursery"] = "trio.Nursery"
         self.typed_calls["anyio.create_task_group"] = "anyio.TaskGroup"
+        self.typed_calls["asyncio.TaskGroup"] = "asyncio.TaskGroup"
 
         self.async_function = False
         self.asynccontextmanager = False
@@ -196,7 +202,10 @@ def is_startable(n: ast.expr, *startable_list: str) -> bool:
             return False
 
         def is_nursery_call(node: ast.expr):
-            if not isinstance(node, ast.Attribute) or node.attr != "start_soon":
+            if not isinstance(node, ast.Attribute) or node.attr not in (
+                "start_soon",
+                "create_task",
+            ):
                 return False
             var = ast.unparse(node.value)
             return ("trio" in self.library and var.endswith("nursery")) or (

tests/eval_files/async102.py

@@ -109,9 +109,19 @@ async def foo():
         pass
     finally:
         myvar = True
-        with trio.open_nursery(10) as s:
-            s.shield = myvar
-            await foo()  # safe in theory, error: 12, Statement("try/finally", lineno-6)
+        with trio.CancelScope(deadline=10) as cs:
+            cs.shield = myvar
+            # safe in theory, but we don't track variable values
+            await foo()  # error: 12, Statement("try/finally", lineno-7)
+    try:
+        pass
+    finally:
+        # false alarm, open_nursery does not block/checkpoint on entry.
+        async with trio.open_nursery() as nursery:  # error: 8, Statement("try/finally", lineno-4)
+            nursery.cancel_scope.deadline = trio.current_time() + 10
+            nursery.cancel_scope.shield = True
+            # false alarm, we currently don't handle nursery.cancel_scope.[deadline/shield]
+            await foo()  # error: 12, Statement("try/finally", lineno-8)
     try:
         pass
     finally:

tests/eval_files/async112.py

@@ -9,34 +9,34 @@
 import trio as noterror
 
 # error
-with trio.open_nursery() as n:  # error: 5, "n"
+with trio.open_nursery() as n:  # error: 5, "n", "nursery"
     n.start(...)
 
-with trio.open_nursery(...) as nurse:  # error: 5, "nurse"
+with trio.open_nursery(...) as nurse:  # error: 5, "nurse", "nursery"
     nurse.start_soon(...)
 
-with trio.open_nursery() as n:  # error: 5, "n"
+with trio.open_nursery() as n:  # error: 5, "n", "nursery"
     n.start_soon(n=7)
 
 
 async def foo():
-    async with trio.open_nursery() as n:  # error: 15, "n"
+    async with trio.open_nursery() as n:  # error: 15, "n", "nursery"
         n.start(...)
 
 
 # weird ones with multiple `withitem`s
 # but if split among several `with` they'd all be treated as error (or ASYNC111), so
 # treating as error for now.
-with trio.open_nursery() as n, trio.open("") as n:  # error: 5, "n"
+with trio.open_nursery() as n, trio.open("") as n:  # error: 5, "n", "nursery"
     n.start(...)
 
-with open("") as o, trio.open_nursery() as n:  # error: 20, "n"
+with open("") as o, trio.open_nursery() as n:  # error: 20, "n", "nursery"
     n.start(o)
 
-with trio.open_nursery() as n, trio.open_nursery() as nurse:  # error: 31, "nurse"
+with trio.open_nursery() as n, trio.open_nursery() as nurse:  # error: 31, "nurse", "nursery"
     nurse.start(n.start(...))
 
-with trio.open_nursery() as n, trio.open_nursery() as n:  # error: 5, "n" # error: 31, "n"
+with trio.open_nursery() as n, trio.open_nursery() as n:  # error: 5, "n", "nursery" # error: 31, "n", "nursery"
     n.start(...)
 
 # safe if passing variable as parameter
@@ -83,7 +83,7 @@ async def foo():
 
 # body is a call to await n.start
 async def foo_1():
-    with trio.open_nursery(...) as n:  # error: 9, "n"
+    with trio.open_nursery(...) as n:  # error: 9, "n", "nursery"
         await n.start(...)
 
 

tests/eval_files/async112_anyio.py

@@ -11,13 +11,13 @@ async def bar(*args): ...
 
 
 async def foo():
-    async with anyio.create_task_group() as tg:  # error: 15, "tg"
+    async with anyio.create_task_group() as tg:  # error: 15, "tg", "taskgroup"
         await tg.start_soon(bar())
 
     async with anyio.create_task_group() as tg:
         await tg.start(bar(tg))
 
-    async with anyio.create_task_group() as tg:  # error: 15, "tg"
+    async with anyio.create_task_group() as tg:  # error: 15, "tg", "taskgroup"
         tg.start_soon(bar())
 
     async with anyio.create_task_group() as tg:

tests/eval_files/async112_asyncio.py

@@ -13,7 +13,7 @@ async def bar(*args): ...
 
 
 async def foo():
-    async with asyncio.TaskGroup() as tg:  # error: 15, "tg"
+    async with asyncio.TaskGroup() as tg:  # error: 15, "tg", "taskgroup"
         tg.create_task(bar())
 
     async with asyncio.TaskGroup() as tg:

tests/eval_files/async113.py

@@ -1,4 +1,6 @@
 # mypy: disable-error-code="arg-type,attr-defined"
+# ARG --startable-in-context-manager=my_startable
+from __future__ import annotations
 from contextlib import asynccontextmanager
 
 import anyio
@@ -22,6 +24,84 @@ async def foo():
     boo.start_soon(trio.run_process)  # ASYNC113: 4
 
     boo_anyio: anyio.TaskGroup = ...  # type: ignore
+    # false alarm - anyio.run_process is not startable
+    # (nor is asyncio.run_process)
     boo_anyio.start_soon(anyio.run_process)  # ASYNC113: 4
 
     yield
+
+
+async def my_startable(task_status: trio.TaskStatus[object] = trio.TASK_STATUS_IGNORED):
+    task_status.started()
+    await trio.lowlevel.checkpoint()
+
+
+# name of variable being [xxx.]nursery triggers it
+class MyCm_named_variable:
+    def __init__(self):
+        self.nursery_manager = trio.open_nursery()
+        self.nursery = None
+
+    async def __aenter__(self):
+        self.nursery = await self.nursery_manager.__aenter__()
+        self.nursery.start_soon(my_startable)  # ASYNC113: 8
+
+    async def __aexit__(self, *args):
+        assert self.nursery is not None
+        await self.nursery_manager.__aexit__(*args)
+
+
+# call chain is not tracked
+# trio.open_nursery -> NurseryManager
+# NurseryManager.__aenter__ -> nursery
+class MyCm_calls:
+    async def __aenter__(self):
+        self.nursery_manager = trio.open_nursery()
+        self.moo = None
+        self.moo = await self.nursery_manager.__aenter__()
+        self.moo.start_soon(my_startable)
+
+    async def __aexit__(self, *args):
+        assert self.moo is not None
+        await self.nursery_manager.__aexit__(*args)
+
+
+# types of class variables are not tracked across functions
+class MyCm_typehint_class_variable:
+    def __init__(self):
+        self.nursery_manager = trio.open_nursery()
+        self.moo: trio.Nursery = None  # type: ignore
+
+    async def __aenter__(self):
+        self.moo = await self.nursery_manager.__aenter__()
+        self.moo.start_soon(my_startable)
+
+    async def __aexit__(self, *args):
+        assert self.moo is not None
+        await self.nursery_manager.__aexit__(*args)
+
+
+# type hint with __or__ is not picked up
+class MyCm_typehint:
+    async def __aenter__(self):
+        self.nursery_manager = trio.open_nursery()
+        self.moo: trio.Nursery | None = None
+        self.moo = await self.nursery_manager.__aenter__()
+        self.moo.start_soon(my_startable)
+
+    async def __aexit__(self, *args):
+        assert self.moo is not None
+        await self.nursery_manager.__aexit__(*args)
+
+
+# only if the type hint is exactly trio.Nursery
+class MyCm_typehint_explicit:
+    async def __aenter__(self):
+        self.nursery_manager = trio.open_nursery()
+        self.moo: trio.Nursery = None  # type: ignore
+        self.moo = await self.nursery_manager.__aenter__()
+        self.moo.start_soon(my_startable)  # ASYNC113: 8
+
+    async def __aexit__(self, *args):
+        assert self.moo is not None
+        await self.nursery_manager.__aexit__(*args)

tests/eval_files/async113_anyio.py

@@ -1,12 +1,22 @@
 # mypy: disable-error-code="arg-type"
+# ARG --startable-in-context-manager=my_startable
 from contextlib import asynccontextmanager
 
 import anyio
 
 
+async def my_startable(
+    task_status: anyio.abc.TaskStatus[object] = anyio.TASK_STATUS_IGNORED,
+):
+    task_status.started()
+    await anyio.lowlevel.checkpoint()
+
+
 @asynccontextmanager
 async def foo():
     # create_task_group only exists in anyio
     async with anyio.create_task_group() as bar_tg:
+        bar_tg.start_soon(my_startable)  # ASYNC113: 8
+        # false alarm - anyio.run_process is not startable
         bar_tg.start_soon(anyio.run_process)  # ASYNC113: 8
-    yield
+        yield

tests/eval_files/async113_asyncio.py

@@ -0,0 +1,17 @@
+# BASE_LIBRARY asyncio
+# ARG --startable-in-context-manager=bar
+# TRIO_NO_ERROR
+# ANYIO_NO_ERROR
+
+from contextlib import asynccontextmanager
+import asyncio
+
+
+async def bar(): ...
+
+
+@asynccontextmanager
+async def my_cm():
+    async with asyncio.TaskGroup() as tg:  # type: ignore[attr-defined]
+        tg.create_task(bar)  # ASYNC113: 8
+        yield

tests/test_flake8_async.py

@@ -308,7 +308,9 @@ def test_eval(
     )
 
     if only_check_not_crash:
-        return
+        # mark it as skipped to indicate we didn't actually test anything in particular
+        # (it confused me once when I didn't notice a file was marked with NOTRIO)
+        pytest.skip()
 
     # Check that error messages refer to current library, or to no library.
     if test not in (