diff --git a/docs/changelog.rst b/docs/changelog.rst index 2e3cc35..95b0875 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,10 @@ Changelog `CalVer, YY.month.patch `_ +FUTURE +====== +- Add :ref:`ASYNC400 ` except-star-invalid-attribute. + 25.5.1 ====== - Fixed :ref:`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 ` constant-absolute-deadline +- Add :ref:`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 ======= diff --git a/docs/rules.rst b/docs/rules.rst index 405249a..db198f6 100644 --- a/docs/rules.rst +++ b/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 ` 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* ` 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 ================================== diff --git a/flake8_async/visitors/__init__.py b/flake8_async/visitors/__init__.py index 652e885..a97345b 100644 --- a/flake8_async/visitors/__init__.py +++ b/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, diff --git a/flake8_async/visitors/visitor4xx.py b/flake8_async/visitors/visitor4xx.py new file mode 100644 index 0000000..602b9c1 --- /dev/null +++ b/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 diff --git a/tests/eval_files/async400_py311.py b/tests/eval_files/async400_py311.py new file mode 100644 index 0000000..f232398 --- /dev/null +++ b/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" diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index adb0d32..771f59d 100644 --- a/tests/test_flake8_async.py +++ b/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)