Revert the changes to Visitor91x in "async100 now ignores trio.open_n… (#326)

* Revert the changes to Visitor91x in "async100 now ignores trio.open_nursery and anyio.create_task_group (#317)"

This partially reverts commit 7a45176.

Author: jakkdl

docs/changelog.rst

@@ -4,15 +4,19 @@ Changelog
 
 `CalVer, YY.month.patch <https://calver.org/>`_
 
+24.11.3
+=======
+- Revert :ref:`ASYNC100 <async100>` ignoring :func:`trio.open_nursery` and :func:`anyio.create_task_group` due to it not viewing `start_soon()` as introducing a :ref:`cancel point <cancel_point>`.
+
 24.11.2
 =======
 - Fix crash in ``Visitor91x`` on ``async with a().b():``.
 
 24.11.1
 =======
 - :ref:`ASYNC100 <async100>` now ignores :func:`trio.open_nursery` and :func:`anyio.create_task_group`
-  as cancellation sources, because they are :ref:`schedule points <schedule_points>` but not
-  :ref:`cancellation points <cancel_points>`.
+  as cancellation sources, because they are :ref:`schedule points <schedule_point>` but not
+  :ref:`cancellation points <cancel_point>`.
 - :ref:`ASYNC101 <async101>` and :ref:`ASYNC119 <async119>` are now silenced for decorators in :ref:`transform-async-generator-decorators`.
 
 24.10.2
@@ -34,7 +38,6 @@ Changelog
 24.9.3
 ======
 - :ref:`ASYNC102 <async102>` and :ref:`ASYNC120 <async120>`:
-
   - handles nested cancel scopes
   - detects internal cancel scopes of nurseries as a way to shield&deadline
   - no longer treats :func:`trio.open_nursery` or :func:`anyio.create_task_group` as cancellation sources

docs/glossary.rst

@@ -99,7 +99,7 @@ functions defined by Trio will either checkpoint or raise an exception when
 iteration, and when exhausting the iterator, and ``async with`` will checkpoint
 on at least one of enter/exit.
 
-The one exception is :func:`trio.open_nursery` and :func:`anyio.create_task_group` which are :ref:`schedule_points` but not :ref:`cancel_points`.
+The one exception is :func:`trio.open_nursery` and :func:`anyio.create_task_group` which are :ref:`schedule points <schedule_point>` but not :ref:`cancel points <cancel_point>`.
 
 asyncio does not place any guarantees on if or when asyncio functions will
 checkpoint. This means that enabling and adhering to :ref:`ASYNC91x <ASYNC910>`
@@ -117,7 +117,6 @@ To insert a checkpoint with no other side effects, you can use
 <asyncio.sleep>`
 
 .. _schedule_point:
-.. _schedule_points:
 
 Schedule Point
 --------------
@@ -137,7 +136,6 @@ asyncio does not have any direct equivalents due to their cancellation model bei
 
 
 .. _cancel_point:
-.. _cancel_points:
 
 Cancel Point
 ------------

docs/rules.rst

@@ -13,7 +13,6 @@ _`ASYNC100` : cancel-scope-no-checkpoint
     A :ref:`timeout_context` does not contain any :ref:`checkpoints <checkpoint>`.
     This makes it pointless, as the timeout can only be triggered by a checkpoint.
     This check also treats ``yield`` as a checkpoint, since checkpoints can happen in the caller we yield to.
-    :func:`trio.open_nursery` and :func:`anyio.create_task_group` are excluded, as they are :ref:`schedule_points` but not :ref:`cancel_points`.
     See :ref:`ASYNC912 <async912>` which will in addition guarantee checkpoints on every code path.
 
 _`ASYNC101` : yield-in-cancel-scope

flake8_async/__init__.py

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

flake8_async/visitors/visitor91x.py

@@ -25,7 +25,6 @@
     flatten_preserving_comments,
     fnmatch_qualified_name_cst,
     func_has_decorator,
-    identifier_to_string,
     iter_guaranteed_once_cst,
     with_has_call,
 )
@@ -492,33 +491,12 @@ def _is_exception_suppressing_context_manager(self, node: cst.With) -> bool:
             is not None
         )
 
-    def _checkpoint_with(self, node: cst.With):
-        """Conditionally checkpoints entry/exit of With.
-
-        If the with only contains calls to open_nursery/create_task_group, it's a schedule
-        point but not a cancellation point, so we treat it as a checkpoint for async91x
-        but not for async100.
-        """
-        if getattr(node, "asynchronous", None):
-            for item in node.items:
-                if not (
-                    isinstance(item.item, cst.Call)
-                    and identifier_to_string(item.item.func)
-                    in (
-                        "trio.open_nursery",
-                        "anyio.create_task_group",
-                    )
-                ):
-                    self.checkpoint()
-                    break
-            else:
-                self.uncheckpointed_statements = set()
-
     # Async context managers can reasonably checkpoint on either or both of entry and
     # exit.  Given that we can't tell which, we assume "both" to avoid raising a
     # missing-checkpoint warning when there might in fact be one (i.e. a false alarm).
     def visit_With_body(self, node: cst.With):
-        self._checkpoint_with(node)
+        if getattr(node, "asynchronous", None):
+            self.checkpoint()
 
         # if this might suppress exceptions, we cannot treat anything inside it as
         # checkpointing.
@@ -577,7 +555,8 @@ def leave_With(self, original_node: cst.With, updated_node: cst.With):
             self.restore_state(original_node)
             self.uncheckpointed_statements.update(prev_checkpoints)
 
-        self._checkpoint_with(original_node)
+        if getattr(original_node, "asynchronous", None):
+            self.checkpoint()
         return updated_node
 
     # error if no checkpoint since earlier yield or function entry

tests/autofix_files/async100.py

@@ -133,9 +133,9 @@ async def fn(timeout):
 
 
 async def nursery_no_cancel_point():
-    # error: 9, "trio", "CancelScope"
-    async with anyio.create_task_group():
-        ...
+    with trio.CancelScope():  # should error, but reverted PR
+        async with anyio.create_task_group():
+            ...
 
 
 async def dont_crash_on_non_name_or_attr_call():

tests/autofix_files/async100.py.diff

@@ -130,16 +130,3 @@
 
      with contextlib.suppress(Exception):
          with open("blah") as file:
-@@ x,9 x,9 @@
-
-
- async def nursery_no_cancel_point():
--    with trio.CancelScope():  # error: 9, "trio", "CancelScope"
--        async with anyio.create_task_group():
--            ...
-+    # error: 9, "trio", "CancelScope"
-+    async with anyio.create_task_group():
-+        ...
-
-
- async def dont_crash_on_non_name_or_attr_call():

tests/autofix_files/async100_trio.py

@@ -5,6 +5,6 @@
 
 
 async def nursery_no_cancel_point():
-    # error: 9, "trio", "CancelScope"
-    async with trio.open_nursery():
-        ...
+    with trio.CancelScope():  # should error, but reverted PR
+        async with trio.open_nursery():
+            ...

tests/autofix_files/async100_trio.py.diff

@@ -1,12 +0,0 @@
----
-+++
-@@ x,6 x,6 @@
-
-
- async def nursery_no_cancel_point():
--    with trio.CancelScope():  # error: 9, "trio", "CancelScope"
--        async with trio.open_nursery():
--            ...
-+    # error: 9, "trio", "CancelScope"
-+    async with trio.open_nursery():
-+        ...

tests/eval_files/async100.py

@@ -133,7 +133,7 @@ async def fn(timeout):
 
 
 async def nursery_no_cancel_point():
-    with trio.CancelScope():  # error: 9, "trio", "CancelScope"
+    with trio.CancelScope():  # should error, but reverted PR
         async with anyio.create_task_group():
             ...