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 2 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/>`_

25.5.2
======
- 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
2 changes: 1 addition & 1 deletion docs/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ adding the following to your ``.pre-commit-config.yaml``:
minimum_pre_commit_version: '2.9.0'
repos:
- repo: https://github.com/python-trio/flake8-async
rev: 25.5.1
rev: 25.5.2
hooks:
- id: flake8-async
# args: ["--enable=ASYNC100,ASYNC112", "--disable=", "--autofix=ASYNC"]
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__ = "25.5.1"
__version__ = "25.5.2"


# taken from https://github.com/Zac-HD/shed
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
96 changes: 96 additions & 0 deletions flake8_async/visitors/visitor4xx.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
"""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_notes",
)
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a test that set(dir(ExceptionGroup)).issubset(EXCGROUP_ATTRS), so that if a new method or attribute is added (e.g. by PEP-785 😅) we'll notice as soon as we test on that version of Python, rather than when users complain.

Copy link
Member Author

Choose a reason for hiding this comment

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

oopie, that caught a typo. Turns out it's add_note and not add_notes ^^;



@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_notes

# 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"
1 change: 1 addition & 0 deletions tests/test_flake8_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ def _parse_eval_file(
"ASYNC123",
"ASYNC125",
"ASYNC300",
"ASYNC400",
"ASYNC912",
}

Expand Down
Loading