From 92b16f38fb67ba5a424afcb8db0de1129a7184c1 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 5 Mar 2025 17:27:51 +0100 Subject: [PATCH 1/3] add except* support to 102, 103, 104, 120, 910, 911 & 912 --- .pre-commit-config.yaml | 2 +- docs/changelog.rst | 4 + docs/usage.rst | 2 +- flake8_async/__init__.py | 2 +- flake8_async/visitors/visitor102_120.py | 4 +- flake8_async/visitors/visitor103_104.py | 4 +- flake8_async/visitors/visitor91x.py | 34 +++++--- tests/autofix_files/async91x_py311.py | 97 ++++++++++++++++++++++ tests/autofix_files/async91x_py311.py.diff | 33 ++++++++ tests/eval_files/async102_120_py311.py | 29 +++++++ tests/eval_files/async103_104_py311.py | 40 +++++++++ tests/eval_files/async912_py311.py | 16 ++++ tests/eval_files/async91x_py311.py | 92 ++++++++++++++++++++ tests/test_flake8_async.py | 4 + 14 files changed, 348 insertions(+), 15 deletions(-) create mode 100644 tests/autofix_files/async91x_py311.py create mode 100644 tests/autofix_files/async91x_py311.py.diff create mode 100644 tests/eval_files/async102_120_py311.py create mode 100644 tests/eval_files/async103_104_py311.py create mode 100644 tests/eval_files/async912_py311.py create mode 100644 tests/eval_files/async91x_py311.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a9ba3959..aa52c57e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -42,7 +42,7 @@ repos: hooks: - id: mypy # uses py311 syntax, mypy configured for py39 - exclude: tests/eval_files/.*_py311.py + exclude: tests/(eval|autofix)_files/.*_py311.py - repo: https://github.com/RobertCraigie/pyright-python rev: v1.1.396 diff --git a/docs/changelog.rst b/docs/changelog.rst index 86ea4ed7..682b5dba 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,10 @@ Changelog `CalVer, YY.month.patch `_ +25.3.1 +====== +- Add except* support to ASYNC102, 103, 104, 120, 910, 911, 912. + 25.2.3 ======= - No longer require ``flake8`` for installation... so if you require support for config files you must install ``flake8-async[flake8]`` diff --git a/docs/usage.rst b/docs/usage.rst index e5b56eb3..950a01f8 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.2.3 + rev: 25.3.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 92af9654..f635c79d 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.2.3" +__version__ = "25.3.1" # taken from https://github.com/Zac-HD/shed diff --git a/flake8_async/visitors/visitor102_120.py b/flake8_async/visitors/visitor102_120.py index 61f1e32f..759161b1 100644 --- a/flake8_async/visitors/visitor102_120.py +++ b/flake8_async/visitors/visitor102_120.py @@ -149,7 +149,7 @@ def visit_AsyncWith(self, node: ast.AsyncWith): break self.visit_With(node) - def visit_Try(self, node: ast.Try): + def visit_Try(self, node: ast.Try | ast.TryStar): # type: ignore[name-defined] self.save_state( node, "_critical_scope", "_trio_context_managers", "cancelled_caught" ) @@ -165,6 +165,8 @@ def visit_Try(self, node: ast.Try): self._critical_scope = Statement("try/finally", node.lineno, node.col_offset) self.visit_nodes(node.finalbody) + visit_TryStar = visit_Try + def visit_ExceptHandler(self, node: ast.ExceptHandler): # if we're inside a critical scope, a nested except should never override that if self._critical_scope is not None and self._critical_scope.name != "except": diff --git a/flake8_async/visitors/visitor103_104.py b/flake8_async/visitors/visitor103_104.py index 951b0888..502fa730 100644 --- a/flake8_async/visitors/visitor103_104.py +++ b/flake8_async/visitors/visitor103_104.py @@ -160,7 +160,7 @@ def visit_Return(self, node: ast.Return | ast.Yield): visit_Yield = visit_Return # Treat Try's as fully covering only if `finally` always raises. - def visit_Try(self, node: ast.Try): + def visit_Try(self, node: ast.Try | ast.TryStar): # type: ignore[name-defined] self.save_state(node, "cancelled_caught", copy=True) self.cancelled_caught = set() @@ -179,6 +179,8 @@ def visit_Try(self, node: ast.Try): # but it's fine if we raise in finally self.visit_nodes(node.finalbody) + visit_TryStar = visit_Try + # Treat if's as fully covering if both `if` and `else` raise. # `elif` is parsed by the ast as a new if statement inside the else. def visit_If(self, node: ast.If): diff --git a/flake8_async/visitors/visitor91x.py b/flake8_async/visitors/visitor91x.py index 6414160a..adee79aa 100644 --- a/flake8_async/visitors/visitor91x.py +++ b/flake8_async/visitors/visitor91x.py @@ -768,7 +768,7 @@ def leave_Yield( # try can jump into any except or into the finally* at any point during it's # execution so we need to make sure except & finally can handle worst-case # * unless there's a bare except / except BaseException - not implemented. - def visit_Try(self, node: cst.Try): + def visit_Try(self, node: cst.Try | cst.TryStar): if not self.async_function: return self.save_state(node, "try_state", copy=True) @@ -784,39 +784,41 @@ def visit_Try(self, node: cst.Try): Statement("yield", pos.line, pos.column) # type: ignore ) - def leave_Try_body(self, node: cst.Try): + def leave_Try_body(self, node: cst.Try | cst.TryStar): # save state at end of try for entering else self.try_state.try_checkpoint = self.uncheckpointed_statements # check that all except handlers checkpoint (await or most likely raise) self.try_state.except_uncheckpointed_statements = set() - def visit_ExceptHandler(self, node: cst.ExceptHandler): + def visit_ExceptHandler(self, node: cst.ExceptHandler | cst.ExceptStarHandler): # enter with worst case of try self.uncheckpointed_statements = ( self.try_state.body_uncheckpointed_statements.copy() ) def leave_ExceptHandler( - self, original_node: cst.ExceptHandler, updated_node: cst.ExceptHandler - ) -> cst.ExceptHandler: + self, + original_node: cst.ExceptHandler | cst.ExceptStarHandler, + updated_node: cst.ExceptHandler | cst.ExceptStarHandler, + ) -> Any: # not worth creating a TypeVar to handle correctly self.try_state.except_uncheckpointed_statements.update( self.uncheckpointed_statements ) return updated_node - def visit_Try_orelse(self, node: cst.Try): + def visit_Try_orelse(self, node: cst.Try | cst.TryStar): # check else # if else runs it's after all of try, so restore state to back then self.uncheckpointed_statements = self.try_state.try_checkpoint - def leave_Try_orelse(self, node: cst.Try): + def leave_Try_orelse(self, node: cst.Try | cst.TryStar): # checkpoint if else checkpoints, and all excepts checkpoint self.uncheckpointed_statements.update( self.try_state.except_uncheckpointed_statements ) - def visit_Try_finalbody(self, node: cst.Try): + def visit_Try_finalbody(self, node: cst.Try | cst.TryStar): if node.finalbody: self.try_state.added = ( self.try_state.body_uncheckpointed_statements.difference( @@ -835,14 +837,26 @@ def visit_Try_finalbody(self, node: cst.Try): ): self.uncheckpointed_statements.update(self.try_state.added) - def leave_Try_finalbody(self, node: cst.Try): + def leave_Try_finalbody(self, node: cst.Try | cst.TryStar): if node.finalbody: self.uncheckpointed_statements.difference_update(self.try_state.added) - def leave_Try(self, original_node: cst.Try, updated_node: cst.Try) -> cst.Try: + def leave_Try( + self, original_node: cst.Try | cst.TryStar, updated_node: cst.Try | cst.TryStar + ) -> cst.Try | cst.TryStar: self.restore_state(original_node) return updated_node + visit_TryStar = visit_Try + leave_TryStar = leave_Try + leave_TryStar_body = leave_Try_body + visit_TryStar_orelse = visit_Try_orelse + leave_TryStar_orelse = leave_Try_orelse + visit_TryStar_finalbody = visit_Try_finalbody + leave_TryStar_finalbody = leave_Try_finalbody + visit_ExceptStarHandler = visit_ExceptHandler + leave_ExceptStarHandler = leave_ExceptHandler + def leave_If_test(self, node: cst.If | cst.IfExp) -> None: if not self.async_function: return diff --git a/tests/autofix_files/async91x_py311.py b/tests/autofix_files/async91x_py311.py new file mode 100644 index 00000000..fad6e2e3 --- /dev/null +++ b/tests/autofix_files/async91x_py311.py @@ -0,0 +1,97 @@ +"""Test for ASYNC91x rules with except* blocks. + +ASYNC910: async-function-no-checkpoint +ASYNC911: async-generator-no-checkpoint +ASYNC913: indefinite-loop-no-guaranteed-checkpoint + +async912 handled in separate file +""" + +# ARG --enable=ASYNC910,ASYNC911,ASYNC913 +# AUTOFIX +# ASYNCIO_NO_AUTOFIX +import trio + + +async def foo(): ... + + +async def foo_try_except_star_1(): # ASYNC910: 0, "exit", Statement("function definition", lineno) + try: + await foo() + except* ValueError: + ... + except* RuntimeError: + raise + else: + await foo() + await trio.lowlevel.checkpoint() + + +async def foo_try_except_star_2(): # safe + try: + ... + except* ValueError: + ... + finally: + await foo() + + +async def foo_try_except_star_3(): # safe + try: + await foo() + except* ValueError: + raise + + +# Multiple except* handlers - should all guarantee checkpoint +async def foo_try_except_star_4(): + try: + await foo() + except* ValueError: + raise + except* TypeError: + raise + except* Exception: + raise + + +async def try_else_no_raise_in_except(): # ASYNC910: 0, "exit", Statement("function definition", lineno) + try: + ... + except* ValueError: + ... + else: + await foo() + await trio.lowlevel.checkpoint() + + +async def try_else_raise_in_except(): + try: + ... + except* ValueError: + raise + else: + await foo() + + +async def check_async911(): # ASYNC911: 0, "exit", Statement("yield", lineno+7) + try: + await foo() + except* ValueError: + ... + except* RuntimeError: + raise + await trio.lowlevel.checkpoint() + yield # ASYNC911: 4, "yield", Statement("function definition", lineno-7) + await trio.lowlevel.checkpoint() + + +async def check_async913(): + while True: # ASYNC913: 4 + await trio.lowlevel.checkpoint() + try: + await foo() + except* ValueError: + # Missing checkpoint + ... diff --git a/tests/autofix_files/async91x_py311.py.diff b/tests/autofix_files/async91x_py311.py.diff new file mode 100644 index 00000000..d45305bc --- /dev/null +++ b/tests/autofix_files/async91x_py311.py.diff @@ -0,0 +1,33 @@ +--- ++++ +@@ x,6 x,7 @@ + raise + else: + await foo() ++ await trio.lowlevel.checkpoint() + + + async def foo_try_except_star_2(): # safe +@@ x,6 x,7 @@ + ... + else: + await foo() ++ await trio.lowlevel.checkpoint() + + + async def try_else_raise_in_except(): +@@ x,11 x,14 @@ + ... + except* RuntimeError: + raise ++ await trio.lowlevel.checkpoint() + yield # ASYNC911: 4, "yield", Statement("function definition", lineno-7) ++ await trio.lowlevel.checkpoint() + + + async def check_async913(): + while True: # ASYNC913: 4 ++ await trio.lowlevel.checkpoint() + try: + await foo() + except* ValueError: diff --git a/tests/eval_files/async102_120_py311.py b/tests/eval_files/async102_120_py311.py new file mode 100644 index 00000000..f9b91d1a --- /dev/null +++ b/tests/eval_files/async102_120_py311.py @@ -0,0 +1,29 @@ +"""Test for ASYNC102/ASYNC120 with except* + +ASYNC102: await-in-finally-or-cancelled + +ASYNC120: await-in-except +""" + +# type: ignore +# ARG --enable=ASYNC102,ASYNC120 +# NOASYNCIO # TODO: support asyncio shields +import trio + + +async def foo(): + try: + ... + except* ValueError: + await foo() # ASYNC120: 8, Statement("except", lineno-1) + raise + except* BaseException: + await foo() # ASYNC102: 8, Statement("BaseException", lineno-1) + finally: + await foo() # ASYNC102: 8, Statement("try/finally", lineno-8) + + try: + ... + except* BaseException: + with trio.move_on_after(30, shield=True): + await foo() diff --git a/tests/eval_files/async103_104_py311.py b/tests/eval_files/async103_104_py311.py new file mode 100644 index 00000000..6477592a --- /dev/null +++ b/tests/eval_files/async103_104_py311.py @@ -0,0 +1,40 @@ +"""Test for ASYNC103/ASYNC104 with except* blocks. + +ASYNC103: no-reraise-cancelled +ASYNC104: cancelled-not-raised +""" + +# ARG --enable=ASYNC103,ASYNC104 + +try: + ... +except* BaseException: # ASYNC103_trio: 8, "BaseException" + ... + +try: + ... +except* BaseException: + raise + +try: + ... +except* ValueError: + ... +except* BaseException: # ASYNC103_trio: 8, "BaseException" + ... + +try: + ... +except* BaseException: + raise ValueError # ASYNC104: 4 + + +def foo(): + try: + ... + except* BaseException: # ASYNC103_trio: 12, "BaseException" + return # ASYNC104: 8 + try: + ... + except* BaseException: + raise ValueError # ASYNC104: 8 diff --git a/tests/eval_files/async912_py311.py b/tests/eval_files/async912_py311.py new file mode 100644 index 00000000..520bcc76 --- /dev/null +++ b/tests/eval_files/async912_py311.py @@ -0,0 +1,16 @@ +# ASYNC912 can't be tested with the other 91x rules since there's no universal +# cancelscope name across trio/asyncio/anyio - so we need ASYNCIO_NO_ERROR + + +# ASYNCIO_NO_ERROR +async def foo(): ... + + +async def check_async912(): + with trio.move_on_after(30): # ASYNC912: 9 + try: + await foo() + except* ValueError: + # Missing checkpoint + ... + await foo() diff --git a/tests/eval_files/async91x_py311.py b/tests/eval_files/async91x_py311.py new file mode 100644 index 00000000..078675c2 --- /dev/null +++ b/tests/eval_files/async91x_py311.py @@ -0,0 +1,92 @@ +"""Test for ASYNC91x rules with except* blocks. + +ASYNC910: async-function-no-checkpoint +ASYNC911: async-generator-no-checkpoint +ASYNC913: indefinite-loop-no-guaranteed-checkpoint + +async912 handled in separate file +""" + +# ARG --enable=ASYNC910,ASYNC911,ASYNC913 +# AUTOFIX +# ASYNCIO_NO_AUTOFIX +import trio + + +async def foo(): ... + + +async def foo_try_except_star_1(): # ASYNC910: 0, "exit", Statement("function definition", lineno) + try: + await foo() + except* ValueError: + ... + except* RuntimeError: + raise + else: + await foo() + + +async def foo_try_except_star_2(): # safe + try: + ... + except* ValueError: + ... + finally: + await foo() + + +async def foo_try_except_star_3(): # safe + try: + await foo() + except* ValueError: + raise + + +# Multiple except* handlers - should all guarantee checkpoint +async def foo_try_except_star_4(): + try: + await foo() + except* ValueError: + raise + except* TypeError: + raise + except* Exception: + raise + + +async def try_else_no_raise_in_except(): # ASYNC910: 0, "exit", Statement("function definition", lineno) + try: + ... + except* ValueError: + ... + else: + await foo() + + +async def try_else_raise_in_except(): + try: + ... + except* ValueError: + raise + else: + await foo() + + +async def check_async911(): # ASYNC911: 0, "exit", Statement("yield", lineno+7) + try: + await foo() + except* ValueError: + ... + except* RuntimeError: + raise + yield # ASYNC911: 4, "yield", Statement("function definition", lineno-7) + + +async def check_async913(): + while True: # ASYNC913: 4 + try: + await foo() + except* ValueError: + # Missing checkpoint + ... diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index 02863b66..c9085de2 100644 --- a/tests/test_flake8_async.py +++ b/tests/test_flake8_async.py @@ -422,6 +422,10 @@ def _parse_eval_file( if not line or line[0] == "#": continue + # skip lines that *don't* have a comment + if "#" not in line: + continue + # get text between `error:` and (end of line or another comment) k = re.findall(r"(error|ASYNC...)(_.*)?:([^#]*)(?=#|$)", line) From 2ee0d44f8701ba9fa8ee134b628ce600098a5e02 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 5 Mar 2025 17:49:04 +0100 Subject: [PATCH 2/3] skip noerror_on_sync_code if python version is too low to parse the code --- tests/test_flake8_async.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index c9085de2..cf0c995c 100644 --- a/tests/test_flake8_async.py +++ b/tests/test_flake8_async.py @@ -543,6 +543,7 @@ def visit_AsyncFor(self, node: ast.AsyncFor): def test_noerror_on_sync_code(test: str, path: Path): if any(e in test for e in error_codes_ignored_when_checking_transformed_sync_code): return + check_version(test) with tokenize.open(path) as f: source = f.read() tree = SyncTransformer().visit(ast.parse(source)) From 465e13d40014005c6b4d3e4dd12129985bac87ba Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 6 Mar 2025 10:48:32 +0100 Subject: [PATCH 3/3] update comment after review --- tests/autofix_files/async91x_py311.py | 4 ++-- tests/eval_files/async91x_py311.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/autofix_files/async91x_py311.py b/tests/autofix_files/async91x_py311.py index fad6e2e3..a5889644 100644 --- a/tests/autofix_files/async91x_py311.py +++ b/tests/autofix_files/async91x_py311.py @@ -44,12 +44,12 @@ async def foo_try_except_star_3(): # safe raise -# Multiple except* handlers - should all guarantee checkpoint +# Multiple except* handlers - should all guarantee checkpoint/raise async def foo_try_except_star_4(): try: await foo() except* ValueError: - raise + await foo() except* TypeError: raise except* Exception: diff --git a/tests/eval_files/async91x_py311.py b/tests/eval_files/async91x_py311.py index 078675c2..97c0e35d 100644 --- a/tests/eval_files/async91x_py311.py +++ b/tests/eval_files/async91x_py311.py @@ -43,12 +43,12 @@ async def foo_try_except_star_3(): # safe raise -# Multiple except* handlers - should all guarantee checkpoint +# Multiple except* handlers - should all guarantee checkpoint/raise async def foo_try_except_star_4(): try: await foo() except* ValueError: - raise + await foo() except* TypeError: raise except* Exception: