From 4578f06221437f5221ac6fa8017bf83c72be8bb6 Mon Sep 17 00:00:00 2001 From: Zac Hatfield-Dodds Date: Mon, 27 May 2024 10:46:35 -0700 Subject: [PATCH] Rename 120->300, add rules notes --- docs/changelog.rst | 2 +- docs/rules.rst | 30 +++++++++++++++---- flake8_async/visitors/visitors.py | 4 +-- tests/eval_files/{async120.py => async300.py} | 12 ++++---- tests/test_flake8_async.py | 2 +- 5 files changed, 35 insertions(+), 15 deletions(-) rename tests/eval_files/{async120.py => async300.py} (85%) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4c12398..bf0f20b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,7 +6,7 @@ Changelog 24.5.5 ====== -- Add :ref:`ASYNC120 ` create-task-no-reference +- Add :ref:`ASYNC300 ` create-task-no-reference 24.5.4 ====== diff --git a/docs/rules.rst b/docs/rules.rst index d05490b..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. @@ -70,15 +73,14 @@ _`ASYNC119` : yield-in-cm-in-async-gen ``yield`` in context manager in async generator is unsafe, the cleanup may be delayed until ``await`` is no longer allowed. 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 `. -_`ASYNC120` : 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. - 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 @@ -130,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/visitors/visitors.py b/flake8_async/visitors/visitors.py index ccc9780..200bb07 100644 --- a/flake8_async/visitors/visitors.py +++ b/flake8_async/visitors/visitors.py @@ -342,9 +342,9 @@ def visit_Yield(self, node: ast.Yield): @error_class_cst -class Visitor120(Flake8AsyncVisitor_cst): +class Visitor300(Flake8AsyncVisitor_cst): error_codes: Mapping[str, str] = { - "ASYNC120": "asyncio.create_task() called without saving the result" + "ASYNC300": "asyncio.create_task() called without saving the result" } def __init__(self, *args: Any, **kwargs: Any): diff --git a/tests/eval_files/async120.py b/tests/eval_files/async300.py similarity index 85% rename from tests/eval_files/async120.py rename to tests/eval_files/async300.py index 5f04040..3a28bfd 100644 --- a/tests/eval_files/async120.py +++ b/tests/eval_files/async300.py @@ -23,7 +23,7 @@ def __iadd__(self, obj: object): async def foo(): args: Any - asyncio.create_task(*args) # ASYNC120: 4 + asyncio.create_task(*args) # ASYNC300: 4 k = asyncio.create_task(*args) @@ -48,16 +48,16 @@ async def foo(): # more or less esoteric ways of not saving the value - [asyncio.create_task(*args)] # ASYNC120: 5 + [asyncio.create_task(*args)] # ASYNC300: 5 - (asyncio.create_task(*args) for i in range(10)) # ASYNC120: 5 + (asyncio.create_task(*args) for i in range(10)) # ASYNC300: 5 - args = 1 if asyncio.create_task(*args) else 2 # ASYNC120: 16 + args = 1 if asyncio.create_task(*args) else 2 # ASYNC300: 16 - args = (i for i in range(10) if asyncio.create_task(*args)) # ASYNC120: 36 + 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] # ASYNC120: 9 + 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) diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index f7dcb68..c79dc4f 100644 --- a/tests/test_flake8_async.py +++ b/tests/test_flake8_async.py @@ -476,7 +476,7 @@ def _parse_eval_file( "ASYNC116", "ASYNC117", "ASYNC118", - "ASYNC120", + "ASYNC300", "ASYNC912", }