21X and 22X, blocking sync calls in async function

CHANGELOG.md

@@ -1,6 +1,10 @@
 # Changelog
 *[CalVer, YY.month.patch](https://calver.org/)*
 
+## 23.1.1
+- Add TRIO210, TRIO211 - blocking sync call in async function, using network packages (requests, httpx, urllib3)
+- Add TRIO220, TRIO221 - blocking sync call in async function, using subprocess or os.
+
 ## 22.12.5
 - The `--startable-in-context-manager` and `--trio200-blocking-calls` options now handle spaces and newlines.
 - Now compatible with  [flake8-noqa](https://pypi.org/project/flake8-noqa/)'s NQA102 and NQA103 checks.

CONTRIBUTING.md

@@ -40,6 +40,13 @@ You can instead of `error` specify the error code.
 With `# ARG` lines you can also specify command-line arguments that should be passed to the plugin when parsing a file. Can be specified multiple times for several different arguments.  
 Generated tests will by default `--select` the error code of the file, which will enable any visitors that can generate that code (and if those visitors can raise other codes they might be raised too). This can be overriden by adding an `# ARG --select=...` line.
 
+## Running pytest outside tox
+If you don't want to bother with tox to quickly test stuff, you'll need to install the following dependencies:
+```
+pip install -e .
+pip install pytest pytest-cov hypothesis hypothesmith flake8
+```
+
 ## Style Guide
 
 **Code style:** code review should focus on correctness, performance, and readability.

README.md

@@ -40,6 +40,10 @@ pip install flake8-trio
 - **TRIO115**: Replace `trio.sleep(0)` with the more suggestive `trio.lowlevel.checkpoint()`.
 - **TRIO116**: `trio.sleep()` with >24 hour interval should usually be`trio.sleep_forever()`.
 - **TRIO200**: User-configured error for blocking sync calls in async functions. Does nothing by default, see [`trio200-blocking-calls`](#trio200-blocking-calls) for how to configure it.
+- **TRIO210**: Sync HTTP call {} in async function, use `httpx.AsyncClient`.
+- **TRIO211**: Likely sync HTTP call {} in async function, use `httpx.AsyncClient`. Looks for `urllib3` method calls on pool objects, but only matching on the method signature and not the object.
+- **TRIO220**: Sync call {} in async function, use `await nursery.start(trio.run_process, ...)`.
+- **TRIO221**: Sync call {} in async function, use `await trio.run_process(...)`.
 
 
 ## Configuration

flake8_trio.py

@@ -18,14 +18,14 @@
 from argparse import ArgumentTypeError, Namespace
 from collections.abc import Iterable
 from fnmatch import fnmatch
-from typing import TYPE_CHECKING, Any, NamedTuple, Union, cast
+from typing import TYPE_CHECKING, Any, NamedTuple, TypeVar, Union, cast
 
 # guard against internal flake8 changes
 if TYPE_CHECKING:
     from flake8.options.manager import OptionManager
 
 # CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
-__version__ = "22.12.5"
+__version__ = "23.1.1"
 
 
 class Statement(NamedTuple):
@@ -103,7 +103,7 @@ def __init__(self, options: Namespace):
 
         self.visitors = {
             v(options, self._problems)
-            for v in Flake8TrioVisitor.__subclasses__()
+            for v in ERROR_CLASSES
             if self.selected(v.error_codes)
         }
 
@@ -271,6 +271,16 @@ def walk(self, *body: ast.AST) -> Iterable[ast.AST]:
             yield from ast.walk(b)
 
 
+ERROR_CLASSES: set[type[Flake8TrioVisitor]] = set()
+
+T = TypeVar("T", bound=Flake8TrioVisitor)
+
+
+def error_class(error_class: type[T]) -> type[T]:
+    ERROR_CLASSES.add(error_class)
+    return error_class
+
+
 # ignores module and only checks the unqualified name of the decorator
 # used in 101 and 107/108
 def has_decorator(decorator_list: list[ast.expr], *names: str):
@@ -309,6 +319,7 @@ def fnmatch_qualified_name(name_list: list[ast.expr], *patterns: str) -> str | N
 )
 
 
+@error_class
 class Visitor100(Flake8TrioVisitor):
     error_codes = {
         "TRIO100": (
@@ -328,6 +339,7 @@ def visit_With(self, node: ast.With | ast.AsyncWith):
     visit_AsyncWith = visit_With
 
 
+@error_class
 class Visitor101(Flake8TrioVisitor):
     error_codes = {
         "TRIO101": (
@@ -403,6 +415,7 @@ def has_exception(node: ast.expr | None) -> str:
     return None
 
 
+@error_class
 class Visitor102(Flake8TrioVisitor):
     error_codes = {
         "TRIO102": (
@@ -505,6 +518,7 @@ def visit_Assign(self, node: ast.Assign):
 
 # Never have an except Cancelled or except BaseException block with a code path that
 # doesn't re-raise the error
+@error_class
 class Visitor103_104(Flake8TrioVisitor):
     error_codes = {
         "TRIO103": "{} block with a code path that doesn't re-raise the error",
@@ -722,6 +736,7 @@ def is_nursery_call(node: ast.AST, name: str) -> bool:
     return False
 
 
+@error_class
 class Visitor105(Flake8TrioVisitor):
     error_codes = {
         "TRIO105": "trio async function {} must be immediately awaited",
@@ -740,6 +755,7 @@ def visit_Call(self, node: ast.Call):
             self.error(node, node.func.attr)
 
 
+@error_class
 class Visitor106(Flake8TrioVisitor):
     error_codes = {
         "TRIO106": "trio must be imported with `import trio` for the linter to work",
@@ -769,6 +785,7 @@ def empty_body(body: list[ast.stmt]) -> bool:
     )
 
 
+@error_class
 class Visitor107_108(Flake8TrioVisitor):
     error_codes = {
         "TRIO107": (
@@ -1097,6 +1114,7 @@ def visit_BoolOp(self, node: ast.BoolOp):
         self.uncheckpointed_statements = worst_case_shortcut
 
 
+@error_class
 class Visitor109(Flake8TrioVisitor):
     error_codes = {
         "TRIO109": (
@@ -1117,6 +1135,7 @@ def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef):
                 self.error(arg)
 
 
+@error_class
 class Visitor110(Flake8TrioVisitor):
     error_codes = {
         "TRIO110": (
@@ -1135,6 +1154,7 @@ def visit_While(self, node: ast.While):
             self.error(node)
 
 
+@error_class
 class Visitor111(Flake8TrioVisitor):
     error_codes = {
         "TRIO111": (
@@ -1229,6 +1249,7 @@ def visit_Name(self, node: ast.Name):
                 )
 
 
+@error_class
 class Visitor112(Flake8TrioVisitor):
     error_codes = {
         "TRIO112": (
@@ -1289,6 +1310,7 @@ def _get_identifier(node: ast.expr) -> str:
 )
 
 
+@error_class
 class Visitor113(Flake8TrioVisitor):
     error_codes = {
         "TRIO113": (
@@ -1340,6 +1362,7 @@ def is_startable(n: ast.expr, *startable_list: str) -> bool:
 # --startable-in-context-manager. Will only match against the last part of the option
 # name, so may miss cases where functions are named the same in different modules/classes
 # and option names are specified including the module name.
+@error_class
 class Visitor114(Flake8TrioVisitor):
     error_codes = {
         "TRIO114": (
@@ -1363,6 +1386,7 @@ def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef):
 
 # Suggests replacing all `trio.sleep(0)` with the more suggestive
 # `trio.lowlevel.checkpoint()`
+@error_class
 class Visitor115(Flake8TrioVisitor):
     error_codes = {
         "TRIO115": "Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)`.",
@@ -1378,6 +1402,7 @@ def visit_Call(self, node: ast.Call):
             self.error(node)
 
 
+@error_class
 class Visitor116(Flake8TrioVisitor):
     error_codes = {
         "TRIO116": (
@@ -1420,6 +1445,7 @@ def visit_Call(self, node: ast.Call):
                 self.error(node)
 
 
+@error_class
 class Visitor200(Flake8TrioVisitor):
     error_codes = {
         "TRIO200": "User-configured blocking sync call {0} in async function, consider "
@@ -1441,9 +1467,127 @@ def visit_AsyncFunctionDef(
 
     def visit_Call(self, node: ast.Call):
         if self.async_function and not getattr(node, "awaited", False):
-            blocking_calls = self.options.trio200_blocking_calls
-            if key := fnmatch_qualified_name([node.func], *blocking_calls):
-                self.error(node, key, blocking_calls[key])
+            self.visit_blocking_call(node)
+
+    def visit_blocking_call(self, node: ast.Call):
+        blocking_calls = self.options.trio200_blocking_calls
+        if key := fnmatch_qualified_name([node.func], *blocking_calls):
+            self.error(node, key, blocking_calls[key])
+
+
+@error_class
+class Visitor21X(Visitor200):
+    error_codes = {
+        "TRIO210": "Sync HTTP call {} in async function, use httpx.AsyncClient",
+        "TRIO211": (
+            "Likely sync HTTP call {} in async function, use httpx.AsyncClient"
+        ),
+    }
+
+    def __init__(self, *args: Any, **kwargs: Any):
+        super().__init__(*args, **kwargs)
+        self.imports: set[str] = set()
+
+    def visit_ImportFrom(self, node: ast.ImportFrom):
+        if node.module == "urllib3":
+            self.imports.add(node.module)
+
+    def visit_Import(self, node: ast.Import):
+        for name in node.names:
+            if name.name == "urllib3":
+                # Could also save the name.asname for matching
+                self.imports.add(name.name)
+
+    def visit_blocking_call(self, node: ast.Call):
+        http_methods = {
+            "get",
+            "options",
+            "head",
+            "post",
+            "put",
+            "patch",
+            "delete",
+        }
+        func_name = ast.unparse(node.func)
+        for http_package in "requests", "httpx":
+            if get_matching_call(node, *http_methods | {"request"}, base=http_package):
+                self.error(node, func_name, error_code="TRIO210")
+                return
+
+        if func_name in (
+            "urllib3.request",
+            "urllib.request.urlopen",
+            "request.urlopen",
+            "urlopen",
+        ):
+            self.error(node, func_name, error_code="TRIO210")
+
+        elif (
+            "urllib3" in self.imports
+            and isinstance(node.func, ast.Attribute)
+            and node.func.attr == "request"
+            and node.args
+            and isinstance(node.args[0], ast.Constant)
+            and isinstance(node.args[0].value, str)
+            and node.args[0].value.lower() in http_methods | {"trace", "connect"}
+        ):
+            self.error(node, func_name, error_code="TRIO211")
+
+
+# Process invocations 202
+@error_class
+class Visitor22X(Visitor200):
+    error_codes = {
+        "TRIO220": (
+            "Sync call {} in async function, use "
+            "`await nursery.start(trio.run_process, ...)`"
+        ),
+        "TRIO221": "Sync call {} in async function, use `await trio.run_process(...)`",
+    }
+
+    def visit_blocking_call(self, node: ast.Call):
+        def is_p_wait(arg: ast.expr) -> bool:
+            return (isinstance(arg, ast.Attribute) and arg.attr == "P_WAIT") or (
+                isinstance(arg, ast.Name) and arg.id == "P_WAIT"
+            )
+
+        subprocess_calls = {
+            "run",
+            "call",
+            "check_call",
+            "check_output",
+            "getoutput",
+            "getstatusoutput",
+        }
+
+        func_name = ast.unparse(node.func)
+        if func_name in ("subprocess.Popen", "os.popen"):
+            self.error(node, func_name, error_code="TRIO220")
+            return
+
+        if func_name == "os.system" or get_matching_call(
+            node, *subprocess_calls, base="subprocess"
+        ):
+            self.error(node, func_name, error_code="TRIO221")
+            return
+
+        if re.match("os.spawn[vl]p?e?", func_name):
+            # if mode= is given and not [os.]P_WAIT: TRIO220
+            # 1. as a positional parameter
+            if node.args:
+                arg = node.args[0]
+                if not is_p_wait(arg):
+                    self.error(node, func_name, error_code="TRIO220")
+                    return
+            # 2. as a keyword parameter
+            for kw in node.keywords:
+                if kw.arg == "mode" and not is_p_wait(kw.value):
+                    self.error(node, func_name, error_code="TRIO220")
+                    return
+
+            # otherwise, TRIO221
+            self.error(node, func_name, error_code="TRIO221")
+            return
 
 
 # flake8 ignores type parameters if using comma_separated_list

tests/test_changelog_and_version.py

@@ -1,14 +1,13 @@
 """Tests for flake8-trio package metadata."""
 from __future__ import annotations
 
-import os
 import re
 import unittest
 from collections.abc import Iterable
 from pathlib import Path
 from typing import NamedTuple
 
-from test_flake8_trio import ERROR_CODES, trio_test_files_regex
+from test_flake8_trio import ERROR_CODES
 
 import flake8_trio
 
@@ -68,11 +67,28 @@ def runTest(self):
 
         documented_errors["flake8_trio.py"] = set(ERROR_CODES)
 
-        documented_errors["tests/trio*.py"] = {
-            os.path.splitext(f)[0].upper().split("_")[0]
-            for f in os.listdir("tests")
-            if re.match(trio_test_files_regex, f)
-        }
+        # get tested error codes from file names and from `INCLUDE` lines
+        documented_errors["tests/trio*.py"] = set()
+        p = Path(__file__).parent
+        for file_path in p.iterdir():
+            if not file_path.is_file():
+                continue
+
+            if m := re.search(r"trio\d\d\d", str(file_path)):
+                documented_errors["tests/trio*.py"].add(m.group().upper())
+
+            with open(file_path) as file:
+                for line in file:
+                    if line.startswith("# ARG --enable-visitor-codes-regex"):
+                        for m in re.findall(r"trio\d\d\d", line, re.IGNORECASE):
+                            # pyright types m as `Any` (as it is in typeshed)
+                            # mypy types it as Optional[Match[str]]
+                            # but afaict it should be something like str|Tuple[str,...]
+                            # depending on whether there's a group in the pattern or not.
+                            # (or bytes, if both inputs are bytes)
+                            assert isinstance(m, str)
+                            documented_errors["tests/trio*.py"].add(m)
+                        break
 
         unique_errors: dict[str, set[str]] = {}
         missing_errors: dict[str, set[str]] = {}