Skip to content

add async400 exceptiongroup-invalid-access #379

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

Merged
merged 3 commits into from
May 7, 2025
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
8 changes: 6 additions & 2 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ Changelog

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

FUTURE
======
- Add :ref:`ASYNC400 <async400>` except-star-invalid-attribute.

25.5.1
======
- Fixed :ref:`ASYNC113 <async113>` false alarms if the ``start_soon`` calls are in a nursery cm that was closed before the yield point.
Expand All @@ -19,7 +23,7 @@ Changelog

25.4.2
======
- Add :ref:`ASYNC125 <async125>` constant-absolute-deadline
- Add :ref:`ASYNC125 <async125>` constant-absolute-deadline.

25.4.1
======
Expand All @@ -31,7 +35,7 @@ Changelog

25.2.3
=======
- No longer require ``flake8`` for installation... so if you require support for config files you must install ``flake8-async[flake8]``
- No longer require ``flake8`` for installation... so if you require support for config files you must install ``flake8-async[flake8]``.

25.2.2
=======
Expand Down
5 changes: 5 additions & 0 deletions docs/rules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ _`ASYNC300` : create-task-no-reference
Note that this rule won't check whether the variable the result is saved in is susceptible to being garbage-collected itself. See the asyncio documentation for best practices.
You might consider instead using a :ref:`TaskGroup <taskgroup_nursery>` and calling :meth:`asyncio.TaskGroup.create_task` to avoid this problem, and gain the advantages of structured concurrency with e.g. better cancellation semantics.

ExceptionGroup rules
====================

_`ASYNC400` : except-star-invalid-attribute
When converting a codebase to use `except* <except_star>` it's easy to miss that the caught exception(s) are wrapped in a group, so accessing attributes on the caught exception must now check the contained exceptions. This checks for any attribute access on a caught ``except*`` that's not a known valid attribute on `ExceptionGroup`. This can be safely disabled on a type-checked or coverage-covered code base.

Optional rules disabled by default
==================================
Expand Down
1 change: 1 addition & 0 deletions flake8_async/visitors/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
# This has to be done at the end to avoid circular imports
from . import (
visitor2xx,
visitor4xx,
visitor91x,
visitor101,
visitor102_120,
Expand Down
98 changes: 98 additions & 0 deletions flake8_async/visitors/visitor4xx.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
"""4XX error classes, which handle exception groups.

ASYNC400 except-star-invalid-attribute checks for invalid attribute access on except*
"""

from __future__ import annotations

import ast
from typing import TYPE_CHECKING, Any

from .flake8asyncvisitor import Flake8AsyncVisitor
from .helpers import error_class

if TYPE_CHECKING:
from collections.abc import Mapping

EXCGROUP_ATTRS = (
# from ExceptionGroup
"message",
"exceptions",
"subgroup",
"split",
"derive",
# from BaseException
"args",
"with_traceback",
"add_note",
# in the backport
"_is_protocol",
)


@error_class
class Visitor4xx(Flake8AsyncVisitor):

error_codes: Mapping[str, str] = {
"ASYNC400": (
"Accessing attribute {} on ExceptionGroup as if it was a bare Exception."
)
}

def __init__(self, *args: Any, **kwargs: Any):
super().__init__(*args, **kwargs)
self.exception_groups: list[str] = []
self.trystar = False

def visit_TryStar(self, node: ast.TryStar): # type: ignore[name-defined]
self.save_state(node, "trystar")
self.trystar = True

def visit_Try(self, node: ast.Try):
self.save_state(node, "trystar")
self.trystar = False

def visit_ExceptHandler(self, node: ast.ExceptHandler):
if not self.trystar or node.name is None:
return
self.save_state(node, "exception_groups", copy=True)
self.exception_groups.append(node.name)
self.visit_nodes(node.body)

def visit_Attribute(self, node: ast.Attribute):
if (
isinstance(node.value, ast.Name)
and node.value.id in self.exception_groups
and node.attr not in EXCGROUP_ATTRS
and not (node.attr.startswith("__") and node.attr.endswith("__"))
):
self.error(node, node.attr)

def _clear_if_name(self, node: ast.AST | None):
if isinstance(node, ast.Name) and node.id in self.exception_groups:
self.exception_groups.remove(node.id)

def _walk_and_clear(self, node: ast.AST | None):
if node is None:
return
for n in ast.walk(node):
self._clear_if_name(n)

def visit_Assign(self, node: ast.Assign):
for t in node.targets:
self._walk_and_clear(t)

def visit_AnnAssign(self, node: ast.AnnAssign):
self._clear_if_name(node.target)

def visit_withitem(self, node: ast.withitem):
self._walk_and_clear(node.optional_vars)

def visit_FunctionDef(
self, node: ast.FunctionDef | ast.AsyncFunctionDef | ast.Lambda
):
self.save_state(node, "exception_groups", "trystar", copy=False)
self.exception_groups = []

visit_AsyncFunctionDef = visit_FunctionDef
visit_Lambda = visit_FunctionDef
84 changes: 84 additions & 0 deletions tests/eval_files/async400_py311.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
try:
...
except* ValueError as e:
e.anything # error: 4, "anything"
e.foo() # error: 4, "foo"
e.bar.zee # error: 4, "bar"

# from ExceptionGroup
e.message
e.exceptions
e.subgroup
e.split
e.derive

# from BaseException
e.args
e.with_traceback
e.add_note

# ignore anything that looks like a dunder
e.__foo__
e.__bar__

e.anything # safe

# assigning to the variable clears it
try:
...
except* ValueError as e:
e = e.exceptions[0]
e.ignore # safe
except* ValueError as e:
e, f = 1, 2
e.anything # safe
except* TypeError as e:
(e, f) = (1, 2)
e.anything # safe
except* ValueError as e:
with blah as e:
e.anything
e.anything
except* ValueError as e:
e: int = 1
e.real
except* ValueError as e:
with blah as (e, f):
e.anything

# check state saving
try:
...
except* ValueError as e:
...
except* BaseException:
e.error # safe

try:
...
except* ValueError as e:
try:
...
except* TypeError as e:
...
e.anything # error: 4, "anything"

try:
...
except* ValueError as e:

def foo():
# possibly problematic, but we minimize false alarms
e.anything

e.anything # error: 4, "anything"

def foo(e):
# this one is more clear it should be treated as safe
e.anything

e.anything # error: 4, "anything"

lambda e: e.anything

e.anything # error: 4, "anything"
12 changes: 12 additions & 0 deletions tests/test_flake8_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
from flake8_async import Plugin
from flake8_async.base import Error, Statement
from flake8_async.visitors import ERROR_CLASSES, ERROR_CLASSES_CST
from flake8_async.visitors.visitor4xx import EXCGROUP_ATTRS

if sys.version_info < (3, 11):
from exceptiongroup import ExceptionGroup

if TYPE_CHECKING:
from collections.abc import Iterable, Sequence
Expand Down Expand Up @@ -512,6 +516,7 @@ def _parse_eval_file(
"ASYNC123",
"ASYNC125",
"ASYNC300",
"ASYNC400",
"ASYNC912",
}

Expand Down Expand Up @@ -845,6 +850,13 @@ async def foo():
assert not errors, "# false alarm:\n" + function_str


def test_async400_excgroup_attributes():
for attr in dir(ExceptionGroup):
if attr.startswith("__") and attr.endswith("__"):
continue
assert attr in EXCGROUP_ATTRS


# from https://docs.python.org/3/library/itertools.html#itertools-recipes
def consume(iterator: Iterable[Any]):
deque(iterator, maxlen=0)
Expand Down
Loading