From e9b804874c616c81f3a0498a65c0ede27ae2966c Mon Sep 17 00:00:00 2001 From: jakkdl Date: Tue, 6 May 2025 11:38:45 +0200 Subject: [PATCH 1/3] add async400 exceptiongroup-invalid-access --- docs/changelog.rst | 8 ++- docs/rules.rst | 5 ++ docs/usage.rst | 2 +- flake8_async/__init__.py | 2 +- flake8_async/visitors/__init__.py | 1 + flake8_async/visitors/visitor4xx.py | 96 +++++++++++++++++++++++++++++ tests/eval_files/async400_py311.py | 84 +++++++++++++++++++++++++ tests/test_flake8_async.py | 1 + 8 files changed, 195 insertions(+), 4 deletions(-) create mode 100644 flake8_async/visitors/visitor4xx.py create mode 100644 tests/eval_files/async400_py311.py diff --git a/docs/changelog.rst b/docs/changelog.rst index 2e3cc355..8cd6cd23 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,10 @@ Changelog `CalVer, YY.month.patch `_ +25.5.2 +====== +- 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 405249a2..db198f6e 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/docs/usage.rst b/docs/usage.rst index 407ffbaf..a3101c5b 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -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"] diff --git a/flake8_async/__init__.py b/flake8_async/__init__.py index ff251269..15583f6a 100644 --- a/flake8_async/__init__.py +++ b/flake8_async/__init__.py @@ -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 diff --git a/flake8_async/visitors/__init__.py b/flake8_async/visitors/__init__.py index 652e8850..a97345bc 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 00000000..eb8ce94c --- /dev/null +++ b/flake8_async/visitors/visitor4xx.py @@ -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", +) + + +@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 00000000..d33359cc --- /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_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" diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index adb0d32e..15233270 100644 --- a/tests/test_flake8_async.py +++ b/tests/test_flake8_async.py @@ -512,6 +512,7 @@ def _parse_eval_file( "ASYNC123", "ASYNC125", "ASYNC300", + "ASYNC400", "ASYNC912", } From 373a8602d4729e985a54eea21b861db0eb9e2687 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Tue, 6 May 2025 12:00:11 +0200 Subject: [PATCH 2/3] add period at the end of error message --- flake8_async/visitors/visitor4xx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flake8_async/visitors/visitor4xx.py b/flake8_async/visitors/visitor4xx.py index eb8ce94c..3b3bd49d 100644 --- a/flake8_async/visitors/visitor4xx.py +++ b/flake8_async/visitors/visitor4xx.py @@ -33,7 +33,7 @@ class Visitor4xx(Flake8AsyncVisitor): error_codes: Mapping[str, str] = { "ASYNC400": ( - "Accessing attribute {} on ExceptionGroup as if it was a bare Exception" + "Accessing attribute {} on ExceptionGroup as if it was a bare Exception." ) } From 2e5e9526a501443d86c5127c4a7c271699df3c5e Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 7 May 2025 12:35:35 +0200 Subject: [PATCH 3/3] add test, fix add_notes->add_note, add _is_protocol --- docs/changelog.rst | 2 +- docs/usage.rst | 2 +- flake8_async/__init__.py | 2 +- flake8_async/visitors/visitor4xx.py | 4 +++- tests/eval_files/async400_py311.py | 2 +- tests/test_flake8_async.py | 11 +++++++++++ 6 files changed, 18 insertions(+), 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 8cd6cd23..95b08755 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,7 +4,7 @@ Changelog `CalVer, YY.month.patch `_ -25.5.2 +FUTURE ====== - Add :ref:`ASYNC400 ` except-star-invalid-attribute. diff --git a/docs/usage.rst b/docs/usage.rst index a3101c5b..407ffbaf 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -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.2 + rev: 25.5.1 hooks: - id: flake8-async # args: ["--enable=ASYNC100,ASYNC112", "--disable=", "--autofix=ASYNC"] diff --git a/flake8_async/__init__.py b/flake8_async/__init__.py index 15583f6a..ff251269 100644 --- a/flake8_async/__init__.py +++ b/flake8_async/__init__.py @@ -38,7 +38,7 @@ # CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1" -__version__ = "25.5.2" +__version__ = "25.5.1" # taken from https://github.com/Zac-HD/shed diff --git a/flake8_async/visitors/visitor4xx.py b/flake8_async/visitors/visitor4xx.py index 3b3bd49d..602b9c12 100644 --- a/flake8_async/visitors/visitor4xx.py +++ b/flake8_async/visitors/visitor4xx.py @@ -24,7 +24,9 @@ # from BaseException "args", "with_traceback", - "add_notes", + "add_note", + # in the backport + "_is_protocol", ) diff --git a/tests/eval_files/async400_py311.py b/tests/eval_files/async400_py311.py index d33359cc..f2323983 100644 --- a/tests/eval_files/async400_py311.py +++ b/tests/eval_files/async400_py311.py @@ -15,7 +15,7 @@ # from BaseException e.args e.with_traceback - e.add_notes + e.add_note # ignore anything that looks like a dunder e.__foo__ diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index 15233270..771f59d3 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 @@ -846,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)