diff --git a/docs/changelog.rst b/docs/changelog.rst index 2672b08..8ed36ea 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,10 @@ Changelog `CalVer, YY.month.patch `_ +24.10.3 +======= +- Add :ref:`ASYNC124 ` async-function-could-be-sync + 24.10.2 ======= - :ref:`ASYNC102 ` now also warns about ``await()`` inside ``__aexit__``. diff --git a/flake8_async/visitors/visitor91x.py b/flake8_async/visitors/visitor91x.py index 539fcee..ea79e1d 100644 --- a/flake8_async/visitors/visitor91x.py +++ b/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 diff --git a/tests/autofix_files/async124.py b/tests/autofix_files/async124.py new file mode 100644 index 0000000..cc0e8f4 --- /dev/null +++ b/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() diff --git a/tests/autofix_files/async124.py.diff b/tests/autofix_files/async124.py.diff new file mode 100644 index 0000000..e796987 --- /dev/null +++ b/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() diff --git a/tests/eval_files/async124.py b/tests/eval_files/async124.py new file mode 100644 index 0000000..2484c19 --- /dev/null +++ b/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 diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index 9676bbc..4435649 100644 --- a/tests/test_flake8_async.py +++ b/tests/test_flake8_async.py @@ -619,13 +619,14 @@ 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, @@ -633,6 +634,7 @@ def assert_correct_lines_and_codes(errors: Iterable[Error], expected: Iterable[E sep="\n", file=sys.stderr, ) + printed_header = True print( f"| {line:4}",