diff --git a/docs/changelog.rst b/docs/changelog.rst index 57a511f..bf0f20b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,13 +4,17 @@ Changelog *[CalVer, YY.month.patch](https://calver.org/)* +24.5.5 +====== +- Add :ref:`ASYNC300 ` create-task-no-reference + 24.5.4 ====== -- Add ASYNC913: Indefinite loop with no guaranteed checkpoint. -- Fix bugs in ASYNC910 and ASYNC911 autofixing where they sometimes didn't add a library import. -- Fix crash in ASYNC911 when trying to autofix a one-line ``while ...: yield`` +- Add :ref:`ASYNC913 `: Indefinite loop with no guaranteed checkpoint. +- Fix bugs in :ref:`ASYNC910 ` and :ref:`ASYNC911 ` autofixing where they sometimes didn't add a library import. +- Fix crash in :ref:`ASYNC911 ` when trying to autofix a one-line ``while ...: yield`` - Add :ref:`exception-suppress-context-managers`. Contextmanagers that may suppress exceptions. -- ASYNC91x now treats checkpoints inside ``with contextlib.suppress`` as unreliable. +- :ref:`ASYNC91x ` now treats checkpoints inside ``with contextlib.suppress`` as unreliable. 24.5.3 ====== diff --git a/docs/rules.rst b/docs/rules.rst index 64162d7..14e0610 100644 --- a/docs/rules.rst +++ b/docs/rules.rst @@ -6,6 +6,9 @@ Rules General rules ============= +Our ``ASYNC1xx`` rules check for semantic problems ranging from fatal errors (e.g. 101), +to idioms for clearer code (e.g. 116). + _`ASYNC100` : cancel-scope-no-checkpoint A :ref:`timeout_context` does not contain any :ref:`checkpoints `. This makes it pointless, as the timeout can only be triggered by a checkpoint. @@ -71,10 +74,13 @@ _`ASYNC119` : yield-in-cm-in-async-gen We strongly encourage you to read `PEP 533 `_ and use `async with aclosing(...) `_, or better yet avoid async generators entirely (see `ASYNC900`_ ) in favor of context managers which return an iterable :ref:`channel/stream/queue `. - Blocking sync calls in async functions ====================================== +Our 2xx lint rules warn you to use the async equivalent for slow sync calls which +would otherwise block the event loop (and therefore cause performance problems, +or even deadlock). + .. _httpx.Client: https://www.python-httpx.org/api/#client .. _httpx.AsyncClient: https://www.python-httpx.org/api/#asyncclient .. _urllib3: https://github.com/urllib3/urllib3 @@ -126,9 +132,27 @@ ASYNC251 : blocking-sleep Use :func:`trio.sleep`/:func:`anyio.sleep`/:func:`asyncio.sleep`. +Asyncio-specific rules +====================== + +Asyncio *encourages* structured concurrency, with :obj:`asyncio.TaskGroup`, but does not *require* it. +We therefore provide some additional lint rules for common problems - although we'd also recommend a +gradual migration to AnyIO, which is much less error-prone. + +_`ASYNC300` : create-task-no-reference + Calling :func:`asyncio.create_task` without saving the result. A task that isn't referenced elsewhere may get garbage collected at any time, even before it's done. + Note that this rule won't check whether the variable the result is saved in is susceptible to being garbage-collected itself. See the asyncio documentation for best practices. + You might consider instead using a :ref:`TaskGroup ` and calling :meth:`asyncio.TaskGroup.create_task` to avoid this problem, and gain the advantages of structured concurrency with e.g. better cancellation semantics. + + Optional rules disabled by default ================================== +Our 9xx rules check for semantics issues, like 1xx rules, but are disabled by default due +to the higher volume of warnings. We encourage you to enable them - without guaranteed +:ref:`checkpoint`\ s timeouts and cancellation can be arbitrarily delayed, and async +generators are prone to the problems described in :pep:`533`. + _`ASYNC900` : unsafe-async-generator Async generator without :func:`@asynccontextmanager ` not allowed. You might want to enable this on a codebase since async generators are inherently unsafe and cleanup logic might not be performed. diff --git a/flake8_async/__init__.py b/flake8_async/__init__.py index 749584c..587a4ef 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.5.4" +__version__ = "24.5.5" # taken from https://github.com/Zac-HD/shed diff --git a/flake8_async/visitors/helpers.py b/flake8_async/visitors/helpers.py index f1bf871..be39809 100644 --- a/flake8_async/visitors/helpers.py +++ b/flake8_async/visitors/helpers.py @@ -337,11 +337,15 @@ def build_cst_matcher(attr: str) -> m.BaseExpression: return m.Attribute(value=build_cst_matcher(body), attr=m.Name(value=tail)) -def identifier_to_string(attr: cst.Name | cst.Attribute) -> str: +def identifier_to_string(attr: cst.Name | cst.Attribute) -> str | None: if isinstance(attr, cst.Name): return attr.value - assert isinstance(attr.value, (cst.Attribute, cst.Name)) - return identifier_to_string(attr.value) + "." + attr.attr.value + if not isinstance(attr.value, (cst.Attribute, cst.Name)): + return None + lhs = identifier_to_string(attr.value) + if lhs is None: + return None + return lhs + "." + attr.attr.value def with_has_call( @@ -383,10 +387,10 @@ def with_has_call( assert isinstance(item.item, cst.Call) assert isinstance(res["base"], (cst.Name, cst.Attribute)) assert isinstance(res["function"], cst.Name) + base_string = identifier_to_string(res["base"]) + assert base_string is not None, "subscripts should never get matched" res_list.append( - AttributeCall( - item.item, identifier_to_string(res["base"]), res["function"].value - ) + AttributeCall(item.item, base_string, res["function"].value) ) return res_list diff --git a/flake8_async/visitors/visitors.py b/flake8_async/visitors/visitors.py index 58d8d1f..200bb07 100644 --- a/flake8_async/visitors/visitors.py +++ b/flake8_async/visitors/visitors.py @@ -5,8 +5,17 @@ import ast from typing import TYPE_CHECKING, Any, cast -from .flake8asyncvisitor import Flake8AsyncVisitor -from .helpers import disabled_by_default, error_class, get_matching_call, has_decorator +import libcst as cst + +from .flake8asyncvisitor import Flake8AsyncVisitor, Flake8AsyncVisitor_cst +from .helpers import ( + disabled_by_default, + error_class, + error_class_cst, + get_matching_call, + has_decorator, + identifier_to_string, +) if TYPE_CHECKING: from collections.abc import Mapping @@ -332,6 +341,53 @@ def visit_Yield(self, node: ast.Yield): visit_Lambda = visit_AsyncFunctionDef +@error_class_cst +class Visitor300(Flake8AsyncVisitor_cst): + error_codes: Mapping[str, str] = { + "ASYNC300": "asyncio.create_task() called without saving the result" + } + + def __init__(self, *args: Any, **kwargs: Any): + super().__init__(*args, **kwargs) + self.safe_to_create_task: bool = False + + def visit_Assign(self, node: cst.CSTNode): + self.save_state(node, "safe_to_create_task") + self.safe_to_create_task = True + + def visit_CompIf(self, node: cst.CSTNode): + self.save_state(node, "safe_to_create_task") + self.safe_to_create_task = False + + def visit_Call(self, node: cst.Call): + if ( + isinstance(node.func, (cst.Name, cst.Attribute)) + and identifier_to_string(node.func) == "asyncio.create_task" + and not self.safe_to_create_task + ): + self.error(node) + self.visit_Assign(node) + + visit_NamedExpr = visit_Assign + visit_AugAssign = visit_Assign + visit_IfExp_test = visit_CompIf + + # because this is a Flake8AsyncVisitor_cst, we need to manually call restore_state + def leave_Assign( + self, original_node: cst.CSTNode, updated_node: cst.CSTNode + ) -> Any: + self.restore_state(original_node) + return updated_node + + leave_Call = leave_Assign + leave_CompIf = leave_Assign + leave_NamedExpr = leave_Assign + leave_AugAssign = leave_Assign + + def leave_IfExp_test(self, node: cst.IfExp): + self.restore_state(node) + + @error_class @disabled_by_default class Visitor900(Flake8AsyncVisitor): diff --git a/tests/eval_files/async300.py b/tests/eval_files/async300.py new file mode 100644 index 0000000..3a28bfd --- /dev/null +++ b/tests/eval_files/async300.py @@ -0,0 +1,80 @@ +# BASE_LIBRARY asyncio +# TRIO_NO_ERROR +# ANYIO_NO_ERROR + +from typing import Any + +import asyncio + + +def handle_things(*args: object): ... + + +class TaskStorer: + def __init__(self): + self.tasks: set[Any] = set() + + def __ior__(self, obj: object): + self.tasks.add(obj) + + def __iadd__(self, obj: object): + self.tasks.add(obj) + + +async def foo(): + args: Any + asyncio.create_task(*args) # ASYNC300: 4 + + k = asyncio.create_task(*args) + + mylist = [] + mylist.append(asyncio.create_task(*args)) + + handle_things(asyncio.create_task(*args)) + + (l := asyncio.create_task(*args)) + + mylist = [asyncio.create_task(*args)] + + task_storer = TaskStorer() + task_storer |= asyncio.create_task(*args) + task_storer += asyncio.create_task(*args) + + mylist = [asyncio.create_task(*args) for i in range(10)] + + # non-call usage is fine + asyncio.create_task + asyncio.create_task = args + + # more or less esoteric ways of not saving the value + + [asyncio.create_task(*args)] # ASYNC300: 5 + + (asyncio.create_task(*args) for i in range(10)) # ASYNC300: 5 + + args = 1 if asyncio.create_task(*args) else 2 # ASYNC300: 16 + + args = (i for i in range(10) if asyncio.create_task(*args)) # ASYNC300: 36 + + # not supported, it can't be used as a context manager + with asyncio.create_task(*args) as k: # type: ignore[attr-defined] # ASYNC300: 9 + ... + + # import aliasing is not supported (this would raise ASYNC106 bad-async-library-import) + from asyncio import create_task + + create_task(*args) + + # nor is assigning it + boo = asyncio.create_task + boo(*args) + + # or any lambda thing + my_lambda = lambda: asyncio.create_task(*args) + my_lambda(*args) + + # don't crash + + args.nodes[args].append(args) + args[1].nodes() + args[1].abc.nodes() diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index 3520c50..c79dc4f 100644 --- a/tests/test_flake8_async.py +++ b/tests/test_flake8_async.py @@ -476,6 +476,7 @@ def _parse_eval_file( "ASYNC116", "ASYNC117", "ASYNC118", + "ASYNC300", "ASYNC912", }