From d9078544e041d90b900d5c764390b4563e751ca5 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 26 Feb 2024 17:10:45 +0100 Subject: [PATCH 1/8] initial support for asyncio. 106 and 9xx are working --- README.md | 2 +- flake8_async/__init__.py | 14 ++++ flake8_async/base.py | 1 + flake8_async/visitors/visitor_utility.py | 4 +- flake8_async/visitors/visitors.py | 6 +- tests/autofix_files/async100.py | 1 + .../autofix_files/async100_simple_autofix.py | 1 + tests/autofix_files/noqa.py | 1 + tests/eval_files/anyio_trio.py | 2 + tests/eval_files/async100.py | 1 + tests/eval_files/async100_noautofix.py | 1 + tests/eval_files/async100_simple_autofix.py | 1 + tests/eval_files/async101.py | 1 + tests/eval_files/async102.py | 1 + tests/eval_files/async102_anyio.py | 3 +- tests/eval_files/async102_trio.py | 1 + tests/eval_files/async103.py | 1 + tests/eval_files/async103_both_imported.py | 1 + tests/eval_files/async103_no_104.py | 1 + tests/eval_files/async103_trio.py | 1 + tests/eval_files/async104.py | 1 + tests/eval_files/async104_anyio.py | 1 + tests/eval_files/async104_trio.py | 1 + tests/eval_files/async105.py | 1 + tests/eval_files/async105_anyio.py | 2 + tests/eval_files/async109.py | 1 + tests/eval_files/async110.py | 1 + tests/eval_files/async111.py | 1 + tests/eval_files/async112.py | 1 + tests/eval_files/async113.py | 1 + tests/eval_files/async113_trio.py | 1 + tests/eval_files/async114.py | 1 + tests/eval_files/async115.py | 1 + tests/eval_files/async116.py | 1 + tests/eval_files/async118.py | 1 + tests/eval_files/async200.py | 1 + tests/eval_files/async210.py | 1 + tests/eval_files/async211.py | 1 + tests/eval_files/async212.py | 1 + tests/eval_files/async22x.py | 1 + tests/eval_files/async232.py | 1 + tests/eval_files/async23x.py | 1 + tests/eval_files/async240.py | 1 + tests/eval_files/async900.py | 1 + tests/eval_files/no_library.py | 1 + tests/eval_files/noqa.py | 1 + tests/eval_files/trio_anyio.py | 1 + tests/test_config_and_args.py | 2 +- tests/test_flake8_async.py | 68 ++++++++++++------- 49 files changed, 114 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 1e58a5b5..b3ce8801 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ pip install flake8-async - **ASYNC103**: `except BaseException`, `except trio.Cancelled` or a bare `except:` with a code path that doesn't re-raise. If you don't want to re-raise `BaseException`, add a separate handler for `trio.Cancelled` before. - **ASYNC104**: `Cancelled` and `BaseException` must be re-raised - when a user tries to `return` or `raise` a different exception. - **ASYNC105**: Calling a trio async function without immediately `await`ing it. -- **ASYNC106**: `trio`/`anyio` must be imported with `import trio`/`import anyio` for the linter to work. +- **ASYNC106**: `trio`/`anyio`/`asyncio` must be imported with `import trio`/`import anyio`/`import asyncio` for the linter to work. - **ASYNC109**: Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead - **ASYNC110**: `while : await trio.sleep()` should be replaced by a `trio.Event`. - **ASYNC111**: Variable, from context manager opened inside nursery, passed to `start[_soon]` might be invalidly accessed while in use, due to context manager closing before the nursery. This is usually a bug, and nurseries should generally be the inner-most context manager. diff --git a/flake8_async/__init__.py b/flake8_async/__init__.py index 9bb13784..7d69d5b2 100644 --- a/flake8_async/__init__.py +++ b/flake8_async/__init__.py @@ -298,6 +298,19 @@ def add_options(option_manager: OptionManager | ArgumentParser): " suggestions with [anyio|trio]." ), ) + add_argument( + "--asyncio", + # action=store_true + parse_from_config does seem to work here, despite + # https://github.com/PyCQA/flake8/issues/1770 + action="store_true", + required=False, + default=False, + help=( + "Change the default library to be asyncio instead of trio." + " If anyio/trio is imported it will assume that is also available and" + " print suggestions with [asyncio|anyio/trio]." + ), + ) @staticmethod def parse_options(options: Namespace): @@ -342,6 +355,7 @@ def get_matching_codes( startable_in_context_manager=options.startable_in_context_manager, trio200_blocking_calls=options.trio200_blocking_calls, anyio=options.anyio, + asyncio=options.asyncio, disable_noqa=options.disable_noqa, ) diff --git a/flake8_async/base.py b/flake8_async/base.py index 4ebead4a..3f167d38 100644 --- a/flake8_async/base.py +++ b/flake8_async/base.py @@ -32,6 +32,7 @@ class Options: startable_in_context_manager: Collection[str] trio200_blocking_calls: dict[str, str] anyio: bool + asyncio: bool disable_noqa: bool diff --git a/flake8_async/visitors/visitor_utility.py b/flake8_async/visitors/visitor_utility.py index 58784a95..cd3a66cf 100644 --- a/flake8_async/visitors/visitor_utility.py +++ b/flake8_async/visitors/visitor_utility.py @@ -117,11 +117,13 @@ def __init__(self, *args: Any, **kwargs: Any): # see imports if self.options.anyio: self.add_library("anyio") + if self.options.asyncio: + self.add_library("anyio") def visit_Import(self, node: ast.Import): for alias in node.names: name = alias.name - if name in ("trio", "anyio") and alias.asname is None: + if name in ("trio", "anyio", "asyncio") and alias.asname is None: self.add_library(name) diff --git a/flake8_async/visitors/visitors.py b/flake8_async/visitors/visitors.py index 9293ab4b..259eb156 100644 --- a/flake8_async/visitors/visitors.py +++ b/flake8_async/visitors/visitors.py @@ -11,6 +11,8 @@ if TYPE_CHECKING: from collections.abc import Mapping +LIBRARIES = ("trio", "anyio", "asyncio") + @error_class class Visitor106(Flake8AsyncVisitor): @@ -19,12 +21,12 @@ class Visitor106(Flake8AsyncVisitor): } def visit_ImportFrom(self, node: ast.ImportFrom): - if node.module in ("trio", "anyio"): + if node.module in LIBRARIES: self.error(node, node.module) def visit_Import(self, node: ast.Import): for name in node.names: - if name.name in ("trio", "anyio") and name.asname is not None: + if name.name in LIBRARIES and name.asname is not None: self.error(node, name.name) diff --git a/tests/autofix_files/async100.py b/tests/autofix_files/async100.py index f899fe37..f15dd43e 100644 --- a/tests/autofix_files/async100.py +++ b/tests/autofix_files/async100.py @@ -1,5 +1,6 @@ # type: ignore # AUTOFIX +# NOASYNCIO import trio diff --git a/tests/autofix_files/async100_simple_autofix.py b/tests/autofix_files/async100_simple_autofix.py index 450e9c1d..71742344 100644 --- a/tests/autofix_files/async100_simple_autofix.py +++ b/tests/autofix_files/async100_simple_autofix.py @@ -1,3 +1,4 @@ +# NOASYNCIO # AUTOFIX import trio diff --git a/tests/autofix_files/noqa.py b/tests/autofix_files/noqa.py index 7d69ee07..a0ca87bc 100644 --- a/tests/autofix_files/noqa.py +++ b/tests/autofix_files/noqa.py @@ -1,5 +1,6 @@ # AUTOFIX # NOANYIO # TODO +# NOASYNCIO # ARG --enable=ASYNC100,ASYNC911 from typing import Any diff --git a/tests/eval_files/anyio_trio.py b/tests/eval_files/anyio_trio.py index 93e8dba6..2c48ab4c 100644 --- a/tests/eval_files/anyio_trio.py +++ b/tests/eval_files/anyio_trio.py @@ -1,6 +1,8 @@ # type: ignore # ARG --enable=ASYNC220 # NOTRIO +# NOASYNCIO +# BASE_LIBRARY anyio # anyio eval will automatically prepend this test with `--anyio` import trio # isort: skip diff --git a/tests/eval_files/async100.py b/tests/eval_files/async100.py index acf6e706..a9b39c67 100644 --- a/tests/eval_files/async100.py +++ b/tests/eval_files/async100.py @@ -1,5 +1,6 @@ # type: ignore # AUTOFIX +# NOASYNCIO import trio diff --git a/tests/eval_files/async100_noautofix.py b/tests/eval_files/async100_noautofix.py index f16d77b1..2f3624e7 100644 --- a/tests/eval_files/async100_noautofix.py +++ b/tests/eval_files/async100_noautofix.py @@ -1,3 +1,4 @@ +# NOASYNCIO import trio diff --git a/tests/eval_files/async100_simple_autofix.py b/tests/eval_files/async100_simple_autofix.py index 95286b2a..c783f71d 100644 --- a/tests/eval_files/async100_simple_autofix.py +++ b/tests/eval_files/async100_simple_autofix.py @@ -1,3 +1,4 @@ +# NOASYNCIO # AUTOFIX import trio diff --git a/tests/eval_files/async101.py b/tests/eval_files/async101.py index 9404807a..fb60825c 100644 --- a/tests/eval_files/async101.py +++ b/tests/eval_files/async101.py @@ -1,3 +1,4 @@ +# NOASYNCIO # type: ignore import contextlib import contextlib as bla diff --git a/tests/eval_files/async102.py b/tests/eval_files/async102.py index c891bdc9..53381832 100644 --- a/tests/eval_files/async102.py +++ b/tests/eval_files/async102.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO from contextlib import asynccontextmanager import trio diff --git a/tests/eval_files/async102_anyio.py b/tests/eval_files/async102_anyio.py index 61102257..8de353b2 100644 --- a/tests/eval_files/async102_anyio.py +++ b/tests/eval_files/async102_anyio.py @@ -1,8 +1,9 @@ +# NOASYNCIO # type: ignore import anyio from anyio import get_cancelled_exc_class -# this one is fine to also run with ASYNC +# this one is fine to also run with trio async def foo(): ... diff --git a/tests/eval_files/async102_trio.py b/tests/eval_files/async102_trio.py index 58d58a5b..a672bd22 100644 --- a/tests/eval_files/async102_trio.py +++ b/tests/eval_files/async102_trio.py @@ -1,3 +1,4 @@ +# NOASYNCIO # NOANYIO - since anyio.Cancelled does not exist import trio diff --git a/tests/eval_files/async103.py b/tests/eval_files/async103.py index 82bf8e1b..5b010bff 100644 --- a/tests/eval_files/async103.py +++ b/tests/eval_files/async103.py @@ -1,3 +1,4 @@ +# NOASYNCIO # ARG --enable=ASYNC103,ASYNC104 from typing import Any diff --git a/tests/eval_files/async103_both_imported.py b/tests/eval_files/async103_both_imported.py index 720fa811..64ea9476 100644 --- a/tests/eval_files/async103_both_imported.py +++ b/tests/eval_files/async103_both_imported.py @@ -1,3 +1,4 @@ +# NOASYNCIO # NOANYIO - don't run it with substitutions import anyio import trio diff --git a/tests/eval_files/async103_no_104.py b/tests/eval_files/async103_no_104.py index 4eb7e7e4..94d13372 100644 --- a/tests/eval_files/async103_no_104.py +++ b/tests/eval_files/async103_no_104.py @@ -1,3 +1,4 @@ +# NOASYNCIO # ARG --enable=ASYNC103 # check that partly disabling a visitor works from typing import Any diff --git a/tests/eval_files/async103_trio.py b/tests/eval_files/async103_trio.py index 82aad9ba..9ac4a2c0 100644 --- a/tests/eval_files/async103_trio.py +++ b/tests/eval_files/async103_trio.py @@ -1,4 +1,5 @@ # ARG --enable=ASYNC103,ASYNC104 +# NOASYNCIO # NOANYIO from typing import Any diff --git a/tests/eval_files/async104.py b/tests/eval_files/async104.py index 1701f26a..874fe15b 100644 --- a/tests/eval_files/async104.py +++ b/tests/eval_files/async104.py @@ -1,4 +1,5 @@ # ARG --enable=ASYNC103,ASYNC104 +# NOASYNCIO try: ... # raise different exception diff --git a/tests/eval_files/async104_anyio.py b/tests/eval_files/async104_anyio.py index 4e8101f3..d2b84d73 100644 --- a/tests/eval_files/async104_anyio.py +++ b/tests/eval_files/async104_anyio.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO import anyio try: diff --git a/tests/eval_files/async104_trio.py b/tests/eval_files/async104_trio.py index e81c0faf..487169f6 100644 --- a/tests/eval_files/async104_trio.py +++ b/tests/eval_files/async104_trio.py @@ -1,4 +1,5 @@ # NOANYIO +# NOASYNCIO import trio try: diff --git a/tests/eval_files/async105.py b/tests/eval_files/async105.py index 8e18caee..93123bb3 100644 --- a/tests/eval_files/async105.py +++ b/tests/eval_files/async105.py @@ -1,4 +1,5 @@ # NOANYIO +# NOASYNCIO from typing import Any from collections.abc import Coroutine diff --git a/tests/eval_files/async105_anyio.py b/tests/eval_files/async105_anyio.py index bba17e5a..334a25c0 100644 --- a/tests/eval_files/async105_anyio.py +++ b/tests/eval_files/async105_anyio.py @@ -1,4 +1,6 @@ # NOTRIO +# NOASYNCIO +# BASE_LIBRARY anyio import anyio diff --git a/tests/eval_files/async109.py b/tests/eval_files/async109.py index 139e2e5d..4c691836 100644 --- a/tests/eval_files/async109.py +++ b/tests/eval_files/async109.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO import trio import trio as anything diff --git a/tests/eval_files/async110.py b/tests/eval_files/async110.py index 2052f8da..ec65f54b 100644 --- a/tests/eval_files/async110.py +++ b/tests/eval_files/async110.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO import trio import trio as noerror diff --git a/tests/eval_files/async111.py b/tests/eval_files/async111.py index 40477102..7044c62a 100644 --- a/tests/eval_files/async111.py +++ b/tests/eval_files/async111.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO from typing import Any import trio diff --git a/tests/eval_files/async112.py b/tests/eval_files/async112.py index bb97bd21..f13b6ec5 100644 --- a/tests/eval_files/async112.py +++ b/tests/eval_files/async112.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO import functools from functools import partial diff --git a/tests/eval_files/async113.py b/tests/eval_files/async113.py index 998e9e39..d0920ab6 100644 --- a/tests/eval_files/async113.py +++ b/tests/eval_files/async113.py @@ -1,4 +1,5 @@ # mypy: disable-error-code="arg-type,attr-defined" +# NOASYNCIO from contextlib import asynccontextmanager import anyio diff --git a/tests/eval_files/async113_trio.py b/tests/eval_files/async113_trio.py index b03dda96..bf4891f6 100644 --- a/tests/eval_files/async113_trio.py +++ b/tests/eval_files/async113_trio.py @@ -1,4 +1,5 @@ # mypy: disable-error-code="arg-type,call-overload,misc" +# NOASYNCIO import contextlib import contextlib as arbitrary_import_alias_for_contextlib import functools diff --git a/tests/eval_files/async114.py b/tests/eval_files/async114.py index c4f7b5c7..30a6b871 100644 --- a/tests/eval_files/async114.py +++ b/tests/eval_files/async114.py @@ -1,3 +1,4 @@ +# NOASYNCIO import trio # ARG --startable-in-context-manager=foo diff --git a/tests/eval_files/async115.py b/tests/eval_files/async115.py index 63cabb02..0d747c80 100644 --- a/tests/eval_files/async115.py +++ b/tests/eval_files/async115.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO import time import trio diff --git a/tests/eval_files/async116.py b/tests/eval_files/async116.py index ab2f849e..48fac988 100644 --- a/tests/eval_files/async116.py +++ b/tests/eval_files/async116.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO import math from math import inf diff --git a/tests/eval_files/async118.py b/tests/eval_files/async118.py index eebc00a5..8dbf715a 100644 --- a/tests/eval_files/async118.py +++ b/tests/eval_files/async118.py @@ -1,3 +1,4 @@ +# NOASYNCIO from typing import Any import anyio diff --git a/tests/eval_files/async200.py b/tests/eval_files/async200.py index f9e1a947..f084711e 100644 --- a/tests/eval_files/async200.py +++ b/tests/eval_files/async200.py @@ -3,6 +3,7 @@ # Test spaces in options, and trailing comma # Cannot test newlines, since argparse splits on those if passed on the CLI # ARG --trio200-blocking-calls=bar -> BAR, bee-> SHOULD_NOT_BE_PRINTED,bonnet ->SHOULD_NOT_BE_PRINTED,bee.bonnet->BEEBONNET,*.postwild->POSTWILD,prewild.*->PREWILD,*.*.*->TRIPLEDOT, +# NOASYNCIO # don't error in sync function diff --git a/tests/eval_files/async210.py b/tests/eval_files/async210.py index e898e058..f98d246f 100644 --- a/tests/eval_files/async210.py +++ b/tests/eval_files/async210.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO import urllib import httpx diff --git a/tests/eval_files/async211.py b/tests/eval_files/async211.py index 1fa24e0a..9cd76e50 100644 --- a/tests/eval_files/async211.py +++ b/tests/eval_files/async211.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO from urllib3 import PoolManager diff --git a/tests/eval_files/async212.py b/tests/eval_files/async212.py index 1671c67f..6c215fd0 100644 --- a/tests/eval_files/async212.py +++ b/tests/eval_files/async212.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO import urllib3 diff --git a/tests/eval_files/async22x.py b/tests/eval_files/async22x.py index 40f39b09..b68d2fe8 100644 --- a/tests/eval_files/async22x.py +++ b/tests/eval_files/async22x.py @@ -1,5 +1,6 @@ # type: ignore # ARG --enable=ASYNC220,ASYNC221,ASYNC222 +# NOASYNCIO async def foo(): diff --git a/tests/eval_files/async232.py b/tests/eval_files/async232.py index 0fcc0d35..8c211ba6 100644 --- a/tests/eval_files/async232.py +++ b/tests/eval_files/async232.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO import io from io import BufferedRandom, BufferedReader, BufferedWriter, TextIOWrapper from typing import Any, Optional diff --git a/tests/eval_files/async23x.py b/tests/eval_files/async23x.py index e1ea7252..299dd3a1 100644 --- a/tests/eval_files/async23x.py +++ b/tests/eval_files/async23x.py @@ -1,5 +1,6 @@ # type: ignore # ARG --enable=ASYNC230,ASYNC231 +# NOASYNCIO import io import os diff --git a/tests/eval_files/async240.py b/tests/eval_files/async240.py index fbb781cd..3f86d507 100644 --- a/tests/eval_files/async240.py +++ b/tests/eval_files/async240.py @@ -1,4 +1,5 @@ # type: ignore +# NOASYNCIO import os.path from os.path import isfile, normpath, relpath diff --git a/tests/eval_files/async900.py b/tests/eval_files/async900.py index 235d211b..908c31e7 100644 --- a/tests/eval_files/async900.py +++ b/tests/eval_files/async900.py @@ -1,5 +1,6 @@ # type: ignore # ARG --no-checkpoint-warning-decorator=asynccontextmanager,other_context_manager +# NOASYNCIO from contextlib import asynccontextmanager diff --git a/tests/eval_files/no_library.py b/tests/eval_files/no_library.py index de648edc..7db92ba2 100644 --- a/tests/eval_files/no_library.py +++ b/tests/eval_files/no_library.py @@ -1,5 +1,6 @@ # type: ignore # ARG --enable=ASYNC220 # NOANYIO +# NOASYNCIO async def foo(): subprocess.Popen() # ASYNC220: 4, 'subprocess.Popen', "trio" diff --git a/tests/eval_files/noqa.py b/tests/eval_files/noqa.py index 4cde1a73..46585211 100644 --- a/tests/eval_files/noqa.py +++ b/tests/eval_files/noqa.py @@ -1,5 +1,6 @@ # AUTOFIX # NOANYIO # TODO +# NOASYNCIO # ARG --enable=ASYNC100,ASYNC911 from typing import Any diff --git a/tests/eval_files/trio_anyio.py b/tests/eval_files/trio_anyio.py index 2714e29d..25439f26 100644 --- a/tests/eval_files/trio_anyio.py +++ b/tests/eval_files/trio_anyio.py @@ -1,6 +1,7 @@ # type: ignore # ARG --enable=ASYNC220 # NOANYIO +# NOASYNCIO # TODO import trio # isort: skip import anyio # isort: skip diff --git a/tests/test_config_and_args.py b/tests/test_config_and_args.py index ba3841b3..7d3df935 100644 --- a/tests/test_config_and_args.py +++ b/tests/test_config_and_args.py @@ -175,7 +175,7 @@ def test_anyio_from_config(tmp_path: Path, capsys: pytest.CaptureFixture[str]): "[anyio|trio]", ) err_file = str(Path(__file__).parent / "eval_files" / "anyio_trio.py") - expected = f"{err_file}:10:5: ASYNC220 {err_msg}\n" + expected = f"{err_file}:12:5: ASYNC220 {err_msg}\n" from flake8.main.cli import main returnvalue = main( diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index 2b53bcee..c16dd55c 100644 --- a/tests/test_flake8_async.py +++ b/tests/test_flake8_async.py @@ -109,7 +109,8 @@ def check_autofix( plugin: Plugin, unfixed_code: str, generate_autofix: bool, - anyio: bool = False, + library: str = "trio", + base_library: str = "trio", ): # the source code after it's been visited by current transformers visited_code = plugin.module.code @@ -144,9 +145,13 @@ def check_autofix( # if running against anyio, since "eval_files/{test.py}" have replaced trio->anyio, # meaning it's replaced in visited_code, we also replace it in previous generated code # and in the previous diff - if anyio: - previous_autofixed = replace_library(previous_autofixed) - autofix_diff_content = replace_library(autofix_diff_content) + if base_library != library: + previous_autofixed = replace_library( + previous_autofixed, original=base_library, new=library + ) + autofix_diff_content = replace_library( + autofix_diff_content, original=base_library, new=library + ) # save any difference in the autofixed code diff = diff_strings(previous_autofixed, visited_code) @@ -160,7 +165,7 @@ def check_autofix( # if --generate-autofix is specified, which it may be during development, # just silently overwrite the content. - if generate_autofix and not anyio: + if generate_autofix and base_library == library: autofix_files[test].write_text(visited_code) autofix_diff_file.write_text(added_autofix_diff) return @@ -183,10 +188,16 @@ def check_autofix( # markers in the same pass as we parse out errors etc. @dataclass class MagicMarkers: + # Exclude checking a library against a file NOANYIO: bool = False NOTRIO: bool = False + NOASYNCIO: bool = False + # File should raise no errors with this library ANYIO_NO_ERROR: bool = False - ASYNC_NO_ERROR: bool = False + TRIO_NO_ERROR: bool = False + ASYNCIO_NO_ERROR: bool = False + # eval file is written using this library, so no substitution is required + BASE_LIBRARY: str = "trio" def find_magic_markers( @@ -204,31 +215,35 @@ def find_magic_markers( # when testing the same file @pytest.mark.parametrize(("test", "path"), test_files, ids=[f[0] for f in test_files]) @pytest.mark.parametrize("autofix", [False, True], ids=["noautofix", "autofix"]) -@pytest.mark.parametrize("anyio", [False, True], ids=["trio", "anyio"]) +@pytest.mark.parametrize("library", ["trio", "anyio", "asyncio"]) @pytest.mark.parametrize("noqa", [False, True], ids=["normal", "noqa"]) def test_eval( test: str, path: Path, autofix: bool, - anyio: bool, + library: str, noqa: bool, generate_autofix: bool, ): content = path.read_text() magic_markers = find_magic_markers(content) - if anyio and magic_markers.NOANYIO: - pytest.skip("file marked with NOANYIO") - # if autofixing, columns may get messed up ignore_column = autofix - if magic_markers.NOTRIO: - if not anyio: - pytest.skip("file marked with NOTRIO") + if library == "anyio" and magic_markers.NOANYIO: + pytest.skip("file marked with NOANYIO") + if library == "asyncio" and magic_markers.NOASYNCIO: + pytest.skip("file marked with NOANYIO") + if library == "trio" and magic_markers.NOTRIO: + pytest.skip("file marked with NOTRIO") + + if library == "asyncio" and autofix: + pytest.skip("no support for asyncio+autofix currently") - # if test is marked NOTRIO, it's not written to require substitution - elif anyio: - content = replace_library(content) + if library != magic_markers.BASE_LIBRARY: + content = replace_library( + content, original=magic_markers.BASE_LIBRARY, new=library + ) # if substituting we're messing up columns ignore_column = True @@ -238,13 +253,13 @@ def test_eval( content = re.sub(r"#[\s]*(error|ASYNC\d\d\d):.*", "# noqa", content) expected, parsed_args, enable = _parse_eval_file(test, content) - if anyio: - parsed_args.insert(0, "--anyio") + if library != "trio": + parsed_args.insert(0, f"--{library}") if autofix: parsed_args.append(f"--autofix={enable}") - if (anyio and magic_markers.ANYIO_NO_ERROR) or ( - not anyio and magic_markers.ASYNC_NO_ERROR + if (library == "anyio" and magic_markers.ANYIO_NO_ERROR) or ( + library == "trio" and magic_markers.TRIO_NO_ERROR ): expected = [] @@ -253,14 +268,21 @@ def test_eval( plugin, *expected, args=parsed_args, ignore_column=ignore_column ) - if anyio: + if library == "anyio": # check that error messages refer to 'anyio', or to neither library for error in errors: message = error.message.format(*error.args) assert "anyio" in message or "trio" not in message if autofix and not noqa: - check_autofix(test, plugin, content, generate_autofix, anyio=anyio) + check_autofix( + test, + plugin, + content, + generate_autofix, + library=library, + base_library=magic_markers.BASE_LIBRARY, + ) else: # make sure content isn't modified assert content == plugin.module.code From b6b022b78e6cb2c5dbbfe731c57002c08d6cab32 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 26 Feb 2024 17:28:12 +0100 Subject: [PATCH 2/8] 200, 21x, 900 work perfectly fine with asyncio --- tests/eval_files/async200.py | 1 - tests/eval_files/async210.py | 1 - tests/eval_files/async211.py | 1 - tests/eval_files/async212.py | 1 - tests/eval_files/async900.py | 1 - 5 files changed, 5 deletions(-) diff --git a/tests/eval_files/async200.py b/tests/eval_files/async200.py index f084711e..f9e1a947 100644 --- a/tests/eval_files/async200.py +++ b/tests/eval_files/async200.py @@ -3,7 +3,6 @@ # Test spaces in options, and trailing comma # Cannot test newlines, since argparse splits on those if passed on the CLI # ARG --trio200-blocking-calls=bar -> BAR, bee-> SHOULD_NOT_BE_PRINTED,bonnet ->SHOULD_NOT_BE_PRINTED,bee.bonnet->BEEBONNET,*.postwild->POSTWILD,prewild.*->PREWILD,*.*.*->TRIPLEDOT, -# NOASYNCIO # don't error in sync function diff --git a/tests/eval_files/async210.py b/tests/eval_files/async210.py index f98d246f..e898e058 100644 --- a/tests/eval_files/async210.py +++ b/tests/eval_files/async210.py @@ -1,5 +1,4 @@ # type: ignore -# NOASYNCIO import urllib import httpx diff --git a/tests/eval_files/async211.py b/tests/eval_files/async211.py index 9cd76e50..1fa24e0a 100644 --- a/tests/eval_files/async211.py +++ b/tests/eval_files/async211.py @@ -1,5 +1,4 @@ # type: ignore -# NOASYNCIO from urllib3 import PoolManager diff --git a/tests/eval_files/async212.py b/tests/eval_files/async212.py index 6c215fd0..1671c67f 100644 --- a/tests/eval_files/async212.py +++ b/tests/eval_files/async212.py @@ -1,5 +1,4 @@ # type: ignore -# NOASYNCIO import urllib3 diff --git a/tests/eval_files/async900.py b/tests/eval_files/async900.py index 908c31e7..235d211b 100644 --- a/tests/eval_files/async900.py +++ b/tests/eval_files/async900.py @@ -1,6 +1,5 @@ # type: ignore # ARG --no-checkpoint-warning-decorator=asynccontextmanager,other_context_manager -# NOASYNCIO from contextlib import asynccontextmanager From b640a1d8f6b9352e00fdce3d5048b248afe6244c Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 1 Mar 2024 11:33:35 +0100 Subject: [PATCH 3/8] update changelog. Removed #ASYNCIO from a bunch of files which are passing on asyncio, though not necessarily for good reasons --- CHANGELOG.md | 6 ++++++ tests/eval_files/anyio_trio.py | 2 +- tests/eval_files/async102_anyio.py | 1 - tests/eval_files/async104_anyio.py | 2 +- tests/eval_files/async105_anyio.py | 2 +- tests/eval_files/async114.py | 4 +++- tests/eval_files/async118.py | 2 +- 7 files changed, 13 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 811204a0..da23b5bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## Future - Removed TRIO117, MultiError removed in trio 0.24.0 +- Renamed the library from flake8-trio to flake8-async, to indicate the checker supports more than just `trio`. +- Renamed all error codes from TRIOxxx to ASYNCxxx +- Renamed the binary from flake8-trio to flake8-async +- Lots of internal renaming. +- Added asyncio support for ASYNC106 +- added `--library` ## 23.5.1 - TRIO91X now supports comprehensions diff --git a/tests/eval_files/anyio_trio.py b/tests/eval_files/anyio_trio.py index 2c48ab4c..4b6dbfcf 100644 --- a/tests/eval_files/anyio_trio.py +++ b/tests/eval_files/anyio_trio.py @@ -1,8 +1,8 @@ # type: ignore # ARG --enable=ASYNC220 # NOTRIO -# NOASYNCIO # BASE_LIBRARY anyio +# TODO: why does this pass with --asyncio # anyio eval will automatically prepend this test with `--anyio` import trio # isort: skip diff --git a/tests/eval_files/async102_anyio.py b/tests/eval_files/async102_anyio.py index 8de353b2..ea73c9c2 100644 --- a/tests/eval_files/async102_anyio.py +++ b/tests/eval_files/async102_anyio.py @@ -1,4 +1,3 @@ -# NOASYNCIO # type: ignore import anyio from anyio import get_cancelled_exc_class diff --git a/tests/eval_files/async104_anyio.py b/tests/eval_files/async104_anyio.py index d2b84d73..054b5160 100644 --- a/tests/eval_files/async104_anyio.py +++ b/tests/eval_files/async104_anyio.py @@ -1,5 +1,5 @@ # type: ignore -# NOASYNCIO +# BASE_LIBRARY ANYIO import anyio try: diff --git a/tests/eval_files/async105_anyio.py b/tests/eval_files/async105_anyio.py index 334a25c0..c51e0afd 100644 --- a/tests/eval_files/async105_anyio.py +++ b/tests/eval_files/async105_anyio.py @@ -1,6 +1,6 @@ # NOTRIO -# NOASYNCIO # BASE_LIBRARY anyio +# asyncio obv will not raise any errors on this file import anyio diff --git a/tests/eval_files/async114.py b/tests/eval_files/async114.py index 30a6b871..cecdb322 100644 --- a/tests/eval_files/async114.py +++ b/tests/eval_files/async114.py @@ -1,6 +1,8 @@ -# NOASYNCIO import trio +# async114 does not care about the imported library, so will raise errors regardless +# of trio/anyio/asyncio + # ARG --startable-in-context-manager=foo diff --git a/tests/eval_files/async118.py b/tests/eval_files/async118.py index 8dbf715a..1323a60b 100644 --- a/tests/eval_files/async118.py +++ b/tests/eval_files/async118.py @@ -1,4 +1,4 @@ -# NOASYNCIO +# This raises the same errors on trio/asyncio, which is a bit silly, but inconsequential from typing import Any import anyio From e97b5d3af8d74fc27742097db4a8f07483099128 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 1 Mar 2024 11:36:59 +0100 Subject: [PATCH 4/8] fix bug from review, which also resolves confusion with unexpected test passing --- flake8_async/visitors/visitor_utility.py | 2 +- tests/eval_files/anyio_trio.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/flake8_async/visitors/visitor_utility.py b/flake8_async/visitors/visitor_utility.py index cd3a66cf..220fc470 100644 --- a/flake8_async/visitors/visitor_utility.py +++ b/flake8_async/visitors/visitor_utility.py @@ -118,7 +118,7 @@ def __init__(self, *args: Any, **kwargs: Any): if self.options.anyio: self.add_library("anyio") if self.options.asyncio: - self.add_library("anyio") + self.add_library("asyncio") def visit_Import(self, node: ast.Import): for alias in node.names: diff --git a/tests/eval_files/anyio_trio.py b/tests/eval_files/anyio_trio.py index 4b6dbfcf..2c48ab4c 100644 --- a/tests/eval_files/anyio_trio.py +++ b/tests/eval_files/anyio_trio.py @@ -1,8 +1,8 @@ # type: ignore # ARG --enable=ASYNC220 # NOTRIO +# NOASYNCIO # BASE_LIBRARY anyio -# TODO: why does this pass with --asyncio # anyio eval will automatically prepend this test with `--anyio` import trio # isort: skip From ced45706284e9bf76bd8deb0afd35d992026d350 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 1 Mar 2024 16:45:25 +0100 Subject: [PATCH 5/8] update error descriptions in README to better reflect which errors work with which library, and minor updates. 102/103/104 now sees asyncio.exceptions.CancelledError as a critical exception. Fix import/arg detection of asyncio. add asyncio102_asyncio. add async103_all_imported. Finally make test_anyio_from_config autodetect the correct line number. Fix BASE_LIBRARY marker being interpreted as a bool. Make #NOTRIO/#NOASYNCIO/#NOANYIO run the visitor but ignore the result, instead of skipping, to check it doesn't crash. Generalize error-message-library-check. --- README.md | 46 ++++++------- flake8_async/visitors/helpers.py | 3 + flake8_async/visitors/visitor103_104.py | 34 +++++++++- flake8_async/visitors/visitor91x.py | 16 ++++- flake8_async/visitors/visitor_utility.py | 8 ++- tests/eval_files/anyio_trio.py | 1 + tests/eval_files/async102.py | 1 + tests/eval_files/async102_anyio.py | 7 +- tests/eval_files/async102_asyncio.py | 39 +++++++++++ tests/eval_files/async103_all_imported.py | 79 +++++++++++++++++++++++ tests/eval_files/async118.py | 3 + tests/test_config_and_args.py | 17 ++++- tests/test_flake8_async.py | 68 +++++++++++++------ 13 files changed, 270 insertions(+), 52 deletions(-) create mode 100644 tests/eval_files/async102_asyncio.py create mode 100644 tests/eval_files/async103_all_imported.py diff --git a/README.md b/README.md index b3ce8801..b6126a22 100644 --- a/README.md +++ b/README.md @@ -23,47 +23,47 @@ pip install flake8-async ``` ## List of warnings - -- **ASYNC100**: A `with trio.fail_after(...):` or `with trio.move_on_after(...):` +- **ASYNC100**: A `with [trio|anyio].fail_after(...):` or `with [trio|anyio].move_on_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. -- **ASYNC101**: `yield` inside a nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling. -- **ASYNC102**: It's unsafe to await inside `finally:` or `except BaseException/trio.Cancelled` unless you use a shielded - cancel scope with a timeout. -- **ASYNC103**: `except BaseException`, `except trio.Cancelled` or a bare `except:` with a code path that doesn't re-raise. If you don't want to re-raise `BaseException`, add a separate handler for `trio.Cancelled` before. -- **ASYNC104**: `Cancelled` and `BaseException` must be re-raised - when a user tries to `return` or `raise` a different exception. -- **ASYNC105**: Calling a trio async function without immediately `await`ing it. +- **ASYNC101**: `yield` inside a trio/anyio nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling. +- **ASYNC102**: It's unsafe to await inside `finally:` or `except BaseException/trio.Cancelled/anyio.get_cancelled_exc_class()/asyncio.exceptions.CancelledError` unless you use a shielded cancel scope with a timeout. This is currently not able to detect asyncio shields. +- **ASYNC103**: `except BaseException/trio.Cancelled/anyio.get_cancelled_exc_class()/asyncio.exceptions.CancelledError`, or a bare `except:` with a code path that doesn't re-raise. If you don't want to re-raise `BaseException`, add a separate handler for `trio.Cancelled`/`anyio.get_cancelled_exc_class()`/`asyncio.exceptions.CancelledError` before. +- **ASYNC104**: `trio.Cancelled`/`anyio.get_cancelled_exc_class()`/`asyncio.exceptions.CancelledError`/`BaseException` must be re-raised. The same as ASYNC103, except specifically triggered on `return` or a different exception being raised. +- **ASYNC105**: Calling a trio async function without immediately `await`ing it. This is only supported with trio functions, but you can get similar functionality with a type-checker. - **ASYNC106**: `trio`/`anyio`/`asyncio` must be imported with `import trio`/`import anyio`/`import asyncio` for the linter to work. -- **ASYNC109**: Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead -- **ASYNC110**: `while : await trio.sleep()` should be replaced by a `trio.Event`. +- **ASYNC109**: Async function definition with a `timeout` parameter - use `[trio/anyio].[fail/move_on]_[after/at]` instead. +- **ASYNC110**: `while : await [trio/anyio].sleep()` should be replaced by a `[trio|anyio].Event`. - **ASYNC111**: Variable, from context manager opened inside nursery, passed to `start[_soon]` might be invalidly accessed while in use, due to context manager closing before the nursery. This is usually a bug, and nurseries should generally be the inner-most context manager. - **ASYNC112**: Nursery body with only a call to `nursery.start[_soon]` and not passing itself as a parameter can be replaced with a regular function call. - **ASYNC113**: Using `nursery.start_soon` in `__aenter__` doesn't wait for the task to begin. Consider replacing with `nursery.start`. - **ASYNC114**: Startable function (i.e. has a `task_status` keyword parameter) not in `--startable-in-context-manager` parameter list, please add it so ASYNC113 can catch errors when using it. -- **ASYNC115**: Replace `trio.sleep(0)` with the more suggestive `trio.lowlevel.checkpoint()`. -- **ASYNC116**: `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`. +- **ASYNC115**: Replace `[trio|anyio].sleep(0)` with the more suggestive `[trio|anyio].lowlevel.checkpoint()`. +- **ASYNC116**: `[trio|anyio].sleep()` with >24 hour interval should usually be `[trio|anyio].sleep_forever()`. - **ASYNC118**: Don't assign the value of `anyio.get_cancelled_exc_class()` to a variable, since that breaks linter checks and multi-backend programs. ### Warnings for blocking sync calls in async functions -- **ASYNC200**: User-configured error for blocking sync calls in async functions. Does nothing by default, see [`trio200-blocking-calls`](#trio200-blocking-calls) for how to configure it. -- **ASYNC210**: Sync HTTP call in async function, use `httpx.AsyncClient`. +Note: 22X, 23X and 24X has not had asyncio-specific suggestions written. +- **ASYNC200**: User-configured error for blocking sync calls in async functions. Does nothing by default, see [`async200-blocking-calls`](#async200-blocking-calls) for how to configure it. +- **ASYNC210**: Sync HTTP call in async function, use `httpx.AsyncClient`. This and the other ASYNC21x checks look for usage of `urllib3` and `httpx.Client`, and recommend using `httpx.AsyncClient` as that's the largest http client supporting anyio/trio. - **ASYNC211**: Likely sync HTTP call in async function, use `httpx.AsyncClient`. Looks for `urllib3` method calls on pool objects, but only matching on the method signature and not the object. - **ASYNC212**: Blocking sync HTTP call on httpx object, use httpx.AsyncClient. -- **ASYNC220**: Sync process call in async function, use `await nursery.start(trio.run_process, ...)`. -- **ASYNC221**: Sync process call in async function, use `await trio.run_process(...)`. -- **ASYNC222**: Sync `os.*` call in async function, wrap in `await trio.to_thread.run_sync()`. -- **ASYNC230**: Sync IO call in async function, use `trio.open_file(...)`. -- **ASYNC231**: Sync IO call in async function, use `trio.wrap_file(...)`. -- **ASYNC232**: Blocking sync call on file object, wrap the file object in `trio.wrap_file()` to get an async file object. -- **ASYNC240**: Avoid using `os.path` in async functions, prefer using `trio.Path` objects. +- **ASYNC220**: Sync process call in async function, use `await nursery.start([trio|anyio].run_process, ...)`. +- **ASYNC221**: Sync process call in async function, use `await [trio|anyio].run_process(...)`. +- **ASYNC222**: Sync `os.*` call in async function, wrap in `await [trio|anyio].to_thread.run_sync()`. +- **ASYNC230**: Sync IO call in async function, use `[trio|anyio].open_file(...)`. +- **ASYNC231**: Sync IO call in async function, use `[trio|anyio].wrap_file(...)`. +- **ASYNC232**: Blocking sync call on file object, wrap the file object in `[trio|anyio].wrap_file()` to get an async file object. +- **ASYNC240**: Avoid using `os.path` in async functions, prefer using `[trio|anyio].Path` objects. ### Warnings disabled by default -- **ASYNC900**: Async generator without `@asynccontextmanager` not allowed. -- **ASYNC910**: Exit or `return` from async function with no guaranteed checkpoint or exception since function definition. +- **ASYNC900**: Async generator without `@asynccontextmanager` not allowed. You might want to enable this on a codebase since async generators are inherently unsafe and cleanup logic might not be performed. See https://github.com/python-trio/flake8-async/issues/211 and https://discuss.python.org/t/using-exceptiongroup-at-anthropic-experience-report/20888/6 for discussion. +- **ASYNC910**: Exit or `return` from async function with no guaranteed checkpoint or exception since function definition. You might want to enable this on a codebase to make it easier to reason about checkpoints, and make the logic of ASYNC911 correct. - **ASYNC911**: Exit, `yield` or `return` from async iterable with no guaranteed checkpoint since possible function entry (yield or function definition) Checkpoints are `await`, `async for`, and `async with` (on one of enter/exit). ### Removed Warnings +- **TRIOxxx**: All error codes are now renamed ASYNCxxx - **TRIO107**: Renamed to TRIO910 - **TRIO108**: Renamed to TRIO911 - **TRIO117**: Don't raise or catch `trio.[NonBase]MultiError`, prefer `[exceptiongroup.]BaseExceptionGroup`. `MultiError` was removed in trio==0.24.0. diff --git a/flake8_async/visitors/helpers.py b/flake8_async/visitors/helpers.py index 40b5451d..f8521b3b 100644 --- a/flake8_async/visitors/helpers.py +++ b/flake8_async/visitors/helpers.py @@ -237,6 +237,9 @@ def has_exception(node: ast.expr) -> str | None: "trio.Cancelled", "anyio.get_cancelled_exc_class()", "get_cancelled_exc_class()", + "asyncio.exceptions.CancelledError", + "exceptions.CancelledError", + "CancelledError", ): return name return None diff --git a/flake8_async/visitors/visitor103_104.py b/flake8_async/visitors/visitor103_104.py index 119d66b6..7bd815e8 100644 --- a/flake8_async/visitors/visitor103_104.py +++ b/flake8_async/visitors/visitor103_104.py @@ -22,8 +22,30 @@ _suggestion_dict: dict[tuple[str, ...], str] = { ("anyio",): "anyio.get_cancelled_exc_class()", ("trio",): "trio.Cancelled", + ("asyncio",): "asyncio.exceptions.CancelledError", } -_suggestion_dict[("anyio", "trio")] = "[" + "|".join(_suggestion_dict.values()) + "]" +# TODO: ugly +for a, b in (("anyio", "trio"), ("anyio", "asyncio"), ("asyncio", "trio")): + _suggestion_dict[(a, b)] = ( + "[" + "|".join((_suggestion_dict[(a,)], _suggestion_dict[(b,)])) + "]" + ) +_suggestion_dict[ + ( + "anyio", + "asyncio", + "trio", + ) +] = ( + "[" + + "|".join( + ( + _suggestion_dict[("anyio",)], + _suggestion_dict[("asyncio",)], + _suggestion_dict[("trio",)], + ) + ) + + "]" +) _error_codes = { "ASYNC103": _async103_common_msg, @@ -56,6 +78,7 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler): marker = critical_except(node) if marker is None: + # not a critical exception handler return # If previous excepts have handled trio.Cancelled, don't do anything - namely @@ -69,6 +92,13 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler): ): error_code = "ASYNC103" self.cancelled_caught.add("anyio") + elif marker.name in ( + "asyncio.exceptions.CancelledError", + "exceptions.CancelledError", + "CancelledError", + ): + error_code = "ASYNC103" + self.cancelled_caught.add("asyncio") else: if self.cancelled_caught: return @@ -76,7 +106,7 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler): error_code = f"ASYNC103_{self.library_str}" else: error_code = f"ASYNC103_{'_'.join(sorted(self.library))}" - self.cancelled_caught.update("trio", "anyio") + self.cancelled_caught.update("trio", "anyio", "asyncio") # Don't save the state of cancelled_caught, that's handled in Try and would # reset it between each except diff --git a/flake8_async/visitors/visitor91x.py b/flake8_async/visitors/visitor91x.py index bbc4739f..9fcc5e77 100644 --- a/flake8_async/visitors/visitor91x.py +++ b/flake8_async/visitors/visitor91x.py @@ -93,6 +93,9 @@ def copy(self): def checkpoint_statement(library: str) -> cst.SimpleStatementLine: + # logic before this should stop code from wanting to insert the non-existing + # asyncio.lowlevel.checkpoint + assert library != "asyncio" return cst.SimpleStatementLine( [cst.Expr(cst.parse_expression(f"await {library}.lowlevel.checkpoint()"))] ) @@ -111,6 +114,7 @@ def __init__(self): self.noautofix: bool = False self.add_statement: cst.SimpleStatementLine | None = None + # used for inserting import if there's none self.explicitly_imported_library: dict[str, bool] = { "trio": False, "anyio": False, @@ -250,8 +254,12 @@ def __init__(self, *args: Any, **kwargs: Any): self.try_state = TryState() def should_autofix(self, node: cst.CSTNode, code: str | None = None) -> bool: - return not self.noautofix and super().should_autofix( - node, "ASYNC911" if self.has_yield else "ASYNC910" + return ( + not self.noautofix + and super().should_autofix( + node, "ASYNC911" if self.has_yield else "ASYNC910" + ) + and self.library != ("asyncio",) ) def checkpoint_statement(self) -> cst.SimpleStatementLine: @@ -359,7 +367,9 @@ def leave_Return( ) -> cst.Return: if not self.async_function: return updated_node - if self.check_function_exit(original_node): + if self.check_function_exit(original_node) and self.should_autofix( + original_node + ): self.add_statement = self.checkpoint_statement() # avoid duplicate error messages self.uncheckpointed_statements = set() diff --git a/flake8_async/visitors/visitor_utility.py b/flake8_async/visitors/visitor_utility.py index 220fc470..bf843541 100644 --- a/flake8_async/visitors/visitor_utility.py +++ b/flake8_async/visitors/visitor_utility.py @@ -136,11 +136,17 @@ def __init__(self, *args: Any, **kwargs: Any): # see imports if self.options.anyio: self.add_library("anyio") + if self.options.asyncio: + self.add_library("asyncio") def visit_Import(self, node: cst.Import): for alias in node.names: if m.matches( - alias, m.ImportAlias(name=m.Name("trio") | m.Name("anyio"), asname=None) + alias, + m.ImportAlias( + name=m.Name("trio") | m.Name("anyio") | m.Name("asyncio"), + asname=None, + ), ): assert isinstance(alias.name.value, str) self.add_library(alias.name.value) diff --git a/tests/eval_files/anyio_trio.py b/tests/eval_files/anyio_trio.py index 2c48ab4c..4d5fff6c 100644 --- a/tests/eval_files/anyio_trio.py +++ b/tests/eval_files/anyio_trio.py @@ -2,6 +2,7 @@ # ARG --enable=ASYNC220 # NOTRIO # NOASYNCIO +# set base library so trio doesn't get replaced when running with anyio # BASE_LIBRARY anyio # anyio eval will automatically prepend this test with `--anyio` diff --git a/tests/eval_files/async102.py b/tests/eval_files/async102.py index 53381832..54823883 100644 --- a/tests/eval_files/async102.py +++ b/tests/eval_files/async102.py @@ -1,5 +1,6 @@ # type: ignore # NOASYNCIO +# asyncio has different mechanisms for shielded scopes, so would raise additional errors in this file. from contextlib import asynccontextmanager import trio diff --git a/tests/eval_files/async102_anyio.py b/tests/eval_files/async102_anyio.py index ea73c9c2..ae1a57da 100644 --- a/tests/eval_files/async102_anyio.py +++ b/tests/eval_files/async102_anyio.py @@ -1,9 +1,12 @@ # type: ignore +# NOTRIO +# NOASYNCIO +# BASE_LIBRARY anyio +# this test will raise the same errors with trio/asyncio, despite [trio|asyncio].get_cancelled_exc_class not existing +# marked not to run the tests though as error messages will only refer to anyio import anyio from anyio import get_cancelled_exc_class -# this one is fine to also run with trio - async def foo(): ... diff --git a/tests/eval_files/async102_asyncio.py b/tests/eval_files/async102_asyncio.py new file mode 100644 index 00000000..4f28feab --- /dev/null +++ b/tests/eval_files/async102_asyncio.py @@ -0,0 +1,39 @@ +# type: ignore +# NOANYIO +# NOTRIO +# BASE_LIBRARY asyncio +from contextlib import asynccontextmanager + +import asyncio + + +async def foo(): + # asyncio.move_on_after does not exist, so this will raise an error + try: + ... + finally: + with asyncio.move_on_after(deadline=30) as s: + s.shield = True + await foo() # error: 12, Statement("try/finally", lineno-5) + + try: + pass + finally: + await foo() # error: 8, Statement("try/finally", lineno-3) + + # asyncio.CancelScope does not exist, so this will raise an error + try: + pass + finally: + with asyncio.CancelScope(deadline=30, shield=True): + await foo() # error: 12, Statement("try/finally", lineno-4) + + # TODO: I think this is the asyncio-equivalent, but functionality to ignore the error + # has not been implemented + + try: + ... + finally: + await asyncio.shield( # error: 8, Statement("try/finally", lineno-3) + asyncio.wait_for(foo()) + ) diff --git a/tests/eval_files/async103_all_imported.py b/tests/eval_files/async103_all_imported.py new file mode 100644 index 00000000..0c1bdc41 --- /dev/null +++ b/tests/eval_files/async103_all_imported.py @@ -0,0 +1,79 @@ +# NOASYNCIO +# NOANYIO - don't run it with substitutions +import anyio +import trio +import asyncio +from asyncio.exceptions import CancelledError +from asyncio import exceptions + +try: + ... +except trio.Cancelled: # ASYNC103: 7, "trio.Cancelled" + ... +except ( + anyio.get_cancelled_exc_class() # ASYNC103: 4, "anyio.get_cancelled_exc_class()" +): + ... +except CancelledError: # ASYNC103: 7, "CancelledError" + ... +except: # safe + ... + +# reordered +try: + ... +except ( + asyncio.exceptions.CancelledError # ASYNC103: 4, "asyncio.exceptions.CancelledError" +): + ... +except ( + anyio.get_cancelled_exc_class() # ASYNC103: 4, "anyio.get_cancelled_exc_class()" +): + ... +except trio.Cancelled: # ASYNC103: 7, "trio.Cancelled" + ... +except: # safe + ... + +# asyncio supports all three ways of importing asyncio.exceptions.CancelledError +try: + ... +except exceptions.CancelledError: # ASYNC103: 7, "exceptions.CancelledError" + ... + +# catching any one of the exceptions in multi-library files will suppress errors on the bare except. It's unlikely a try block contains code that can raise multiple ones. +try: + ... +except ( + anyio.get_cancelled_exc_class() # ASYNC103: 4, "anyio.get_cancelled_exc_class()" +): + ... +except: # safe ? + ... + +try: + ... +except trio.Cancelled: # ASYNC103: 7, "trio.Cancelled" + ... +except: # safe ? + ... + +try: + ... +except ( + asyncio.exceptions.CancelledError # ASYNC103: 4, "asyncio.exceptions.CancelledError" +): + ... +except: # safe ? + ... + +# Check we get the proper suggestion when all are imported +try: + ... +except BaseException: # ASYNC103_anyio_asyncio_trio: 7, "BaseException" + ... + +try: + ... +except: # ASYNC103_anyio_asyncio_trio: 0, "bare except" + ... diff --git a/tests/eval_files/async118.py b/tests/eval_files/async118.py index 1323a60b..25ed3858 100644 --- a/tests/eval_files/async118.py +++ b/tests/eval_files/async118.py @@ -1,4 +1,7 @@ +# NOTRIO +# NOASYNCIO # This raises the same errors on trio/asyncio, which is a bit silly, but inconsequential +# marked not to run the tests though as error messages will only refer to anyio from typing import Any import anyio diff --git a/tests/test_config_and_args.py b/tests/test_config_and_args.py index 7d3df935..f9dcea61 100644 --- a/tests/test_config_and_args.py +++ b/tests/test_config_and_args.py @@ -174,13 +174,24 @@ def test_anyio_from_config(tmp_path: Path, capsys: pytest.CaptureFixture[str]): "subprocess.Popen", "[anyio|trio]", ) - err_file = str(Path(__file__).parent / "eval_files" / "anyio_trio.py") - expected = f"{err_file}:12:5: ASYNC220 {err_msg}\n" + err_file = Path(__file__).parent / "eval_files" / "anyio_trio.py" + + # find the line with the expected error + for i, line in enumerate(err_file.read_text().split("\n")): + if "# ASYNC220: " in line: + # line numbers start at 1, enumerate starts at 0 + lineno = i + 1 + break + else: + raise AssertionError("could not find error in file") + + # construct the full error message + expected = f"{err_file}:{lineno}:5: ASYNC220 {err_msg}\n" from flake8.main.cli import main returnvalue = main( argv=[ - err_file, + str(err_file), "--config", str(tmp_path / ".flake8"), ] diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index c16dd55c..e86e548c 100644 --- a/tests/test_flake8_async.py +++ b/tests/test_flake8_async.py @@ -207,7 +207,12 @@ def find_magic_markers( markers = (f.name for f in fields(found_markers)) pattern = rf'# ({"|".join(markers)})' for f in re.findall(pattern, content): - setattr(found_markers, f, True) + if f == "BASE_LIBRARY": + m = re.search(r"# BASE_LIBRARY (\w*)\n", content) + assert m, "invalid 'BASE_LIBRARY' marker" + found_markers.BASE_LIBRARY = m.groups()[0] + else: + setattr(found_markers, f, True) return found_markers @@ -229,16 +234,16 @@ def test_eval( magic_markers = find_magic_markers(content) # if autofixing, columns may get messed up ignore_column = autofix - - if library == "anyio" and magic_markers.NOANYIO: - pytest.skip("file marked with NOANYIO") - if library == "asyncio" and magic_markers.NOASYNCIO: - pytest.skip("file marked with NOANYIO") - if library == "trio" and magic_markers.NOTRIO: - pytest.skip("file marked with NOTRIO") - - if library == "asyncio" and autofix: - pytest.skip("no support for asyncio+autofix currently") + only_check_not_crash = False + + # file would raise different errors if transformed to a different library + # so we run the checker against it solely to check that it doesn't crash + if ( + (library == "anyio" and magic_markers.NOANYIO) + or (library == "asyncio" and magic_markers.NOASYNCIO) + or (library == "trio" and magic_markers.NOTRIO) + ): + only_check_not_crash = True if library != magic_markers.BASE_LIBRARY: content = replace_library( @@ -252,7 +257,9 @@ def test_eval( # replace all instances of some error with noqa content = re.sub(r"#[\s]*(error|ASYNC\d\d\d):.*", "# noqa", content) - expected, parsed_args, enable = _parse_eval_file(test, content) + expected, parsed_args, enable = _parse_eval_file( + test, content, only_parse_args=only_check_not_crash + ) if library != "trio": parsed_args.insert(0, f"--{library}") if autofix: @@ -265,16 +272,26 @@ def test_eval( plugin = Plugin.from_source(content) errors = assert_expected_errors( - plugin, *expected, args=parsed_args, ignore_column=ignore_column + plugin, + *expected, + args=parsed_args, + ignore_column=ignore_column, + only_check_not_crash=only_check_not_crash, ) - if library == "anyio": - # check that error messages refer to 'anyio', or to neither library + if only_check_not_crash: + return + + # check that error messages refer to current library, or to no library + if test not in ("ASYNC103_BOTH_IMPORTED", "ASYNC103_ALL_IMPORTED"): for error in errors: message = error.message.format(*error.args) - assert "anyio" in message or "trio" not in message + assert library in message or not any( + lib in message for lib in ("anyio", "asyncio", "trio") + ) - if autofix and not noqa: + # asyncio does not support autofix atm, so should not modify content + if autofix and not noqa and library != "asyncio": check_autofix( test, plugin, @@ -310,7 +327,9 @@ def test_autofix(test: str): assert plugin.module.code == content, "autofixed file changed when autofixed again" -def _parse_eval_file(test: str, content: str) -> tuple[list[Error], list[str], str]: +def _parse_eval_file( + test: str, content: str, only_parse_args: bool = False +) -> tuple[list[Error], list[str], str]: # version check check_version(test) test = test.split("_")[0] @@ -340,6 +359,9 @@ def _parse_eval_file(test: str, content: str) -> tuple[list[Error], list[str], s if m := re.match(r"--enable=(.*)", argument): enabled_codes = m.groups()[0] + if only_parse_args: + continue + # skip commented out lines if not line or line[0] == "#": continue @@ -471,6 +493,7 @@ def assert_expected_errors( *expected: Error, args: list[str] | None = None, ignore_column: bool = False, + only_check_not_crash: bool = False, ) -> list[Error]: # initialize default option values initialize_options(plugin, args) @@ -482,6 +505,15 @@ def assert_expected_errors( for e in *errors, *expected_: e.col = -1 + if only_check_not_crash: + # Check that this file in fact does report different errors. + # Exclude empty errors+expected_ due to noqa runs. + assert errors != expected_ or errors == expected_ == [], ( + "eval file appears to give all the correct errors." + " Maybe you can remove the `# NO[ANYIO/TRIO/ASYNCIO]` magic marker?" + ) + return errors + print_first_diff(errors, expected_) assert_correct_lines_and_codes(errors, expected_) if not ignore_column: From a2dcf815d68603d417a337b225f369002fe20807 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 1 Mar 2024 16:59:43 +0100 Subject: [PATCH 6/8] turn now-redundant `should_autofix()` call into an assert --- flake8_async/visitors/visitor91x.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/flake8_async/visitors/visitor91x.py b/flake8_async/visitors/visitor91x.py index 9fcc5e77..52643ee4 100644 --- a/flake8_async/visitors/visitor91x.py +++ b/flake8_async/visitors/visitor91x.py @@ -149,8 +149,11 @@ def leave_SimpleStatementLine( # possible TODO: generate an error if transforming+visiting is done in a # single pass and emit-error-on-transform can be enabled/disabled. The error can't # be generated in the yield/return since it doesn't know if it will be autofixed. - if self.add_statement is None or not self.should_autofix(original_node): + if self.add_statement is None: return updated_node + + # methods setting self.add_statement should have called self.should_autofix + assert self.should_autofix(original_node) curr_add_statement = self.add_statement self.add_statement = None From 3864eb2b72ca052ebe3c68988b38f87f9fde0f1a Mon Sep 17 00:00:00 2001 From: John Litborn <11260241+jakkdl@users.noreply.github.com> Date: Sat, 2 Mar 2024 13:45:59 +0100 Subject: [PATCH 7/8] Update tests/test_config_and_args.py Co-authored-by: Zac Hatfield-Dodds --- tests/test_config_and_args.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_config_and_args.py b/tests/test_config_and_args.py index f9dcea61..bf79a440 100644 --- a/tests/test_config_and_args.py +++ b/tests/test_config_and_args.py @@ -177,10 +177,8 @@ def test_anyio_from_config(tmp_path: Path, capsys: pytest.CaptureFixture[str]): err_file = Path(__file__).parent / "eval_files" / "anyio_trio.py" # find the line with the expected error - for i, line in enumerate(err_file.read_text().split("\n")): + for lineno, line in enumerate(err_file.read_text().split("\n"), start=1): if "# ASYNC220: " in line: - # line numbers start at 1, enumerate starts at 0 - lineno = i + 1 break else: raise AssertionError("could not find error in file") From 86b473d50e7ccd22f52abfbd8ffb3b18b58c917d Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 2 Mar 2024 14:04:24 +0100 Subject: [PATCH 8/8] fix pre-commit. Replace all old URLs to point to new. --- .github/workflows/ci.yml | 2 +- README.md | 2 +- pyproject.toml | 2 +- setup.py | 2 +- tests/autofix_files/async910.py | 4 ++-- tests/autofix_files/async910.py.diff | 2 +- tests/eval_files/async102.py | 2 +- tests/eval_files/async104.py | 2 +- tests/eval_files/async200.py | 2 +- tests/eval_files/async910.py | 4 ++-- tests/test_config_and_args.py | 4 +++- 11 files changed, 15 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cee9e1c5..48e58f8b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -65,7 +65,7 @@ jobs: release: runs-on: ubuntu-latest needs: [pyright, test] - if: github.repository == 'Zac-HD/flake8-trio' && github.ref == 'refs/heads/main' + if: github.repository == 'python-trio/flake8-async' && github.ref == 'refs/heads/main' steps: - uses: actions/checkout@v4 - name: Set up Python 3 diff --git a/README.md b/README.md index b6126a22..9fc0a480 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -[![pre-commit.ci status](https://results.pre-commit.ci/badge/github/python-trio/flake8-trio/main.svg)](https://results.pre-commit.ci/latest/github/python-trio/flake8-trio/main) +[![pre-commit.ci status](https://results.pre-commit.ci/badge/github/python-trio/flake8-async/main.svg)](https://results.pre-commit.ci/latest/github/python-trio/flake8-async/main) [![Checked with pyright](https://microsoft.github.io/pyright/img/pyright_badge.svg)](https://microsoft.github.io/pyright/) # flake8-async diff --git a/pyproject.toml b/pyproject.toml index 1f8562ce..5892babf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ ignore = [ "COM", # flake8-comma, handled by black "ANN", # annotations, handled by pyright/mypy "T20", # flake8-print - "TID252", # relative imports from parent modules https://github.com/python-trio/flake8-trio/pull/196#discussion_r1200413372 + "TID252", # relative imports from parent modules https://github.com/python-trio/flake8-async/pull/196#discussion_r1200413372 "D101", "D102", "D103", diff --git a/setup.py b/setup.py index a1f4cdbb..124abe2a 100755 --- a/setup.py +++ b/setup.py @@ -25,7 +25,7 @@ def local_file(name: str) -> Path: author="Zac Hatfield-Dodds, John Litborn, and Contributors", author_email="zac@zhd.dev", packages=find_packages(include=["flake8_async", "flake8_async.*"]), - url="https://github.com/python-trio/flake8-trio", + url="https://github.com/python-trio/flake8-async", license="MIT", description="A highly opinionated flake8 plugin for Trio-related problems.", zip_safe=False, diff --git a/tests/autofix_files/async910.py b/tests/autofix_files/async910.py index 1a9e698f..88c3f9fe 100644 --- a/tests/autofix_files/async910.py +++ b/tests/autofix_files/async910.py @@ -382,7 +382,7 @@ async def foo_try_7(): # safe pass -# https://github.com/Zac-HD/flake8-trio/issues/45 +# https://github.com/python-trio/flake8-async/issues/45 async def to_queue(iter_func, queue): async with iter_func() as it: async for x in it: @@ -499,7 +499,7 @@ async def foo_range_5(): # error: 0, "exit", Statement("function definition", l await trio.lowlevel.checkpoint() -# https://github.com/Zac-HD/flake8-trio/issues/47 +# https://github.com/python-trio/flake8-async/issues/47 async def f(): while True: if ...: diff --git a/tests/autofix_files/async910.py.diff b/tests/autofix_files/async910.py.diff index d36009bd..002a82da 100644 --- a/tests/autofix_files/async910.py.diff +++ b/tests/autofix_files/async910.py.diff @@ -196,7 +196,7 @@ + await trio.lowlevel.checkpoint() - # https://github.com/Zac-HD/flake8-trio/issues/47 + # https://github.com/python-trio/flake8-async/issues/47 @@ x,6 x,7 @@ # should error async def foo_comprehension_2(): # error: 0, "exit", Statement("function definition", lineno) diff --git a/tests/eval_files/async102.py b/tests/eval_files/async102.py index 54823883..2d90dd7e 100644 --- a/tests/eval_files/async102.py +++ b/tests/eval_files/async102.py @@ -142,7 +142,7 @@ async def foo(): # change of functionality, no longer treated as safe -# https://github.com/Zac-HD/flake8-trio/issues/54 +# https://github.com/python-trio/flake8-async/issues/54 @asynccontextmanager async def foo2(): try: diff --git a/tests/eval_files/async104.py b/tests/eval_files/async104.py index 874fe15b..b7634767 100644 --- a/tests/eval_files/async104.py +++ b/tests/eval_files/async104.py @@ -14,7 +14,7 @@ try: ... except BaseException as e: - # see https://github.com/Zac-HD/flake8-trio/pull/8#discussion_r932737341 + # see https://github.com/python-trio/flake8-async/pull/8#discussion_r932737341 raise BaseException() from e # error: 4 diff --git a/tests/eval_files/async200.py b/tests/eval_files/async200.py index f9e1a947..5e2ddaeb 100644 --- a/tests/eval_files/async200.py +++ b/tests/eval_files/async200.py @@ -46,7 +46,7 @@ async def bar3(): bar() # ASYNC200: 4, "bar", "BAR" # don't error on directly awaited expressions - # https://github.com/Zac-HD/flake8-trio/issues/85 + # https://github.com/python-trio/flake8-async/issues/85 await bar() print(await bar()) diff --git a/tests/eval_files/async910.py b/tests/eval_files/async910.py index 38c003d8..baa02748 100644 --- a/tests/eval_files/async910.py +++ b/tests/eval_files/async910.py @@ -362,7 +362,7 @@ async def foo_try_7(): # safe pass -# https://github.com/Zac-HD/flake8-trio/issues/45 +# https://github.com/python-trio/flake8-async/issues/45 async def to_queue(iter_func, queue): async with iter_func() as it: async for x in it: @@ -472,7 +472,7 @@ async def foo_range_5(): # error: 0, "exit", Statement("function definition", l await foo() -# https://github.com/Zac-HD/flake8-trio/issues/47 +# https://github.com/python-trio/flake8-async/issues/47 async def f(): while True: if ...: diff --git a/tests/test_config_and_args.py b/tests/test_config_and_args.py index bf79a440..90764b88 100644 --- a/tests/test_config_and_args.py +++ b/tests/test_config_and_args.py @@ -177,7 +177,9 @@ def test_anyio_from_config(tmp_path: Path, capsys: pytest.CaptureFixture[str]): err_file = Path(__file__).parent / "eval_files" / "anyio_trio.py" # find the line with the expected error - for lineno, line in enumerate(err_file.read_text().split("\n"), start=1): + for lineno, line in enumerate( # noqa: B007 # lineno not used in loop body + err_file.read_text().split("\n"), start=1 + ): if "# ASYNC220: " in line: break else: