python-trio · Zac-HD · Aug 10, 2022 · Aug 9, 2022 · Aug 10, 2022 · Zac-HD
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,9 @@
 # Changelog
 *[CalVer, YY.month.patch](https://calver.org/)*
 
+## Future
+- 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.
+
 ## 22.8.4
 - Fix TRIO108 raising errors on yields in some sync code.
 - TRIO109 now skips all decorated functions to avoid false alarms

diff --git a/README.md b/README.md
@@ -33,3 +33,4 @@ pip install flake8-trio
   Checkpoints are `await`, `async for`, and `async with` (on one of enter/exit).
 - **TRIO109**: Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead
 - **TRIO110**: `while <condition>: await trio.sleep()` should be replaced by a `trio.Event`.
+- **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.
diff --git a/flake8_trio.py b/flake8_trio.py
@@ -11,7 +11,18 @@
 
 import ast
 import tokenize
-from typing import Any, Dict, Iterable, List, NamedTuple, Optional, Set, Union
+from typing import (
+    Any,
+    Dict,
+    Iterable,
+    List,
+    NamedTuple,
+    Optional,
+    Set,
+    Tuple,
+    Union,
+    cast,
+)
 
 # CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
 __version__ = "22.8.4"
@@ -44,6 +55,7 @@
         "`trio.[fail/move_on]_[after/at]` instead"
     ),
     "TRIO110": "`while <condition>: await trio.sleep()` should be replaced by a `trio.Event`.",
+    "TRIO112": "Redundant nursery {}, consider replacing with a regular function call",
 }
 
 
@@ -63,32 +75,21 @@ def __eq__(self, other: Any) -> bool:
         )
 
 
-HasLineInfo = Union[ast.expr, ast.stmt, ast.arg, ast.excepthandler, Statement]
-
-
-class TrioScope:
-    def __init__(self, node: ast.Call, funcname: str):
-        self.node = node
-        self.funcname = funcname
-        self.variable_name: Optional[str] = None
-        self.shielded: bool = False
-        self.has_timeout: bool = True
+HasLineCol = Union[ast.expr, ast.stmt, ast.arg, ast.excepthandler, Statement]
 
-        # scope.shield is assigned to in visit_Assign
 
-        if self.funcname == "CancelScope":
-            self.has_timeout = False
-            for kw in node.keywords:
-                # Only accepts constant values
-                if kw.arg == "shield" and isinstance(kw.value, ast.Constant):
-                    self.shielded = kw.value.value
-                # sets to True even if timeout is explicitly set to inf
-                if kw.arg == "deadline":
-                    self.has_timeou
8000
t = True
-
-    def __str__(self):
-        # Not supporting other ways of importing trio, per TRIO106
-        return f"trio.{self.funcname}"
+def get_matching_call(
+    node: ast.AST, *names: str, base: str = "trio"
+) -> Optional[Tuple[ast.Call, str]]:
+    if (
+        isinstance(node, ast.Call)
+        and isinstance(node.func, ast.Attribute)
+        and isinstance(node.func.value, ast.Name)
+        and node.func.value.id == base
+        and node.func.attr in names
+    ):
+        return node, node.func.attr
+    return None
 
 
 class Error:
@@ -157,7 +158,7 @@ def visit_nodes(
                 for node in arg:
                     visit(node)
 
-    def error(self, error: str, node: HasLineInfo, *args: object):
+    def error(self, error: str, node: HasLineCol, *args: object):
         if not self.suppress_errors:
             self._problems.append(Error(error, node.lineno, node.col_offset, *args))
 
@@ -177,18 +178,6 @@ def walk(self, *body: ast.AST) -> Iterable[ast.AST]:
             yield from ast.walk(b)
 
 
-def get_trio_scope(node: ast.AST, *names: str) -> Optional[TrioScope]:
-    if (
-        isinstance(node, ast.Call)
-        and isinstance(node.func, ast.Attribute)
-        and isinstance(node.func.value, ast.Name)
-        and node.func.value.id == "trio"
-        and node.func.attr in names
-    ):
-        return TrioScope(node, node.func.attr)
-    return None
-
-
 def has_decorator(decorator_list: List[ast.expr], *names: str):
     for dec in decorator_list:
         if (isinstance(dec, ast.Name) and dec.id in names) or (
@@ -211,6 +200,7 @@ def __init__(self):
     def visit_With(self, node: Union[ast.With, ast.AsyncWith]):
         # 100
         self.check_for_trio100(node)
+        self.check_for_trio112(node)
 
         # 101 for rest of function
         outer = self.get_state("_yield_is_error")
@@ -219,7 +209,7 @@ def visit_With(self, node: Union[ast.With, ast.AsyncWith]):
         if not self._safe_decorator:
             for item in (i.context_expr for i in node.items):
                 if (
-                    get_trio_scope(item, "open_nursery", *cancel_scope_names)
+                    get_matching_call(item, "open_nursery", *cancel_scope_names)
                     is not None
                 ):
                     self._yield_is_error = True
@@ -234,14 +224,14 @@ def visit_With(self, node: Union[ast.With, ast.AsyncWith]):
 
     # ---- 100 ----
     def check_for_trio100(self, node: Union[ast.With, ast.AsyncWith]):
-        # Context manager with no `await` call within
+        # Context manager with no `await trio.X` call within
         for item in (i.context_expr for i in node.items):
-            call = get_trio_scope(item, *cancel_scope_names)
+            call = get_matching_call(item, *cancel_scope_names)
             if call and not any(
                 isinstance(x, checkpoint_node_types) and x != node
                 for x in ast.walk(node)
             ):
-                self.error("TRIO100", item, str(call))
+                self.error("TRIO100", item, f"trio.{call[1]}")
 
     # ---- 101 ----
     def visit_FunctionDef(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef]):
@@ -258,7 +248,7 @@ def visit_FunctionDef(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef]):
 
     # ---- 101, 109 ----
     def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef):
-        self.check_109(node)
+        self.check_for_trio109(node)
         self.visit_FunctionDef(node)
 
     # ---- 101 ----
@@ -269,7 +259,7 @@ def visit_Yield(self, node: ast.Yield):
         self.generic_visit(node)
 
     # ---- 109 ----
-    def check_109(self, node: ast.AsyncFunctionDef):
+    def check_for_trio109(self, node: ast.AsyncFunctionDef):
         if node.decorator_list:
             return
         args = node.args
@@ -290,18 +280,48 @@ def visit_Import(self, node: ast.Import):
 
     # ---- 110 ----
     def visit_While(self, node: ast.While):
-        self.check_for_110(node)
+        self.check_for_trio110(node)
         self.generic_visit(node)
 
-    def check_for_110(self, node: ast.While):
+    def check_for_trio110(self, node: ast.While):
         if (
             len(node.body) == 1
             and isinstance(node.body[0], ast.Expr)
             and isinstance(node.body[0].value, ast.Await)
-            and get_trio_scope(node.body[0].value.value, "sleep", "sleep_until")
+            and get_matching_call(node.body[0].value.value, "sleep", "sleep_until")
         ):
             self.error("TRIO110", node)
 
+    # if with has a withitem `trio.open_nursery() as <X>`,
+    # and the body is only a single expression <X>.start[_soon](),
+    # and does not pass <X> as a parameter to the expression
+    def check_for_trio112(self, node: Union[ast.With, ast.AsyncWith]):
+        # body is single expression
+        if len(node.body) != 1 or not isinstance(node.body[0], ast.Expr):
+            return
+        for item in node.items:
+            # get variable name <X>
+            if not isinstance(item.optional_vars, ast.Name):
+                continue
+            var_name = item.optional_vars.id
+
+            # check for trio.open_nursery
+            nursery = get_matching_call(item.context_expr, "open_nursery")
+
+            # isinstance(..., ast.Call) is done in get_matching_call
+            body_call = cast(ast.Call, node.body[0].value)
+
+            if (
+                nursery is not None
+                and get_matching_call(body_call, "start", "start_soon", base=var_name)
+                # check for presence of <X> as parameter
+                and not any(
+                    (isinstance(n, ast.Name) and n.id == var_name)
+                    for n in self.walk(*body_call.args, *body_call.keywords)
+                )
+            ):
+                self.error("TRIO112", item.context_expr, var_name)
+
 
 def critical_except(node: ast.ExceptHandler) -> Optional[Statement]:
     def has_exception(node: Optional[ast.expr]) -> str:
@@ -333,10 +353,30 @@ def has_exception(node: Optional[ast.expr]) -> str:
 
 
 class Visitor102(Flake8TrioVisitor):
+    class TrioScope:
+        def __init__(self, node: ast.Call, funcname: str):
+            self.node = node
+            self.funcname = funcname
+            self.variable_name: Optional[str] = None
+            self.shielded: bool = False
+            self.has_timeout: bool = True
+
+            # scope.shielded is assigned to in visit_Assign
+
+            if self.funcname == "CancelScope":
+                self.has_timeout = False
+                for kw in node.keywords:
+                    # Only accepts constant values
+                    if kw.arg == "shield" and isinstance(kw.value, ast.Constant):
+                        self.shielded = kw.value.value
+                    # sets to True even if timeout is explicitly set to inf
+                    if kw.arg == "deadline":
+                        self.has_timeout = True
+
     def __init__(self):
         super().__init__()
         self._critical_scope: Optional[Statement] = None
-        self._trio_context_managers: List[TrioScope] = []
+        self._trio_context_managers: List[Visitor102.TrioScope] = []
         self._safe_decorator = False
 
     # 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]):
 
         # Check for a `with trio.<scope_creater>`
         for item in node.items:
-            trio_scope = get_trio_scope(
+            call = get_matching_call(
                 item.context_expr, "open_nursery", *cancel_scope_names
             )
-            if trio_scope is None:
+            if call is None:
                 continue
 
-            self._trio_context_managers.append(trio_scope)
-            has_context_manager = True
+            trio_scope = self.TrioScope(*call)
             # check if it's saved in a variable
             if isinstance(item.optional_vars, ast.Name):
                 trio_scope.variable_name = item.optional_vars.id
+
+            self._trio_context_managers.append(trio_scope)
+            has_context_manager = True
             break
 
         self.generic_visit(node)

diff --git a/tests/trio112.py b/tests/trio112.py
@@ -0,0 +1,88 @@
+import trio
+import trio as noterror
+
+# error
+with trio.open_nursery() as n:  # error: 5, "n"
+    n.start(...)
+
+with trio.open_nursery(...) as nurse:  # error: 5, "nurse"
+    nurse.start_soon(...)
+
+with trio.open_nursery() as n:  # error: 5, "n"
+    n.start_soon(n=7)
+
+
+async def foo():
+    async with trio.open_nursery() as n:  # error: 15, "n"
+        n.start(...)
+
+
+# weird ones with multiple `withitem`s
+# but if split among several `with` they'd all be treated as error (or TRIO111), so
+# treating as error for now.
+with trio.open_nursery() as n, trio.open("") as n:  # error: 5, "n"
+    n.start(...)
+
+with open("") as o, trio.open_nursery() as n:  # error: 20, "n"
+    n.start(o)
+
+with trio.open_nursery() as n, trio.open_nursery() as nurse:  # error: 31, "nurse"
+    nurse.start(n.start(...))
+
+with trio.open_nursery() as n, trio.open_nursery() as n:  # error: 5, "n" # error: 31, "n"
+    n.start(...)
+
+# safe if passing variable as parameter
+with trio.open_nursery() as n:
+    n.start(..., n, ...)
+
+with trio.open_nursery() as n:
+    n.start(..., foo=n + 7, bar=...)
+
+with trio.open_nursery() as n:
+    n.start(foo=tuple(tuple(tuple(tuple(n)))))
+
+# safe if multiple lines
+with trio.open_nursery() as n:
+    ...
+    n.start_soon(...)
+
+with trio.open_nursery() as n:
+    n.start_soon(...)
+    ...
+
+# fmt: off
+with trio.open_nursery() as n:
+    n.start_soon(...) ; ...
+# fmt: on
+
+# n as a parameter to lambda is in fact not using it, but we don't parse
+with trio.open_nursery() as n:
+    n.start_soon(lambda n: n + 1)
+
+# body isn't a call to n.start
+async def foo_1():
+    with trio.open_nursery(...) as n:
+        await n.start(...)
+
+
+# not *trio*.open_nursery
+with noterror.open_nursery(...) as n:
+    n.start(...)
+
+# not trio.*open_nursery*
+with trio.not_error(...) as n:
+    n.start(...)
+
+p = trio.not_error
+# not *n*.start[_soon]
+with trio.open_nursery() as n:
+    p.start(...)
+
+# not n.*start[_soon]*
+with trio.open_nursery() as n:
+    n.start_never(...)
+
+# redundant nursery, not handled
+with trio.open_nursery():
+    pass