bump pyright, introducing warnings for tons of missing docstrings

check.sh

@@ -96,18 +96,8 @@ fi
 
 codespell || EXIT_STATUS=$?
 
-PYRIGHT=0
 echo "::group::Pyright interface tests"
-pyright --verifytypes --ignoreexternal --pythonplatform=Linux --verifytypes=trio \
-    || { echo "* Pyright --verifytypes (Linux) found errors." >> "$GITHUB_STEP_SUMMARY"; PYRIGHT=1; }
-pyright --verifytypes --ignoreexternal --pythonplatform=Darwin --verifytypes=trio \
-    || { echo "* Pyright --verifytypes (Mac) found errors." >> "$GITHUB_STEP_SUMMARY"; PYRIGHT=1; }
-pyright --verifytypes --ignoreexternal --pythonplatform=Windows --verifytypes=trio \
-    || { echo "* Pyright --verifytypes (Windows) found errors." >> "$GITHUB_STEP_SUMMARY"; PYRIGHT=1; }
-if [ $PYRIGHT -ne 0 ]; then
-    echo "::error:: Pyright --verifytypes returned errors."
-    EXIT_STATUS=1
-fi
+python src/trio/_tests/check_type_completeness.py || EXIT_STATUS=$?
 
 pyright src/trio/_tests/type_tests || EXIT_STATUS=$?
 pyright src/trio/_core/_tests/type_tests || EXIT_STATUS=$?

pyproject.toml

@@ -251,6 +251,8 @@ omit = [
     "*/trio/_core/_generated_*",
     # Type tests aren't intended to be run, just passed to type checkers.
     "*/type_tests/*",
+    # Script used to check type completeness that isn't run in tests
+    "*/trio/_tests/check_type_completeness.py",
 ]
 # The test suite spawns subprocesses to test some stuff, so make sure
 # this doesn't corrupt the coverage files

src/trio/_socket.py

@@ -616,6 +616,7 @@
 
     @property
     def did_shutdown_SHUT_WR(self) -> bool:
+        """Return True if the socket has been shut down with the SHUT_WR flag"""
         raise NotImplementedError
 
     def __repr__(self) -> str:
@@ -634,9 +635,11 @@
         raise NotImplementedError
 
     def is_readable(self) -> bool:
+        """Return True if the socket is readable. This is checked with `select.select` on Windows, otherwise `select.poll`."""
         raise NotImplementedError
 
     async def wait_writable(self) -> None:
+        """Convenience method that calls trio.lowlevel.wait_writable for the object."""
         raise NotImplementedError
 
     async def accept(self) -> tuple[SocketType, AddressFormat]:
@@ -724,6 +727,21 @@
             raise NotImplementedError
 
 
+# copy docstrings from socket.SocketType / socket.socket
+for name, obj in SocketType.__dict__.items():
+    # skip dunders and already defined docstrings
+    if name.startswith("__") or obj.__doc__:
+        continue
+    # try both socket.socket and socket.SocketType
+    for stdlib_type in _stdlib_socket.socket, _stdlib_socket.SocketType:
+        stdlib_obj = getattr(stdlib_type, name, None)
+        if stdlib_obj and stdlib_obj.__doc__:
+            break
+    else:
+        continue
+    obj.__doc__ = stdlib_obj.__doc__
+
+
 class _SocketType(SocketType):
     def __init__(self, sock: _stdlib_socket.socket):
         if type(sock) is not _stdlib_socket.socket:

src/trio/_tests/_check_type_completeness.json

@@ -0,0 +1,64 @@
+{
+    "Darwin": [
+        "No docstring found for function \"trio._unix_pipes.FdStream.send_all\"",
+        "No docstring found for function \"trio._unix_pipes.FdStream.wait_send_all_might_not_block\"",
+        "No docstring found for function \"trio._unix_pipes.FdStream.receive_some\"",
+        "No docstring found for function \"trio._unix_pipes.FdStream.close\"",
+        "No docstring found for function \"trio._unix_pipes.FdStream.aclose\"",
+        "No docstring found for function \"trio._unix_pipes.FdStream.fileno\""
+    ],
+    "Linux": [
+        "No docstring found for class \"trio._core._io_epoll._EpollStatistics\"",
+        "No docstring found for function \"trio._unix_pipes.FdStream.send_all\"",
+        "No docstring found for function \"trio._unix_pipes.FdStream.wait_send_all_might_not_block\"",
+        "No docstring found for function \"trio._unix_pipes.FdStream.receive_some\"",
+        "No docstring found for function \"trio._unix_pipes.FdStream.close\"",
+        "No docstring found for function \"trio._unix_pipes.FdStream.aclose\"",
+        "No docstring found for function \"trio._unix_pipes.FdStream.fileno\""
+    ],
+    "Windows": [],
+    "all": [
+        "No docstring found for class \"trio.MemoryReceiveChannel\"",
+        "No docstring found for class \"trio._channel.MemoryReceiveChannel\"",
+        "No docstring found for function \"trio._channel.MemoryReceiveChannel.statistics\"",
+        "No docstring found for class \"trio._channel.MemoryChannelStats\"",
+        "No docstring found for function \"trio._channel.MemoryReceiveChannel.aclose\"",
+        "No docstring found for class \"trio.MemorySendChannel\"",
+        "No docstring found for class \"trio._channel.MemorySendChannel\"",
+        "No docstring found for function \"trio._channel.MemorySendChannel.statistics\"",
+        "No docstring found for function \"trio._channel.MemorySendChannel.aclose\"",
+        "No docstring found for class \"trio._core._run.Task\"",
+        "No docstring found for class \"trio._socket.SocketType\"",
+        "No docstring found for function \"trio._highlevel_socket.SocketStream.send_all\"",
+        "No docstring found for function \"trio._highlevel_socket.SocketStream.wait_send_all_might_not_block\"",
+        "No docstring found for function \"trio._highlevel_socket.SocketStream.send_eof\"",
+        "No docstring found for function \"trio._highlevel_socket.SocketStream.receive_some\"",
+        "No docstring found for function \"trio._highlevel_socket.SocketStream.aclose\"",
+        "No docstring found for function \"trio._path.Path.absolute\"",
+        "No docstring found for class \"trio._path.AsyncAutoWrapperType\"",
+        "No docstring found for function \"trio._path.AsyncAutoWrapperType.generate_forwards\"",
+        "No docstring found for function \"trio._path.AsyncAutoWrapperType.generate_wraps\"",
+        "No docstring found for function \"trio._path.AsyncAutoWrapperType.generate_magic\"",
+        "No docstring found for function \"trio._path.AsyncAutoWrapperType.generate_iter\"",
+        "No docstring found for function \"trio._subprocess.HasFileno.fileno\"",
+        "No docstring found for class \"trio._sync.AsyncContextManagerMixin\"",
+        "No docstring found for function \"trio._sync._HasAcquireRelease.acquire\"",
+        "No docstring found for function \"trio._sync._HasAcquireRelease.release\"",
+        "No docstring found for class \"trio._sync._LockImpl\"",
+        "No docstring found for class \"trio._core._local._NoValue\"",
+        "No docstring found for class \"trio._core._local.RunVarToken\"",
+        "No docstring found for class \"trio.lowlevel.RunVarToken\"",
+        "No docstring found for class \"trio.lowlevel.Task\"",
+        "No docstring found for class \"trio._core._ki.KIProtectionSignature\"",
+        "No docstring found for class \"trio.socket.SocketType\"",
+        "No docstring found for class \"trio.socket.gaierror\"",
+        "No docstring found for class \"trio.socket.herror\"",
+        "No docstring found for function \"trio._core._mock_clock.MockClock.start_clock\"",
+        "No docstring found for function \"trio._core._mock_clock.MockClock.current_time\"",
+        "No docstring found for function \"trio._core._mock_clock.MockClock.deadline_to_sleep_time\"",
+        "No docstring found for function \"trio.testing._raises_group._ExceptionInfo.exconly\"",
+        "No docstring found for function \"trio.testing._raises_group._ExceptionInfo.errisinstance\"",
+        "No docstring found for function \"trio.testing._raises_group._ExceptionInfo.getrepr\"",
+        "No docstring found for function \"trio.testing._raises_group.RaisesGroup.expected_type\""
+    ]
+}

src/trio/_tests/check_type_completeness.py

@@ -0,0 +1,221 @@
+#!/usr/bin/env python3
+"""This is a file that wraps calls to `pyright --verifytypes`, achieving two things:
+1. give an error if docstrings are missing.
+    pyright will give a number of missing docstrings, and error messages, but not exit with a non-zero value.
+2. filter out specific errors we don't care about.
+    this is largely due to 1, but also because Trio does some very complex stuff and --verifytypes has few to no ways of ignoring specific errors.
+"""
+from __future__ import annotations
+
+# this file is not run as part of the tests, instead it's run standalone from check.sh
+import argparse
+import json
+import subprocess
+import sys
+from pathlib import Path
+
+import trio
+import trio.testing
+
+# not needed if everything is working, but if somebody does something to generate
+# tons of errors, we can be nice and stop them from getting 3*tons of output
+printed_diagnostics: set[str] = set()
+
+
+# TODO: consider checking manually without `--ignoreexternal`, and/or
+# removing it from the below call later on.
+def run_pyright(platform: str) -> subprocess.CompletedProcess[bytes]:
+    return subprocess.run(
+        [
+            "pyright",
+            # Specify a platform and version to keep imported modules consistent.
+            f"--pythonplatform={platform}",
+            "--pythonversion=3.8",
+            "--verifytypes=trio",
+            "--outputjson",
+            "--ignoreexternal",
+        ],
+        capture_output=True,
+    )
+
+
+def has_docstring_at_runtime(name: str) -> bool:
+    """Pyright gives us an object identifier of xx.yy.zz
+    This function tries to decompose that into its constituent parts, such that we
+    can resolve it, in order to check whether it has a `__doc__` at runtime and
+    verifytypes misses it because we're doing overly fancy stuff.
+    """
+    assert trio.testing
+
+    # figure out what part of the name is the module, so we can "import" it
+    name_parts = name.split(".")
+    split_i = 1
+    if name_parts[1] == "tests":
+        return True
+    if name_parts[1] in ("_core", "testing"):  # noqa: SIM108
+        split_i = 2
+    else:
+        split_i = 1
+    module = sys.modules[".".join(name_parts[:split_i])]
+
+    # traverse down the remaining identifiers with getattr
+    obj = module
+    try:
+        for obj_name in name_parts[split_i:]:
+            obj = getattr(obj, obj_name)
+    except AttributeError as exc:
+        # asynciowrapper does funky getattr stuff
+        if "AsyncIOWrapper" in str(exc) or name in (
+            # Symbols not existing on all platforms, so we can't dynamically inspect them.
+            # Manually confirmed to have docstrings but pyright doesn't see them due to
+            # export shenanigans. TODO: actually manually confirm that.
+            # darwin
+            "trio.lowlevel.current_kqueue",
+            "trio.lowlevel.monitor_kevent",
+            "trio.lowlevel.wait_kevent",
+            "trio._core._io_kqueue._KqueueStatistics",
+            # windows
+            "trio._socket.SocketType.share",
+            "trio._core._io_windows._WindowsStatistics",
+            "trio._core._windows_cffi.Handle",
+            "trio.lowlevel.current_iocp",
+            "trio.lowlevel.monitor_completion_key",
+            "trio.lowlevel.readinto_overlapped",
+            "trio.lowlevel.register_with_iocp",
+            "trio.lowlevel.wait_overlapped",
+            "trio.lowlevel.write_overlapped",
+            "trio.lowlevel.WaitForSingleObject",
+            "trio.socket.fromshare",
+            # TODO: these are erroring on all platforms, why?
+            "trio._highlevel_generic.StapledStream.send_stream",
+            "trio._highlevel_generic.StapledStream.receive_stream",
+            "trio._ssl.SSLStream.transport_stream",
+            "trio._file_io._HasFileNo",
+            "trio._file_io._HasFileNo.fileno",
+        ):
+            return True
+
+        else:
+            print(
+                f"Pyright sees {name} at runtime, but unable to getattr({obj.__name__}, {obj_name})."
+            )
+            return False
+    return bool(obj.__doc__)
+
+
+def check_type(
+    platform: str, full_diagnostics_file: Path | None, expected_errors: list[object]
+) -> list[object]:
+    # convince isort we use the trio import
+    assert trio
+
+    # run pyright, load output into json
+    res = run_pyright(platform)
+    current_result = json.loads(res.stdout)
+
+    if res.stderr:
+        print(res.stderr)
+
+    if full_diagnostics_file:
+        with open(full_diagnostics_file, "a") as f:
+            json.dump(current_result, f, sort_keys=True, indent=4)
+
+    errors = []
+
+    for symbol in current_result["typeCompleteness"]["symbols"]:
+        diagnostics = symbol["diagnostics"]
+        name = symbol["name"]
+        for diagnostic in diagnostics:
+            message = diagnostic["message"]
+            # ignore errors about missing docstrings if they're available at runtime
+            if message.startswith("No docstring found for"):
+                if has_docstring_at_runtime(symbol["name"]):
+                    continue
+            else:
+                # Missing docstring messages include the name of the object.
+                # Other errors don't, so we add it.
+                message = f"{name}: {message}"
+            if message not in expected_errors and message not in printed_diagnostics:
+                print(f"new error: {message}")
+            errors.append(message)
+            printed_diagnostics.add(message)
+
+        continue
+
+    return errors
+
+
+def main(args: argparse.Namespace) -> int:
+    if args.full_diagnostics_file:
+        full_diagnostics_file = Path(args.full_diagnostics_file)
+        full_diagnostics_file.write_text("")
+    else:
+        full_diagnostics_file = None
+
+    errors_by_platform_file = Path(__file__).parent / "_check_type_completeness.json"
+    if errors_by_platform_file.exists():
+        with open(errors_by_platform_file) as f:
+            errors_by_platform = json.load(f)
+    else:
+        errors_by_platform = {"Linux": [], "Windows": [], "Darwin": [], "all": []}
+
+    changed = False
+    for platform in "Linux", "Windows", "Darwin":
+        platform_errors = errors_by_platform[platform] + errors_by_platform["all"]
+        print("*" * 20, f"\nChecking {platform}...")
+        errors = check_type(platform, full_diagnostics_file, platform_errors)
+
+        new_errors = [e for e in errors if e not in platform_errors]
+        missing_errors = [e for e in platform_errors if e not in errors]
+
+        if new_errors:
+            print(
+                f"New errors introduced in `pyright --verifytypes`. Fix them, or ignore them by modifying {errors_by_platform_file}. The latter can be done by pre-commit CI bot."
+            )
+            changed = True
+        if missing_errors:
+            print(
+                f"Congratulations, you have resolved existing errors! Please remove them from {errors_by_platform_file}, either manually or with the pre-commit CI bot."
+            )
+            changed = True
+            print(missing_errors)
+
+        errors_by_platform[platform] = errors
+    print("*" * 20)
+
+    # cut down the size of the json file by a lot, and make it easier to parse for
+    # humans, by moving errors that appear on all platforms to a separate category
+    errors_by_platform["all"] = []
+    for e in errors_by_platform["Linux"].copy():
+        if e in errors_by_platform["Darwin"] and e in errors_by_platform["Windows"]:
+            for platform in "Linux", "Windows", "Darwin":
+                errors_by_platform[platform].remove(e)
+            errors_by_platform["all"].append(e)
+
+    if changed and args.overwrite_file:
+        with open(errors_by_platform_file, "w") as f:
+            json.dump(errors_by_platform, f, indent=4, sort_keys=True)
+            # newline at end of file
+            f.write("\n")
+
+    # True -> 1 -> non-zero exit value -> error
+    return changed
+
+
+parser = argparse.ArgumentParser()
+parser.add_argument(
+    "--overwrite-file",
+    action="store_true",
+    default=False,
+    help="Use this flag to overwrite the current stored results. Either in CI together with a diff check, or to avoid having to manually correct it.",
+)
+parser.add_argument(
+    "--full-diagnostics-file",
+    type=Path,
+    default=None,
+    help="Use this for debugging, it will dump the output of all three pyright runs by platform into this file.",
+)
+args = parser.parse_args()
+
+assert __name__ == "__main__", "This script should be run standalone"
+sys.exit(main(args))