make ASYNC912 and 913 care about cancel points, and ignore schedule points

Author: jakkdl

docs/changelog.rst

@@ -4,6 +4,10 @@ Changelog
 
 `CalVer, YY.month.patch <https://calver.org/>`_
 
+25.2.1
+=======
+- :ref:`ASYNC912 <async912>` and :ref:`ASYNC913 <async913>` will now trigger if there's no *cancel* points. This means that :func:`trio.open_nursery`/`anyio.create_task_group` will not silence them on their own, unless they're guaranteed to start tasks.
+
 25.1.1
 =======
 - Add :ref:`ASYNC124 <async124>` async-function-could-be-sync

docs/rules.rst

@@ -197,12 +197,13 @@ _`ASYNC911` : async-generator-no-checkpoint
     Exit, ``yield`` or ``return`` from async iterable with no guaranteed :ref:`checkpoint` since possible function entry (``yield`` or function definition).
 
 _`ASYNC912` : cancel-scope-no-guaranteed-checkpoint
-    A timeout/cancelscope has :ref:`checkpoints <checkpoint>`, but they're not guaranteed to run.
-    Similar to `ASYNC100`_, but it does not warn on trivial cases where there is no checkpoint at all.
+    A timeout/cancelscope has :ref:`cancel points <cancel_point>`, but they're not guaranteed to run.
+    Similar to `ASYNC100`_, but it does not warn on trivial cases where there is no cancel point at all.
     It instead shares logic with `ASYNC910`_ and `ASYNC911`_ for parsing conditionals and branches.
 
 _`ASYNC913` : indefinite-loop-no-guaranteed-checkpoint
     An indefinite loop (e.g. ``while True``) has no guaranteed :ref:`checkpoint <checkpoint>`. This could potentially cause a deadlock.
+    This will also error if there's no guaranteed :ref:`cancel point`, where even though it won't deadlock the loop might become an uncancelable dry-run loop.
 
 .. _autofix-support:
 

docs/usage.rst

@@ -33,7 +33,7 @@ adding the following to your ``.pre-commit-config.yaml``:
    minimum_pre_commit_version: '2.9.0'
    repos:
    - repo: https://github.com/python-trio/flake8-async
-     rev: 25.1.1
+     rev: 25.2.1
      hooks:
        - id: flake8-async
          # args: [--enable=ASYNC, --disable=ASYNC9, --autofix=ASYNC]

flake8_async/__init__.py

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

flake8_async/visitors/visitor91x.py

@@ -354,10 +354,10 @@ class Visitor91X(Flake8AsyncVisitor_cst, CommonVisitors):
             "on line {1.lineno}."
         ),
         "ASYNC912": (
-            "CancelScope with no guaranteed checkpoint. This makes it potentially "
+            "CancelScope with no guaranteed cancel point. This makes it potentially "
             "impossible to cancel."
         ),
-        "ASYNC913": ("Indefinite loop with no guaranteed checkpoint."),
+        "ASYNC913": ("Indefinite loop with no guaranteed cancel points."),
         "ASYNC100": (
             "{0}.{1} context contains no checkpoints, remove the context or add"
             " `await {0}.lowlevel.checkpoint()`."
@@ -401,10 +401,16 @@ def checkpoint_cancel_point(self) -> None:
         self.taskgroup_has_start_soon.clear()
 
     def checkpoint_schedule_point(self) -> None:
-        self.uncheckpointed_statements = set()
+        # ASYNC912&ASYNC913 only cares about cancel points, so don't remove
+        # them if we only do a schedule point
+        self.uncheckpointed_statements = {
+            s
+            for s in self.uncheckpointed_statements
+            if isinstance(s, ArtificialStatement)
+        }
 
     def checkpoint(self) -> None:
-        self.checkpoint_schedule_point()
+        self.uncheckpointed_statements = set()
         self.checkpoint_cancel_point()
 
     def checkpoint_statement(self) -> cst.SimpleStatementLine:

tests/autofix_files/async913_trio_anyio.py

@@ -0,0 +1,12 @@
+# ARG --enable=ASYNC913
+# AUTOFIX
+# ASYNCIO_NO_ERROR
+
+import trio
+
+
+async def nursery_no_cancel_point():
+    while True:  # ASYNC913: 4
+        await trio.lowlevel.checkpoint()
+        async with trio.open_nursery():
+            pass

tests/autofix_files/async913_trio_anyio.py.diff

@@ -0,0 +1,9 @@
+---
++++
+@@ x,5 x,6 @@
+
+ async def nursery_no_cancel_point():
+     while True:  # ASYNC913: 4
++        await trio.lowlevel.checkpoint()
+         async with trio.open_nursery():
+             pass

tests/eval_files/async912.py

@@ -125,7 +125,7 @@ def customWrapper(a: T) -> T:
     with (res := trio.fail_at(10)):
         ...
     # but saving with `as` does
-    with trio.fail_at(10) as res:  # ASYNC912: 9
+    with trio.fail_at(10) as res2:  # ASYNC912: 9
         if bar():
             await trio.lowlevel.checkpoint()
 
@@ -189,3 +189,10 @@ async def check_yield_logic():
         if bar():
             await trio.lowlevel.checkpoint()
         yield
+
+
+async def nursery_no_cancel_point():
+    with trio.move_on_after(10):  # ASYNC912: 9
+        async with trio.open_nursery():
+            if bar():
+                await trio.lowlevel.checkpoint()

tests/eval_files/async913_trio_anyio.py

@@ -0,0 +1,11 @@
+# ARG --enable=ASYNC913
+# AUTOFIX
+# ASYNCIO_NO_ERROR
+
+import trio
+
+
+async def nursery_no_cancel_point():
+    while True:  # ASYNC913: 4
+        async with trio.open_nursery():
+            pass

tests/test_flake8_async.py

@@ -103,7 +103,14 @@ def diff_strings(first: str, second: str, /) -> str:
 # replaces all instances of `original` with `new` in string
 # unless it's preceded by a `-`, which indicates it's part of a command-line flag
 def replace_library(string: str, original: str = "trio", new: str = "anyio") -> str:
-    return re.sub(rf"(?<!-){original}", new, string)
+    def replace_str(string: str, original: str, new: str) -> str:
+        return re.sub(rf"(?<!-){original}", new, string)
+
+    if original == "trio" and new == "anyio":
+        string = replace_str(string, "trio.open_nursery", "anyio.create_task_group")
+    elif original == "anyio" and new == "trio":
+        string = replace_str(string, "anyio.create_task_group", "trio.open_nursery")
+    return replace_str(string, original, new)
 
 
 def check_autofix(