diff --git a/docs/changelog.rst b/docs/changelog.rst index 2672b087..4a2a8938 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,12 @@ Changelog `CalVer, YY.month.patch `_ +24.11.1 +======= +- :ref:`ASYNC100 ` now ignores :func:`trio.open_nursery` and :func:`anyio.create_task_group` + as cancellation sources, because they are :ref:`schedule points ` but not + :ref:`cancellation points `. + 24.10.2 ======= - :ref:`ASYNC102 ` now also warns about ``await()`` inside ``__aexit__``. @@ -23,6 +29,7 @@ Changelog 24.9.3 ====== - :ref:`ASYNC102 ` and :ref:`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 diff --git a/docs/glossary.rst b/docs/glossary.rst index 7dde9b7b..36574f32 100644 --- a/docs/glossary.rst +++ b/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 ` and +:ref:`can switch which task is running `, 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` which are :ref:`schedule_points` but not :ref:`cancel_points`. + asyncio does not place any guarantees on if or when asyncio functions will checkpoint. This means that enabling and adhering to :ref:`ASYNC91x ` -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,35 @@ To insert a checkpoint with no other side effects, you can use :func:`trio.lowlevel.checkpoint`/:func:`anyio.lowlevel.checkpoint`/:func:`asyncio.sleep(0) ` +.. _schedule_point: +.. _schedule_points: + +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_points: + +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 diff --git a/docs/rules.rst b/docs/rules.rst index c3b7566d..f7a404b3 100644 --- a/docs/rules.rst +++ b/docs/rules.rst @@ -13,6 +13,7 @@ _`ASYNC100` : cancel-scope-no-checkpoint A :ref:`timeout_context` does not contain any :ref:`checkpoints `. 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 ` which will in addition guarantee checkpoints on every code path. ASYNC101 : yield-in-cancel-scope diff --git a/flake8_async/__init__.py b/flake8_async/__init__.py index 83c5d570..bbe6c353 100644 --- a/flake8_async/__init__.py +++ b/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.1" # taken from https://github.com/Zac-HD/shed diff --git a/flake8_async/visitors/visitor91x.py b/flake8_async/visitors/visitor91x.py index 539fcee6..26980549 100644 --- a/flake8_async/visitors/visitor91x.py +++ b/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, ) @@ -491,12 +492,34 @@ 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) or not isinstance( + item.item.func, (cst.Attribute, cst.Name) + ): + self.checkpoint() + break + + func = identifier_to_string(item.item.func) + assert func is not None + if func not 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): - if getattr(node, "asynchronous", None): - self.checkpoint() + self._checkpoint_with(node) # if this might suppress exceptions, we cannot treat anything inside it as # checkpointing. @@ -555,8 +578,7 @@ def leave_With(self, original_node: cst.With, updated_node: cst.With): self.restore_state(original_node) self.uncheckpointed_statements.update(prev_checkpoints) - if getattr(original_node, "asynchronous", None): - self.checkpoint() + self._checkpoint_with(original_node) return updated_node # error if no checkpoint since earlier yield or function entry diff --git a/tests/autofix_files/async100.py b/tests/autofix_files/async100.py index 524e320f..ecd6946b 100644 --- a/tests/autofix_files/async100.py +++ b/tests/autofix_files/async100.py @@ -130,3 +130,14 @@ async def fn(timeout): if condition(): return await trio.sleep(1) + + +async def nursery_no_cancel_point(): + # error: 9, "trio", "CancelScope" + async with anyio.create_task_group(): + ... + + +async def dont_crash_on_non_name_or_attr_call(): + async with contextlib.asynccontextmanager(agen_fn)(): + ... diff --git a/tests/autofix_files/async100.py.diff b/tests/autofix_files/async100.py.diff index 268f7d82..7f074e60 100644 --- a/tests/autofix_files/async100.py.diff +++ b/tests/autofix_files/async100.py.diff @@ -130,3 +130,16 @@ 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(): diff --git a/tests/autofix_files/async100_trio.py b/tests/autofix_files/async100_trio.py new file mode 100644 index 00000000..8ab2dd5b --- /dev/null +++ b/tests/autofix_files/async100_trio.py @@ -0,0 +1,10 @@ +# AUTOFIX +# ASYNCIO_NO_ERROR # asyncio.open_nursery doesn't exist +# ANYIO_NO_ERROR # anyio.open_nursery doesn't exist +import trio + + +async def nursery_no_cancel_point(): + # error: 9, "trio", "CancelScope" + async with trio.open_nursery(): + ... diff --git a/tests/autofix_files/async100_trio.py.diff b/tests/autofix_files/async100_trio.py.diff new file mode 100644 index 00000000..dd355aae --- /dev/null +++ b/tests/autofix_files/async100_trio.py.diff @@ -0,0 +1,12 @@ +--- ++++ +@@ 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(): ++ ... diff --git a/tests/eval_files/async100.py b/tests/eval_files/async100.py index 1460a545..4dc100f5 100644 --- a/tests/eval_files/async100.py +++ b/tests/eval_files/async100.py @@ -130,3 +130,14 @@ async def fn(timeout): if condition(): return await trio.sleep(1) + + +async def nursery_no_cancel_point(): + with trio.CancelScope(): # error: 9, "trio", "CancelScope" + async with anyio.create_task_group(): + ... + + +async def dont_crash_on_non_name_or_attr_call(): + async with contextlib.asynccontextmanager(agen_fn)(): + ... diff --git a/tests/eval_files/async100_trio.py b/tests/eval_files/async100_trio.py new file mode 100644 index 00000000..ccf5a1ea --- /dev/null +++ b/tests/eval_files/async100_trio.py @@ -0,0 +1,10 @@ +# AUTOFIX +# ASYNCIO_NO_ERROR # asyncio.open_nursery doesn't exist +# ANYIO_NO_ERROR # anyio.open_nursery doesn't exist +import trio + + +async def nursery_no_cancel_point(): + with trio.CancelScope(): # error: 9, "trio", "CancelScope" + async with trio.open_nursery(): + ...