diff --git a/docs/changelog.rst b/docs/changelog.rst index 568319d..2e3cc35 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,10 @@ Changelog `CalVer, YY.month.patch `_ +25.5.1 +====== +- Fixed :ref:`ASYNC113 ` false alarms if the ``start_soon`` calls are in a nursery cm that was closed before the yield point. + 25.4.4 ====== - :ref:`ASYNC900 ` now accepts and recommends :func:`trio.as_safe_channel`. diff --git a/docs/usage.rst b/docs/usage.rst index b76bbf8..407ffba 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.4 + rev: 25.5.1 hooks: - id: flake8-async # args: ["--enable=ASYNC100,ASYNC112", "--disable=", "--autofix=ASYNC"] diff --git a/flake8_async/__init__.py b/flake8_async/__init__.py index b9a1d72..ff25126 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.4" +__version__ = "25.5.1" # taken from https://github.com/Zac-HD/shed diff --git a/flake8_async/visitors/visitors.py b/flake8_async/visitors/visitors.py index 30cb237..f77f2e6 100644 --- a/flake8_async/visitors/visitors.py +++ b/flake8_async/visitors/visitors.py @@ -3,6 +3,7 @@ from __future__ import annotations import ast +from collections import defaultdict from typing import TYPE_CHECKING, Any, cast from .flake8asyncvisitor import Flake8AsyncVisitor, Flake8AsyncVisitor_cst @@ -181,20 +182,25 @@ def __init__(self, *args: Any, **kwargs: Any): self.asynccontextmanager = False self.aenter = False + self.potential_errors: dict[str, list[ast.Call]] = defaultdict(list) + def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef): - self.save_state(node, "aenter") + self.save_state(node, "aenter", "asynccontextmanager", "potential_errors") - self.aenter = node.name == "__aenter__" or has_decorator( - node, "asynccontextmanager" - ) + self.aenter = node.name == "__aenter__" + self.asynccontextmanager = has_decorator(node, "asynccontextmanager") def visit_FunctionDef(self, node: ast.FunctionDef): - self.save_state(node, "aenter") + self.save_state(node, "aenter", "asynccontextmanager", "potential_errors") # sync function should never be named __aenter__ or have @asynccontextmanager - self.aenter = False + self.aenter = self.asynccontextmanager = False def visit_Yield(self, node: ast.Yield): - self.aenter = False + for nodes in self.potential_errors.values(): + for n in nodes: + self.error(n) + self.potential_errors.clear() + self.aenter = self.asynccontextmanager = False def visit_Call(self, node: ast.Call) -> None: def is_startable(n: ast.expr, *startable_list: str) -> bool: @@ -210,14 +216,14 @@ def is_startable(n: ast.expr, *startable_list: str) -> bool: return any(is_startable(nn, *startable_list) for nn in n.args) return False - def is_nursery_call(node: ast.expr): + def is_nursery_call(node: ast.expr) -> str | None: if not isinstance(node, ast.Attribute) or node.attr not in ( "start_soon", "create_task", ): - return False + return None var = ast.unparse(node.value) - return ( + if ( ("trio" in self.library and var.endswith("nursery")) or ("anyio" in self.library and var.endswith("task_group")) or ( @@ -228,11 +234,12 @@ def is_nursery_call(node: ast.expr): "asyncio.TaskGroup", ) ) - ) + ): + return var + return None if ( - self.aenter - and is_nursery_call(node.func) + (var := is_nursery_call(node.func)) is not None and len(node.args) > 0 and is_startable( node.args[0], @@ -241,7 +248,24 @@ def is_nursery_call(node: ast.expr): *self.options.startable_in_context_manager, ) ): - self.error(node) + if self.aenter: + self.error(node) + elif self.asynccontextmanager: + self.potential_errors[var].append(node) + + def visit_AsyncWith(self, node: ast.AsyncWith | ast.With): + # Entirely skip any nurseries that doesn't have any yields in them. + # This fixes an otherwise very thorny false alarm. + # In the worst case this does mean we iterate over the body twice, but might + # actually be a performance gain on average due to setting `novisit` + if not any(isinstance(n, ast.Yield) for b in node.body for n in ast.walk(b)): + self.novisit = True + return + + # open_nursery/create_task_group only works with AsyncWith, but in case somebody + # is doing something very weird we'll be conservative and possibly avoid + # some potential false alarms + visit_With = visit_AsyncWith # Checks that all async functions with a "task_status" parameter have a match in diff --git a/tests/eval_files/async113.py b/tests/eval_files/async113.py index bd45446..815733b 100644 --- a/tests/eval_files/async113.py +++ b/tests/eval_files/async113.py @@ -24,9 +24,18 @@ async def foo(): # we don't check for `async with` with trio.open_nursery() as bar: # type: ignore[attr-defined] bar.start_soon(my_startable) # ASYNC113: 8 + yield + + +@asynccontextmanager +async def foo2(): async with trio.open_nursery() as bar: bar.start_soon(my_startable) # ASYNC113: 8 + yield + +@asynccontextmanager +async def foo3(): boo: trio.Nursery = ... # type: ignore boo.start_soon(my_startable) # ASYNC113: 4 @@ -132,3 +141,52 @@ def non_async_func(): bar.start_soon(my_startable) yield + + +@asynccontextmanager +async def false_alarm(): + async with trio.open_nursery() as nursery: + nursery.start_soon(my_startable) + yield + + +@asynccontextmanager +async def should_error(): + async with trio.open_nursery() as nursery: + nursery.start_soon(my_startable) # ASYNC113: 8 + # overrides the nursery variable + async with trio.open_nursery() as nursery: + nursery.start_soon(my_startable) + yield + + +@asynccontextmanager +async def foo_sync_with_closed(): + # we don't check for `async with` + with trio.open_nursery() as bar: # type: ignore[attr-defined] + bar.start_soon(my_startable) + yield + + +# fixed by entirely skipping nurseries without yields in them +class FalseAlarm: + async def __aenter__(self): + with trio.open_nursery() as nursery: + nursery.start_soon(my_startable) + + +@asynccontextmanager +async def yield_before_start_soon(): + with trio.open_nursery() as bar: + yield + bar.start_soon(my_startable) + + +# This was broken when visit_AsyncWith manually visited subnodes due to not +# letting TypeTrackerVisitor interject. +@asynccontextmanager +async def nested(): + with trio.open_nursery() as foo: + with trio.open_nursery() as bar: + bar.start_soon(my_startable) # error: 12 + yield diff --git a/tests/eval_files/async113_trio.py b/tests/eval_files/async113_trio.py index c538b43..ff089bd 100644 --- a/tests/eval_files/async113_trio.py +++ b/tests/eval_files/async113_trio.py @@ -132,8 +132,10 @@ async def foo4(): ... @asynccontextmanager() # type: ignore[call-arg] async def foo_paren(): nursery.start_soon(trio.run_process) # error: 4 + yield @asynccontextmanager(1, 2, 3) # type: ignore[call-arg] async def foo_params(): nursery.start_soon(trio.run_process) # error: 4 + yield