diff --git a/README.md b/README.md index cc437909..638da953 100644 --- a/README.md +++ b/README.md @@ -27,3 +27,4 @@ pip install git+https://github.com/Zac-HD/flake8-trio - **TRIO100**: a `with trio.fail_after(...):` or `with trio.move_on_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. +- **TRIO101** `yield` inside a nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling. diff --git a/flake8_trio.py b/flake8_trio.py index 0b9145fe..fa328d78 100644 --- a/flake8_trio.py +++ b/flake8_trio.py @@ -11,13 +11,24 @@ import ast import tokenize -from typing import Any, Generator, List, Optional, Tuple, Type, Union +from typing import Any, Generator, List, Optional, Set, Tuple, Type, Union # CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1" __version__ = "22.7.1" Error = Tuple[int, int, str, Type[Any]] +cancel_scope_names = ( + "fail_after", + "fail_at", + "move_on_after", + "move_at", + "CancelScope", +) +context_manager_names = ( + "contextmanager", + "asynccontextmanager", +) def make_error(error: str, lineno: int, col: int, *args: Any, **kwargs: Any) -> Error: @@ -40,19 +51,61 @@ class Visitor(ast.NodeVisitor): def __init__(self) -> None: super().__init__() self.problems: List[Error] = [] + self.safe_yields: Set[ast.Yield] = set() + self._yield_is_error = False + self._context_manager = False - def visit_With(self, node: ast.With) -> None: + def visit_generic_with(self, node: Union[ast.With, ast.AsyncWith]): self.check_for_trio100(node) + + outer = self._yield_is_error + if not self._context_manager and any( + is_trio_call(item, "open_nursery", *cancel_scope_names) + for item in (i.context_expr for i in node.items) + ): + self._yield_is_error = True + self.generic_visit(node) + self._yield_is_error = outer + + def visit_With(self, node: ast.With) -> None: + self.visit_generic_with(node) def visit_AsyncWith(self, node: ast.AsyncWith) -> None: - self.check_for_trio100(node) + self.visit_generic_with(node) + + def visit_generic_FunctionDef( + self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef] + ): + outer_cm = self._context_manager + outer_yie = self._yield_is_error + self._yield_is_error = False + if any( + (isinstance(d, ast.Name) and d.id in context_manager_names) + or (isinstance(d, ast.Attribute) and d.attr in context_manager_names) + for d in node.decorator_list + ): + self._context_manager = True + self.generic_visit(node) + self._context_manager = outer_cm + self._yield_is_error = outer_yie + + def visit_FunctionDef(self, node: ast.FunctionDef) -> None: + self.visit_generic_FunctionDef(node) + + def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: + self.visit_generic_FunctionDef(node) + + def visit_Yield(self, node: ast.Yield) -> None: + if self._yield_is_error: + self.problems.append(make_error(TRIO101, node.lineno, node.col_offset)) + self.generic_visit(node) def check_for_trio100(self, node: Union[ast.With, ast.AsyncWith]) -> None: # Context manager with no `await` call within for item in (i.context_expr for i in node.items): - call = is_trio_call(item, "fail_after", "move_on_after") + call = is_trio_call(item, *cancel_scope_names) if call and not any(isinstance(x, ast.Await) for x in ast.walk(node)): self.problems.append( make_error(TRIO100, item.lineno, item.col_offset, call) @@ -79,3 +132,4 @@ def run(self) -> Generator[Tuple[int, int, str, Type[Any]], None, None]: TRIO100 = "TRIO100: {} context contains no checkpoints, add `await trio.sleep(0)`" +TRIO101 = "TRIO101: yield inside a nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling" diff --git a/tests/test_flake8_trio.py b/tests/test_flake8_trio.py index 19c8d253..22b31886 100644 --- a/tests/test_flake8_trio.py +++ b/tests/test_flake8_trio.py @@ -9,7 +9,7 @@ from hypothesis import HealthCheck, given, settings from hypothesmith import from_grammar, from_node -from flake8_trio import TRIO100, Error, Plugin, Visitor, make_error +from flake8_trio import TRIO100, TRIO101, Error, Plugin, Visitor, make_error class Flake8TrioTestCase(unittest.TestCase): @@ -40,10 +40,20 @@ def test_trio100_py39(self): make_error(TRIO100, 14, 8, "trio.move_on_after"), ) + def test_trio101(self): + self.maxDiff = None + self.assert_expected_errors( + "trio101.py", + make_error(TRIO101, 10, 8), + make_error(TRIO101, 15, 8), + make_error(TRIO101, 27, 8), + make_error(TRIO101, 38, 8), + ) + @pytest.mark.fuzz class TestFuzz(unittest.TestCase): - @settings(max_examples=10_000, suppress_health_check=[HealthCheck.too_slow]) + @settings(max_examples=1_000, suppress_health_check=[HealthCheck.too_slow]) @given((from_grammar() | from_node()).map(ast.parse)) def test_does_not_crash_on_any_valid_code(self, syntax_tree: ast.AST): # Given any syntatically-valid source code, the checker should diff --git a/tests/trio101.py b/tests/trio101.py new file mode 100644 index 00000000..9c68f5ec --- /dev/null +++ b/tests/trio101.py @@ -0,0 +1,53 @@ +import contextlib +import contextlib as bla +from contextlib import asynccontextmanager, contextmanager + +import trio + + +def foo0(): + with trio.open_nursery() as _: + yield 1 # error + + +async def foo1(): + async with trio.open_nursery() as _: + yield 1 # error + + +@contextmanager +def foo2(): + with trio.open_nursery() as _: + yield 1 # safe + + +async def foo3(): + async with trio.CancelScope() as _: + await trio.sleep(1) # so trio100 doesn't complain + yield 1 # error + + +@asynccontextmanager +async def foo4(): + async with trio.open_nursery() as _: + yield 1 # safe + + +async def foo5(): + async with trio.open_nursery(): + yield 1 # error + + def foo6(): + yield 1 # safe + + +@contextlib.asynccontextmanager +async def foo7(): + async with trio.open_nursery() as _: + yield 1 # safe + + +@bla.contextmanager +def foo8(): + with trio.open_nursery() as _: + yield 1 # safe diff --git a/tox.ini b/tox.ini index e1016fcd..7dd2a8a4 100644 --- a/tox.ini +++ b/tox.ini @@ -28,11 +28,11 @@ description = Runs pytest, optionally with posargs deps = pytest pytest-cov - pytest-xdist + #pytest-xdist hypothesis hypothesmith commands = - pytest {posargs:-n auto} + pytest #{posargs:-n auto} # Settings for other tools