From 860b83c9988dd07af0b72efe4a1d5b762820b132 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 2 Apr 2025 11:23:30 +0200 Subject: [PATCH 1/2] add ASYNC125 constant-absolute-deadline --- docs/changelog.rst | 4 ++ docs/rules.rst | 8 ++++ docs/usage.rst | 2 +- flake8_async/__init__.py | 2 +- flake8_async/visitors/helpers.py | 16 ++++++-- flake8_async/visitors/visitor102_120.py | 4 +- flake8_async/visitors/visitors.py | 49 +++++++++++++++++++++++-- tests/eval_files/async125.py | 33 +++++++++++++++++ tests/test_flake8_async.py | 1 + 9 files changed, 108 insertions(+), 11 deletions(-) create mode 100644 tests/eval_files/async125.py diff --git a/docs/changelog.rst b/docs/changelog.rst index 0cf1f937..1d299702 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,10 @@ Changelog `CalVer, YY.month.patch `_ +25.4.2 +====== +- Add :ref:`ASYNC125 ` constant-absolute-deadline + 25.4.1 ====== - Add match-case (structural pattern matching) support to ASYNC103, 104, 910, 911 & 912. diff --git a/docs/rules.rst b/docs/rules.rst index 974d99cc..18baec81 100644 --- a/docs/rules.rst +++ b/docs/rules.rst @@ -102,6 +102,14 @@ _`ASYNC124`: async-function-could-be-sync This currently overlaps with :ref:`ASYNC910 ` and :ref:`ASYNC911 ` which, if enabled, will autofix the function to have :ref:`checkpoint`. This excludes class methods as they often have to be async for other reasons, if you really do want to check those you could manually run :ref:`ASYNC910 ` and/or :ref:`ASYNC911 ` and check the methods they trigger on. +_`ASYNC125`: constant-absolute-deadline + Passing constant values (other than :const:`math.inf`) to timeouts expecting absolute + deadlines is nonsensical. These should always be defined relative to + :func:`trio.current_time`/:func:`anyio.current_time`, or you might want to use + :func:`trio.fail_after`/`:func:`trio.move_on_after`/:func:`anyio.fail_after`/ + :func:`anyio.move_on_after`, or the ``relative_deadline`` parameter to + :class:`trio.CancelScope`. + Blocking sync calls in async functions ====================================== diff --git a/docs/usage.rst b/docs/usage.rst index 4ba6d70d..429408e3 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -33,7 +33,7 @@ adding the following to your ``.pre-commit-config.yaml``: minimum_pre_commit_version: '2.9.0' repos: - repo: https://github.com/python-trio/flake8-async - rev: 25.4.1 + rev: 25.4.2 hooks: - id: flake8-async # args: ["--enable=ASYNC100,ASYNC112", "--disable=", "--autofix=ASYNC"] diff --git a/flake8_async/__init__.py b/flake8_async/__init__.py index a1f55466..a477ce30 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__ = "25.4.1" +__version__ = "25.4.2" # taken from https://github.com/Zac-HD/shed diff --git a/flake8_async/visitors/helpers.py b/flake8_async/visitors/helpers.py index 4646fc4f..2203e2c4 100644 --- a/flake8_async/visitors/helpers.py +++ b/flake8_async/visitors/helpers.py @@ -6,6 +6,7 @@ from __future__ import annotations import ast +from dataclasses import dataclass from fnmatch import fnmatch from typing import TYPE_CHECKING, NamedTuple, TypeVar, Union @@ -287,11 +288,20 @@ def has_exception(node: ast.expr) -> str | None: ) +@dataclass +class MatchingCall: + node: ast.Call + name: str + base: str + + def __str__(self) -> str: + return self.base + "." + self.name + + # convenience function used in a lot of visitors -# should probably return a named tuple def get_matching_call( node: ast.AST, *names: str, base: Iterable[str] = ("trio", "anyio") -) -> tuple[ast.Call, str, str] | None: +) -> MatchingCall | None: if isinstance(base, str): base = (base,) if ( @@ -301,7 +311,7 @@ def get_matching_call( and node.func.value.id in base and node.func.attr in names ): - return node, node.func.attr, node.func.value.id + return MatchingCall(node, node.func.attr, node.func.value.id) return None diff --git a/flake8_async/visitors/visitor102_120.py b/flake8_async/visitors/visitor102_120.py index 759161b1..97c665e1 100644 --- a/flake8_async/visitors/visitor102_120.py +++ b/flake8_async/visitors/visitor102_120.py @@ -34,7 +34,7 @@ class Visitor102(Flake8AsyncVisitor): } class TrioScope: - def __init__(self, node: ast.Call, funcname: str, _): + def __init__(self, node: ast.Call, funcname: str): super().__init__() self.node = node self.funcname = funcname @@ -126,7 +126,7 @@ def visit_With(self, node: ast.With | ast.AsyncWith): if call is None: continue - trio_scope = self.TrioScope(*call) + trio_scope = self.TrioScope(call.node, call.name) # check if it's saved in a variable if isinstance(item.optional_vars, ast.Name): trio_scope.variable_name = item.optional_vars.id diff --git a/flake8_async/visitors/visitors.py b/flake8_async/visitors/visitors.py index 92ed2b19..ace5b075 100644 --- a/flake8_async/visitors/visitors.py +++ b/flake8_async/visitors/visitors.py @@ -287,8 +287,7 @@ def visit_Call(self, node: ast.Call): and isinstance(node.args[0], ast.Constant) and node.args[0].value == 0 ): - # m[2] is set to node.func.value.id - self.error(node, m[2]) + self.error(node, m.base) @error_class @@ -326,7 +325,7 @@ def visit_Call(self, node: ast.Call): and arg.value > 86400 ) ): - self.error(node, m[2]) + self.error(node, m.base) @error_class @@ -452,7 +451,49 @@ def visit_Call(self, node: ast.Call): node, "fail_after", "move_on_after", base=("trio", "anyio") ) ): - self.error(node, f"{match[2]}.{match[1]}") + self.error(node, str(match)) + + +@error_class +class Visitor125(Flake8AsyncVisitor): + error_codes: Mapping[str, str] = { + "ASYNC125": ( + "Using {} with a constant value is nonsensical, as the value is relative " + "to the runner clock. Use ``fail_after(...)``, ``move_on_after(...)``, " + "``CancelScope(relative_deadline=...)`` or calculate it relative to " + "``{}.current_time()``." + ) + } + + def visit_Call(self, node: ast.Call): + def is_constant(value: ast.expr) -> bool: + if isinstance(value, ast.Constant): + return True + if isinstance(value, ast.BinOp): + return is_constant(value.left) and is_constant(value.right) + return False + + match = get_matching_call( + node, "fail_at", "move_on_at", "CancelScope", base=("trio", "anyio") + ) + if match is None: + return + + if match.name in ("fail_at", "move_on_at") and len(node.args) == 1: + value = node.args[0] + else: + for kw in node.keywords: + if kw.arg == "deadline": + value = kw.value + break + else: + return + if is_constant(value): + self.error( + value, + str(match), + match.base, + ) @error_class_cst diff --git a/tests/eval_files/async125.py b/tests/eval_files/async125.py new file mode 100644 index 00000000..eed3344c --- /dev/null +++ b/tests/eval_files/async125.py @@ -0,0 +1,33 @@ +import trio +from typing import Final + +# ASYNCIO_NO_ERROR +# anyio.[fail/move_on]_at doesn't exist, but no harm in erroring if we encounter them + +trio.fail_at(5) # ASYNC125: 13, "trio.fail_at", "trio" +trio.fail_at(deadline=5) # ASYNC125: 22, "trio.fail_at", "trio" +trio.move_on_at(10**3) # ASYNC125: 16, "trio.move_on_at", "trio" +trio.fail_at(7 * 3 + 2 / 5 - (8**7)) # ASYNC125: 13, "trio.fail_at", "trio" + +trio.CancelScope(deadline=7) # ASYNC125: 26, "trio.CancelScope", "trio" +trio.CancelScope(shield=True, deadline=7) # ASYNC125: 39, "trio.CancelScope", "trio" + +# we *could* tell them to use math.inf here ... +trio.fail_at(10**1000) # ASYNC125: 13, "trio.fail_at", "trio" + +# _after is fine +trio.fail_after(5) +trio.move_on_after(2.3) + +trio.fail_at(trio.current_time()) +trio.fail_at(trio.current_time() + 7) + +# relative_deadline is fine, though anyio doesn't have it +trio.CancelScope(relative_deadline=7) + +# does not trigger on other "constants".. but we could opt to trigger on +# any all-caps variable, or on :Final +MY_CONST_VALUE = 7 +trio.fail_at(MY_CONST_VALUE) +my_final_value: Final = 3 +trio.fail_at(my_final_value) diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index cf0c995c..adb0d32e 100644 --- a/tests/test_flake8_async.py +++ b/tests/test_flake8_async.py @@ -510,6 +510,7 @@ def _parse_eval_file( "ASYNC121", "ASYNC122", "ASYNC123", + "ASYNC125", "ASYNC300", "ASYNC912", } From 010f170bc229e1d999fb8508627d54b05e4392a4 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 2 Apr 2025 11:29:35 +0200 Subject: [PATCH 2/2] fix rtd --- docs/rules.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules.rst b/docs/rules.rst index 18baec81..d662fcf2 100644 --- a/docs/rules.rst +++ b/docs/rules.rst @@ -103,7 +103,7 @@ _`ASYNC124`: async-function-could-be-sync This excludes class methods as they often have to be async for other reasons, if you really do want to check those you could manually run :ref:`ASYNC910 ` and/or :ref:`ASYNC911 ` and check the methods they trigger on. _`ASYNC125`: constant-absolute-deadline - Passing constant values (other than :const:`math.inf`) to timeouts expecting absolute + Passing constant values (other than :data:`math.inf`) to timeouts expecting absolute deadlines is nonsensical. These should always be defined relative to :func:`trio.current_time`/:func:`anyio.current_time`, or you might want to use :func:`trio.fail_after`/`:func:`trio.move_on_after`/:func:`anyio.fail_after`/