From 99b7309e74b6b0ee08e78457507d610b734f4d6e Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 5 Aug 2022 16:02:46 +0200 Subject: [PATCH 1/3] add trio[301], don't await trio.sleep inside loop --- CHANGELOG.md | 1 + README.md | 1 + flake8_trio.py | 24 ++++++++++++++++++++++ tests/trio301.py | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+) create mode 100644 tests/trio301.py diff --git a/CHANGELOG.md b/CHANGELOG.md index afd64fc4..2cab147a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Future - Added TRIO109: Async definitions should not have a `timeout` parameter. Use `trio.[fail/move_on]_[at/after]` +- Added TRIO301: `while : 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` diff --git a/README.md b/README.md index c978f84b..d5bac34d 100644 --- a/README.md +++ b/README.md @@ -32,3 +32,4 @@ pip install flake8-trio - **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 with` `async for`. - **TRIO109**: Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead +- **TRIO301**: `while : await trio.sleep()` should be replaced by a `trio.Event`. diff --git a/flake8_trio.py b/flake8_trio.py index 9717f25e..929a2389 100644 --- a/flake8_trio.py +++ b/flake8_trio.py @@ -127,6 +127,7 @@ def __init__(self): super().__init__() self._yield_is_error = False self._safe_decorator = False + self._inside_loop: bool = False def visit_With(self, node: Union[ast.With, ast.AsyncWith]): self.check_for_trio100(node) @@ -154,6 +155,7 @@ def visit_AsyncWith(self, node: ast.AsyncWith): def visit_FunctionDef(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef]): outer = self.get_state() self._yield_is_error = False + self._inside_loop = False # check for @ and @. if has_decorator(node.decorator_list, *context_manager_names): @@ -198,6 +200,27 @@ def check_109(self, args: ast.arguments): if arg.arg == "timeout": self.error(TRIO109, arg.lineno, arg.col_offset) + # 301 + def visit_While(self, node: Union[ast.While, ast.For, ast.AsyncFor]): + outer = self._inside_loop + self._inside_loop = True + self.generic_visit(node) + self._inside_loop = outer + + visit_For = visit_While + visit_AsyncFor = visit_While + + def visit_Await(self, node: ast.Await): + if ( + self._inside_loop + and isinstance(node.value, ast.Call) + and isinstance(node.value.func, ast.Attribute) + and node.value.func.attr == "sleep" + and isinstance(node.value.func.value, ast.Name) + and node.value.func.value.id == "trio" + ): + self.error(TRIO301, node.lineno, node.col_offset) + def critical_except(node: ast.ExceptHandler) -> Optional[Tuple[int, int, str]]: def has_exception(node: Optional[ast.expr]) -> str: @@ -639,3 +662,4 @@ def run(self) -> Iterable[Error]: 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" +TRIO301 = "TRIO301: `while : await trio.sleep()` should be replaced by a `trio.Event`." diff --git a/tests/trio301.py b/tests/trio301.py new file mode 100644 index 00000000..07b2e03d --- /dev/null +++ b/tests/trio301.py @@ -0,0 +1,52 @@ +import trio +import trio as noerror + + +async def sleep(): + return + + +async def foo(): + await trio.sleep() # ok + await sleep() + await noerror.sleep() + await trio.Event() + + while ...: + await trio.sleep() # error: 8 + await trio.sleep() # error: 8 + await sleep() + await noerror.sleep() + await trio.Event() + + for _ in "": + await trio.sleep() # error: 8 + await sleep() + await noerror.sleep() + await trio.Event() + + async for _ in trio.blah: + await trio.sleep() # error: 8 + await sleep() + await noerror.sleep() + await trio.Event() + + while ...: + await trio.sleep() # error: 8 + + async def blah(): + await trio.sleep() + + await trio.sleep() # error: 8 + + while ...: + while ...: + await trio.sleep() # error: 12 + await trio.sleep() # error: 8 + + while ...: + if ...: + await trio.sleep() # error: 12 + + while await trio.sleep(): # error: 10 + ... From 5bafeaae79bc78cdf59d8b8613731d8df0b972c0 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 5 Aug 2022 20:58:00 +0200 Subject: [PATCH 2/3] restrict test to only trigger on while loop with a single statement in body --- flake8_trio.py | 23 +++++---------- tests/trio301.py | 76 ++++++++++++++++++++++++++++++------------------ 2 files changed, 54 insertions(+), 45 deletions(-) diff --git a/flake8_trio.py b/flake8_trio.py index 929a2389..feb94163 100644 --- a/flake8_trio.py +++ b/flake8_trio.py @@ -127,7 +127,6 @@ def __init__(self): super().__init__() self._yield_is_error = False self._safe_decorator = False - self._inside_loop: bool = False def visit_With(self, node: Union[ast.With, ast.AsyncWith]): self.check_for_trio100(node) @@ -200,24 +199,16 @@ def check_109(self, args: ast.arguments): if arg.arg == "timeout": self.error(TRIO109, arg.lineno, arg.col_offset) - # 301 - def visit_While(self, node: Union[ast.While, ast.For, ast.AsyncFor]): - outer = self._inside_loop - self._inside_loop = True + def visit_While(self, node: ast.While): + self.check_for_301(node) self.generic_visit(node) - self._inside_loop = outer - visit_For = visit_While - visit_AsyncFor = visit_While - - def visit_Await(self, node: ast.Await): + def check_for_301(self, node: ast.While): if ( - self._inside_loop - and isinstance(node.value, ast.Call) - and isinstance(node.value.func, ast.Attribute) - and node.value.func.attr == "sleep" - and isinstance(node.value.func.value, ast.Name) - and node.value.func.value.id == "trio" + 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(TRIO301, node.lineno, node.col_offset) diff --git a/tests/trio301.py b/tests/trio301.py index 07b2e03d..84e19c47 100644 --- a/tests/trio301.py +++ b/tests/trio301.py @@ -2,51 +2,69 @@ import trio as noerror -async def sleep(): - return +async def foo(): + # only trigger on while loop with body being exactly one sleep[_until] statement + while ...: # error: 4 + await trio.sleep() + while ...: # error: 4 + await trio.sleep_until() -async def foo(): - await trio.sleep() # ok - await sleep() - await noerror.sleep() - await trio.Event() + # nested + + while ...: # safe + while ...: # error: 8 + await trio.sleep() + await trio.sleep() + while ...: # safe + while ...: # error: 8 + await trio.sleep() + + ### the rest are all safe + + # don't trigger on bodies with more than one statement while ...: - await trio.sleep() # error: 8 - await trio.sleep() # error: 8 - await sleep() - await noerror.sleep() - await trio.Event() + await trio.sleep() + await trio.sleep() - for _ in "": - await trio.sleep() # error: 8 - await sleep() + while ...: # safe + ... + await trio.sleep() + + while ...: + await trio.sleep() + await trio.sleep_until() + + # check library name + while ...: await noerror.sleep() - await trio.Event() - async for _ in trio.blah: - await trio.sleep() # error: 8 + async def sleep(): + ... + + while ...: await sleep() - await noerror.sleep() - await trio.Event() + # check function name while ...: - await trio.sleep() # error: 8 + await trio.sleepies() - async def blah(): - await trio.sleep() + # don't trigger on [async] for + for _ in "": + await trio.sleep() - await trio.sleep() # error: 8 + async for _ in trio.blah: + await trio.sleep() while ...: - while ...: - await trio.sleep() # error: 12 - await trio.sleep() # error: 8 + + async def blah(): + await trio.sleep() while ...: if ...: - await trio.sleep() # error: 12 + await trio.sleep() - while await trio.sleep(): # error: 10 + while await trio.sleep(): ... From 3d8fdf45e569880eda8fb6bb45428e2cd87650da Mon Sep 17 00:00:00 2001 From: Zac Hatfield-Dodds Date: Fri, 5 Aug 2022 14:58:10 -0700 Subject: [PATCH 3/3] Assign trio110 --- CHANGELOG.md | 2 +- README.md | 2 +- flake8_trio.py | 8 ++++---- tests/{trio300.py => trio109.py} | 0 tests/{trio301.py => trio110.py} | 0 5 files changed, 6 insertions(+), 6 deletions(-) rename tests/{trio300.py => trio109.py} (100%) rename tests/{trio301.py => trio110.py} (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cab147a..3189b5c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## Future - Added TRIO109: Async definitions should not have a `timeout` parameter. Use `trio.[fail/move_on]_[at/after]` -- Added TRIO301: `while : await trio.sleep()` should be replaced by a `trio.Event`. +- Added TRIO110: `while : 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` diff --git a/README.md b/README.md index d5bac34d..517e5dd1 100644 --- a/README.md +++ b/README.md @@ -32,4 +32,4 @@ pip install flake8-trio - **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 with` `async for`. - **TRIO109**: Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead -- **TRIO301**: `while : await trio.sleep()` should be replaced by a `trio.Event`. +- **TRIO110**: `while : await trio.sleep()` should be replaced by a `trio.Event`. diff --git a/flake8_trio.py b/flake8_trio.py index feb94163..1392496e 100644 --- a/flake8_trio.py +++ b/flake8_trio.py @@ -200,17 +200,17 @@ def check_109(self, args: ast.arguments): self.error(TRIO109, arg.lineno, arg.col_offset) def visit_While(self, node: ast.While): - self.check_for_301(node) + self.check_for_110(node) self.generic_visit(node) - def check_for_301(self, node: ast.While): + 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(TRIO301, node.lineno, node.col_offset) + self.error(TRIO110, node.lineno, node.col_offset) def critical_except(node: ast.ExceptHandler) -> Optional[Tuple[int, int, str]]: @@ -653,4 +653,4 @@ def run(self) -> Iterable[Error]: 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" -TRIO301 = "TRIO301: `while : await trio.sleep()` should be replaced by a `trio.Event`." +TRIO110 = "TRIO110: `while : await trio.sleep()` should be replaced by a `trio.Event`." diff --git a/tests/trio300.py b/tests/trio109.py similarity index 100% rename from tests/trio300.py rename to tests/trio109.py diff --git a/tests/trio301.py b/tests/trio110.py similarity index 100% rename from tests/trio301.py rename to tests/trio110.py