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 <[email protected]>
python-trio · Mar 8, 2024 · 8b1a1cf · 8b1a1cf
1 parent fe751bc
commit 8b1a1cf
Show file tree

Hide file tree

Showing 42 changed files with 576 additions and 77 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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>.*)", 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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