Merge remote-tracking branch 'origin/main' into fix_release_stuff

Author: jakkdl

.pre-commit-config.yaml

@@ -9,7 +9,7 @@ ci:
 
 repos:
     - repo: https://github.com/astral-sh/ruff-pre-commit
-      rev: v0.7.3
+      rev: v0.7.4
       hooks:
           - id: ruff
             args: [--fix]
@@ -45,7 +45,7 @@ repos:
             exclude: tests/eval_files/.*_py311.py
 
     - repo: https://github.com/RobertCraigie/pyright-python
-      rev: v1.1.388
+      rev: v1.1.389
       hooks:
           - id: pyright
             # ignore warnings about new version being available, no other warnings

docs/changelog.rst

@@ -4,17 +4,32 @@ Changelog
 
 `CalVer, YY.month.patch <https://calver.org/>`_
 
-Future
-======
-- :ref:`ASYNC101 <async101>` and :ref:`ASYNC119 <async119>` are now silenced for decorators in :ref:`transform-async-generator-decorators`
+24.11.4
+=======
+- :ref:`ASYNC100 <async100>` once again ignores :func:`trio.open_nursery` and :func:`anyio.create_task_group`, unless we find a call to ``.start_soon()``.
+
+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_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
 =======
 - :ref:`ASYNC102 <async102>` now also warns about ``await()`` inside ``__aexit__``.
 
 24.10.1
 =======
-- Add :ref:`ASYNC123 <async123>` bad-exception-group-flattening
+- Add :ref:`ASYNC123 <async123>` bad-exception-group-flattening.
 
 24.9.5
 ======

docs/glossary.rst

@@ -88,8 +88,8 @@ Exception classes:
 
 Checkpoint
 ----------
-Checkpoints are points where the async backend checks for cancellation and
-can switch which task is running, in an ``await``, ``async for``, or ``async with``
+Checkpoints are points where the async backend checks for :ref:`cancellation <cancel_point>` and
+:ref:`can switch which task is running <schedule_point>`, in an ``await``, ``async for``, or ``async with``
 expression.  Regular checkpoints can be important for both performance and correctness.
 
 Trio has extensive and detailed documentation on the concept of
@@ -99,11 +99,11 @@ 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`. They do not checkpoint on entry, and on exit they insert a :ref:`schedule point <schedule_point>`. However, if sub-tasks are cancelled they will be propagated on exit, so if you're starting tasks you can usually treat the exit as a :ref:`cancel point <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>`
-will still not guarantee checkpoints.
-
-For anyio it will depend on the current backend.
+will still not guarantee checkpoints on asyncio (even if used via anyio).
 
 When using Trio (or an AnyIO library that people might use on Trio), it can be
 very helpful to ensure that your own code adheres to the same guarantees as
@@ -116,6 +116,33 @@ To insert a checkpoint with no other side effects, you can use
 :func:`trio.lowlevel.checkpoint`/:func:`anyio.lowlevel.checkpoint`/:func:`asyncio.sleep(0)
 <asyncio.sleep>`
 
+.. _schedule_point:
+
+Schedule Point
+--------------
+A schedule point is half of a full :ref:`checkpoint`, which allows the async backend to switch the running task, but doesn't check for cancellation (the other half is a :ref:`cancel_point`).
+While you are unlikely to need one, they are available as :func:`trio.lowlevel.cancel_shielded_checkpoint`/:func:`anyio.lowlevel.cancel_shielded_checkpoint`, and equivalent to
+
+.. code-block:: python
+
+   from trio import CancelScope, lowlevel
+   # or
+   # from anyio import CancelScope, lowlevel
+
+   with CancelScope(shield=True):
+       await lowlevel.checkpoint()
+
+asyncio does not have any direct equivalents due to their cancellation model being different.
+
+
+.. _cancel_point:
+
+Cancel Point
+------------
+A schedule point is half of a full :ref:`checkpoint`, which will raise :ref:`cancelled` if the enclosing cancel scope has been cancelled, but does not allow the scheduler to switch to a different task (the other half is a :ref:`schedule_point`).
+While you are unlikely to need one, they are available as :func:`trio.lowlevel.checkpoint_if_cancelled`/:func:`anyio.lowlevel.checkpoint_if_cancelled`.
+Users of asyncio might want to use :meth:`asyncio.Task.cancelled`.
+
 .. _channel_stream_queue:
 
 Channel / Stream / Queue

docs/rules.rst

@@ -13,6 +13,7 @@ _`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 <schedule_point>` but not :ref:`cancel points <cancel_point>` (unless they have tasks started in them).
     See :ref:`ASYNC912 <async912>` which will in addition guarantee checkpoints on every code path.
 
 _`ASYNC101` : yield-in-cancel-scope

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: 24.10.2
+     rev: 24.11.4
      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__ = "24.10.2"
+__version__ = "24.11.4"
 
 
 # taken from https://github.com/Zac-HD/shed

flake8_async/visitors/helpers.py

@@ -337,15 +337,20 @@ def build_cst_matcher(attr: str) -> m.BaseExpression:
     return m.Attribute(value=build_cst_matcher(body), attr=m.Name(value=tail))
 
 
-def identifier_to_string(attr: cst.Name | cst.Attribute) -> str | None:
-    if isinstance(attr, cst.Name):
-        return attr.value
-    if not isinstance(attr.value, (cst.Attribute, cst.Name)):
-        return None
-    lhs = identifier_to_string(attr.value)
-    if lhs is None:
-        return None
-    return lhs + "." + attr.attr.value
+def identifier_to_string(node: cst.CSTNode) -> str | None:
+    """Convert a simple identifier to a string.
+
+    If the node is composed of anything but cst.Name + cst.Attribute it returns None.
+    """
+    if isinstance(node, cst.Name):
+        return node.value
+    if (
+        isinstance(node, cst.Attribute)
+        and (lhs := identifier_to_string(node.value)) is not None
+    ):
+        return lhs + "." + node.attr.value
+
+    return None
 
 
 def with_has_call(

flake8_async/visitors/visitor91x.py

@@ -25,6 +25,7 @@
     flatten_preserving_comments,
     fnmatch_qualified_name_cst,
     func_has_decorator,
+    identifier_to_string,
     iter_guaranteed_once_cst,
     with_has_call,
 )
@@ -285,6 +286,7 @@ def __init__(self, *args: Any, **kwargs: Any):
         # ASYNC100
         self.has_checkpoint_stack: list[bool] = []
         self.node_dict: dict[cst.With, list[AttributeCall]] = {}
+        self.taskgroup_has_start_soon: dict[str, bool] = {}
 
         # --exception-suppress-context-manager
         self.suppress_imported_as: list[str] = []
@@ -299,13 +301,31 @@ def should_autofix(self, node: cst.CSTNode, code: str | None = None) -> bool:
             and self.library != ("asyncio",)
         )
 
-    def checkpoint(self) -> None:
-        self.uncheckpointed_statements = set()
+    def checkpoint_cancel_point(self) -> None:
         self.has_checkpoint_stack = [True] * len(self.has_checkpoint_stack)
+        # don't need to look for any .start_soon() calls
+        self.taskgroup_has_start_soon.clear()
+
+    def checkpoint_schedule_point(self) -> None:
+        self.uncheckpointed_statements = set()
+
+    def checkpoint(self) -> None:
+        self.checkpoint_schedule_point()
+        self.checkpoint_cancel_point()
 
     def checkpoint_statement(self) -> cst.SimpleStatementLine:
         return checkpoint_statement(self.library[0])
 
+    def visit_Call(self, node: cst.Call) -> None:
+        # [Nursery/TaskGroup].start_soon introduces a cancel point
+        if (
+            isinstance(node.func, cst.Attribute)
+            and isinstance(node.func.value, cst.Name)
+            and node.func.attr.value == "start_soon"
+            and node.func.value.value in self.taskgroup_has_start_soon
+        ):
+            self.taskgroup_has_start_soon[node.func.value.value] = True
+
     def visit_ImportFrom(self, node: cst.ImportFrom) -> None:
         # Semi-crude approach to handle `from contextlib import suppress`.
         # It does not handle the identifier being overridden, or assigned
@@ -341,16 +361,21 @@ def visit_FunctionDef(self, node: cst.FunctionDef) -> bool:
             "safe_decorator",
             "async_function",
             "uncheckpointed_statements",
+            # comp_unknown does not need to be saved
             "loop_state",
             "try_state",
             "has_checkpoint_stack",
-            "suppress_imported_as",
+            # node_dict is cleaned up and don't need to be saved
+            "taskgroup_has_start_soon",
+            "suppress_imported_as",  # a copy is saved, but state is not reset
             copy=True,
         )
-        self.uncheckpointed_statements = set()
-        self.has_checkpoint_stack = []
         self.has_yield = self.safe_decorator = False
+        self.uncheckpointed_statements = set()
         self.loop_state = LoopState()
+        # try_state is reset upon entering try
+        self.has_checkpoint_stack = []
+        self.taskgroup_has_start_soon = {}
 
         self.async_function = (
             node.asynchronous is not None
@@ -440,8 +465,8 @@ def leave_Return(
         ):
             self.add_statement = self.checkpoint_statement()
         # avoid duplicate error messages
-        self.uncheckpointed_statements = set()
-        # we don't treat it as a checkpoint for ASYNC100
+        # but don't see it as a cancel point for ASYNC100
+        self.checkpoint_schedule_point()
 
         # return original node to avoid problems with identity equality
         assert original_node.deep_equals(updated_node)
@@ -491,12 +516,47 @@ def _is_exception_suppressing_context_manager(self, node: cst.With) -> bool:
             is not None
         )
 
+    def _checkpoint_with(self, node: cst.With, entry: bool):
+        """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.
+
+        Saves the name of the taskgroup/nursery if entry is set
+        """
+        if not getattr(node, "asynchronous", None):
+            return
+
+        for item in node.items:
+            if isinstance(item.item, cst.Call) and identifier_to_string(
+                item.item.func
+            ) in (
+                "trio.open_nursery",
+                "anyio.create_task_group",
+            ):
+                if item.asname is not None and isinstance(item.asname.name, cst.Name):
+                    # save the nursery/taskgroup to see if it has a `.start_soon`
+                    if entry:
+                        self.taskgroup_has_start_soon[item.asname.name.value] = False
+                    elif self.taskgroup_has_start_soon.pop(
+                        item.asname.name.value, False
+                    ):
+                        self.checkpoint()
+                        return
+            else:
+                self.checkpoint()
+                break
+        else:
+            # only taskgroup/nursery calls
+            self.checkpoint_schedule_point()
+
     # 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):
-        if getattr(node, "asynchronous", None):
-            self.checkpoint()
+        self.save_state(node, "taskgroup_has_start_soon", copy=True)
+        self._checkpoint_with(node, entry=True)
 
         # if this might suppress exceptions, we cannot treat anything inside it as
         # checkpointing.
@@ -548,15 +608,16 @@ def leave_With(self, original_node: cst.With, updated_node: cst.With):
                 for res in self.node_dict[original_node]:
                     self.error(res.node, error_code="ASYNC912")
 
+        self.node_dict.pop(original_node, None)
+
         # if exception-suppressing, restore all uncheckpointed statements from
         # before the `with`.
         if self._is_exception_suppressing_context_manager(original_node):
             prev_checkpoints = self.uncheckpointed_statements
             self.restore_state(original_node)
             self.uncheckpointed_statements.update(prev_checkpoints)
 
-        if getattr(original_node, "asynchronous", None):
-            self.checkpoint()
+        self._checkpoint_with(original_node, entry=False)
         return updated_node
 
     # error if no checkpoint since earlier yield or function entry
@@ -569,7 +630,7 @@ def leave_Yield(
 
         # Treat as a checkpoint for ASYNC100, since the context we yield to
         # may checkpoint.
-        self.has_checkpoint_stack = [True] * len(self.has_checkpoint_stack)
+        self.checkpoint_cancel_point()
 
         if self.check_function_exit(original_node) and self.should_autofix(
             original_node

flake8_async/visitors/visitors.py

@@ -5,8 +5,6 @@
 import ast
 from typing import TYPE_CHECKING, Any, cast
 
-import libcst as cst
-
 from .flake8asyncvisitor import Flake8AsyncVisitor, Flake8AsyncVisitor_cst
 from .helpers import (
     disabled_by_default,
@@ -20,6 +18,8 @@
 if TYPE_CHECKING:
     from collections.abc import Mapping
 
+    import libcst as cst
+
 LIBRARIES = ("trio", "anyio", "asyncio")
 
 
@@ -460,8 +460,7 @@ def visit_CompIf(self, node: cst.CSTNode):
 
     def visit_Call(self, node: cst.Call):
         if (
-            isinstance(node.func, (cst.Name, cst.Attribute))
-            and identifier_to_string(node.func) == "asyncio.create_task"
+            identifier_to_string(node.func) == "asyncio.create_task"
             and not self.safe_to_create_task
         ):
             self.error(node)

tests/autofix_files/async100.py

@@ -130,3 +130,13 @@ async def fn(timeout):
             if condition():
                 return
             await trio.sleep(1)
+
+
+async def dont_crash_on_non_name_or_attr_call():
+    async with contextlib.asynccontextmanager(agen_fn)():
+        ...
+
+
+async def another_weird_with_call():
+    async with a().b():
+        ...