Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 1 addition & 3 deletions docs/glossary.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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>`
Expand All @@ -117,7 +117,6 @@ To insert a checkpoint with no other side effects, you can use
<asyncio.sleep>`

.. _schedule_point:
.. _schedule_points:

Schedule Point
--------------
Expand All @@ -137,7 +136,6 @@ asyncio does not have any direct equivalents due to their cancellation model bei


.. _cancel_point:
.. _cancel_points:

Cancel Point
------------
Expand Down
1 change: 0 additions & 1 deletion docs/rules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion flake8_async/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 4 additions & 25 deletions flake8_async/visitors/visitor91x.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
flatten_preserving_comments,
fnmatch_qualified_name_cst,
func_has_decorator,
identifier_to_string,
iter_guaranteed_once_cst,
with_has_call,
)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions tests/autofix_files/async100.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
13 changes: 0 additions & 13 deletions tests/autofix_files/async100.py.diff
Original file line number Diff line number Diff line change
Expand Up @@ -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():
6 changes: 3 additions & 3 deletions tests/autofix_files/async100_trio.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
...
12 changes: 0 additions & 12 deletions tests/autofix_files/async100_trio.py.diff
Original file line number Diff line number Diff line change
@@ -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():
+ ...
2 changes: 1 addition & 1 deletion tests/eval_files/async100.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
...

Expand Down
2 changes: 1 addition & 1 deletion tests/eval_files/async100_trio.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@


async def nursery_no_cancel_point():
with trio.CancelScope(): # error: 9, "trio", "CancelScope"
with trio.CancelScope(): # should error, but reverted PR
async with trio.open_nursery():
...
2 changes: 2 additions & 0 deletions tests/test_flake8_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ def format_difflib_line(s: str) -> str:


def diff_strings(first: str, second: str, /) -> str:
if first == second:
return ""
return (
"".join(
map(
Expand Down
Loading