From 071ee2456105237bea79402fad1d307ceb12fef3 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 28 Jul 2022 12:57:58 +0200 Subject: [PATCH 1/4] Implemented TRIO105: Calling trio async function without await --- CHANGELOG.md | 3 +++ README.md | 1 + flake8_trio.py | 51 +++++++++++++++++++++++++++++++++++++-- tests/test_flake8_trio.py | 31 +++++++++++++++++++++++- tests/trio105.py | 46 +++++++++++++++++++++++++++++++++++ 5 files changed, 129 insertions(+), 3 deletions(-) create mode 100644 tests/trio105.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ad2a4b4c..09c1ed6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog *[CalVer, YY.month.patch](https://calver.org/)* +## 22.7.5 +- Added TRIO105 check for not `await`ing async trio functions. + ## 22.7.3 - Added TRIO102 check for unsafe checkpoints inside `finally:` blocks diff --git a/README.md b/README.md index 615b3cba..e714719f 100644 --- a/README.md +++ b/README.md @@ -24,3 +24,4 @@ pip install flake8-trio - **TRIO101** `yield` inside a nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling. - **TRIO102** it's unsafe to await inside `finally:` unless you use a shielded cancel scope with a timeout" +- **TRIO105** Calling a trio async function without `await`ing it. diff --git a/flake8_trio.py b/flake8_trio.py index 9e0ee737..765d9ce5 100644 --- a/flake8_trio.py +++ b/flake8_trio.py @@ -14,7 +14,7 @@ from typing import Any, Collection, Generator, List, Optional, Tuple, Type, Union # CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1" -__version__ = "22.7.3" +__version__ = "22.7.5" Error = Tuple[int, int, str, Type[Any]] @@ -255,6 +255,52 @@ def check_for_trio100(self, node: Union[ast.With, ast.AsyncWith]) -> None: ) +trio_async_functions = ( + "aclose_forcefully", + "open_file", + "open_ssl_over_tcp_listeners", + "open_ssl_over_tcp_stream", + "open_tcp_listeners", + "open_tcp_stream", + "open_unix_socket", + "run_process", + "server_listeners", + "serve_ssl_over_tcp", + "serve_tcp", + "sleep", + "sleep_forever", + "sleep_until", +) + + +class Visitor105(ast.NodeVisitor): + def __init__(self) -> None: + super().__init__() + self.problems: List[Error] = [] + self.node_stack: List[ast.AST] = [] + + def visit(self, node: ast.AST): + self.node_stack.append(node) + super().visit(node) + self.node_stack.pop() + + def visit_Call(self, node: ast.Call): + if ( + isinstance(node.func, ast.Attribute) + and isinstance(node.func.value, ast.Name) + and node.func.value.id == "trio" + and node.func.attr in trio_async_functions + and ( + len(self.node_stack) < 2 + or not isinstance(self.node_stack[-2], ast.Await) + ) + ): + self.problems.append( + make_error(TRIO105, node.lineno, node.col_offset, node.func.attr) + ) + self.generic_visit(node) + + class Plugin: name = __name__ version = __version__ @@ -269,7 +315,7 @@ def from_filename(cls, filename: str) -> "Plugin": return cls(ast.parse(source)) def run(self) -> Generator[Tuple[int, int, str, Type[Any]], None, None]: - for v in (Visitor, Visitor102): + for v in (Visitor, Visitor102, Visitor105): visitor = v() visitor.visit(self._tree) yield from visitor.problems @@ -278,3 +324,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" TRIO102 = "TRIO102: it's unsafe to await inside `finally:` unless you use a shielded cancel scope with a timeout" +TRIO105 = "TRIO105: Trio async function {} must be awaited" diff --git a/tests/test_flake8_trio.py b/tests/test_flake8_trio.py index dfe4acfc..88ed9942 100644 --- a/tests/test_flake8_trio.py +++ b/tests/test_flake8_trio.py @@ -9,7 +9,16 @@ from hypothesis import HealthCheck, given, settings from hypothesmith import from_grammar, from_node -from flake8_trio import TRIO100, TRIO101, TRIO102, Error, Plugin, Visitor, make_error +from flake8_trio import ( + TRIO100, + TRIO101, + TRIO102, + TRIO105, + Error, + Plugin, + Visitor, + make_error, +) class Flake8TrioTestCase(unittest.TestCase): @@ -71,6 +80,26 @@ def test_trio102(self): make_error(TRIO102, 123, 12), ) + def test_trio105(self): + self.assert_expected_errors( + "trio105.py", + make_error(TRIO105, 25, 4, "aclose_forcefully"), + make_error(TRIO105, 26, 4, "open_file"), + make_error(TRIO105, 27, 4, "open_ssl_over_tcp_listeners"), + make_error(TRIO105, 28, 4, "open_ssl_over_tcp_stream"), + make_error(TRIO105, 29, 4, "open_tcp_listeners"), + make_error(TRIO105, 30, 4, "open_tcp_stream"), + make_error(TRIO105, 31, 4, "open_unix_socket"), + make_error(TRIO105, 32, 4, "run_process"), + make_error(TRIO105, 33, 4, "server_listeners"), + make_error(TRIO105, 34, 4, "serve_ssl_over_tcp"), + make_error(TRIO105, 35, 4, "serve_tcp"), + make_error(TRIO105, 36, 4, "sleep"), + make_error(TRIO105, 37, 4, "sleep_forever"), + make_error(TRIO105, 38, 4, "sleep_until"), + make_error(TRIO105, 45, 15, "open_file"), + ) + @pytest.mark.fuzz class TestFuzz(unittest.TestCase): diff --git a/tests/trio105.py b/tests/trio105.py new file mode 100644 index 00000000..a88990f2 --- /dev/null +++ b/tests/trio105.py @@ -0,0 +1,46 @@ +import trio + + +async def foo(): + # not async + trio.run() + + # safely awaited + await trio.aclose_forcefully() + await trio.open_file() + await trio.open_ssl_over_tcp_listeners() + await trio.open_ssl_over_tcp_stream() + await trio.open_tcp_listeners() + await trio.open_tcp_stream() + await trio.open_unix_socket() + await trio.run_process() + await trio.server_listeners() + await trio.serve_ssl_over_tcp() + await trio.serve_tcp() + await trio.sleep() + await trio.sleep_forever() + await trio.sleep_until() + + # errors + trio.aclose_forcefully() + trio.open_file() + trio.open_ssl_over_tcp_listeners() + trio.open_ssl_over_tcp_stream() + trio.open_tcp_listeners() + trio.open_tcp_stream() + trio.open_unix_socket() + trio.run_process() + trio.server_listeners() + trio.serve_ssl_over_tcp() + trio.serve_tcp() + trio.sleep() + trio.sleep_forever() + trio.sleep_until() + + # safe + async with await trio.open_file() as f: + pass + + # error + async with trio.open_file() as f: + pass From 6337c9e1afeac84ed21c3a96f0197bd334bc2414 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 28 Jul 2022 13:25:01 +0200 Subject: [PATCH 2/4] I'm quite happy I figured out how to get the async functions generated, one less bug pushed to production --- flake8_trio.py | 2 +- tests/test_flake8_trio.py | 14 +++++++++++++- tests/trio105.py | 4 ++-- tox.ini | 1 + 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/flake8_trio.py b/flake8_trio.py index 765d9ce5..a943dd28 100644 --- a/flake8_trio.py +++ b/flake8_trio.py @@ -264,7 +264,7 @@ def check_for_trio100(self, node: Union[ast.With, ast.AsyncWith]) -> None: "open_tcp_stream", "open_unix_socket", "run_process", - "server_listeners", + "serve_listeners", "serve_ssl_over_tcp", "serve_tcp", "sleep", diff --git a/tests/test_flake8_trio.py b/tests/test_flake8_trio.py index 88ed9942..42d5b85e 100644 --- a/tests/test_flake8_trio.py +++ b/tests/test_flake8_trio.py @@ -1,4 +1,5 @@ import ast +import inspect import os import site import sys @@ -6,6 +7,7 @@ from pathlib import Path import pytest +import trio # type: ignore from hypothesis import HealthCheck, given, settings from hypothesmith import from_grammar, from_node @@ -18,6 +20,7 @@ Plugin, Visitor, make_error, + trio_async_functions, ) @@ -91,7 +94,7 @@ def test_trio105(self): make_error(TRIO105, 30, 4, "open_tcp_stream"), make_error(TRIO105, 31, 4, "open_unix_socket"), make_error(TRIO105, 32, 4, "run_process"), - make_error(TRIO105, 33, 4, "server_listeners"), + make_error(TRIO105, 33, 4, "serve_listeners"), make_error(TRIO105, 34, 4, "serve_ssl_over_tcp"), make_error(TRIO105, 35, 4, "serve_tcp"), make_error(TRIO105, 36, 4, "sleep"), @@ -100,6 +103,15 @@ def test_trio105(self): make_error(TRIO105, 45, 15, "open_file"), ) + self.assertEqual( + set(trio_async_functions), + { + o[0] + for o in inspect.getmembers(trio) # type: ignore + if inspect.iscoroutinefunction(o[1]) + }, + ) + @pytest.mark.fuzz class TestFuzz(unittest.TestCase): diff --git a/tests/trio105.py b/tests/trio105.py index a88990f2..95681213 100644 --- a/tests/trio105.py +++ b/tests/trio105.py @@ -14,7 +14,7 @@ async def foo(): await trio.open_tcp_stream() await trio.open_unix_socket() await trio.run_process() - await trio.server_listeners() + await trio.serve_listeners() await trio.serve_ssl_over_tcp() await trio.serve_tcp() await trio.sleep() @@ -30,7 +30,7 @@ async def foo(): trio.open_tcp_stream() trio.open_unix_socket() trio.run_process() - trio.server_listeners() + trio.serve_listeners() trio.serve_ssl_over_tcp() trio.serve_tcp() trio.sleep() diff --git a/tox.ini b/tox.ini index 03e0f268..6d3be4f3 100644 --- a/tox.ini +++ b/tox.ini @@ -35,6 +35,7 @@ deps = #pytest-xdist hypothesis hypothesmith + trio commands = pytest #{posargs:-n auto} From 02a8da113937369604e34a4dabd7bdc0d15b5bce Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 28 Jul 2022 13:37:09 +0200 Subject: [PATCH 3/4] yay I figured out a tricky case --- tests/test_flake8_trio.py | 1 + tests/trio105.py | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/tests/test_flake8_trio.py b/tests/test_flake8_trio.py index 42d5b85e..0a261ea1 100644 --- a/tests/test_flake8_trio.py +++ b/tests/test_flake8_trio.py @@ -101,6 +101,7 @@ def test_trio105(self): make_error(TRIO105, 37, 4, "sleep_forever"), make_error(TRIO105, 38, 4, "sleep_until"), make_error(TRIO105, 45, 15, "open_file"), + make_error(TRIO105, 52, 8, "open_file"), ) self.assertEqual( diff --git a/tests/trio105.py b/tests/trio105.py index 95681213..091e713f 100644 --- a/tests/trio105.py +++ b/tests/trio105.py @@ -44,3 +44,10 @@ async def foo(): # error async with trio.open_file() as f: pass + + # currently errors out, but is unclear if user intended to save the + # awaitable or the result. Not too tricky to do a super basic parsing + # (save variable name, check if a variable with that name is ever awaited) + # but I suspect that isn't very useful regardless? + k = trio.open_file() + await k From 3ee1eb84056e1bb5e5baca6df9f1789ecf773d9b Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 29 Jul 2022 00:04:16 +0200 Subject: [PATCH 4/4] updated error message, comment. Un-updated version and moved changelog entry to ## Future --- CHANGELOG.md | 4 ++-- README.md | 2 +- flake8_trio.py | 4 ++-- tests/test_flake8_trio.py | 2 +- tests/trio105.py | 6 ++---- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09c1ed6b..ea84525e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,8 @@ # Changelog *[CalVer, YY.month.patch](https://calver.org/)* -## 22.7.5 -- Added TRIO105 check for not `await`ing async trio functions. +## Future +- Added TRIO105 check for not immediately `await`ing async trio functions. ## 22.7.3 - Added TRIO102 check for unsafe checkpoints inside `finally:` blocks diff --git a/README.md b/README.md index e714719f..33f8546c 100644 --- a/README.md +++ b/README.md @@ -24,4 +24,4 @@ pip install flake8-trio - **TRIO101** `yield` inside a nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling. - **TRIO102** it's unsafe to await inside `finally:` unless you use a shielded cancel scope with a timeout" -- **TRIO105** Calling a trio async function without `await`ing it. +- **TRIO105** Calling a trio async function without immediately `await`ing it. diff --git a/flake8_trio.py b/flake8_trio.py index a943dd28..f6ad578b 100644 --- a/flake8_trio.py +++ b/flake8_trio.py @@ -14,7 +14,7 @@ from typing import Any, Collection, Generator, List, Optional, Tuple, Type, Union # CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1" -__version__ = "22.7.5" +__version__ = "22.7.3" Error = Tuple[int, int, str, Type[Any]] @@ -324,4 +324,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" TRIO102 = "TRIO102: it's unsafe to await inside `finally:` unless you use a shielded cancel scope with a timeout" -TRIO105 = "TRIO105: Trio async function {} must be awaited" +TRIO105 = "TRIO105: Trio async function {} must be immediately awaited" diff --git a/tests/test_flake8_trio.py b/tests/test_flake8_trio.py index 0a261ea1..7f24a2bf 100644 --- a/tests/test_flake8_trio.py +++ b/tests/test_flake8_trio.py @@ -101,7 +101,7 @@ def test_trio105(self): make_error(TRIO105, 37, 4, "sleep_forever"), make_error(TRIO105, 38, 4, "sleep_until"), make_error(TRIO105, 45, 15, "open_file"), - make_error(TRIO105, 52, 8, "open_file"), + make_error(TRIO105, 50, 8, "open_file"), ) self.assertEqual( diff --git a/tests/trio105.py b/tests/trio105.py index 091e713f..d1213617 100644 --- a/tests/trio105.py +++ b/tests/trio105.py @@ -45,9 +45,7 @@ async def foo(): async with trio.open_file() as f: pass - # currently errors out, but is unclear if user intended to save the - # awaitable or the result. Not too tricky to do a super basic parsing - # (save variable name, check if a variable with that name is ever awaited) - # but I suspect that isn't very useful regardless? + # safe in theory, but deemed sufficiently poor style that parsing + # it isn't supported k = trio.open_file() await k