diff --git a/docs/changelog.rst b/docs/changelog.rst
index 2672b08..496d2f8 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -4,6 +4,12 @@ Changelog
`CalVer, YY.month.patch `_
+24.10.3
+=======
+- Add :ref:`ASYNC124 ` async-function-could-be-sync
+- :ref:`ASYNC91x ` now correctly handles ``await()`` in parameter lists.
+- Fixed a bug with :ref:`ASYNC91x ` and nested empty functions.
+
24.10.2
=======
- :ref:`ASYNC102 ` now also warns about ``await()`` inside ``__aexit__``.
diff --git a/docs/rules.rst b/docs/rules.rst
index c3b7566..da3b943 100644
--- a/docs/rules.rst
+++ b/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 ` and :ref:`ASYNC911 ` which, if enabled, will autofix the function to have :ref:`checkpoint`.
+
Blocking sync calls in async functions
======================================
diff --git a/flake8_async/__init__.py b/flake8_async/__init__.py
index 83c5d57..d36800f 100644
--- a/flake8_async/__init__.py
+++ b/flake8_async/__init__.py
@@ -38,7 +38,7 @@
# CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
-__version__ = "24.10.2"
+__version__ = "24.10.3"
# taken from https://github.com/Zac-HD/shed
diff --git a/flake8_async/visitors/visitor91x.py b/flake8_async/visitors/visitor91x.py
index 539fcee..613cdf9 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,97 @@ def func_empty_body(node: cst.FunctionDef) -> bool:
)
+# this could've been implemented as part of visitor91x, but /shrug
+@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
+ self.in_class = False
+
+ def visit_ClassDef(self, node: cst.ClassDef):
+ self.save_state(node, "in_class", copy=False)
+ self.in_class = True
+
+ def leave_ClassDef(
+ self, original_node: cst.ClassDef, updated_node: cst.ClassDef
+ ) -> cst.ClassDef:
+ self.restore_state(original_node)
+ return updated_node
+
+ # await in sync defs are not valid, but handling this will make ASYNC124
+ # correctly pop up in parent func as if the child function was async
+ def visit_FunctionDef(self, node: cst.FunctionDef):
+ # default values are evaluated in parent scope
+ # this visitor has no autofixes, so we can throw away return value
+ _ = node.params.visit(self)
+
+ self.save_state(node, "has_await", "in_class", copy=False)
+
+ # ignore class methods
+ self.has_await = self.in_class
+
+ # but not nested functions
+ self.in_class = False
+
+ _ = node.body.visit(self)
+
+ # we've manually visited subnodes (that we care about).
+ return 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)
+ and not func_has_decorator(original_node, "overload")
+ # skip functions named 'text_xxx' with params, since they may be relying
+ # on async fixtures. This is esp bad as sync funcs relying on async fixtures
+ # is not well handled: https://github.com/pytest-dev/pytest/issues/10839
+ # also skip funcs with @fixture and params
+ and not (
+ original_node.params.params
+ and (
+ original_node.name.value.startswith("test_")
+ or func_has_decorator(original_node, "fixture")
+ )
+ )
+ # ignore functions with no_checkpoint_warning_decorators
+ and not fnmatch_qualified_name_cst(
+ original_node.decorators, *self.options.no_checkpoint_warning_decorators
+ )
+ ):
+ 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 | cst.CompFor):
+ if node.asynchronous is not None:
+ self.has_await = True
+
+ visit_For = visit_With
+ visit_CompFor = visit_With
+
+ # The generator target will be immediately evaluated, but the other
+ # elements will not be evaluated at the point of defining the GenExp.
+ # To consume those needs an explicit syntactic checkpoint
+ def visit_GeneratorExp(self, node: cst.GeneratorExp):
+ node.for_in.iter.visit(self)
+ return False
+
+
@dataclass
class LoopState:
infinite_loop: bool = False
@@ -274,7 +371,6 @@ class Visitor91X(Flake8AsyncVisitor_cst, CommonVisitors):
def __init__(self, *args: Any, **kwargs: Any):
super().__init__(*args, **kwargs)
self.has_yield = False
- self.safe_decorator = False
self.async_function = False
self.uncheckpointed_statements: set[Statement] = set()
self.comp_unknown = False
@@ -289,6 +385,9 @@ def __init__(self, *args: Any, **kwargs: Any):
# --exception-suppress-context-manager
self.suppress_imported_as: list[str] = []
+ # used to transfer new body between visit_FunctionDef and leave_FunctionDef
+ self.new_body: cst.BaseSuite | None = None
+
def should_autofix(self, node: cst.CSTNode, code: str | None = None) -> bool:
if code is None: # pragma: no branch
code = "ASYNC911" if self.has_yield else "ASYNC910"
@@ -329,6 +428,10 @@ def visit_ImportFrom(self, node: cst.ImportFrom) -> None:
return
def visit_FunctionDef(self, node: cst.FunctionDef) -> bool:
+ # `await` in default values happen in parent scope
+ # we also know we don't ever modify parameters so we can ignore the return value
+ _ = node.params.visit(self)
+
# don't lint functions whose bodies solely consist of pass or ellipsis
# @overload functions are also guaranteed to be empty
# we also ignore pytest fixtures
@@ -338,7 +441,6 @@ def visit_FunctionDef(self, node: cst.FunctionDef) -> bool:
self.save_state(
node,
"has_yield",
- "safe_decorator",
"async_function",
"uncheckpointed_statements",
"loop_state",
@@ -349,7 +451,7 @@ def visit_FunctionDef(self, node: cst.FunctionDef) -> bool:
)
self.uncheckpointed_statements = set()
self.has_checkpoint_stack = []
- self.has_yield = self.safe_decorator = False
+ self.has_yield = False
self.loop_state = LoopState()
self.async_function = (
@@ -358,36 +460,49 @@ def visit_FunctionDef(self, node: cst.FunctionDef) -> bool:
node.decorators, *self.options.no_checkpoint_warning_decorators
)
)
- if not self.async_function:
- # only visit subnodes if there is an async function defined inside
- # this should improve performance on codebases with many sync functions
- return any(m.findall(node, m.FunctionDef(asynchronous=m.Asynchronous())))
+ # only visit subnodes if there is an async function defined inside
+ # this should improve performance on codebases with many sync functions
+ if not self.async_function and not any(
+ m.findall(node, m.FunctionDef(asynchronous=m.Asynchronous()))
+ ):
+ return False
pos = self.get_metadata(PositionProvider, node).start # type: ignore
self.uncheckpointed_statements = {
Statement("function definition", pos.line, pos.column) # type: ignore
}
- return True
+
+ # visit body
+ # we're not gonna get FlattenSentinel or RemovalSentinel
+ self.new_body = cast(cst.BaseSuite, node.body.visit(self))
+
+ # we know that leave_FunctionDef for this FunctionDef will run immediately after
+ # this function exits so we don't need to worry about save_state for new_body
+ return False
def leave_FunctionDef(
self, original_node: cst.FunctionDef, updated_node: cst.FunctionDef
) -> cst.FunctionDef:
if (
- self.async_function
+ self.new_body is not None
+ and self.async_function
# updated_node does not have a position, so we must send original_node
and self.check_function_exit(original_node)
and self.should_autofix(original_node)
- and isinstance(updated_node.body, cst.IndentedBlock)
+ and isinstance(self.new_body, cst.IndentedBlock)
):
# insert checkpoint at the end of body
- new_body = list(updated_node.body.body)
- new_body.append(self.checkpoint_statement())
- indentedblock = updated_node.body.with_changes(body=new_body)
- updated_node = updated_node.with_changes(body=indentedblock)
+ new_body_block = list(self.new_body.body)
+ new_body_block.append(self.checkpoint_statement())
+ self.new_body = self.new_body.with_changes(body=new_body_block)
self.ensure_imported_library()
+ if self.new_body is not None:
+ updated_node = updated_node.with_changes(body=self.new_body)
self.restore_state(original_node)
+ # reset self.new_body
+ self.new_body = None
return updated_node
# error if function exit/return/yields with uncheckpointed statements
@@ -978,8 +1093,8 @@ def visit_CompFor(self, node: cst.CompFor):
return False
# The generator target will be immediately evaluated, but the other
- # elements will be lazily evaluated as the generator is consumed so we don't
- # visit them as any checkpoints in them are not guaranteed to execute.
+ # elements will not be evaluated at the point of defining the GenExp.
+ # To consume those needs an explicit syntactic checkpoint
def visit_GeneratorExp(self, node: cst.GeneratorExp):
node.for_in.iter.visit(self)
return False
diff --git a/tests/autofix_files/async910.py b/tests/autofix_files/async910.py
index e8bcf77..4d97ec3 100644
--- a/tests/autofix_files/async910.py
+++ b/tests/autofix_files/async910.py
@@ -134,9 +134,12 @@ def foo_normal_func_1():
def foo_normal_func_2(): ...
-# overload decorator
+# overload decorator is skipped
+# overload functions should always be empty, so the check is somewhat redundant,
+# but making one non-empty to check the logic.
@overload
-async def foo_overload_1(_: bytes): ...
+async def foo_overload_1(_: bytes):
+ raise NotImplementedError
@typing.overload
@@ -616,3 +619,19 @@ async def fn_226(): # error: 0, "exit", Statement("function definition", lineno
except Exception:
pass
await trio.lowlevel.checkpoint()
+
+
+# the await() is evaluated in the parent scope
+async def foo_default_value_await():
+ async def bar( # error: 4, "exit", Statement("function definition", lineno)
+ arg=await foo(),
+ ):
+ print()
+ await trio.lowlevel.checkpoint()
+
+
+async def foo_nested_empty_async():
+ # this previously errored because leave_FunctionDef assumed a non-empty body
+ async def bar(): ...
+
+ await foo()
diff --git a/tests/autofix_files/async910.py.diff b/tests/autofix_files/async910.py.diff
index 8974f7f..3fb6131 100644
--- a/tests/autofix_files/async910.py.diff
+++ b/tests/autofix_files/async910.py.diff
@@ -213,8 +213,19 @@
# Issue #226
-@@ x,3 x,4 @@
+@@ x,6 x,7 @@
pass
except Exception:
pass
+ await trio.lowlevel.checkpoint()
+
+
+ # the await() is evaluated in the parent scope
+@@ x,6 x,7 @@
+ arg=await foo(),
+ ):
+ print()
++ await trio.lowlevel.checkpoint()
+
+
+ async def foo_nested_empty_async():
diff --git a/tests/eval_files/async124.py b/tests/eval_files/async124.py
new file mode 100644
index 0000000..5815f16
--- /dev/null
+++ b/tests/eval_files/async124.py
@@ -0,0 +1,146 @@
+"""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
+# ARG --no-checkpoint-warning-decorator=custom_disabled_decorator
+
+# 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.
+# NOAUTOFIX # all errors get "fixed" except for foo_fix_no_subfix
+from typing import Any, overload
+from pytest import fixture
+
+custom_disabled_decorator: 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
+
+
+# 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( # ASYNC910: 0, "exit", Statement("function definition", lineno)
+ my_async_fixture,
+):
+ assert my_async_fixture.setup_worked_correctly
+
+
+# no params -> no async fixtures
+async def test_no_fixture(): # ASYNC124: 0 # ASYNC910: 0, "exit", Statement("function definition", lineno)
+ print("blah")
+
+
+# skip @overload. They should always be empty, but /shrug
+@overload
+async def foo_overload():
+ print("blah")
+
+
+async def foo_overload(): ...
+
+
+# skip @[pytest.]fixture if they have any params, since they might depend on other
+# async fixtures
+@fixture
+async def foo_fix(my_async_fixture):
+ print("blah")
+
+
+# but @fixture with no params can be converted to sync
+@fixture
+async def foo_fix_no_subfix(): # ASYNC124: 0
+ print("blah")
+
+
+async def default_value():
+ def foo(arg=await foo()): ...
+
+
+# only the expression in genexp's get checked
+async def foo_async_gen(): # ASYNC124: 0
+ return ( # ASYNC910: 4, "return", Statement("function definition", lineno-1)
+ await a async for a in foo_gen()
+ )
+
+
+async def foo_async_gen_await():
+ return (a for a in await foo_gen())
+
+
+async def foo_async_for_comprehension():
+ return [a async for a in foo_gen()]
+
+
+class Foo:
+ # async124 ignores class methods
+ async def bar( # ASYNC910: 4, "exit", Statement("function definition", lineno)
+ self,
+ ):
+ async def bee(): # ASYNC124: 8 # ASYNC910: 8, "exit", Statement("function definition", lineno)
+ print("blah")
+
+ async def later_in_class( # ASYNC910: 4, "exit", Statement("function definition", lineno)
+ self,
+ ):
+ print()
+
+
+async def after_class(): # ASYNC124: 0 # ASYNC910: 0, "exit", Statement("function definition", lineno)
+ print()
+
+
+@custom_disabled_decorator
+async def foo_has_custom_disabled_decorator():
+ print()
diff --git a/tests/eval_files/async910.py b/tests/eval_files/async910.py
index 4853437..f8d7680 100644
--- a/tests/eval_files/async910.py
+++ b/tests/eval_files/async910.py
@@ -126,9 +126,12 @@ def foo_normal_func_1():
def foo_normal_func_2(): ...
-# overload decorator
+# overload decorator is skipped
+# overload functions should always be empty, so the check is somewhat redundant,
+# but making one non-empty to check the logic.
@overload
-async def foo_overload_1(_: bytes): ...
+async def foo_overload_1(_: bytes):
+ raise NotImplementedError
@typing.overload
@@ -586,3 +589,18 @@ async def fn_226(): # error: 0, "exit", Statement("function definition", lineno
pass
except Exception:
pass
+
+
+# the await() is evaluated in the parent scope
+async def foo_default_value_await():
+ async def bar( # error: 4, "exit", Statement("function definition", lineno)
+ arg=await foo(),
+ ):
+ print()
+
+
+async def foo_nested_empty_async():
+ # this previously errored because leave_FunctionDef assumed a non-empty body
+ async def bar(): ...
+
+ await foo()
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}",
diff --git a/tests/trio_options.py b/tests/trio_options.py
index 34d9e68..daad4f8 100644
--- a/tests/trio_options.py
+++ b/tests/trio_options.py
@@ -1,8 +1,15 @@
"""Test file used in tests/test_decator.py to check decorator and command line."""
+import asyncio
+
app = None
+def condition() -> bool:
+ return False
+
+
@app.route # type: ignore
async def f():
- print("hello world")
+ if condition():
+ await asyncio.sleep(0)