add async400 exceptiongroup-invalid-access (#379)

Author: jakkdl

docs/changelog.rst

@@ -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.
@@ -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
 ======
@@ -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
 =======

docs/rules.rst

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

flake8_async/visitors/__init__.py

@@ -29,6 +29,7 @@
 # This has to be done at the end to avoid circular imports
 from . import (
     visitor2xx,
+    visitor4xx,
     visitor91x,
     visitor101,
     visitor102_120,

flake8_async/visitors/visitor4xx.py

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

tests/eval_files/async400_py311.py

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

tests/test_flake8_async.py

@@ -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
@@ -512,6 +516,7 @@ def _parse_eval_file(
     "ASYNC123",
     "ASYNC125",
     "ASYNC300",
+    "ASYNC400",
     "ASYNC912",
 }
 
@@ -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)