8000 add ASYNC300 create-task-no-reference (#256) · python-trio/flake8-async@6a35f4a · GitHub
[go: up one dir, main page]
Skip to content

Commit 6a35f4a

Browse files
jakkdlZac-HD
jakkdl
and
Zac-HD
authored
add ASYNC300 create-task-no-reference (#256)
* add ASYNC120 create-task-no-reference * fix crash when subscripts are in attributes * codecov * Rename 120->300, add rules notes --------- Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com>
1 parent 85fe612 commit 6a35f4a

File tree

7 files changed

+183
-14
lines changed

7 files changed

+183
-14
lines changed

‎docs/changelog.rst

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,17 @@ Changelog
44

55
*[CalVer, YY.month.patch](https://calver.org/)*
66

7+
24.5.5
8+
======
9+
- Add :ref:`ASYNC300 <async300>` create-task-no-reference
10+
711
24.5.4
812
======
9-
- Add ASYNC913: Indefinite loop with no guaranteed checkpoint.
10-
- Fix bugs in ASYNC910 and ASYNC911 autofixing where they sometimes didn't add a library import.
11-
- Fix crash in ASYNC911 when trying to autofix a one-line ``while ...: yield``
13+
- Add :ref:`ASYNC913 <async913>`: Indefinite loop with no guaranteed checkpoint.
14+
- Fix bugs in :ref:`ASYNC910 <async910>` and :ref:`ASYNC911 <async911>` autofixing where they sometimes didn't add a library import.
15+
- Fix crash in :ref:`ASYNC911 <async911>` when trying to autofix a one-line ``while ...: yield``
1216
- Add :ref:`exception-suppress-context-managers`. Contextmanagers that may suppress exceptions.
13-
- ASYNC91x now treats checkpoints inside ``with contextlib.suppress`` as unreliable.
17+
- :ref:`ASYNC91x <ASYNC910>` now treats checkpoints inside ``with contextlib.suppress`` as unreliable.
1418

1519
24.5.3
1620
======

‎docs/rules.rst

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ Rules
66
General rules
77
=============
88

9+
Our ``ASYNC1xx`` rules check for semantic problems ranging from fatal errors (e.g. 101),
10+
to idioms for clearer code (e.g. 116).
11+
912
_`ASYNC100` : cancel-scope-no-checkpoint
1013
A :ref:`timeout_context` does not contain any :ref:`checkpoints <checkpoint>`.
1114
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
7174
We strongly encourage you to read `PEP 533 <https://peps.python.org/pep-0533/>`_ and use `async with aclosing(...) <https://docs.python.org/3/library/contextlib.html#contextlib.aclosing>`_, or better yet avoid async generators entirely (see `ASYNC900`_ ) in favor of context managers which return an iterable :ref:`channel/stream/queue <channel_stream_queue>`.
7275

7376

74-
7577
Blocking sync calls in async functions
7678
======================================
7779

80+
Our 2xx lint rules warn you to use the async equivalent for slow sync calls which
81+
would otherwise block the event loop (and therefore cause performance problems,
82+
or even deadlock).
83+
7884
.. _httpx.Client: https://www.python-httpx.org/api/#client
7985
.. _httpx.AsyncClient: https://www.python-httpx.org/api/#asyncclient
8086
.. _urllib3: https://github.com/urllib3/urllib3
@@ -126,9 +132,27 @@ ASYNC251 : blocking-sleep
126132
Use :func:`trio.sleep`/:func:`anyio.sleep`/:func:`asyncio.sleep`.
127133

128134

135+
Asyncio-specific rules
136+
======================
137+
138+
Asyncio *encourages* structured concurrency, with :obj:`asyncio.TaskGroup`, but does not *require* it.
139+
We therefore provide some additional lint rules for common problems - although we'd also recommend a
140+
gradual migration to AnyIO, which is much less error-prone.
141+
142+
_`ASYNC300` : create-task-no-reference
143+
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.
144+
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.
145+
You might consider instead using a :ref:`TaskGroup <taskgroup_nursery>` and calling :meth:`asyncio.TaskGroup.create_task` to avoid this problem, and gain the advantages of structured concurrency with e.g. better cancellation semantics.
146+
147+
129148
Optional rules disabled by default
130149
==================================
131150

151+
Our 9xx rules check for semantics issues, like 1xx rules, but are disabled by default due
152+
to the higher volume of warnings. We encourage you to enable them - without guaranteed
153+
:ref:`checkpoint`\ s timeouts and cancellation can be arbitrarily delayed, and async
154+
generators are prone to the problems described in :pep:`533`.
155+
132156
_`ASYNC900` : unsafe-async-generator
133157
Async generator without :func:`@asynccontextmanager <contextlib.asynccontextmanager>` not allowed.
134158
You might want to enable this on a codebase since async generators are inherently unsafe and cleanup logic might not be performed.

‎flake8_async/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838

3939

4040
# CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
41-
__version__ = "24.5.4"
41+
__version__ = "24.5.5"
4242

4343

4444
# taken from https://github.com/Zac-HD/shed

‎flake8_async/visitors/helpers.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -337,11 +337,15 @@ def build_cst_matcher(attr: str) -> m.BaseExpression:
337337
return m.Attribute(value=build_cst_matcher(body), attr=m.Name(value=tail))
338338

339339

340-
def identifier_to_string(attr: cst.Name | cst.Attribute) -> str:
340+
def identifier_to_string(attr: cst.Name | cst.Attribute) -> str | None:
341341
if isinstance(attr, cst.Name):
342342
return attr.value
343-
assert isinstance(attr.value, (cst.Attribute, cst.Name))
344-
return identifier_to_string(attr.value) + "." + attr.attr.value
343+
if not isinstance(attr.value, (cst.Attribute, cst.Name)):
344+
return None
345+
lhs = identifier_to_string(attr.value)
346+
if lhs is None:
347+
return None
348+
return lhs + "." + attr.attr.value
345349

346350

347351
def with_has_call(
@@ -383,10 +387,10 @@ def with_has_call(
383387
assert isinstance(item.item, cst.Call)
384388
assert isinstance(res["base"], (cst.Name, cst.Attribute))
385389
assert isinstance(res["function"], cst.Name)
390+
base_string = identifier_to_string(res["base"])
391+
assert base_string is not None, "subscripts should never get matched"
386392
res_list.append(
387-
AttributeCall(
388-
item.item, identifier_to_string(res["base"]), res["function"].value
389-
)
393+
AttributeCall(item.item, base_string, res["function"].value)
390394
)
391395
return res_list
392396

‎flake8_async/visitors/visitors.py

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,17 @@
55
import ast
66
from typing import TYPE_CHECKING, Any, cast
77

8-
from .flake8asyncvisitor import Flake8AsyncVisitor
9-
from .helpers import disabled_by_default, error_class, get_matching_call, has_decorator
8+
import libcst as cst
9+
10+
from .flake8asyncvisitor import Flake8AsyncVisitor, Flake8AsyncVisitor_cst
11+
from .helpers import (
12+
disabled_by_default,
13+
error_class,
14+
error_class_cst,
15+
get_matching_call,
16+
has_decorator,
17+
identifier_to_string,
18+
)
1019

1120
if TYPE_CHECKING:
1221
from collections.abc import Mapping
@@ -332,6 +341,53 @@ def visit_Yield(self, node: ast.Yield):
332341
visit_Lambda = visit_AsyncFunctionDef
333342

334343

344+
@error_class_cst
345+
class Visitor300(Flake8AsyncVisitor_cst):
346+
error_codes: Mapping[str, str] = {
347+
"ASYNC300": "asyncio.create_task() called without saving the result"
348+
}
349+
350+
def __init__(self, *args: Any, **kwargs: Any):
351+
super().__init__(*args, **kwargs)
352+
self.safe_to_create_task: bool = False
353+
354+
def visit_Assign(self, node: cst.CSTNode):
355+
self.save_state(node, "safe_to_create_task")
356+
self.safe_to_create_task = True
357+
358+
def visit_CompIf(self, node: cst.CSTNode):
359+
self.save_state(node, "safe_to_create_task")
360+
self.safe_to_create_task = False
361+
362+
def visit_Call(self, node: cst.Call):
363+
if (
364+
isinstance(node.func, (cst.Name, cst.Attribute))
365+
and identifier_to_string(node.func) == "asyncio.create_task"
366+
and not self.safe_to_create_task
367+
):
368+
self.error(node)
369+
self.visit_Assign(node)
370+
371+
visit_NamedExpr = visit_Assign
372+
visit_AugAssign = visit_Assign
373+
visit_IfExp_test = visit_CompIf
374+
375+
# because this is a Flake8AsyncVisitor_cst, we need to manually call restore_state
376+
def leave_Assign(
377+
self, original_node: cst.CSTNode, updated_node: cst.CSTNode
378+
) -> Any:
379+
self.restore_state(original_node)
380+
return updated_node
381+
382+
leave_Call = leave_Assign
383+
leave_CompIf = leave_Assign
384+
leave_NamedExpr = leave_Assign
385+
leave_AugAssign = leave_Assign
386+
387+
def leave_IfExp_test(self, node: cst.IfExp):
388+
self.restore_state(node)
389+
390+
335391
@error_class
336392
@disabled_by_default
337393
class Visitor900(Flake8AsyncVisitor):

‎tests/eval_files/async300.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
# BASE_LIBRARY asyncio
2+
# TRIO_NO_ERROR
3+
# ANYIO_NO_ERROR
4+
5+
from typing import Any
6+
7+
import asyncio
8+
9+
10+
def handle_things(*args: object): ...
11+
12+
13+
class TaskStorer:
14+
def __init__(self):
15+
self.tasks: set[Any] = set()
16+
17+
def __ior__(self, obj: object):
18+
self.tasks.add(obj)
19+
20+
def __iadd__(self, obj: object):
21+
self.tasks.add(obj)
22+
23+
24+
async def foo():
25+
args: Any
26+
asyncio.create_task(*args) # ASYNC300: 4
27+
28+
k = asyncio.create_task(*args)
29+
30+
mylist = []
31+
mylist.append(asyncio.create_task(*args))
32+
33+
handle_things(asyncio.create_task(*args))
34+
35+
(l := asyncio.create_task(*args))
36+
37+
mylist = [asyncio.create_task(*args)]
38+
39+
task_storer = TaskStorer()
40+
task_storer |= asyncio.create_task(*args)
41+
task_storer += asyncio.create_task(*args)
42+
43+
mylist = [asyncio.create_task(*args) for i in range(10)]
44+
45+
# non-call usage is fine
46+
asyncio.create_task
47+
asyncio.create_task = args
48+
49+
# more or less esoteric ways of not saving the value
50+
51+
[asyncio.create_task(*args)] # ASYNC300: 5
52+
53+
(asyncio.create_task(*args) for i in range(10)) # ASYNC300: 5
54+
55+
args = 1 if asyncio.create_task(*args) else 2 # ASYNC300: 16
56+
57+
args = (i for i in range(10) if asyncio.create_task(*args)) # ASYNC300: 36
58+
59+
# not supported, it can't be used as a context manager
60+
with asyncio.create_task(*args) as k: # type: ignore[attr-defined] # ASYNC300: 9
61+
...
62+
63+
# import aliasing is not supported (this would raise ASYNC106 bad-async-library-import)
64+
from asyncio import create_task
65+
66+
create_task(*args)
67+
68+
# nor is assigning it
69+
boo = asyncio.create_task
70+
boo(*args)
71+
72+
# or any lambda thing
73+
my_lambda = lambda: asyncio.create_task(*args)
74+
my_lambda(*args)
75+
76+
# don't crash
77+
78+
args.nodes[args].append(args)
79+
args[1].nodes()
80+
args[1].abc.nodes()

‎tests/test_flake8_async.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,7 @@ def _parse_eval_file(
476476
"ASYNC116",
477477
"ASYNC117&q 50F4 uot;,
478478
"ASYNC118",
479+
"ASYNC300",
479480
"ASYNC912",
480481
}
481482

0 commit comments

Comments
 (0)
0