From 8b1a1cf91b09d8c34dbfb2e954eafcc727d288e2 Mon Sep 17 00:00:00 2001 From: John Litborn <11260241+jakkdl@users.noreply.github.com> Date: Fri, 8 Mar 2024 12:50:40 +0100 Subject: [PATCH] moar asyncio support, test infra improvements (#213) * Add asyncio-specific error messages for 2xx. Add check in tests to detect if [library]_NO_ERROR should be used, add it to several checks. Specify why NO_[library] and [library]_NO_ERROR is specified in a lot of eval files. * update error codes in readme * * added 113 support for asyncio * split off anyio_create_task_group testing into async113_anyio.py * improved comments --------- Co-authored-by: Zac Hatfield-Dodds --- CHANGELOG.md | 4 +- README.md | 12 +- flake8_async/__init__.py | 2 +- flake8_async/base.py | 2 +- flake8_async/visitors/visitor2xx.py | 67 ++++- flake8_async/visitors/visitors.py | 7 +- tests/autofix_files/async100.py | 2 +- .../autofix_files/async100_simple_autofix.py | 2 +- tests/autofix_files/noqa.py | 3 +- tests/autofix_files/noqa_testing.py | 1 - tests/eval_files/anyio_trio.py | 1 - tests/eval_files/async100.py | 2 +- tests/eval_files/async100_asyncio.py | 23 ++ tests/eval_files/async100_noautofix.py | 2 +- tests/eval_files/async100_simple_autofix.py | 2 +- tests/eval_files/async101.py | 4 +- tests/eval_files/async102.py | 3 +- tests/eval_files/async103.py | 1 - tests/eval_files/async103_no_104.py | 1 - tests/eval_files/async104.py | 1 - tests/eval_files/async105.py | 4 +- tests/eval_files/async109.py | 1 - tests/eval_files/async110.py | 2 +- tests/eval_files/async111.py | 4 +- tests/eval_files/async112.py | 4 +- tests/eval_files/async113.py | 10 +- tests/eval_files/async113_anyio.py | 12 + tests/eval_files/async113_trio.py | 6 +- tests/eval_files/async115.py | 2 +- tests/eval_files/async116.py | 2 +- tests/eval_files/async118.py | 9 +- tests/eval_files/async22x.py | 2 +- tests/eval_files/async22x_asyncio.py | 80 ++++++ tests/eval_files/async232.py | 7 +- tests/eval_files/async232_asyncio.py | 265 ++++++++++++++++++ tests/eval_files/async23x.py | 2 +- tests/eval_files/async23x_asyncio.py | 48 ++++ tests/eval_files/no_library.py | 1 - tests/eval_files/noqa.py | 3 +- tests/eval_files/noqa_testing.py | 1 - tests/eval_files/trio_anyio.py | 1 - tests/test_flake8_async.py | 45 +-- 42 files changed, 576 insertions(+), 77 deletions(-) create mode 100644 tests/eval_files/async100_asyncio.py create mode 100644 tests/eval_files/async113_anyio.py create mode 100644 tests/eval_files/async22x_asyncio.py create mode 100644 tests/eval_files/async232_asyncio.py create mode 100644 tests/eval_files/async23x_asyncio.py diff --git a/CHANGELOG.md b/CHANGELOG.md index da23b5b..b56cabb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,13 @@ # Changelog *[CalVer, YY.month.patch](https://calver.org/)* -## Future +## 24.3.1 - 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 asyncio support for several error codes - added `--library` ## 23.5.1 diff --git a/README.md b/README.md index f6f1eaa..4837ca2 100644 --- a/README.md +++ b/README.md @@ -48,13 +48,13 @@ Note: 22X, 23X and 24X has not had asyncio-specific suggestions written. - **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/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(...)`. +- **ASYNC220**: Sync process call in async function, use `await nursery.start([trio/anyio].run_process, ...)`. `asyncio` users can use [`asyncio.create_subprocess_[exec/shell]`](https://docs.python.org/3/library/asyncio-subprocess.html). +- **ASYNC221**: Sync process call in async function, use `await [trio/anyio].run_process(...)`. `asyncio` users can use [`asyncio.create_subprocess_[exec/shell]`](https://docs.python.org/3/library/asyncio-subprocess.html). +- **ASYNC222**: Sync `os.*` call in async function, wrap in `await [trio/anyio].to_thread.run_sync()`. `asyncio` users can use [`asyncio.loop.run_in_executor`](https://docs.python.org/3/library/asyncio-subprocess.html). +- **ASYNC230**: Sync IO call in async function, use `[trio/anyio].open_file(...)`. `asyncio` users need to use a library such as [aiofiles](https://pypi.org/project/aiofiles/), or switch to [anyio](https://github.com/agronholm/anyio). +- **ASYNC231**: Sync IO call in async function, use `[trio/anyio].wrap_file(...)`. `asyncio` users need to use a library such as [aiofiles](https://pypi.org/project/aiofiles/), or switch to [anyio](https://github.com/agronholm/anyio). - **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. +- **ASYNC240**: Avoid using `os.path` in async functions, prefer using `[trio/anyio].Path` objects. `asyncio` users should consider [aiopath](https://pypi.org/project/aiopath) or [anyio](https://github.com/agronholm/anyio). ### Warnings disabled by default - **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. diff --git a/flake8_async/__init__.py b/flake8_async/__init__.py index 15ad2c1..ca74009 100644 --- a/flake8_async/__init__.py +++ b/flake8_async/__init__.py @@ -37,7 +37,7 @@ # CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1" -__version__ = "23.5.1" +__version__ = "24.3.1" # taken from https://github.com/Zac-HD/shed diff --git a/flake8_async/base.py b/flake8_async/base.py index 3f167d3..56c5a96 100644 --- a/flake8_async/base.py +++ b/flake8_async/base.py @@ -83,7 +83,7 @@ def __iter__(self): def cmp(self): # column may be ignored/modified when autofixing, so sort on that last - return self.line, self.code, self.args, self.col + return self.line, self.code, self.args, self.message, self.col # for sorting in tests def __lt__(self, other: Error) -> bool: diff --git a/flake8_async/visitors/visitor2xx.py b/flake8_async/visitors/visitor2xx.py index 86a48ec..0b68131 100644 --- a/flake8_async/visitors/visitor2xx.py +++ b/flake8_async/visitors/visitor2xx.py @@ -169,13 +169,26 @@ def visit_blocking_call(self, node: ast.Call): # Process invocations 202 @error_class class Visitor22X(Visitor200): + # not the prettiest, as it requires duplicating eval files, and will give + # nonsensical error messages if using asyncio+trio/anyio error_codes: Mapping[str, str] = { "ASYNC220": ( "Sync call {} in async function, use " "`await nursery.start({}.run_process, ...)`." ), + "ASYNC220_asyncio": ( + "Sync call {} in async function, use " + "`asyncio.create_subprocess_[exec/shell]." + ), "ASYNC221": "Sync call {} in async function, use `await {}.run_process(...)`.", + "ASYNC221_asyncio": ( + "Sync call {} in async function, use " + "`asyncio.create_subprocess_[exec/shell]." + ), "ASYNC222": "Sync call {} in async function, wrap in `{}.to_thread.run_sync()`.", + "ASYNC222_asyncio": ( + "Sync call {} in async function, use `asyncio.loop.run_in_executor`." + ), } def visit_blocking_call(self, node: ast.Call): @@ -222,7 +235,14 @@ def is_p_wait(arg: ast.expr) -> bool: error_code = "ASYNC220" break - if error_code is not None: + if error_code is None: + return + if self.library == ("asyncio",): + self.error(node, func_name, error_code=error_code + "_asyncio") + else: + # the asyncio + anyio/trio case is probably not worth special casing, + # so we simply suggest e.g. `[trio/asyncio/anyio].run_process()` despite + # asyncio.run_process not existing self.error(node, func_name, self.library_str, error_code=error_code) @@ -231,6 +251,14 @@ class Visitor23X(Visitor200): error_codes: Mapping[str, str] = { "ASYNC230": "Sync call {0} in async function, use `{1}.open_file(...)`.", "ASYNC231": "Sync call {0} in async function, use `{1}.wrap_file({0})`.", + "ASYNC230_asyncio": ( + "Sync call {0} in async function, " + " use a library such as aiofiles or anyio." + ), + "ASYNC231_asyncio": ( + "Sync call {0} in async function, " + " use a library such as aiofiles or anyio." + ), } def visit_Call(self, node: ast.Call): @@ -249,16 +277,23 @@ def visit_blocking_call(self, node: ast.Call): error_code = "ASYNC231" else: return - self.error(node, func_name, self.library_str, error_code=error_code) + if self.library == ("asyncio",): + self.error(node, func_name, error_code=error_code + "_asyncio") + else: + self.error(node, func_name, self.library_str, error_code=error_code) @error_class class Visitor232(Visitor200): error_codes: Mapping[str, str] = { "ASYNC232": ( - "Blocking sync call {1} on file object {0}, wrap the file object" + "Blocking sync call {1} on file object {0}, wrap the file object " "in `{2}.wrap_file()` to get an async file object." - ) + ), + "ASYNC232_asyncio": ( + "Blocking sync call {1} on file object {0}, wrap the file object " + " to get an async file object." + ), } def __init__(self, *args: Any, **kwargs: Any): @@ -279,13 +314,30 @@ def visit_blocking_call(self, node: ast.Call): and (anno := self.variables.get(node.func.value.id)) and (anno in io_file_types or f"io.{anno}" in io_file_types) ): - self.error(node, node.func.attr, node.func.value.id, self.library_str) + if self.library == ("asyncio",): + self.error( + node, + node.func.attr, + node.func.value.id, + error_code="ASYNC232_asyncio", + ) + else: + self.error( + node, + node.func.attr, + node.func.value.id, + self.library_str, + error_code="ASYNC232", + ) @error_class class Visitor24X(Visitor200): error_codes: Mapping[str, str] = { "ASYNC240": "Avoid using os.path, prefer using {1}.Path objects.", + "ASYNC240_asyncio": ( + "Avoid using os.path, use a library such as aiopath or anyio." + ), } def __init__(self, *args: Any, **kwargs: Any): @@ -324,10 +376,11 @@ def visit_ImportFrom(self, node: ast.ImportFrom): def visit_Call(self, node: ast.Call): if not self.async_function: return + error_code = "ASYNC240_asyncio" if self.library == ("asyncio",) else "ASYNC240" func_name = ast.unparse(node.func) if func_name in self.imports_from_ospath: - self.error(node, func_name, self.library_str) + self.error(node, func_name, self.library_str, error_code=error_code) elif (m := re.fullmatch(r"os\.path\.(?P.*)", func_name)) and m.group( "func" ) in self.os_funcs: - self.error(node, m.group("func"), self.library_str) + self.error(node, m.group("func"), self.library_str, error_code=error_code) diff --git a/flake8_async/visitors/visitors.py b/flake8_async/visitors/visitors.py index 259eb15..2160ee9 100644 --- a/flake8_async/visitors/visitors.py +++ b/flake8_async/visitors/visitors.py @@ -93,7 +93,9 @@ def visit_With(self, node: ast.With | ast.AsyncWith): var_name = item.optional_vars.id # check for trio.open_nursery - nursery = get_matching_call(item.context_expr, "open_nursery") + nursery = get_matching_call( + item.context_expr, "open_nursery", base=("trio",) + ) # `isinstance(..., ast.Call)` is done in get_matching_call body_call = cast("ast.Call", node.body[0].value) @@ -169,6 +171,7 @@ def is_nursery_call(node: ast.expr): in ( "trio.Nursery", "anyio.TaskGroup", + "asyncio.TaskGroup", ) ) @@ -209,7 +212,7 @@ def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef): self.error(node, node.name) -# Suggests replacing all `trio.sleep(0)` with the more suggestive +# Suggests replacing all `[trio|anyio].sleep(0)` with the more suggestive # `trio.lowlevel.checkpoint()` @error_class class Visitor115(Flake8AsyncVisitor): diff --git a/tests/autofix_files/async100.py b/tests/autofix_files/async100.py index f15dd43..a6d373e 100644 --- a/tests/autofix_files/async100.py +++ b/tests/autofix_files/async100.py @@ -1,6 +1,6 @@ # type: ignore # AUTOFIX -# NOASYNCIO +# ASYNCIO_NO_ERROR # timeout primitives are named differently in asyncio import trio diff --git a/tests/autofix_files/async100_simple_autofix.py b/tests/autofix_files/async100_simple_autofix.py index 7174234..b60e5f9 100644 --- a/tests/autofix_files/async100_simple_autofix.py +++ b/tests/autofix_files/async100_simple_autofix.py @@ -1,4 +1,4 @@ -# NOASYNCIO +# ASYNCIO_NO_ERROR - no asyncio.move_on_after # AUTOFIX import trio diff --git a/tests/autofix_files/noqa.py b/tests/autofix_files/noqa.py index a0ca87b..b335bc7 100644 --- a/tests/autofix_files/noqa.py +++ b/tests/autofix_files/noqa.py @@ -1,6 +1,5 @@ # AUTOFIX -# NOANYIO # TODO -# NOASYNCIO +# NOASYNCIO - does not trigger on ASYNC100 # ARG --enable=ASYNC100,ASYNC911 from typing import Any diff --git a/tests/autofix_files/noqa_testing.py b/tests/autofix_files/noqa_testing.py index 703a48c..9bb4e45 100644 --- a/tests/autofix_files/noqa_testing.py +++ b/tests/autofix_files/noqa_testing.py @@ -1,5 +1,4 @@ # AUTOFIX -# NOANYIO # TODO # ARG --enable=ASYNC911 import trio diff --git a/tests/eval_files/anyio_trio.py b/tests/eval_files/anyio_trio.py index 2e5dd2e..fcbd338 100644 --- a/tests/eval_files/anyio_trio.py +++ b/tests/eval_files/anyio_trio.py @@ -1,7 +1,6 @@ # type: ignore # ARG --enable=ASYNC220 # NOTRIO -# NOASYNCIO # set base library so trio doesn't get replaced when running with anyio # BASE_LIBRARY anyio diff --git a/tests/eval_files/async100.py b/tests/eval_files/async100.py index a9b39c6..a16cca2 100644 --- a/tests/eval_files/async100.py +++ b/tests/eval_files/async100.py @@ -1,6 +1,6 @@ # type: ignore # AUTOFIX -# NOASYNCIO +# ASYNCIO_NO_ERROR # timeout primitives are named differently in asyncio import trio diff --git a/tests/eval_files/async100_asyncio.py b/tests/eval_files/async100_asyncio.py new file mode 100644 index 0000000..9dd743c --- /dev/null +++ b/tests/eval_files/async100_asyncio.py @@ -0,0 +1,23 @@ +# TRIO_NO_ERROR +# ANYIO_NO_ERROR +# BASE_LIBRARY asyncio +# ASYNCIO_NO_ERROR # TODO + +import asyncio +import asyncio.timeouts + + +async def foo(): + # py>=3.11 re-exports these in the main asyncio namespace + with asyncio.timeout_at(10): # type: ignore[attr-defined] + ... + with asyncio.timeout_at(10): # type: ignore[attr-defined] + ... + with asyncio.timeout(10): # type: ignore[attr-defined] + ... + with asyncio.timeouts.timeout_at(10): + ... + with asyncio.timeouts.timeout_at(10): + ... + with asyncio.timeouts.timeout(10): + ... diff --git a/tests/eval_files/async100_noautofix.py b/tests/eval_files/async100_noautofix.py index 2f3624e..6da4ed6 100644 --- a/tests/eval_files/async100_noautofix.py +++ b/tests/eval_files/async100_noautofix.py @@ -1,4 +1,4 @@ -# NOASYNCIO +# ASYNCIO_NO_ERROR - no asyncio.move_on_after import trio diff --git a/tests/eval_files/async100_simple_autofix.py b/tests/eval_files/async100_simple_autofix.py index c783f71..e267dd4 100644 --- a/tests/eval_files/async100_simple_autofix.py +++ b/tests/eval_files/async100_simple_autofix.py @@ -1,4 +1,4 @@ -# NOASYNCIO +# ASYNCIO_NO_ERROR - no asyncio.move_on_after # AUTOFIX import trio diff --git a/tests/eval_files/async101.py b/tests/eval_files/async101.py index fb60825..b0bdf76 100644 --- a/tests/eval_files/async101.py +++ b/tests/eval_files/async101.py @@ -1,4 +1,6 @@ -# NOASYNCIO +# ASYNCIO_NO_ERROR - nursery/cancelscope in asyncio is called taskgroup/timeout +# Note: this one *shouldn't* be giving the same errors for anyio (but it currently does) +# since it has TaskGroups instead of nurseries. #TODO #215 # type: ignore import contextlib import contextlib as bla diff --git a/tests/eval_files/async102.py b/tests/eval_files/async102.py index 2d90dd7..a846716 100644 --- a/tests/eval_files/async102.py +++ b/tests/eval_files/async102.py @@ -1,6 +1,5 @@ # type: ignore -# NOASYNCIO -# asyncio has different mechanisms for shielded scopes, so would raise additional errors in this file. +# NOASYNCIO # TODO: support asyncio shields from contextlib import asynccontextmanager import trio diff --git a/tests/eval_files/async103.py b/tests/eval_files/async103.py index 5b010bf..82bf8e1 100644 --- a/tests/eval_files/async103.py +++ b/tests/eval_files/async103.py @@ -1,4 +1,3 @@ -# NOASYNCIO # ARG --enable=ASYNC103,ASYNC104 from typing import Any diff --git a/tests/eval_files/async103_no_104.py b/tests/eval_files/async103_no_104.py index 94d1337..4eb7e7e 100644 --- a/tests/eval_files/async103_no_104.py +++ b/tests/eval_files/async103_no_104.py @@ -1,4 +1,3 @@ -# NOASYNCIO # ARG --enable=ASYNC103 # check that partly disabling a visitor works from typing import Any diff --git a/tests/eval_files/async104.py b/tests/eval_files/async104.py index b763476..8b845c2 100644 --- a/tests/eval_files/async104.py +++ b/tests/eval_files/async104.py @@ -1,5 +1,4 @@ # ARG --enable=ASYNC103,ASYNC104 -# NOASYNCIO try: ... # raise different exception diff --git a/tests/eval_files/async105.py b/tests/eval_files/async105.py index 93123bb..62a0c3c 100644 --- a/tests/eval_files/async105.py +++ b/tests/eval_files/async105.py @@ -1,5 +1,5 @@ -# NOANYIO -# NOASYNCIO +# ANYIO_NO_ERROR # not supported, and no plans to +# ASYNCIO_NO_ERROR # not supported, and no plans to from typing import Any from collections.abc import Coroutine diff --git a/tests/eval_files/async109.py b/tests/eval_files/async109.py index 4c69183..139e2e5 100644 --- a/tests/eval_files/async109.py +++ b/tests/eval_files/async109.py @@ -1,5 +1,4 @@ # type: ignore -# NOASYNCIO import trio import trio as anything diff --git a/tests/eval_files/async110.py b/tests/eval_files/async110.py index ec65f54..2e8c8bc 100644 --- a/tests/eval_files/async110.py +++ b/tests/eval_files/async110.py @@ -1,5 +1,5 @@ # type: ignore -# NOASYNCIO +# ASYNCIO_NO_ERROR - not yet supported # TODO import trio import trio as noerror diff --git a/tests/eval_files/async111.py b/tests/eval_files/async111.py index 7044c62..cd751fb 100644 --- a/tests/eval_files/async111.py +++ b/tests/eval_files/async111.py @@ -1,5 +1,7 @@ # type: ignore -# NOASYNCIO +# 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. +# It's possible there's an equivalent asyncio construction/gotcha, but methods are differently named, so this file will not raise any errors +# ASYNCIO_NO_ERROR # no nurseries in asyncio. Though maybe the bug is relevant for TaskGroups from typing import Any import trio diff --git a/tests/eval_files/async112.py b/tests/eval_files/async112.py index f13b6ec..ce4ad93 100644 --- a/tests/eval_files/async112.py +++ b/tests/eval_files/async112.py @@ -1,5 +1,7 @@ # type: ignore -# NOASYNCIO +# 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. +# ASYNCIO_NO_ERROR - # TODO: expand check to work with asyncio.TaskGroup +# ANYIO_NO_ERROR - # TODO: expand check to work with anyio.TaskGroup import functools from functools import partial diff --git a/tests/eval_files/async113.py b/tests/eval_files/async113.py index d0920ab..63d1c41 100644 --- a/tests/eval_files/async113.py +++ b/tests/eval_files/async113.py @@ -1,11 +1,14 @@ # mypy: disable-error-code="arg-type,attr-defined" -# NOASYNCIO from contextlib import asynccontextmanager import anyio import trio -# NOANYIO - requires no substitution check +# set base library to anyio, so we can replace anyio->asyncio and get correct errors +# BASE_LIBRARY anyio + +# NOTRIO - replacing anyio->trio would give mismatching errors. +# This file tests basic trio errors, and async113_trio checks trio-specific errors @asynccontextmanager @@ -15,9 +18,6 @@ async def foo(): async with trio.open_nursery() as bar: bar.start_soon(trio.run_process) # ASYNC113: 8 - async with anyio.create_task_group() as bar_tg: - bar_tg.start_soon(anyio.run_process) # ASYNC113: 8 - boo: trio.Nursery = ... # type: ignore boo.start_soon(trio.run_process) # ASYNC113: 4 diff --git a/tests/eval_files/async113_anyio.py b/tests/eval_files/async113_anyio.py new file mode 100644 index 0000000..7fd1a95 --- /dev/null +++ b/tests/eval_files/async113_anyio.py @@ -0,0 +1,12 @@ +# mypy: disable-error-code="arg-type" +from contextlib import asynccontextmanager + +import anyio + + +@asynccontextmanager +async def foo(): + # create_task_group only exists in anyio + async with anyio.create_task_group() as bar_tg: + bar_tg.start_soon(anyio.run_process) # ASYNC113: 8 + yield diff --git a/tests/eval_files/async113_trio.py b/tests/eval_files/async113_trio.py index bf4891f..0875519 100644 --- a/tests/eval_files/async113_trio.py +++ b/tests/eval_files/async113_trio.py @@ -1,5 +1,5 @@ # mypy: disable-error-code="arg-type,call-overload,misc" -# NOASYNCIO +# ASYNC113: Using nursery.start_soon in __aenter__ doesn't wait for the task to begin. Consider replacing with nursery.start. import contextlib import contextlib as arbitrary_import_alias_for_contextlib import functools @@ -11,7 +11,9 @@ import trio # ARG --startable-in-context-manager=custom_startable_function -# NOANYIO + +# ANYIO_NO_ERROR - anyio uses TaskGroups. Checked in async113.py & async113_anyio.py +# ASYNCIO_NO_ERROR - asyncio uses TaskGroups. Checked in async113.py @asynccontextmanager diff --git a/tests/eval_files/async115.py b/tests/eval_files/async115.py index 0d747c8..fbfa7d9 100644 --- a/tests/eval_files/async115.py +++ b/tests/eval_files/async115.py @@ -1,5 +1,5 @@ # type: ignore -# NOASYNCIO +# ASYNCIO_NO_ERROR # no asyncio.lowlevel.checkpoint() import time import trio diff --git a/tests/eval_files/async116.py b/tests/eval_files/async116.py index 48fac98..60bf8b9 100644 --- a/tests/eval_files/async116.py +++ b/tests/eval_files/async116.py @@ -1,5 +1,5 @@ # type: ignore -# NOASYNCIO +# ASYNCIO_NO_ERROR - no asyncio.sleep_forever, so check intentionally doesn't trigger. import math from math import inf diff --git a/tests/eval_files/async118.py b/tests/eval_files/async118.py index 25ed385..e60b9b8 100644 --- a/tests/eval_files/async118.py +++ b/tests/eval_files/async118.py @@ -1,7 +1,8 @@ -# 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 +# `get_cancelled_exc_class` will trigger errors even if anyio isn't imported, out of +# an abundance of caution. But `anyio.get_cancelled_exc_class` will not. +# NOTRIO # anyio-specific check, see above +# NOASYNCIO # anyio-specific check, see above +# BASE_LIBRARY anyio from typing import Any import anyio diff --git a/tests/eval_files/async22x.py b/tests/eval_files/async22x.py index b68d2fe..7092dd0 100644 --- a/tests/eval_files/async22x.py +++ b/tests/eval_files/async22x.py @@ -1,6 +1,6 @@ # type: ignore # ARG --enable=ASYNC220,ASYNC221,ASYNC222 -# NOASYNCIO +# NOASYNCIO - specifies error message differently async def foo(): diff --git a/tests/eval_files/async22x_asyncio.py b/tests/eval_files/async22x_asyncio.py new file mode 100644 index 0000000..c493f31 --- /dev/null +++ b/tests/eval_files/async22x_asyncio.py @@ -0,0 +1,80 @@ +# type: ignore +# ARG --enable=ASYNC220,ASYNC221,ASYNC222 +# NOANYIO +# NOTRIO +# BASE_LIBRARY asyncio + +# same content as async22x.py, but different error message + + +async def foo(): + await async_fun( + subprocess.getoutput() # ASYNC221_asyncio: 8, 'subprocess.getoutput' + ) + subprocess.Popen() # ASYNC220_asyncio: 4, 'subprocess.Popen' + os.system() # ASYNC221_asyncio: 4, 'os.system' + + system() + os.system.anything() + os.anything() + + subprocess.run() # ASYNC221_asyncio: 4, 'subprocess.run' + subprocess.call() # ASYNC221_asyncio: 4, 'subprocess.call' + subprocess.check_call() # ASYNC221_asyncio: 4, 'subprocess.check_call' + subprocess.check_output() # ASYNC221_asyncio: 4, 'subprocess.check_output' + subprocess.getoutput() # ASYNC221_asyncio: 4, 'subprocess.getoutput' + subprocess.getstatusoutput() # ASYNC221_asyncio: 4, 'subprocess.getstatusoutput' + + await async_fun( + subprocess.getoutput() # ASYNC221_asyncio: 8, 'subprocess.getoutput' + ) + + subprocess.anything() + subprocess.foo() + subprocess.bar.foo() + subprocess() + + os.posix_spawn() # ASYNC221_asyncio: 4, 'os.posix_spawn' + os.posix_spawnp() # ASYNC221_asyncio: 4, 'os.posix_spawnp' + + os.spawn() + os.spawn + os.spawnllll() + + os.spawnl() # ASYNC221_asyncio: 4, 'os.spawnl' + os.spawnle() # ASYNC221_asyncio: 4, 'os.spawnle' + os.spawnlp() # ASYNC221_asyncio: 4, 'os.spawnlp' + os.spawnlpe() # ASYNC221_asyncio: 4, 'os.spawnlpe' + os.spawnv() # ASYNC221_asyncio: 4, 'os.spawnv' + os.spawnve() # ASYNC221_asyncio: 4, 'os.spawnve' + os.spawnvp() # ASYNC221_asyncio: 4, 'os.spawnvp' + os.spawnvpe() # ASYNC221_asyncio: 4, 'os.spawnvpe' + + # if mode is given, and is not os.P_WAIT: ASYNC220 + os.spawnl(os.P_NOWAIT) # ASYNC220_asyncio: 4, 'os.spawnl' + os.spawnl(P_NOWAIT) # ASYNC220_asyncio: 4, 'os.spawnl' + os.spawnl(mode=os.P_NOWAIT) # ASYNC220_asyncio: 4, 'os.spawnl' + os.spawnl(mode=P_NOWAIT) # ASYNC220_asyncio: 4, 'os.spawnl' + + # if it is P_WAIT, ASYNC221 + os.spawnl(os.P_WAIT) # ASYNC221_asyncio: 4, 'os.spawnl' + os.spawnl(P_WAIT) # ASYNC221_asyncio: 4, 'os.spawnl' + os.spawnl(mode=os.P_WAIT) # ASYNC221_asyncio: 4, 'os.spawnl' + os.spawnl(mode=P_WAIT) # ASYNC221_asyncio: 4, 'os.spawnl' + # treating this as 221 to simplify code, and see no real reason not to + os.spawnl(foo.P_WAIT) # ASYNC221_asyncio: 4, 'os.spawnl' + + # other weird cases: ASYNC220 + os.spawnl(0) # ASYNC220_asyncio: 4, 'os.spawnl' + os.spawnl(1) # ASYNC220_asyncio: 4, 'os.spawnl' + os.spawnl(foo()) # ASYNC220_asyncio: 4, 'os.spawnl' + + # ASYNC222 + os.wait() # ASYNC222_asyncio: 4, 'os.wait' + os.wait3() # ASYNC222_asyncio: 4, 'os.wait3' + os.wait4() # ASYNC222_asyncio: 4, 'os.wait4' + os.waitid() # ASYNC222_asyncio: 4, 'os.waitid' + os.waitpid() # ASYNC222_asyncio: 4, 'os.waitpid' + + os.waitpi() + os.waiti() diff --git a/tests/eval_files/async232.py b/tests/eval_files/async232.py index 8c211ba..5fd52f4 100644 --- a/tests/eval_files/async232.py +++ b/tests/eval_files/async232.py @@ -1,5 +1,5 @@ # type: ignore -# NOASYNCIO +# NOASYNCIO # see async232_asyncio.py import io from io import BufferedRandom, BufferedReader, BufferedWriter, TextIOWrapper from typing import Any, Optional @@ -14,6 +14,11 @@ async def file_text(f: io.TextIOWrapper): # there might be non-sync calls on TextIOWrappers? - but it will currently trigger # on all calls f.anything() # ASYNC232: 4, 'anything', 'f', "trio" + f.aoeuaoeuaoeuaoeu() # ASYNC232: 4, 'aoeuaoeuaoeuaoeu', 'f', "trio" + + +# Test different file types for the type tracker +# We use f.read() in all of them for simplicity, but any method call would trigger the same error async def file_binary_read(f: io.BufferedReader): diff --git a/tests/eval_files/async232_asyncio.py b/tests/eval_files/async232_asyncio.py new file mode 100644 index 0000000..ae070b7 --- /dev/null +++ b/tests/eval_files/async232_asyncio.py @@ -0,0 +1,265 @@ +# type: ignore +# NOTRIO # see async232.py +# NOANYIO # see async232.py +import io +from io import BufferedRandom, BufferedReader, BufferedWriter, TextIOWrapper +from typing import Any, Optional + +blah: Any = None + + +async def file_text(f: io.TextIOWrapper): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + f.readlines() # ASYNC232_asyncio: 4, 'readlines', 'f' + + # there might be non-sync calls on TextIOWrappers? - but it will currently trigger + # on all calls + f.anything() # ASYNC232_asyncio: 4, 'anything', 'f' + f.aoeuaoeuaoeuaoeuaoeu() # ASYNC232_asyncio: 4, 'aoeuaoeuaoeuaoeuaoeu', 'f' + + +# Test different file types for the type tracker +# We use f.read() in all of them for simplicity, but any method call would trigger the same error + + +async def file_binary_read(f: io.BufferedReader): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +async def file_binary_write(f: io.BufferedWriter): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +async def file_binary_readwrite(f: io.BufferedRandom): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +async def file_text_(f: TextIOWrapper): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +async def file_binary_read_(f: BufferedReader): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +async def file_binary_write_(f: BufferedWriter): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +async def file_binary_readwrite_(f: BufferedRandom): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +async def file_text_3(f: TextIOWrapper = blah): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +async def file_text_4(f: TextIOWrapper | None): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + if f: + f.read() # ASYNC232_asyncio: 8, 'read', 'f' + + +async def file_text_4_left(f: None | TextIOWrapper): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + if f: + f.read() # ASYNC232_asyncio: 8, 'read', 'f' + + +# not handled +async def file_text_4_both(f: None | TextIOWrapper | None): + f.read() + + +# not handled +async def file_text_4_non_none(f: TextIOWrapper | int): + f.read() + + +async def file_text_5(f: TextIOWrapper | None = None): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + if f: + f.read() # ASYNC232_asyncio: 8, 'read', 'f' + + +async def file_text_6(f: Optional[TextIOWrapper] = None): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + if f: + f.read() # ASYNC232_asyncio: 8, 'read', 'f' + + +# posonly +async def file_text_7(f: TextIOWrapper = blah, /): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +# keyword-only +async def file_text_8(*, f: TextIOWrapper = blah): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +async def file_text_9(lf: list[TextIOWrapper]): + for f in lf: + f.read() # not handled + + +async def open_file_1(): + with open(""): + ... + + +async def open_file_2(): + with open("") as f: + print(f) + + +async def open_file_3(): + with open("") as f: + f.read() # ASYNC232_asyncio: 8, 'read', 'f' + + +async def open_file_4(): + f = open("") + + +async def open_file_5(): + f = open("") + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +async def open_file_6(): + ff = open("") + f = ff + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + ff.read() # ASYNC232_asyncio: 4, 'read', 'ff' + + +async def noerror(): + with blah("") as f: + f.read() + + +def sync_fun(f: TextIOWrapper): + async def async_fun(): + f.read() # ASYNC232_asyncio: 8, 'read', 'f' + + +def sync_fun_2(): + f = open("") + + async def async_fun(): + f.read() # ASYNC232_asyncio: 8, 'read', 'f' + + +async def type_assign(): + f: TextIOWrapper = ... + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +# define global variables +f = open("") +g: TextIOWrapper = ... + + +# Test state handling on nested functions +async def async_wrapper(f: TextIOWrapper): + def inside(f: int): + f.read() + + async def inception(): + f.read() + + f.read() + + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + lambda f: f.read() + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +# and show that they're still marked as TextIOWrappers +async def global_vars(): + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + g.read() # ASYNC232_asyncio: 4, 'read', 'g' + + +# If the type is explicitly overridden, it will not error +async def overridden_type(f: TextIOWrapper): + f: int = 7 + f.read() + f: TextIOWrapper = ... + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + f: int = 7 + f.read() + f: TextIOWrapper = ... + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +# ***** Known unhandled cases ***** + + +# It will error on non-explicit assignments +async def implicit_overridden_type(): + f: TextIOWrapper = ... + f = arbitrary_function() + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + f = 7 + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +# Tuple assignments are completely ignored +async def multi_assign(): + x, y = open(""), open("") + x.read() # should error + y.read() # should error + + +# as are attribute assignments +async def attribute_assign(): + x.y = open("") + x.y.read() # should error + + +# or calling on subattributes of an object +async def attribute_access_on_object(): + f: TextIOWrapper = ... + f.any.thing() # should error + + +# The type checker is very naive, and will not do any parsing of logic pertaining +# to the type +async def type_restricting_1(f: Optional[TextIOWrapper] = None): + if f is None: + f.read() # ASYNC232_asyncio: 8, 'read', 'f' + + +async def type_restricting_2(f: Optional[TextIOWrapper] = None): + if isinstance(f, TextIOWrapper): + return + f.read() # ASYNC232_asyncio: 4, 'read', 'f' + + +# Classes are not supported, partly due to not handling attributes at all, +# but would require additional logic +class myclass: + x: TextIOWrapper + + async def foo(self): + self.x.read() # should error + + +class myclass_2: + def __init__(self): + self.x: TextIOWrapper = open("") + + async def foo(self): + self.x.read() # should error + + +# Return types are not parsed +async def call_function_with_return_type(): + def return_textiowrapper() -> TextIOWrapper: + return open("") + + k = return_textiowrapper() + k.read() # should error diff --git a/tests/eval_files/async23x.py b/tests/eval_files/async23x.py index 299dd3a..63e9fbd 100644 --- a/tests/eval_files/async23x.py +++ b/tests/eval_files/async23x.py @@ -1,6 +1,6 @@ # type: ignore # ARG --enable=ASYNC230,ASYNC231 -# NOASYNCIO +# NOASYNCIO # see async23x_asyncio.py import io import os diff --git a/tests/eval_files/async23x_asyncio.py b/tests/eval_files/async23x_asyncio.py new file mode 100644 index 0000000..419eb27 --- /dev/null +++ b/tests/eval_files/async23x_asyncio.py @@ -0,0 +1,48 @@ +# type: ignore +# ARG --enable=ASYNC230,ASYNC231 +# NOTRIO # see async23x.py +# NOANYIO # see async23x.py +# BASE_LIBRARY asyncio +import io +import os + +import asyncio + + +async def foo(): + open("") # ASYNC230_asyncio: 4, 'open' + io.open_code("") # ASYNC230_asyncio: 4, 'io.open_code' + os.fdopen(0) # ASYNC231_asyncio: 4, 'os.fdopen' + + # sync call is awaited, so it can't be sync + await open("") + + # asyncio.wrap_file does not exist, so check that it doesn't trigger the + # protection that e.g. `trio.wrap_file` would give. + # In theory we could support detecting equivalent `wrap_file`s from other libraries + await asyncio.wrap_file(open("")) # ASYNC230_asyncio: 28, 'open' + await asyncio.wrap_file(os.fdopen(0)) # ASYNC231_asyncio: 28, 'os.fdopen' + + # with uses the same code & logic + with os.fdopen(0): # ASYNC231_asyncio: 9, 'os.fdopen' + ... + with open(""): # ASYNC230_asyncio: 9, 'open' + ... + with open("") as f: # ASYNC230_asyncio: 9, 'open' + ... + with foo(), open(""): # ASYNC230_asyncio: 16, 'open' + ... + async with open(""): # ASYNC230_asyncio: 15, 'open' + ... + async with asyncio.wrap_file(open("")): # ASYNC230_asyncio: 33, 'open' + ... + + # test io.open + # pyupgrade removes the unnecessary `io.` + # https://github.com/asottile/pyupgrade#open-alias + # and afaict neither respects fmt:off nor #noqa - so I don't know how to test it + open("") # ASYNC230_asyncio: 4, 'open' + + +def foo_sync(): + open("") diff --git a/tests/eval_files/no_library.py b/tests/eval_files/no_library.py index 7db92ba..a79b250 100644 --- a/tests/eval_files/no_library.py +++ b/tests/eval_files/no_library.py @@ -1,6 +1,5 @@ # 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 4658521..71a3627 100644 --- a/tests/eval_files/noqa.py +++ b/tests/eval_files/noqa.py @@ -1,6 +1,5 @@ # AUTOFIX -# NOANYIO # TODO -# NOASYNCIO +# NOASYNCIO - does not trigger on ASYNC100 # ARG --enable=ASYNC100,ASYNC911 from typing import Any diff --git a/tests/eval_files/noqa_testing.py b/tests/eval_files/noqa_testing.py index 746f2c6..1c6ea8f 100644 --- a/tests/eval_files/noqa_testing.py +++ b/tests/eval_files/noqa_testing.py @@ -1,5 +1,4 @@ # AUTOFIX -# NOANYIO # TODO # ARG --enable=ASYNC911 import trio diff --git a/tests/eval_files/trio_anyio.py b/tests/eval_files/trio_anyio.py index 37918f0..5adce03 100644 --- a/tests/eval_files/trio_anyio.py +++ b/tests/eval_files/trio_anyio.py @@ -1,7 +1,6 @@ # type: ignore # ARG --enable=ASYNC220 # NOANYIO -# NOASYNCIO # TODO import trio # isort: skip import anyio # isort: skip diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index e86e548..f18d270 100644 --- a/tests/test_flake8_async.py +++ b/tests/test_flake8_async.py @@ -208,7 +208,7 @@ def find_magic_markers( pattern = rf'# ({"|".join(markers)})' for f in re.findall(pattern, content): if f == "BASE_LIBRARY": - m = re.search(r"# BASE_LIBRARY (\w*)\n", content) + m = re.search(r"# BASE_LIBRARY (\w*)", content) assert m, "invalid 'BASE_LIBRARY' marker" found_markers.BASE_LIBRARY = m.groups()[0] else: @@ -216,12 +216,11 @@ def find_magic_markers( return found_markers -# This could be optimized not to reopen+reread+reparse the same file over and over -# when testing the same file -@pytest.mark.parametrize(("test", "path"), test_files, ids=[f[0] for f in test_files]) +# Caching test file content makes ~0 difference to runtime +@pytest.mark.parametrize("noqa", [False, True], ids=["normal", "noqa"]) @pytest.mark.parametrize("autofix", [False, True], ids=["noautofix", "autofix"]) @pytest.mark.parametrize("library", ["trio", "anyio", "asyncio"]) -@pytest.mark.parametrize("noqa", [False, True], ids=["normal", "noqa"]) +@pytest.mark.parametrize(("test", "path"), test_files, ids=[f[0] for f in test_files]) def test_eval( test: str, path: Path, @@ -232,6 +231,7 @@ def test_eval( ): content = path.read_text() magic_markers = find_magic_markers(content) + # if autofixing, columns may get messed up ignore_column = autofix only_check_not_crash = False @@ -258,15 +258,17 @@ def test_eval( content = re.sub(r"#[\s]*(error|ASYNC\d\d\d):.*", "# noqa", content) expected, parsed_args, enable = _parse_eval_file( - test, content, only_parse_args=only_check_not_crash + test, content, only_check_not_crash=only_check_not_crash ) if library != "trio": parsed_args.insert(0, f"--{library}") if autofix: parsed_args.append(f"--autofix={enable}") - if (library == "anyio" and magic_markers.ANYIO_NO_ERROR) or ( - library == "trio" and magic_markers.TRIO_NO_ERROR + if ( + (library == "anyio" and magic_markers.ANYIO_NO_ERROR) + or (library == "trio" and magic_markers.TRIO_NO_ERROR) + or (library == "asyncio" and magic_markers.ASYNCIO_NO_ERROR) ): expected = [] @@ -277,13 +279,21 @@ def test_eval( args=parsed_args, ignore_column=ignore_column, only_check_not_crash=only_check_not_crash, + noqa=noqa, ) 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"): + # Check that error messages refer to current library, or to no library. + if test not in ( + # 103_[BOTH/ALL]_IMPORTED will contain messages that refer to anyio regardless of + # current library + "ASYNC103_BOTH_IMPORTED", + "ASYNC103_ALL_IMPORTED", + # 23X_asyncio messages does not mention asyncio + "ASYNC23X_ASYNCIO", + ): for error in errors: message = error.message.format(*error.args) assert library in message or not any( @@ -328,7 +338,7 @@ def test_autofix(test: str): def _parse_eval_file( - test: str, content: str, only_parse_args: bool = False + test: str, content: str, only_check_not_crash: bool = False ) -> tuple[list[Error], list[str], str]: # version check check_version(test) @@ -359,9 +369,6 @@ def _parse_eval_file( 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 @@ -399,6 +406,8 @@ def _parse_eval_file( if err_code == "error": err_code = test + if only_check_not_crash and err_code + alt_code not in ERROR_CODES: + continue error_class = ERROR_CODES[err_code + alt_code] message = error_class.error_codes[err_code + alt_code] try: @@ -494,6 +503,7 @@ def assert_expected_errors( args: list[str] | None = None, ignore_column: bool = False, only_check_not_crash: bool = False, + noqa: bool = False, ) -> list[Error]: # initialize default option values initialize_options(plugin, args) @@ -506,9 +516,12 @@ def assert_expected_errors( e.col = -1 if only_check_not_crash: + assert noqa or errors != [], ( + "eval file giving no errors w/ only_check_not_crash, " + "consider adding [library]NOERROR" + ) # Check that this file in fact does report different errors. - # Exclude empty errors+expected_ due to noqa runs. - assert errors != expected_ or errors == expected_ == [], ( + assert errors != expected_ or (errors == expected_ == [] and noqa), ( "eval file appears to give all the correct errors." " Maybe you can remove the `# NO[ANYIO/TRIO/ASYNCIO]` magic marker?" )