Skip to content

Commit 051204c

Browse files
jakkdlZac-HD
andauthored
initial implementation of ASYNC123 (#303)
* initial implementation of ASYNC123 * add docs, version * fix code coverage * split out py311+-specific async123 test code to not crash CI runs on previous python versions * Update docs/rules.rst Co-authored-by: Zac Hatfield-Dodds <[email protected]> * Update docs/rules.rst --------- Co-authored-by: Zac Hatfield-Dodds <[email protected]>
1 parent ab022b1 commit 051204c

9 files changed

+278
-1
lines changed

.pre-commit-config.yaml

+2
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ repos:
4040
rev: v1.11.2
4141
hooks:
4242
- id: mypy
43+
# uses py311 syntax, mypy configured for py39
44+
exclude: tests/eval_files/.*_py311.py
4345

4446
- repo: https://github.com/RobertCraigie/pyright-python
4547
rev: v1.1.384

docs/changelog.rst

+4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ Changelog
44

55
`CalVer, YY.month.patch <https://calver.org/>`_
66

7+
24.10.1
8+
=======
9+
- Add :ref:`ASYNC123 <async123>` bad-exception-group-flattening
10+
711
24.9.5
812
======
913
- Fix crash when analyzing code with infinite loop inside context manager.

docs/rules.rst

+5
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@ _`ASYNC121`: control-flow-in-taskgroup
8989
_`ASYNC122`: delayed-entry-of-relative-cancelscope
9090
:func:`trio.move_on_after`, :func:`trio.fail_after`, :func:`anyio.move_on_after` and :func:`anyio.fail_after` behaves unintuitively if initialization and entry are separated, with the timeout starting on initialization. Trio>=0.27 changes this behaviour, so if you don't support older versions you should disable this check. See `Trio issue #2512 <https://github.com/python-trio/trio/issues/2512>`_.
9191

92+
_`ASYNC123`: bad-exception-group-flattening
93+
Raising one of the exceptions contained in an exception group will mutate it, replacing the original ``.__context__`` with the group, and erasing the ``.__traceback__``.
94+
Dropping this information makes diagnosing errors much more difficult.
95+
We recommend ``raise SomeNewError(...) from group`` if possible; or consider using `copy.copy` to shallow-copy the exception before re-raising (for copyable types), or re-raising the error from outside the `except` block.
96+
9297
Blocking sync calls in async functions
9398
======================================
9499

flake8_async/__init__.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838

3939

4040
# CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
41-
__version__ = "24.9.5"
41+
__version__ = "24.10.1"
4242

4343

4444
# taken from https://github.com/Zac-HD/shed

flake8_async/visitors/__init__.py

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
visitor105,
3737
visitor111,
3838
visitor118,
39+
visitor123,
3940
visitor_utility,
4041
visitors,
4142
)

flake8_async/visitors/visitor123.py

+111
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
"""foo."""
2+
3+
from __future__ import annotations
4+
5+
import ast
6+
from typing import TYPE_CHECKING, Any
7+
8+
from .flake8asyncvisitor import Flake8AsyncVisitor
9+
from .helpers import error_class
10+
11+
if TYPE_CHECKING:
12+
from collections.abc import Mapping
13+
14+
15+
@error_class
16+
class Visitor123(Flake8AsyncVisitor):
17+
error_codes: Mapping[str, str] = {
18+
"ASYNC123": (
19+
"Raising a child exception of an exception group loses"
20+
" context, cause, and/or traceback of the exception inside the group."
21+
)
22+
}
23+
24+
def __init__(self, *args: Any, **kwargs: Any):
25+
super().__init__(*args, **kwargs)
26+
self.try_star = False
27+
self.exception_group_names: set[str] = set()
28+
self.child_exception_list_names: set[str] = set()
29+
self.child_exception_names: set[str] = set()
30+
31+
def _is_exception_group(self, node: ast.expr) -> bool:
32+
return (
33+
(isinstance(node, ast.Name) and node.id in self.exception_group_names)
34+
or (
35+
# a child exception might be an ExceptionGroup
36+
self._is_child_exception(node)
37+
)
38+
or (
39+
isinstance(node, ast.Call)
40+
and isinstance(node.func, ast.Attribute)
41+
and self._is_exception_group(node.func.value)
42+
and node.func.attr in ("subgroup", "split")
43+
)
44+
)
45+
46+
def _is_exception_list(self, node: ast.expr | None) -> bool:
47+
return (
48+
isinstance(node, ast.Name) and node.id in self.child_exception_list_names
49+
) or (
50+
isinstance(node, ast.Attribute)
51+
and node.attr == "exceptions"
52+
and self._is_exception_group(node.value)
53+
)
54+
55+
def _is_child_exception(self, node: ast.expr | None) -> bool:
56+
return (
57+
isinstance(node, ast.Name) and node.id in self.child_exception_names
58+
) or (isinstance(node, ast.Subscript) and self._is_exception_list(node.value))
59+
60+
def visit_Raise(self, node: ast.Raise):
61+
if self._is_child_exception(node.exc):
62+
self.error(node)
63+
64+
def visit_ExceptHandler(self, node: ast.ExceptHandler):
65+
self.save_state(
66+
node,
67+
"exception_group_names",
68+
"child_exception_list_names",
69+
"child_exception_names",
70+
copy=True,
71+
)
72+
if node.name is None or (
73+
not self.try_star
74+
and (node.type is None or "ExceptionGroup" not in ast.unparse(node.type))
75+
):
76+
self.novisit = True
77+
return
78+
self.exception_group_names = {node.name}
79+
80+
# ast.TryStar added in py311
81+
# we run strict codecov on all python versions, this one doesn't run on <py311
82+
def visit_TryStar(self, node: ast.TryStar): # type: ignore[name-defined] # pragma: no cover
83+
self.save_state(node, "try_star", copy=False)
84+
self.try_star = True
85+
86+
def visit_Assign(self, node: ast.Assign | ast.AnnAssign):
87+
if node.value is None or not self.exception_group_names:
88+
return
89+
targets = (node.target,) if isinstance(node, ast.AnnAssign) else node.targets
90+
if self._is_child_exception(node.value):
91+
# not normally possible to assign single exception to multiple targets
92+
if len(targets) == 1 and isinstance(targets[0], ast.Name):
93+
self.child_exception_names.add(targets[0].id)
94+
elif self._is_exception_list(node.value):
95+
if len(targets) == 1 and isinstance(targets[0], ast.Name):
96+
self.child_exception_list_names.add(targets[0].id)
97+
# unpacking tuples and Starred and shit. Not implemented
98+
elif self._is_exception_group(node.value):
99+
for target in targets:
100+
if isinstance(target, ast.Name):
101+
self.exception_group_names.add(target.id)
102+
elif isinstance(target, ast.Tuple):
103+
for t in target.elts:
104+
if isinstance(t, ast.Name):
105+
self.exception_group_names.add(t.id)
106+
107+
visit_AnnAssign = visit_Assign
108+
109+
def visit_For(self, node: ast.For):
110+
if self._is_exception_list(node.iter) and isinstance(node.target, ast.Name):
111+
self.child_exception_names.add(node.target.id)

tests/eval_files/async123.py

+149
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
import copy
2+
import sys
3+
from typing import Any
4+
5+
if sys.version_info < (3, 11):
6+
from exceptiongroup import BaseExceptionGroup, ExceptionGroup
7+
8+
9+
def condition() -> bool:
10+
return True
11+
12+
13+
def any_fun(arg: Exception) -> Exception:
14+
return arg
15+
16+
17+
try:
18+
...
19+
except ExceptionGroup as e:
20+
if condition():
21+
raise e.exceptions[0] # error: 8
22+
elif condition():
23+
raise copy.copy(e.exceptions[0]) # safe
24+
elif condition():
25+
raise copy.deepcopy(e.exceptions[0]) # safe
26+
else:
27+
raise any_fun(e.exceptions[0]) # safe
28+
try:
29+
...
30+
except BaseExceptionGroup as e:
31+
raise e.exceptions[0] # error: 4
32+
try:
33+
...
34+
except ExceptionGroup as e:
35+
my_e = e.exceptions[0]
36+
raise my_e # error: 4
37+
try:
38+
...
39+
except ExceptionGroup as e:
40+
excs = e.exceptions
41+
my_e = excs[0]
42+
raise my_e # error: 4
43+
try:
44+
...
45+
except ExceptionGroup as e:
46+
excs_2 = e.subgroup(bool)
47+
if excs_2:
48+
raise excs_2.exceptions[0] # error: 8
49+
try:
50+
...
51+
except ExceptionGroup as e:
52+
excs_1, excs_2 = e.split(bool)
53+
if excs_1:
54+
raise excs_1.exceptions[0] # error: 8
55+
if excs_2:
56+
raise excs_2.exceptions[0] # error: 8
57+
58+
try:
59+
...
60+
except ExceptionGroup as e:
61+
f = e
62+
raise f.exceptions[0] # error: 4
63+
try:
64+
...
65+
except ExceptionGroup as e:
66+
excs = e.exceptions
67+
excs2 = excs
68+
raise excs2[0] # error: 4
69+
try:
70+
...
71+
except ExceptionGroup as e:
72+
my_exc = e.exceptions[0]
73+
my_exc2 = my_exc
74+
raise my_exc2 # error: 4
75+
76+
try:
77+
...
78+
except ExceptionGroup as e:
79+
raise e.exceptions[0].exceptions[0] # error: 4
80+
try:
81+
...
82+
except ExceptionGroup as e:
83+
excs = e.exceptions
84+
for exc in excs:
85+
if ...:
86+
raise exc # error: 12
87+
raise
88+
try:
89+
...
90+
except ExceptionGroup as e:
91+
ff: ExceptionGroup[Exception] = e
92+
raise ff.exceptions[0] # error: 4
93+
try:
94+
...
95+
except ExceptionGroup as e:
96+
raise e.subgroup(bool).exceptions[0] # type: ignore # error: 4
97+
98+
# not implemented
99+
try:
100+
...
101+
except ExceptionGroup as e:
102+
a, *b = e.exceptions
103+
raise a
104+
105+
# not implemented
106+
try:
107+
...
108+
except ExceptionGroup as e:
109+
x: Any = object()
110+
x.y = e
111+
raise x.y.exceptions[0]
112+
113+
# coverage
114+
try:
115+
...
116+
except ExceptionGroup:
117+
...
118+
119+
# not implemented
120+
try:
121+
...
122+
except ExceptionGroup as e:
123+
(a, *b), (c, *d) = e.split(bool)
124+
if condition():
125+
raise a
126+
if condition():
127+
raise b[0]
128+
if condition():
129+
raise c
130+
if condition():
131+
raise d[0]
132+
133+
# coverage (skip irrelevant assignments)
134+
x = 0
135+
136+
# coverage (ignore multiple targets when assign target is child exception)
137+
try:
138+
...
139+
except ExceptionGroup as e:
140+
exc = e.exceptions[0]
141+
b, c = exc
142+
if condition():
143+
raise b # not handled, and probably shouldn't raise
144+
else:
145+
raise c # same
146+
147+
# coverage (skip irrelevant loop)
148+
for x in range(5):
149+
...

tests/eval_files/async123_py311.py

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
try:
2+
...
3+
except* Exception as e:
4+
raise e.exceptions[0] # error: 4

tests/test_flake8_async.py

+1
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,7 @@ def _parse_eval_file(
482482
# doesn't check for it
483483
"ASYNC121",
484484
"ASYNC122",
485+
"ASYNC123",
485486
"ASYNC300",
486487
"ASYNC912",
487488
}

0 commit comments

Comments
 (0)