add ASYNC122 delayed-entry-of-relative-cancelscope (#292)

Author: jakkdl

docs/changelog.rst

@@ -4,6 +4,10 @@ Changelog
 
 `CalVer, YY.month.patch <https://calver.org/>`_
 
+24.9.4
+======
+- Add :ref:`ASYNC122 <async122>` delayed-entry-of-relative-cancelscope.
+
 24.9.3
 ======
 - :ref:`ASYNC102 <async102>` and :ref:`ASYNC120 <async120>`:
@@ -19,7 +23,7 @@ Changelog
 
 24.9.1
 ======
-- Add :ref:`ASYNC121 <async121>` control-flow-in-taskgroup
+- Add :ref:`ASYNC121 <async121>` control-flow-in-taskgroup.
 
 24.8.1
 ======
@@ -37,7 +41,7 @@ Changelog
 
 24.5.5
 ======
-- Add :ref:`ASYNC300 <async300>` create-task-no-reference
+- Add :ref:`ASYNC300 <async300>` create-task-no-reference.
 
 24.5.4
 ======

docs/rules.rst

@@ -86,6 +86,8 @@ _`ASYNC120` : await-in-except
 _`ASYNC121`: control-flow-in-taskgroup
     `return`, `continue`, and `break` inside a :ref:`taskgroup_nursery` can lead to counterintuitive behaviour. Refactor the code to instead cancel the :ref:`cancel_scope` inside the TaskGroup/Nursery and place the statement outside of the TaskGroup/Nursery block. In asyncio a user might expect the statement to have an immediate effect, but it will wait for all tasks to finish before having an effect. See `Trio issue #1493 <https://github.com/python-trio/trio/issues/1493>`_ for further issues specific to trio/anyio.
 
+_`ASYNC122`: delayed-entry-of-relative-cancelscope
+    :func:`trio.move_on_after`, :func:`trio.fail_after`, :func:`anyio.move_on_after` and :func:`anyio.fail_after` behaves unintuitively if initialization and entry are separated, with the timeout starting on initialization. Trio>=0.27 changes this behaviour, so if you don't support older versions you should disable this check. See `Trio issue #2512 <https://github.com/python-trio/trio/issues/2512>`_.
 
 Blocking sync calls in async functions
 ======================================

flake8_async/__init__.py

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

flake8_async/visitors/visitors.py

@@ -410,6 +410,33 @@ def visit_FunctionDef(self, node: ast.FunctionDef | ast.AsyncFunctionDef):
     visit_AsyncFunctionDef = visit_FunctionDef
 
 
+@error_class
+class Visitor122(Flake8AsyncVisitor):
+    error_codes: Mapping[str, str] = {
+        "ASYNC122": (
+            "Separating initialization from entry of {} changed behavior in Trio"
+            " 0.27, and was unintuitive before then. If you only support"
+            " trio>=0.27 you should disable this check."
+        )
+    }
+
+    def __init__(self, *args: Any, **kwargs: Any):
+        super().__init__(*args, **kwargs)
+        self.in_withitem = False
+
+    def visit_withitem(self, node: ast.withitem):
+        self.save_state(node, "in_withitem")
+        self.in_withitem = True
+
+    def visit_Call(self, node: ast.Call):
+        if not self.in_withitem and (
+            match := get_matching_call(
+                node, "fail_after", "move_on_after", base=("trio", "anyio")
+            )
+        ):
+            self.error(node, f"{match[2]}.{match[1]}")
+
+
 @error_class_cst
 class Visitor300(Flake8AsyncVisitor_cst):
     error_codes: Mapping[str, str] = {

tests/eval_files/async122.py

@@ -0,0 +1,35 @@
+# ASYNCIO_NO_ERROR
+
+import trio
+
+
+def safe():
+    with trio.move_on_after(5):
+        ...
+    with open("hello"), trio.move_on_after(5):
+        ...
+
+
+def separated():
+    k = trio.move_on_after(5)  # ASYNC122: 8, "trio.move_on_after"
+
+    with k:
+        ...
+
+    l = trio.fail_after(5)  # ASYNC122: 8, "trio.fail_after"
+    with l:
+        ...
+
+
+def fancy_thing_we_dont_cover():
+    # it's hard to distinguish this bad case
+    kk = trio.fail_after
+
+    ll = kk(5)
+
+    with ll:
+        ...
+    # from this good case
+    with kk(5):
+        ...
+    # so we don't bother

tests/test_flake8_async.py

@@ -481,6 +481,7 @@ def _parse_eval_file(
     # opening nurseries & taskgroups can only be done in async context, so ASYNC121
     # doesn't check for it
     "ASYNC121",
+    "ASYNC122",
     "ASYNC300",
     "ASYNC912",
 }