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

ASYNC100 no longer triggers on context managers containing a yield #228

Merged
merged 2 commits into from
Apr 3, 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Changelog
*[CalVer, YY.month.patch](https://calver.org/)*

## 24.3.6
- ASYNC100 no longer triggers if a context manager contains a `yield`.

## 24.3.5
- ASYNC102 (no await inside finally or critical except) no longer raises warnings for calls to `aclose()` on objects in trio/anyio code. See https://github.com/python-trio/flake8-async/issues/156

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pip install flake8-async
## List of warnings
- **ASYNC100**: A `with [trio/anyio].fail_after(...):` or `with [trio/anyio].move_on_after(...):`
context does not contain any `await` statements. This makes it pointless, as
the timeout can only be triggered by a checkpoint.
the timeout can only be triggered by a checkpoint. This check also allows `yield` statements, since checkpoints can happen in the caller we yield to.
- **ASYNC101**: `yield` inside a trio/anyio nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling.
- **ASYNC102**: It's unsafe to await inside `finally:` or `except BaseException/trio.Cancelled/anyio.get_cancelled_exc_class()/asyncio.exceptions.CancelledError` unless you use a shielded cancel scope with a timeout. This is currently not able to detect asyncio shields.
- **ASYNC103**: `except BaseException/trio.Cancelled/anyio.get_cancelled_exc_class()/asyncio.exceptions.CancelledError`, or a bare `except:` with a code path that doesn't re-raise. If you don't want to re-raise `BaseException`, add a separate handler for `trio.Cancelled`/`anyio.get_cancelled_exc_class()`/`asyncio.exceptions.CancelledError` before.
Expand Down
2 changes: 1 addition & 1 deletion flake8_async/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@


# CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
__version__ = "24.3.5"
__version__ = "24.3.6"


# taken from https://github.com/Zac-HD/shed
Expand Down
4 changes: 3 additions & 1 deletion flake8_async/visitors/visitor100.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,11 @@ def visit_For(self, node: cst.For):
if node.asynchronous is not None:
self.checkpoint()

def visit_Await(self, node: cst.Await | cst.For | cst.With):
def visit_Await(self, node: cst.Await | cst.Yield):
self.checkpoint()

visit_Yield = visit_Await

def visit_FunctionDef(self, node: cst.FunctionDef):
self.save_state(node, "has_checkpoint_stack", copy=True)
self.has_checkpoint_stack = []
Expand Down
6 changes: 6 additions & 0 deletions tests/autofix_files/async100.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,9 @@ async def more_nested_tests():
with contextlib.suppress(Exception):
with open("blah") as file:
print("foo")


# Don't trigger for blocks with a yield statement
async def foo():
with trio.fail_after(1):
yield
Comment on lines +119 to +122
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. This should indeed be allowed by ASYNC100...

...but now I'm wondering about a non-optional rule to warn when you yield across a known cancelscope and the function isn't a context-manager, because that really doesn't work. Probably not worth the trouble, since njs and I are planning to get runtime enforcement, the rule would have many missed alarms, and ASYNC900 covers it anyway for people who care.

28 changes: 19 additions & 9 deletions tests/autofix_files/noqa.py
Original file line number Diff line number Diff line change
@@ -1,47 +1,57 @@
# AUTOFIX
# NOASYNCIO - does not trigger on ASYNC100
# ARG --enable=ASYNC100,ASYNC911
# ARG --enable=ASYNC100,ASYNC911,ASYNC910
from typing import Any

import trio


def foo() -> bool:
return False


# fmt: off
async def foo_no_noqa():
await trio.lowlevel.checkpoint()
# ASYNC100: 9, 'trio', 'fail_after'
yield # ASYNC911: 8, "yield", Statement("function definition", lineno-2)
if foo():
await trio.lowlevel.checkpoint()
return # ASYNC910: 12, "return", Statement("function definition", lineno-3)
await trio.lowlevel.checkpoint()


async def foo_noqa_bare():
with trio.fail_after(5): # noqa
yield # noqa
if foo():
return # noqa
await trio.lowlevel.checkpoint()


async def foo_noqa_100():
with trio.fail_after(5): # noqa: ASYNC100
await trio.lowlevel.checkpoint()
yield # ASYNC911: 8, "yield", Statement("function definition", lineno-2)
if foo():
await trio.lowlevel.checkpoint()
return # ASYNC910: 12, "return", Statement("function definition", lineno-3)
await trio.lowlevel.checkpoint()


async def foo_noqa_911():
# ASYNC100: 9, 'trio', 'fail_after'
yield # noqa: ASYNC911
if foo():
return # noqa: ASYNC910
await trio.lowlevel.checkpoint()


async def foo_noqa_100_911():
with trio.fail_after(5): # noqa: ASYNC100, ASYNC911
yield # noqa: ASYNC911
if foo():
return # noqa: ASYNC910
await trio.lowlevel.checkpoint()


async def foo_noqa_100_911_500():
with trio.fail_after(5): # noqa: ASYNC100, ASYNC911 , ASYNC500,,,
yield # noqa: ASYNC100, ASYNC911 , ASYNC500,,,
if foo():
return # noqa: ASYNC100, ASYNC910 , ASYNC500,,,
await trio.lowlevel.checkpoint()
# fmt: on

Expand Down
24 changes: 14 additions & 10 deletions tests/autofix_files/noqa.py.diff
Original file line number Diff line number Diff line change
@@ -1,31 +1,35 @@
---
+++
@@ x,8 x,9 @@
@@ x,9 x,10 @@

# fmt: off
async def foo_no_noqa():
- with trio.fail_after(5): # ASYNC100: 9, 'trio', 'fail_after'
- yield # ASYNC911: 8, "yield", Statement("function definition", lineno-2)
+ await trio.lowlevel.checkpoint()
- if foo():
- return # ASYNC910: 12, "return", Statement("function definition", lineno-3)
+ # ASYNC100: 9, 'trio', 'fail_after'
+ yield # ASYNC911: 8, "yield", Statement("function definition", lineno-2)
+ if foo():
+ await trio.lowlevel.checkpoint()
+ return # ASYNC910: 12, "return", Statement("function definition", lineno-3)
await trio.lowlevel.checkpoint()


@@ x,13 x,14 @@

@@ x,14 x,15 @@
async def foo_noqa_100():
with trio.fail_after(5): # noqa: ASYNC100
+ await trio.lowlevel.checkpoint()
yield # ASYNC911: 8, "yield", Statement("function definition", lineno-2)
if foo():
+ await trio.lowlevel.checkpoint()
return # ASYNC910: 12, "return", Statement("function definition", lineno-3)
await trio.lowlevel.checkpoint()


async def foo_noqa_911():
- with trio.fail_after(5): # ASYNC100: 9, 'trio', 'fail_after'
- yield # noqa: ASYNC911
- if foo():
- return # noqa: ASYNC910
+ # ASYNC100: 9, 'trio', 'fail_after'
+ yield # noqa: ASYNC911
+ if foo():
+ return # noqa: ASYNC910
await trio.lowlevel.checkpoint()


Expand Down
6 changes: 6 additions & 0 deletions tests/eval_files/async100.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,9 @@ async def more_nested_tests():
with contextlib.suppress(Exception):
with open("blah") as file:
print("foo")


# Don't trigger for blocks with a yield statement
async def foo():
with trio.fail_after(1):
yield
24 changes: 17 additions & 7 deletions tests/eval_files/noqa.py
Original file line number Diff line number Diff line change
@@ -1,45 +1,55 @@
# AUTOFIX
# NOASYNCIO - does not trigger on ASYNC100
# ARG --enable=ASYNC100,ASYNC911
# ARG --enable=ASYNC100,ASYNC911,ASYNC910
from typing import Any

import trio


def foo() -> bool:
return False


# fmt: off
async def foo_no_noqa():
with trio.fail_after(5): # ASYNC100: 9, 'trio', 'fail_after'
yield # ASYNC911: 8, "yield", Statement("function definition", lineno-2)
if foo():
return # ASYNC910: 12, "return", Statement("function definition", lineno-3)
await trio.lowlevel.checkpoint()


async def foo_noqa_bare():
with trio.fail_after(5): # noqa
yield # noqa
if foo():
return # noqa
await trio.lowlevel.checkpoint()


async def foo_noqa_100():
with trio.fail_after(5): # noqa: ASYNC100
yield # ASYNC911: 8, "yield", Statement("function definition", lineno-2)
if foo():
return # ASYNC910: 12, "return", Statement("function definition", lineno-3)
await trio.lowlevel.checkpoint()


async def foo_noqa_911():
with trio.fail_after(5): # ASYNC100: 9, 'trio', 'fail_after'
yield # noqa: ASYNC911
if foo():
return # noqa: ASYNC910
await trio.lowlevel.checkpoint()


async def foo_noqa_100_911():
with trio.fail_after(5): # noqa: ASYNC100, ASYNC911
yield # noqa: ASYNC911
if foo():
return # noqa: ASYNC910
await trio.lowlevel.checkpoint()


async def foo_noqa_100_911_500():
with trio.fail_after(5): # noqa: ASYNC100, ASYNC911 , ASYNC500,,,
yield # noqa: ASYNC100, ASYNC911 , ASYNC500,,,
if foo():
return # noqa: ASYNC100, ASYNC910 , ASYNC500,,,
await trio.lowlevel.checkpoint()
# fmt: on

Expand Down
Loading