8000 Merge pull request #34 from jakkdl/trio112_single_statement_nursery · python-trio/flake8-async@d8acd4d · GitHub
[go: up one dir, main page]
Skip to content

Commit d8acd4d

Browse files 8000
Zac-HDZac-HD
authored
Merge pull request #34 from jakkdl/trio112_single_statement_nursery
2 parents e0f77a7 + c57790b commit d8acd4d

File tree

4 files changed

+186
-52
lines changed

4 files changed

+186
-52
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
# Changelog
22
*[CalVer, YY.month.patch](https://calver.org/)*
33

4+
## Future
5+
- add TRIO112, nursery body with only a call to `nursery.start[_soon]` and not passing itself as a parameter can be replaced with a regular function call.
6+
47
## 22.8.4
58
- Fix TRIO108 raising errors on yields in some sync code.
69
- TRIO109 now skips all decorated functions to avoid false alarms

‎README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,4 @@ pip install flake8-trio
3333
Checkpoints are `await`, `async for`, and `async with` (on one of enter/exit).
3434
- **TRIO109**: Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead
3535
- **TRIO110**: `while <condition>: await trio.sleep()` should be replaced by a `trio.Event`.
36+
- **TRIO112**: nursery body with only a call to `nursery.start[_soon]` and not passing itself as a parameter can be replaced with a regular function call.

‎flake8_trio.py

Lines changed: 94 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,18 @@
1111

1212
import ast
1313
import tokenize
14-
from typing import Any, Dict, Iterable, List, NamedTuple, Optional, Set, Union
14+
from typing import (
15+
Any,
16+
Dict,
17+
Iterable,
18+
List,
19+
NamedTuple,
20+
Optional,
21+
Set,
22+
Tuple,
23+
Union,
24+
cast,
25+
)
1526

1627
# CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
1728
__version__ = "22.8.4"
@@ -44,6 +55,7 @@
4455
"`trio.[fail/move_on]_[after/at]` instead"
4556
),
4657
"TRIO110": "`while <condition>: await trio.sleep()` should be replaced by a `trio.Event`.",
58+
"TRIO112": "Redundant nursery {}, consider replacing with a regular function call",
4759
}
4860

4961

@@ -63,32 +75,21 @@ def __eq__(self, other: Any) -> bool:
6375
)
6476

6577

66-
HasLineInfo = Union[ast.expr, ast.stmt, ast.arg, ast.excepthandler, Statement]
67-
68-
69-
class TrioScope:
70-
def __init__(self, node: ast.Call, funcname: str):
71-
self.node = node
72-
self.funcname = funcname
73-
self.variable_name: Optional[str] = None
74-
self.shielded: bool = False
75-
self.has_timeout: bool = True
78+
HasLineCol = Union[ast.expr, ast.stmt, ast.arg, ast.excepthandler, Statement]
7679

77-
# scope.shield is assigned to in visit_Assign
7880

79-
if self.funcname == "CancelScope":
80-
self.has_timeout = False
81-
for kw in node.keywords:
82-
# Only accepts constant values
83-
if kw.arg == "shield" and isinstance(kw.value, ast.Constant):
84-
self.shielded = kw.value.value
85-
# sets to True even if timeout is explicitly set to inf
86-
if kw.arg == "deadline":
87-
self.has_timeout = True
88-
89-
def __str__(self):
90-
# Not supporting other ways of importing trio, per TRIO106
91-
return f"trio.{self.funcname}"
81+
def get_matching_call(
82+
node: ast.AST, *names: str, base: str = "trio"
83+
) -> Optional[Tuple[ast.Call, str]]:
84+
if (
85+
isinstance(node, ast.Call)
86+
and isinstance(node.func, ast.Attribute)
87+
and isinstance(node.func.value, ast.Name)
88+
and node.func.value.id == base
89+
and node.func.attr in names
90+
):
91+
return node, node.func.attr
92+
return None
9293

9394

9495
class Error:
@@ -157,7 +158,7 @@ def visit_nodes(
157158
for node in arg:
158159
visit(node)
159160

160-
def error(self, error: str, node: HasLineInfo, *args: object):
161+
def error(self, error: str, node: HasLineCol, *args: object):
161162
if not self.suppress_errors:
162163
self._problems.append(Error(error, node.lineno, node.col_offset, *args))
163164

@@ -177,18 +178,6 @@ def walk(self, *body: ast.AST) -> Iterable[ast.AST]:
177178
yield from ast.walk(b)
178179

179180

180-
def get_trio_scope(node: ast.AST, *names: str) -> Optional[TrioScope]:
181-
if (
182-
isinstance(node, ast.Call)
183-
and isinstance(node.func, ast.Attribute)
184-
and isinstance(node.func.value, ast.Name)
185-
and node.func.value.id == "trio"
186-
and node.func.attr in names
187-
):
188-
return TrioScope(node, node.func.attr)
189-
return None
190-
191-
192181
def has_decorator(decorator_list: List[ast.expr], *names: str):
193182
for dec in decorator_list:
194183
if (isinstance(dec, ast.Name) and dec.id in names) or (
@@ -211,6 +200,7 @@ def __init__(self):
211200
def visit_With(self, node: Union[ast.With, ast.AsyncWith]):
212201
# 100
213202
self.check_for_trio100(node)
203+
self.check_for_trio112(node)
214204

215205
# 101 for rest of function
216206
outer = self.get_state("_yield_is_error")
@@ -219,7 +209,7 @@ def visit_With(self, node: Union[ast.With, ast.AsyncWith]):
219209
if not self._safe_decorator:
220210
for item in (i.context_expr for i in node.items):
221211
if (
222-
get_trio_scope(item, "open_nursery", *cancel_scope_names)
212+
get_matching_call(item, "open_nursery", *cancel_scope_names)
223213
is not None
224214
):
225215
self._yield_is_error = True
@@ -234,14 +224,14 @@ def visit_With(self, node: Union[ast.With, ast.AsyncWith]):
234224

235225
# ---- 100 ----
236226
def check_for_trio100(self, node: Union[ast.With, ast.AsyncWith]):
237-
# Context manager with no `await` call within
227+
# Context manager with no `await trio.X` call within
238228
for item in (i.context_expr for i in node.items):
239-
call = get_trio_scope(item, *cancel_scope_names)
229+
call = get_matching_call(item, *cancel_scope_names)
240230
if call and not any(
241231
isinstance(x, checkpoint_node_types) and x != node
242232
for x in ast.walk(node)
243233
):
244-
self.error("TRIO100", item, str(call))
234+
self.error("TRIO100", item, f"trio.{call[1]}")
245235

246236
# ---- 101 ----
247237
def visit_FunctionDef(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef]):
@@ -258,7 +248,7 @@ def visit_FunctionDef(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef]):
258248

259249
# ---- 101, 109 ----
260250
def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef):
261-
self.check_109(node)
251+
self.check_for_trio109(node)
262252
self.visit_FunctionDef(node)
263253

264254
# ---- 101 ----
@@ -269,7 +259,7 @@ def visit_Yield(self, node: ast.Yield):
269259
self.generic_visit(node)
270260

271261
# ---- 109 ----
272 F987 -
def check_109(self, node: ast.AsyncFunctionDef):
262+
def check_for_trio109(self, node: ast.AsyncFunctionDef):
273263
if node.decorator_list:
274264
return
275265
args = node.args
@@ -290,18 +280,48 @@ def visit_Import(self, node: ast.Import):
290280

291281
# ---- 110 ----
292282
def visit_While(self, node: ast.While):
293-
self.check_for_110(node)
283+
self.check_for_trio110(node)
294284
self.generic_visit(node)
295285

296-
def check_for_110(self, node: ast.While):
286+
def check_for_trio110(self, node: ast.While):
297287
if (
298288
len(node.body) == 1
299289
and isinstance(node.body[0], ast.Expr)
300290
and isinstance(node.body[0].value, ast.Await)
301-
and get_trio_scope(node.body[0].value.value, "sleep", "sleep_until")
291+
and get_matching_call(node.body[0].value.value, "sleep", "sleep_until")
302292
):
303293
self.error("TRIO110", node)
304294

295+
# if with has a withitem `trio.open_nursery() as <X>`,
296+
# and the body is only a single expression <X>.start[_soon](),
297+
# and does not pass <X> as a parameter to the expression
298+
def check_for_trio112(self, node: Union[ast.With, ast.AsyncWith]):
299+
# body is single expression
300+
if len(node.body) != 1 or not isinstance(node.body[0], ast.Expr):
301+
return
302+
for item in node.items:
303+
# get variable name <X>
304+
if not isinstance(item.optional_vars, ast.Name):
305+
continue
306+
var_name = item.optional_vars.id
307+
308+
# check for trio.open_nursery
309+
nursery = get_matching_call(item.context_expr, "open_nursery")
310+
311+
# isinstance(..., ast.Call) is done in get_matching_call
312+
body_call = cast(ast.Call, node.body[0].value)
313+
314+
if (
315+
nursery is not None
316+
and get_matching_call(body_call, "start", "start_soon", base=var_name)
317+
# check for presence of <X> as parameter
318+
and not any(
319+
(isinstance(n, ast.Name) and n.id == var_name)
320+
for n in self.walk(*body_call.args, *body_call.keywords)
321+
)
322+
):
323+
self.error("TRIO112", item.context_expr, var_name)
324+
305325

306326
def critical_except(node: ast.ExceptHandler) -> Optional[Statement]:
307327
def has_exception(node: Optional[ast.expr]) -> str:
@@ -333,10 +353,30 @@ def has_exception(node: Optional[ast.expr]) -> str:
333353

334354

335355
class Visitor102(Flake8TrioVisitor):
356+
class TrioScope:
357+
def __init__(self, node: ast.Call, funcname: str):
358+
self.node = node
359+
self.funcname = funcname
360+
self.variable_name: Optional[str] = None
361+
self.shielded: bool = False
362+
self.has_timeout: bool = True
363+
364+
# scope.shielded is assigned to in visit_Assign
365+
366+
if self.funcname == "CancelScope":
367+
self.has_timeout = False
368+
for kw in node.keywords:
369+
# Only accepts constant values
370+
if kw.arg == "shield" and isinstance(kw.value, ast.Constant):
371+
self.shielded = kw.value.value
372+
# sets to True even if timeout is explicitly set to inf
373+
if kw.arg == "deadline":
374+
self.has_timeout = True
375+
336376
def __init__(self):
337377
super().__init__()
338378
self._critical_scope: Optional[Statement] = None
339-
self._trio_context_managers: List[TrioScope] = []
379+
self._trio_context_managers: List[Visitor102.TrioScope] = []
340380
self._safe_decorator = False
341381

342382
# if we're inside a finally, and not inside a context_manager, and we're not
@@ -364,17 +404,19 @@ def visit_With(self, node: Union[ast.With, ast.AsyncWith]):
364404

365405
# Check for a `with trio.<scope_creater>`
366406
for item in node.items:
367-
trio_scope = get_trio_scope(
407+
call = get_matching_call(
368408
item.context_expr, "open_nursery", *cancel_scope_names
369409
)
370-
if trio_scope is None:
410+
if call is None:
371411
continue
372412

373-
self._trio_context_managers.append(trio_scope)
374-
has_context_manager = True
413+
trio_scope = self.TrioScope(*call)
375414
# check if it's saved in a variable
376415
if isinstance(item.optional_vars, ast.Name):
377416
trio_scope.variable_name = item.optional_vars.id
417+
418+
self._trio_context_managers.append(trio_scope)
419+
has_context_manager = True
378420
break
379421

380422
self.generic_visit(node)

‎tests/trio112.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import trio
2+
import trio as noterror
3+
4+
# error
5+
with trio.open_nursery() as n: # error: 5, "n"
6+
n.start(...)
7+
8+
with trio.open_nursery(...) as nurse: # error: 5, "nurse"
9+
nurse.start_soon(...)
10+
11+
with trio.open_nursery() as n: # error: 5, "n"
12+
n.start_soon(n=7)
13+
14+
15+
async def foo():
16+
async with trio.open_nursery() as n: # error: 15, "n"
17+
n.start(...)
18+
19+
20+
# weird ones with multiple `withitem`s
21+
# but if split among several `with` they'd all be treated as error (or TRIO111), so
22+
# treating as error for now.
23+
with trio.open_nursery() as n, trio.open("") as n: # error: 5, "n"
24+
n.start(...)
25+
26+
with open("") as o, trio.open_nursery() as n: # error: 20, "n"
27+
n.start(o)
28+
29+
with trio.open_nursery() as n, trio.open_nursery() as nurse: # error: 31, "nurse"
30+
nurse.start(n.start(...))
31+
32+
with trio.open_nursery() as n, trio.open_nursery() as n: # error: 5, "n" # error: 31, "n"
33+
n.start(...)
34+
35+
# safe if passing variable as parameter
36+
with trio.open_nursery() as n:
37+
n.start(..., n, ...)
38+
39+
with trio.open_nursery() as n:
40+
n.start(..., foo=n + 7, bar=...)
41+
42+
with trio.open_nursery() as n:
43+
n.start(foo=tuple(tuple(tuple(tuple(n)))))
44+
45+
# safe if multiple lines
46+
with trio.open_nursery() as n:
47+
...
48+
n.start_soon(...)
49+
50+
with trio.open_nursery() as n:
51+
n.start_soon(...)
52+
...
53+
54+
# fmt: off
55+
with trio.open_nursery() as n:
56+
n.start_soon(...) ; ...
57+
# fmt: on
58+
59+
# n as a parameter to lambda is in fact not using it, but we don't parse
60+
with trio.open_nursery() as n:
61+
n.start_soon(lambda n: n + 1)
62+
63+
# body isn't a call to n.start
64+
async def foo_1():
65+
with trio.open_nursery(...) as n:
66+
await n.start(...)
67+
68+
69+
# not *trio*.open_nursery
70+
with noterror.open_nursery(...) as n:
71+
n.start(...)
72+
73+
# not trio.*open_nursery*
74+
with trio.not_error(...) as n:
75+
n.start(...)
76+
77+
p = trio.not_error
78+
# not *n*.start[_soon]
79+
with trio.open_nursery() as n:
80+
p.start(...)
81+
82+
# not n.*start[_soon]*
83+
with trio.open_nursery() as n:
84+
n.start_never(...)
85+
86+
# redundant nursery, not handled
87+
with trio.open_nursery():
88+
pass

0 commit comments

Comments
 (0)
0