Merge remote-tracking branch 'origin/main' into 7_async_iterable_checkpoints

Author: jakkdl

CHANGELOG.md

@@ -4,6 +4,10 @@
 ## Future
 - Extend TRIO107 and 108 to also handle yields
 
+## 22.8.1
+- Added TRIO109: Async definitions should not have a `timeout` parameter. Use `trio.[fail/move_on]_[at/after]`
+- Added TRIO110: `while <condition>: await trio.sleep()` should be replaced by a `trio.Event`.
+
 ## 22.7.6
 - Extend TRIO102 to also check inside `except BaseException` and `except trio.Cancelled`
 - Extend TRIO104 to also check for `yield`

README.md

@@ -28,6 +28,12 @@ pip install flake8-trio
 - **TRIO104**: `Cancelled` and `BaseException` must be re-raised - when a user tries to `return` or `raise` a different exception.
 - **TRIO105**: Calling a trio async function without immediately `await`ing it.
 - **TRIO106**: trio must be imported with `import trio` for the linter to work.
+<!-- TODO: fix
 - **TRIO107**: Async functions and iterables must have at least one checkpoint on every code path, unless an exception is raised.
 - **TRIO108**: `return` or `yield` from async function must have at least one checkpoint on every code path before it, unless an exception is raised.
-  Checkpoints are `await`, `async for`, and `async with` (on one of enter/exit).
+# - **TRIO107**: Async functions must have at least one checkpoint on every code path, unless an exception is raised.
+# - **TRIO108**: Early return from async function must have at least one checkpoint on every code path before it, unless an exception is raised.
+  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`.

flake8_trio.py

@@ -14,7 +14,11 @@
 from typing import Any, Dict, Iterable, List, Optional, Tuple, Type, Union
 
 # CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
+<<<<<<< HEAD
 __version__ = "22.7.6"
+=======
+__version__ = "22.8.1"
+>>>>>>> origin/main
 
 Error = Tuple[int, int, str, Type[Any]]
 
@@ -40,14 +44,14 @@ def make_error(error: str, lineno: int, col: int, *args: Any, **kwargs: Any) ->
 class Flake8TrioVisitor(ast.NodeVisitor):
     def __init__(self):
         super().__init__()
-        self.problems: List[Error] = []
+        self._problems: List[Error] = []
         self.suppress_errors = False
 
     @classmethod
     def run(cls, tree: ast.AST) -> Iterable[Error]:
         visitor = cls()
         visitor.visit(tree)
-        yield from visitor.problems
+        yield from visitor._problems
 
     def visit_nodes(
         self, *nodes: Union[ast.AST, Iterable[ast.AST]], generic: bool = False
@@ -65,12 +69,12 @@ def visit_nodes(
 
     def error(self, error: str, lineno: int, col: int, *args: Any, **kwargs: Any):
         if not self.suppress_errors:
-            self.problems.append(make_error(error, lineno, col, *args, **kwargs))
+            self._problems.append(make_error(error, lineno, col, *args, **kwargs))
 
     def get_state(self, *attrs: str) -> Dict[str, Any]:
         if not attrs:
             attrs = tuple(self.__dict__.keys())
-        return {attr: getattr(self, attr) for attr in attrs if attr != "problems"}
+        return {attr: getattr(self, attr) for attr in attrs if attr != "_problems"}
 
     def set_state(self, attrs: Dict[str, Any]):
         for attr, value in attrs.items():
@@ -158,8 +162,9 @@ def visit_AsyncWith(self, node: ast.AsyncWith):
         self.visit_With(node)
 
     def visit_FunctionDef(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef]):
-        outer = self.get_state("_safe_decorator", "_yield_is_error")
+        outer = self.get_state()
         self._yield_is_error = False
+        self._inside_loop = False
 
         # check for @<context_manager_name> and @<library>.<context_manager_name>
         if has_decorator(node.decorator_list, *context_manager_names):
@@ -170,11 +175,12 @@ def visit_FunctionDef(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef]):
         self.set_state(outer)
 
     def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef):
+        self.check_109(node.args)
         self.visit_FunctionDef(node)
 
     def visit_Yield(self, node: ast.Yield):
         if self._yield_is_error:
-            self.problems.append(make_error(TRIO101, node.lineno, node.col_offset))
+            self.error(TRIO101, node.lineno, node.col_offset)
 
         self.generic_visit(node)
 
@@ -186,19 +192,35 @@ def check_for_trio100(self, node: Union[ast.With, ast.AsyncWith]):
                 isinstance(x, checkpoint_node_types) and x != node
                 for x in ast.walk(node)
             ):
-                self.problems.append(
-                    make_error(TRIO100, item.lineno, item.col_offset, call)
-                )
+                self.error(TRIO100, item.lineno, item.col_offset, call)
 
     def visit_ImportFrom(self, node: ast.ImportFrom):
         if node.module == "trio":
-            self.problems.append(make_error(TRIO106, node.lineno, node.col_offset))
+            self.error(TRIO106, node.lineno, node.col_offset)
         self.generic_visit(node)
 
     def visit_Import(self, node: ast.Import):
         for name in node.names:
             if name.name == "trio" and name.asname is not None:
-                self.problems.append(make_error(TRIO106, node.lineno, node.col_offset))
+                self.error(TRIO106, node.lineno, node.col_offset)
+
+    def check_109(self, args: ast.arguments):
+        for arg in (*args.posonlyargs, *args.args, *args.kwonlyargs):
+            if arg.arg == "timeout":
+                self.error(TRIO109, arg.lineno, arg.col_offset)
+
+    def visit_While(self, node: ast.While):
+        self.check_for_110(node)
+        self.generic_visit(node)
+
+    def check_for_110(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")
+        ):
+            self.error(TRIO110, node.lineno, node.col_offset)
 
 
 def critical_except(node: ast.ExceptHandler) -> Optional[Tuple[int, int, str]]:
@@ -252,9 +274,7 @@ def visit_Await(
                 cm.has_timeout and cm.shielded for cm in self._trio_context_managers
             )
         ):
-            self.problems.append(
-                make_error(TRIO102, node.lineno, node.col_offset, *self._critical_scope)
-            )
+            self.error(TRIO102, node.lineno, node.col_offset, *self._critical_scope)
         if visit_children:
             self.generic_visit(node)
 
@@ -288,14 +308,15 @@ def visit_AsyncWith(self, node: ast.AsyncWith):
         self.visit_With(node)
 
     def visit_FunctionDef(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef]):
-        outer_cm = self._safe_decorator
+        outer = self.get_state("_safe_decorator")
 
         # check for @<context_manager_name> and @<library>.<context_manager_name>
         if has_decorator(node.decorator_list, *context_manager_names):
             self._safe_decorator = True
 
         self.generic_visit(node)
-        self._safe_decorator = outer_cm
+
+        self.set_state(outer)
 
     visit_AsyncFunctionDef = visit_FunctionDef
 
@@ -305,13 +326,13 @@ def critical_visit(
         block: Tuple[int, int, str],
         generic: bool = False,
     ):
-        outer = self._critical_scope, self._trio_context_managers
+        outer = self.get_state("_critical_scope", "_trio_context_managers")
 
         self._trio_context_managers = []
         self._critical_scope = block
 
         self.visit_nodes(node, generic=generic)
-        self._critical_scope, self._trio_context_managers = outer
+        self.set_state(outer)
 
     def visit_Try(self, node: ast.Try):
         # There's no visit_Finally, so we need to manually visit the Try fields.
@@ -358,7 +379,7 @@ def __init__(self):
     # then there might be a code path that doesn't re-raise.
     def visit_ExceptHandler(self, node: ast.ExceptHandler):
 
-        outer = self.get_state("unraised", "except_name", "loop_depth")
+        outer = self.get_state()
         marker = critical_except(node)
 
         # we need to *not* unset self.unraised if this is non-critical, to still
@@ -375,7 +396,7 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler):
         self.generic_visit(node)
 
         if self.unraised and marker is not None:
-            self.problems.append(make_error(TRIO103, *marker))
+            self.error(TRIO103, *marker)
 
         self.set_state(outer)
 
@@ -387,7 +408,7 @@ def visit_Raise(self, node: ast.Raise):
             and node.exc is not None
             and not (isinstance(node.exc, ast.Name) and node.exc.id == self.except_name)
         ):
-            self.problems.append(make_error(TRIO104, node.lineno, node.col_offset))
+            self.error(TRIO104, node.lineno, node.col_offset)
 
         # treat it as safe regardless, to avoid unnecessary error messages.
         self.unraised = False
@@ -397,7 +418,7 @@ def visit_Raise(self, node: ast.Raise):
     def visit_Return(self, node: Union[ast.Return, ast.Yield]):
         if self.unraised:
             # Error: must re-raise
-            self.problems.append(make_error(TRIO104, node.lineno, node.col_offset))
+            self.error(TRIO104, node.lineno, node.col_offset)
         self.generic_visit(node)
 
     visit_Yield = visit_Return
@@ -446,7 +467,7 @@ def visit_If(self, node: ast.If):
     # we completely disregard them when checking coverage by resetting the
     # effects of them afterwards
     def visit_For(self, node: Union[ast.For, ast.While]):
-        outer_unraised = self.unraised
+        outer = self.get_state("unraised")
 
         self.loop_depth += 1
         for n in node.body:
@@ -455,13 +476,13 @@ def visit_For(self, node: Union[ast.For, ast.While]):
         for n in node.orelse:
             self.visit(n)
 
-        self.unraised = outer_unraised
+        self.set_state(outer)
 
     visit_While = visit_For
 
     def visit_Break(self, node: Union[ast.Break, ast.Continue]):
         if self.unraised and self.loop_depth == 0:
-            self.problems.append(make_error(TRIO104, node.lineno, node.col_offset))
+            self.error(TRIO104, node.lineno, node.col_offset)
         self.generic_visit(node)
 
     visit_Continue = visit_Break
@@ -506,9 +527,7 @@ def visit_Call(self, node: ast.Call):
                 or not isinstance(self.node_stack[-2], ast.Await)
             )
         ):
-            self.problems.append(
-                make_error(TRIO105, node.lineno, node.col_offset, node.func.attr)
-            )
+            self.error(TRIO105, node.lineno, node.col_offset, node.func.attr)
         self.generic_visit(node)
 
 
@@ -779,3 +798,7 @@ def run(self) -> Iterable[Error]:
 TRIO108 = (
     "TRIO108: {} from async function with no guaranteed checkpoint since {} on line {}"
 )
+#TRIO107 = "TRIO107: Async functions must have at least one checkpoint on every code path, unless an exception is raised"
+#TRIO108 = "TRIO108: Early return from async function must have at least one checkpoint on every code path before it."
+TRIO109 = "TRIO109: Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead"
+TRIO110 = "TRIO110: `while <condition>: await trio.sleep()` should be replaced by a `trio.Event`."

tests/test_flake8_trio.py

@@ -46,8 +46,8 @@ def test_eval(test: str, path: str):
     expected: List[Error] = []
     with open(os.path.join("tests", path)) as file:
         for lineno, line in enumerate(file, start=1):
-            # get text between `error: ` and newline
-            k = re.search(r"(?<=error: ).*(?=\n)", line)
+            # get text between `error:` and end of line
+            k = re.search(r"(?<=error:).*$", line)
             if not k or line.strip()[0] == "#":
                 continue
             # Append a bunch of empty strings so string formatting gives garbage instead
@@ -60,6 +60,7 @@ def test_eval(test: str, path: str):
             assert col.isdigit(), f'invalid column "{col}" @L{lineno}, in "{line}"'
             expected.append(make_error(error_msg, lineno, int(col), *args))
 
+    assert expected, "failed to parse any errors in file"
     assert_expected_errors(path, test, *expected)
 
 

tests/trio109.py

@@ -0,0 +1,78 @@
+timeout = 10
+
+
+async def foo():
+    ...
+
+
+# args
+async def foo_1(timeout):  # error: 16
+    ...
+
+
+# arg in args wih default & annotation
+async def foo_2(timeout: int = 3):  # error: 16
+    ...
+
+
+# vararg
+async def foo_3(*timeout):  # ignored
+    ...
+
+
+# kwarg
+async def foo_4(**timeout):  # ignored
+    ...
+
+
+# correct line/col
+async def foo_5(
+    bar,
+    timeouts,
+    my_timeout,
+    timeout_,
+    timeout,  # error: 4
+):
+    ...
+
+
+# posonlyargs
+async def foo_6(
+    timeout,  # error: 4
+    /,
+    bar,
+):
+    ...
+
+
+# kwonlyargs
+async def foo_7(
+    *,
+    timeout,  # error: 4
+):
+    ...
+
+
+# kwonlyargs (and kw_defaults)
+async def foo_8(
+    *,
+    timeout=5,  # error: 4
+):
+    ...
+
+
+async def foo_9(k=timeout):
+    ...
+
+
+# normal functions are not checked
+def foo_10(timeout):
+    ...
+
+
+def foo_11(timeout, /):
+    ...
+
+
+def foo_12(*, timeout):
+    ...