Skip to content

Commit ced4570

Browse files
committed
update error descriptions in README to better reflect which errors work with which library, and minor updates. 102/103/104 now sees asyncio.exceptions.CancelledError as a critical exception. Fix import/arg detection of asyncio. add asyncio102_asyncio. add async103_all_imported. Finally make test_anyio_from_config autodetect the correct line number. Fix BASE_LIBRARY marker being interpreted as a bool. Make #NOTRIO/#NOASYNCIO/#NOANYIO run the visitor but ignore the result, instead of skipping, to check it doesn't crash. Generalize error-message-library-check.
1 parent e97b5d3 commit ced4570

13 files changed

+270
-52
lines changed

README.md

+23-23
Original file line numberDiff line numberDiff line change
@@ -23,47 +23,47 @@ pip install flake8-async
2323
```
2424

2525
## List of warnings
26-
27-
- **ASYNC100**: A `with trio.fail_after(...):` or `with trio.move_on_after(...):`
26+
- **ASYNC100**: A `with [trio|anyio].fail_after(...):` or `with [trio|anyio].move_on_after(...):`
2827
context does not contain any `await` statements. This makes it pointless, as
2928
the timeout can only be triggered by a checkpoint.
30-
- **ASYNC101**: `yield` inside a nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling.
31-
- **ASYNC102**: It's unsafe to await inside `finally:` or `except BaseException/trio.Cancelled` unless you use a shielded
32-
cancel scope with a timeout.
33-
- **ASYNC103**: `except BaseException`, `except trio.Cancelled` 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` before.
34-
- **ASYNC104**: `Cancelled` and `BaseException` must be re-raised - when a user tries to `return` or `raise` a different exception.
35-
- **ASYNC105**: Calling a trio async function without immediately `await`ing it.
29+
- **ASYNC101**: `yield` inside a trio/anyio nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling.
30+
- **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.
31+
- **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.
32+
- **ASYNC104**: `trio.Cancelled`/`anyio.get_cancelled_exc_class()`/`asyncio.exceptions.CancelledError`/`BaseException` must be re-raised. The same as ASYNC103, except specifically triggered on `return` or a different exception being raised.
33+
- **ASYNC105**: Calling a trio async function without immediately `await`ing it. This is only supported with trio functions, but you can get similar functionality with a type-checker.
3634
- **ASYNC106**: `trio`/`anyio`/`asyncio` must be imported with `import trio`/`import anyio`/`import asyncio` for the linter to work.
37-
- **ASYNC109**: Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead
38-
- **ASYNC110**: `while <condition>: await trio.sleep()` should be replaced by a `trio.Event`.
35+
- **ASYNC109**: Async function definition with a `timeout` parameter - use `[trio/anyio].[fail/move_on]_[after/at]` instead.
36+
- **ASYNC110**: `while <condition>: await [trio/anyio].sleep()` should be replaced by a `[trio|anyio].Event`.
3937
- **ASYNC111**: Variable, from context manager opened inside nursery, passed to `start[_soon]` might be invalidly accessed while in use, due to context manager closing before the nursery. This is usually a bug, and nurseries should generally be the inner-most context manager.
4038
- **ASYNC112**: Nursery body with only a call to `nursery.start[_soon]` and not passing itself as a parameter can be replaced with a regular function call.
4139
- **ASYNC113**: Using `nursery.start_soon` in `__aenter__` doesn't wait for the task to begin. Consider replacing with `nursery.start`.
4240
- **ASYNC114**: Startable function (i.e. has a `task_status` keyword parameter) not in `--startable-in-context-manager` parameter list, please add it so ASYNC113 can catch errors when using it.
43-
- **ASYNC115**: Replace `trio.sleep(0)` with the more suggestive `trio.lowlevel.checkpoint()`.
44-
- **ASYNC116**: `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`.
41+
- **ASYNC115**: Replace `[trio|anyio].sleep(0)` with the more suggestive `[trio|anyio].lowlevel.checkpoint()`.
42+
- **ASYNC116**: `[trio|anyio].sleep()` with >24 hour interval should usually be `[trio|anyio].sleep_forever()`.
4543
- **ASYNC118**: Don't assign the value of `anyio.get_cancelled_exc_class()` to a variable, since that breaks linter checks and multi-backend programs.
4644

4745
### Warnings for blocking sync calls in async functions
48-
- **ASYNC200**: User-configured error for blocking sync calls in async functions. Does nothing by default, see [`trio200-blocking-calls`](#trio200-blocking-calls) for how to configure it.
49-
- **ASYNC210**: Sync HTTP call in async function, use `httpx.AsyncClient`.
46+
Note: 22X, 23X and 24X has not had asyncio-specific suggestions written.
47+
- **ASYNC200**: User-configured error for blocking sync calls in async functions. Does nothing by default, see [`async200-blocking-calls`](#async200-blocking-calls) for how to configure it.
48+
- **ASYNC210**: Sync HTTP call in async function, use `httpx.AsyncClient`. This and the other ASYNC21x checks look for usage of `urllib3` and `httpx.Client`, and recommend using `httpx.AsyncClient` as that's the largest http client supporting anyio/trio.
5049
- **ASYNC211**: Likely sync HTTP call in async function, use `httpx.AsyncClient`. Looks for `urllib3` method calls on pool objects, but only matching on the method signature and not the object.
5150
- **ASYNC212**: Blocking sync HTTP call on httpx object, use httpx.AsyncClient.
52-
- **ASYNC220**: Sync process call in async function, use `await nursery.start(trio.run_process, ...)`.
53-
- **ASYNC221**: Sync process call in async function, use `await trio.run_process(...)`.
54-
- **ASYNC222**: Sync `os.*` call in async function, wrap in `await trio.to_thread.run_sync()`.
55-
- **ASYNC230**: Sync IO call in async function, use `trio.open_file(...)`.
56-
- **ASYNC231**: Sync IO call in async function, use `trio.wrap_file(...)`.
57-
- **ASYNC232**: Blocking sync call on file object, wrap the file object in `trio.wrap_file()` to get an async file object.
58-
- **ASYNC240**: Avoid using `os.path` in async functions, prefer using `trio.Path` objects.
51+
- **ASYNC220**: Sync process call in async function, use `await nursery.start([trio|anyio].run_process, ...)`.
52+
- **ASYNC221**: Sync process call in async function, use `await [trio|anyio].run_process(...)`.
53+
- **ASYNC222**: Sync `os.*` call in async function, wrap in `await [trio|anyio].to_thread.run_sync()`.
54+
- **ASYNC230**: Sync IO call in async function, use `[trio|anyio].open_file(...)`.
55+
- **ASYNC231**: Sync IO call in async function, use `[trio|anyio].wrap_file(...)`.
56+
- **ASYNC232**: Blocking sync call on file object, wrap the file object in `[trio|anyio].wrap_file()` to get an async file object.
57+
- **ASYNC240**: Avoid using `os.path` in async functions, prefer using `[trio|anyio].Path` objects.
5958

6059
### Warnings disabled by default
61-
- **ASYNC900**: Async generator without `@asynccontextmanager` not allowed.
62-
- **ASYNC910**: Exit or `return` from async function with no guaranteed checkpoint or exception since function definition.
60+
- **ASYNC900**: Async generator without `@asynccontextmanager` not allowed. You might want to enable this on a codebase since async generators are inherently unsafe and cleanup logic might not be performed. See https://github.com/python-trio/flake8-async/issues/211 and https://discuss.python.org/t/using-exceptiongroup-at-anthropic-experience-report/20888/6 for discussion.
61+
- **ASYNC910**: Exit or `return` from async function with no guaranteed checkpoint or exception since function definition. You might want to enable this on a codebase to make it easier to reason about checkpoints, and make the logic of ASYNC911 correct.
6362
- **ASYNC911**: Exit, `yield` or `return` from async iterable with no guaranteed checkpoint since possible function entry (yield or function definition)
6463
Checkpoints are `await`, `async for`, and `async with` (on one of enter/exit).
6564

6665
### Removed Warnings
66+
- **TRIOxxx**: All error codes are now renamed ASYNCxxx
6767
- **TRIO107**: Renamed to TRIO910
6868
- **TRIO108**: Renamed to TRIO911
6969
- **TRIO117**: Don't raise or catch `trio.[NonBase]MultiError`, prefer `[exceptiongroup.]BaseExceptionGroup`. `MultiError` was removed in trio==0.24.0.

flake8_async/visitors/helpers.py

+3
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,9 @@ def has_exception(node: ast.expr) -> str | None:
237237
"trio.Cancelled",
238238
"anyio.get_cancelled_exc_class()",
239239
"get_cancelled_exc_class()",
240+
"asyncio.exceptions.CancelledError",
241+
"exceptions.CancelledError",
242+
"CancelledError",
240243
):
241244
return name
242245
return None

flake8_async/visitors/visitor103_104.py

+32-2
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,30 @@
2222
_suggestion_dict: dict[tuple[str, ...], str] = {
2323
("anyio",): "anyio.get_cancelled_exc_class()",
2424
("trio",): "trio.Cancelled",
25+
("asyncio",): "asyncio.exceptions.CancelledError",
2526
}
26-
_suggestion_dict[("anyio", "trio")] = "[" + "|".join(_suggestion_dict.values()) + "]"
27+
# TODO: ugly
28+
for a, b in (("anyio", "trio"), ("anyio", "asyncio"), ("asyncio", "trio")):
29+
_suggestion_dict[(a, b)] = (
30+
"[" + "|".join((_suggestion_dict[(a,)], _suggestion_dict[(b,)])) + "]"
31+
)
32+
_suggestion_dict[
33+
(
34+
"anyio",
35+
"asyncio",
36+
"trio",
37+
)
38+
] = (
39+
"["
40+
+ "|".join(
41+
(
42+
_suggestion_dict[("anyio",)],
43+
_suggestion_dict[("asyncio",)],
44+
_suggestion_dict[("trio",)],
45+
)
46+
)
47+
+ "]"
48+
)
2749

2850
_error_codes = {
2951
"ASYNC103": _async103_common_msg,
@@ -56,6 +78,7 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler):
5678
marker = critical_except(node)
5779

5880
if marker is None:
81+
# not a critical exception handler
5982
return
6083

6184
# If previous excepts have handled trio.Cancelled, don't do anything - namely
@@ -69,14 +92,21 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler):
6992
):
7093
error_code = "ASYNC103"
7194
self.cancelled_caught.add("anyio")
95+
elif marker.name in (
96+
"asyncio.exceptions.CancelledError",
97+
"exceptions.CancelledError",
98+
"CancelledError",
99+
):
100+
error_code = "ASYNC103"
101+
self.cancelled_caught.add("asyncio")
72102
else:
73103
if self.cancelled_caught:
74104
return
75105
if len(self.library) < 2:
76106
error_code = f"ASYNC103_{self.library_str}"
77107
else:
78108
error_code = f"ASYNC103_{'_'.join(sorted(self.library))}"
79-
self.cancelled_caught.update("trio", "anyio")
109+
self.cancelled_caught.update("trio", "anyio", "asyncio")
80110

81111
# Don't save the state of cancelled_caught, that's handled in Try and would
82112
# reset it between each except

flake8_async/visitors/visitor91x.py

+13-3
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ def copy(self):
9393

9494

9595
def checkpoint_statement(library: str) -> cst.SimpleStatementLine:
96+
# logic before this should stop code from wanting to insert the non-existing
97+
# asyncio.lowlevel.checkpoint
98+
assert library != "asyncio"
9699
return cst.SimpleStatementLine(
97100
[cst.Expr(cst.parse_expression(f"await {library}.lowlevel.checkpoint()"))]
98101
)
@@ -111,6 +114,7 @@ def __init__(self):
111114
self.noautofix: bool = False
112115
self.add_statement: cst.SimpleStatementLine | None = None
113116

117+
# used for inserting import if there's none
114118
self.explicitly_imported_library: dict[str, bool] = {
115119
"trio": False,
116120
"anyio": False,
@@ -250,8 +254,12 @@ def __init__(self, *args: Any, **kwargs: Any):
250254
self.try_state = TryState()
251255

252256
def should_autofix(self, node: cst.CSTNode, code: str | None = None) -> bool:
253-
return not self.noautofix and super().should_autofix(
254-
node, "ASYNC911" if self.has_yield else "ASYNC910"
257+
return (
258+
not self.noautofix
259+
and super().should_autofix(
260+
node, "ASYNC911" if self.has_yield else "ASYNC910"
261+
)
262+
and self.library != ("asyncio",)
255263
)
256264

257265
def checkpoint_statement(self) -> cst.SimpleStatementLine:
@@ -359,7 +367,9 @@ def leave_Return(
359367
) -> cst.Return:
360368
if not self.async_function:
361369
return updated_node
362-
if self.check_function_exit(original_node):
370+
if self.check_function_exit(original_node) and self.should_autofix(
371+
original_node
372+
):
363373
self.add_statement = self.checkpoint_statement()
364374
# avoid duplicate error messages
365375
self.uncheckpointed_statements = set()

flake8_async/visitors/visitor_utility.py

+7-1
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,17 @@ def __init__(self, *args: Any, **kwargs: Any):
136136
# see imports
137137
if self.options.anyio:
138138
self.add_library("anyio")
139+
if self.options.asyncio:
140+
self.add_library("asyncio")
139141

140142
def visit_Import(self, node: cst.Import):
141143
for alias in node.names:
142144
if m.matches(
143-
alias, m.ImportAlias(name=m.Name("trio") | m.Name("anyio"), asname=None)
145+
alias,
146+
m.ImportAlias(
147+
name=m.Name("trio") | m.Name("anyio") | m.Name("asyncio"),
148+
asname=None,
149+
),
144150
):
145151
assert isinstance(alias.name.value, str)
146152
self.add_library(alias.name.value)

tests/eval_files/anyio_trio.py

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
# ARG --enable=ASYNC220
33
# NOTRIO
44
# NOASYNCIO
5+
# set base library so trio doesn't get replaced when running with anyio
56
# BASE_LIBRARY anyio
67

78
# anyio eval will automatically prepend this test with `--anyio`

tests/eval_files/async102.py

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# type: ignore
22
# NOASYNCIO
3+
# asyncio has different mechanisms for shielded scopes, so would raise additional errors in this file.
34
from contextlib import asynccontextmanager
45

56
import trio

tests/eval_files/async102_anyio.py

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
# type: ignore
2+
# NOTRIO
3+
# NOASYNCIO
4+
# BASE_LIBRARY anyio
5+
# this test will raise the same errors with trio/asyncio, despite [trio|asyncio].get_cancelled_exc_class not existing
6+
# marked not to run the tests though as error messages will only refer to anyio
27
import anyio
38
from anyio import get_cancelled_exc_class
49

5-
# this one is fine to also run with trio
6-
710

811
async def foo(): ...
912

tests/eval_files/async102_asyncio.py

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# type: ignore
2+
# NOANYIO
3+
# NOTRIO
4+
# BASE_LIBRARY asyncio
5+
from contextlib import asynccontextmanager
6+
7+
import asyncio
8+
9+
10+
async def foo():
11+
# asyncio.move_on_after does not exist, so this will raise an error
12+
try:
13+
...
14+
finally:
15+
with asyncio.move_on_after(deadline=30) as s:
16+
s.shield = True
17+
await foo() # error: 12, Statement("try/finally", lineno-5)
18+
19+
try:
20+
pass
21+
finally:
22+
await foo() # error: 8, Statement("try/finally", lineno-3)
23+
24+
# asyncio.CancelScope does not exist, so this will raise an error
25+
try:
26+
pass
27+
finally:
28+
with asyncio.CancelScope(deadline=30, shield=True):
29+
await foo() # error: 12, Statement("try/finally", lineno-4)
30+
31+
# TODO: I think this is the asyncio-equivalent, but functionality to ignore the error
32+
# has not been implemented
33+
34+
try:
35+
...
36+
finally:
37+
await asyncio.shield( # error: 8, Statement("try/finally", lineno-3)
38+
asyncio.wait_for(foo())
39+
)
+79
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
# NOASYNCIO
2+
# NOANYIO - don't run it with substitutions
3+
import anyio
4+
import trio
5+
import asyncio
6+
from asyncio.exceptions import CancelledError
7+
from asyncio import exceptions
8+
9+
try:
10+
...
11+
except trio.Cancelled: # ASYNC103: 7, "trio.Cancelled"
12+
...
13+
except (
14+
anyio.get_cancelled_exc_class() # ASYNC103: 4, "anyio.get_cancelled_exc_class()"
15+
):
16+
...
17+
except CancelledError: # ASYNC103: 7, "CancelledError"
18+
...
19+
except: # safe
20+
...
21+
22+
# reordered
23+
try:
24+
...
25+
except (
26+
asyncio.exceptions.CancelledError # ASYNC103: 4, "asyncio.exceptions.CancelledError"
27+
):
28+
...
29+
except (
30+
anyio.get_cancelled_exc_class() # ASYNC103: 4, "anyio.get_cancelled_exc_class()"
31+
):
32+
...
33+
except trio.Cancelled: # ASYNC103: 7, "trio.Cancelled"
34+
...
35+
except: # safe
36+
...
37+
38+
# asyncio supports all three ways of importing asyncio.exceptions.CancelledError
39+
try:
40+
...
41+
except exceptions.CancelledError: # ASYNC103: 7, "exceptions.CancelledError"
42+
...
43+
44+
# catching any one of the exceptions in multi-library files will suppress errors on the bare except. It's unlikely a try block contains code that can raise multiple ones.
45+
try:
46+
...
47+
except (
48+
anyio.get_cancelled_exc_class() # ASYNC103: 4, "anyio.get_cancelled_exc_class()"
49+
):
50+
...
51+
except: # safe ?
52+
...
53+
54+
try:
55+
...
56+
except trio.Cancelled: # ASYNC103: 7, "trio.Cancelled"
57+
...
58+
except: # safe ?
59+
...
60+
61+
try:
62+
...
63+
except (
64+
asyncio.exceptions.CancelledError # ASYNC103: 4, "asyncio.exceptions.CancelledError"
65+
):
66+
...
67+
except: # safe ?
68+
...
69+
70+
# Check we get the proper suggestion when all are imported
71+
try:
72+
...
73+
except BaseException: # ASYNC103_anyio_asyncio_trio: 7, "BaseException"
74+
...
75+
76+
try:
77+
...
78+
except: # ASYNC103_anyio_asyncio_trio: 0, "bare except"
79+
...

tests/eval_files/async118.py

+3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1+
# NOTRIO
2+
# NOASYNCIO
13
# This raises the same errors on trio/asyncio, which is a bit silly, but inconsequential
4+
# marked not to run the tests though as error messages will only refer to anyio
25
from typing import Any
36

47
import anyio

tests/test_config_and_args.py

+14-3
Original file line numberDiff line numberDiff line change
@@ -174,13 +174,24 @@ def test_anyio_from_config(tmp_path: Path, capsys: pytest.CaptureFixture[str]):
174174
"subprocess.Popen",
175175
"[anyio|trio]",
176176
)
177-
err_file = str(Path(__file__).parent / "eval_files" / "anyio_trio.py")
178-
expected = f"{err_file}:12:5: ASYNC220 {err_msg}\n"
177+
err_file = Path(__file__).parent / "eval_files" / "anyio_trio.py"
178+
179+
# find the line with the expected error
180+
for i, line in enumerate(err_file.read_text().split("\n")):
181+
if "# ASYNC220: " in line:
182+
# line numbers start at 1, enumerate starts at 0
183+
lineno = i + 1
184+
break
185+
else:
186+
raise AssertionError("could not find error in file")
187+
188+
# construct the full error message
189+
expected = f"{err_file}:{lineno}:5: ASYNC220 {err_msg}\n"
179190
from flake8.main.cli import main
180191

181192
returnvalue = main(
182193
argv=[
183-
err_file,
194+
str(err_file),
184195
"--config",
185196
str(tmp_path / ".flake8"),
186197
]

0 commit comments

Comments
 (0)