add async124 async-function-could-be-sync

Author: jakkdl

docs/changelog.rst

@@ -4,6 +4,10 @@ Changelog
 
 `CalVer, YY.month.patch <https://calver.org/>`_
 
+24.10.3
+=======
+- Add :ref:`ASYNC124 <async124>` async-function-could-be-sync
+
 24.10.2
 =======
 - :ref:`ASYNC102 <async102>` now also warns about ``await()`` inside ``__aexit__``.

docs/rules.rst

@@ -94,6 +94,12 @@ _`ASYNC123`: bad-exception-group-flattening
     Dropping this information makes diagnosing errors much more difficult.
     We recommend ``raise SomeNewError(...) from group`` if possible; or consider using `copy.copy` to shallow-copy the exception before re-raising (for copyable types), or re-raising the error from outside the `except` block.
 
+_`ASYNC124`: async-function-could-be-sync
+    Triggers when an async function contain none of ``await``, ``async for`` or ``async with``.
+    Calling an async function is slower than calling regular functions, so if possible you
+    might want to convert your function to be synchronous.
+    This currently overlaps with :ref:`ASYNC910 <ASYNC910>` and :ref:`ASYNC911 <ASYNC911>` which, if enabled, will autofix the function to have :ref:`checkpoint`.
+
 Blocking sync calls in async functions
 ======================================
 

flake8_async/visitors/visitor91x.py

@@ -1,8 +1,14 @@
-"""Contains Visitor91X.
+"""Contains Visitor91X and Visitor124.
 
-910 looks for async functions without guaranteed checkpoints (or exceptions), and 911 does
-the same except for async iterables - while also requiring that they checkpoint between
-each yield.
+Visitor91X contains checks for
+* ASYNC100 cancel-scope-no-checkpoint
+* ASYNC910 async-function-no-checkpoint
+* ASYNC911 async-generator-no-checkpoint
+* ASYNC912 cancel-scope-no-guaranteed-checkpoint
+* ASYNC913 indefinite-loop-no-guaranteed-checkpoint
+
+Visitor124 contains
+* ASYNC124 async-function-could-be-sync
 """
 
 from __future__ import annotations
@@ -63,6 +69,47 @@ def func_empty_body(node: cst.FunctionDef) -> bool:
     )
 
 
+@error_class_cst
+class Visitor124(Flake8AsyncVisitor_cst):
+    error_codes: Mapping[str, str] = {
+        "ASYNC124": (
+            "Async function with no `await` could be sync."
+            " Async functions are more expensive to call."
+        )
+    }
+
+    def __init__(self, *args: Any, **kwargs: Any):
+        super().__init__(*args, **kwargs)
+        self.has_await = False
+
+    def visit_FunctionDef(self, node: cst.FunctionDef):
+        # await in sync defs are not valid, but handling this will make ASYNC124
+        # pop up in parent func as if the child function was async
+        self.save_state(node, "has_await", copy=False)
+        self.has_await = False
+
+    def leave_FunctionDef(
+        self, original_node: cst.FunctionDef, updated_node: cst.FunctionDef
+    ) -> cst.FunctionDef:
+        if (
+            original_node.asynchronous is not None
+            and not self.has_await
+            and not func_empty_body(original_node)
+        ):
+            self.error(original_node)
+        self.restore_state(original_node)
+        return updated_node
+
+    def visit_Await(self, node: cst.Await):
+        self.has_await = True
+
+    def visit_With(self, node: cst.With | cst.For):
+        if node.asynchronous is not None:
+            self.has_await = True
+
+    visit_For = visit_With
+
+
 @dataclass
 class LoopState:
     infinite_loop: bool = False

tests/autofix_files/async124.py

@@ -0,0 +1,87 @@
+"""Async function with no awaits could be sync.
+It currently does not care if 910/911 would also be triggered."""
+
+# ARG --enable=ASYNC124,ASYNC910,ASYNC911
+
+# 910/911 will also autofix async124, in the sense of adding a checkpoint. This is perhaps
+# not what the user wants though, so this would be a case in favor of making 910/911 not
+# trigger when async124 does.
+# AUTOFIX
+# ASYNCIO_NO_AUTOFIX
+from typing import Any
+import trio
+
+
+def condition() -> bool:
+    return False
+
+
+async def foo() -> Any:
+    await foo()
+
+
+async def foo_print():  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+    print("hello")
+    await trio.lowlevel.checkpoint()
+
+
+async def conditional_wait():  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+    if condition():
+        await foo()
+    await trio.lowlevel.checkpoint()
+
+
+async def foo_gen():  # ASYNC124: 0  # ASYNC911: 0, "exit", Statement("yield", lineno+1)
+    await trio.lowlevel.checkpoint()
+    yield  # ASYNC911: 4, "yield", Statement("function definition", lineno-1)
+    await trio.lowlevel.checkpoint()
+
+
+async def foo_async_with():
+    async with foo_gen():
+        ...
+
+
+async def foo_async_for():
+    async for i in foo_gen():
+        ...
+
+
+async def foo_nested():  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+    async def foo_nested_2():
+        await foo()
+    await trio.lowlevel.checkpoint()
+
+
+async def foo_nested_sync():  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+    def foo_nested_sync_child():
+        await foo()  # type: ignore[await-not-async]
+    await trio.lowlevel.checkpoint()
+
+
+# We don't want to trigger on empty/pass functions because of inheritance.
+# Uses same logic as async91x.
+
+
+async def foo_empty():
+    "blah"
+    ...
+
+
+async def foo_empty_pass():
+    "foo"
+    pass
+
+
+# we could consider filtering out functions named `test_.*` to not give false alarms on
+# tests that use async fixtures.
+# For ruff and for running through flake8 we could expect users to use per-file-ignores,
+# but running as standalone we don't currently support that. (though probs wouldn't be
+# too bad to add support).
+# See e.g. https://github.com/agronholm/anyio/issues/803 for why one might want an async
+# test without awaits.
+
+
+async def test_async_fixture(my_anyio_fixture):  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+    assert my_anyio_fixture.setup_worked_correctly
+    await trio.lowlevel.checkpoint()

tests/autofix_files/async124.py.diff

@@ -0,0 +1,49 @@
+---
++++
+@@ x,6 x,7 @@
+ # AUTOFIX
+ # ASYNCIO_NO_AUTOFIX
+ from typing import Any
++import trio
+
+
+ def condition() -> bool:
+@@ x,15 x,19 @@
+
+ async def foo_print():  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+     print("hello")
++    await trio.lowlevel.checkpoint()
+
+
+ async def conditional_wait():  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+     if condition():
+         await foo()
++    await trio.lowlevel.checkpoint()
+
+
+ async def foo_gen():  # ASYNC124: 0  # ASYNC911: 0, "exit", Statement("yield", lineno+1)
++    await trio.lowlevel.checkpoint()
+     yield  # ASYNC911: 4, "yield", Statement("function definition", lineno-1)
++    await trio.lowlevel.checkpoint()
+
+
+ async def foo_async_with():
+@@ x,11 x,13 @@
+ async def foo_nested():  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+     async def foo_nested_2():
+         await foo()
++    await trio.lowlevel.checkpoint()
+
+
+ async def foo_nested_sync():  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+     def foo_nested_sync_child():
+         await foo()  # type: ignore[await-not-async]
++    await trio.lowlevel.checkpoint()
+
+
+ # We don't want to trigger on empty/pass functions because of inheritance.
+@@ x,3 x,4 @@
+
+ async def test_async_fixture(my_anyio_fixture):  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+     assert my_anyio_fixture.setup_worked_correctly
++    await trio.lowlevel.checkpoint()

tests/eval_files/async124.py

@@ -0,0 +1,81 @@
+"""Async function with no awaits could be sync.
+It currently does not care if 910/911 would also be triggered."""
+
+# ARG --enable=ASYNC124,ASYNC910,ASYNC911
+
+# 910/911 will also autofix async124, in the sense of adding a checkpoint. This is perhaps
+# not what the user wants though, so this would be a case in favor of making 910/911 not
+# trigger when async124 does.
+# AUTOFIX
+# ASYNCIO_NO_AUTOFIX
+from typing import Any
+
+
+def condition() -> bool:
+    return False
+
+
+async def foo() -> Any:
+    await foo()
+
+
+async def foo_print():  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+    print("hello")
+
+
+async def conditional_wait():  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+    if condition():
+        await foo()
+
+
+async def foo_gen():  # ASYNC124: 0  # ASYNC911: 0, "exit", Statement("yield", lineno+1)
+    yield  # ASYNC911: 4, "yield", Statement("function definition", lineno-1)
+
+
+async def foo_async_with():
+    async with foo_gen():
+        ...
+
+
+async def foo_async_for():
+    async for i in foo_gen():
+        ...
+
+
+async def foo_nested():  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+    async def foo_nested_2():
+        await foo()
+
+
+async def foo_nested_sync():  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+    def foo_nested_sync_child():
+        await foo()  # type: ignore[await-not-async]
+
+
+# We don't want to trigger on empty/pass functions because of inheritance.
+# Uses same logic as async91x.
+
+
+async def foo_empty():
+    "blah"
+    ...
+
+
+async def foo_empty_pass():
+    "foo"
+    pass
+
+
+# we could consider filtering out functions named `test_.*` to not give false alarms on
+# tests that use async fixtures.
+# For ruff and for running through flake8 we could expect users to use per-file-ignores,
+# but running as standalone we don't currently support that. (though probs wouldn't be
+# too bad to add support).
+# See e.g. https://github.com/agronholm/anyio/issues/803 for why one might want an async
+# test without awaits.
+
+
+async def test_async_fixture(
+    my_anyio_fixture,
+):  # ASYNC124: 0  # ASYNC910: 0, "exit", Statement("function definition", lineno)
+    assert my_anyio_fixture.setup_worked_correctly

tests/test_flake8_async.py

@@ -619,20 +619,22 @@ def assert_correct_lines_and_codes(errors: Iterable[Error], expected: Iterable[E
         expected_dict[e.line][e.code] += 1
 
     error_count = 0
+    printed_header = False
     for line in all_lines:
         if error_dict[line] == expected_dict[line]:
             continue
 
         # go through all the codes on the line
         for code in sorted({*error_dict[line], *expected_dict[line]}):
-            if error_count == 0:
+            if not printed_header:
                 print(
                     "Lines with different # of errors:",
                     "-" * 38,
                     f"| line | {'code':8} | actual | expected |",
                     sep="\n",
                     file=sys.stderr,
                 )
+                printed_header = True
 
             print(
                 f"| {line:4}",