Suggestions from code review

Co-authored-by: jakkdl <[email protected]>

Author: CoolCat467

autobahn/client.py

@@ -26,7 +26,7 @@ async def get_case_count(url: str) -> int:
     return int(case_count)
 
 
-async def get_case_info(url: str, case: str) -> Any:
+async def get_case_info(url: str, case: str) -> object:
     url = f'{url}/getCaseInfo?case={case}'
     async with open_websocket_url(url) as conn:
         return json.loads(await conn.get_message())
@@ -63,7 +63,10 @@ async def run_tests(args: argparse.Namespace) -> None:
         test_cases = list(range(1, case_count + 1))
     exception_cases = []
     for case in test_cases:
-        case_id = (await get_case_info(args.url, case))['id']
+        result = await get_case_info(args.url, case)
+        assert isinstance(result, dict)
+        case_id = result['id']
+        assert isinstance(case_id, int)
         if case_count:
             logger.info("Running test case %s (%d of %d)", case_id, case, case_count)
         else:

mypy.ini

@@ -0,0 +1,13 @@
+[tool.mypy]
+explicit_package_bases = true
+files = ["trio_websocket", "tests", "autobahn", "examples"]
+show_column_numbers = true
+show_error_codes = true
+show_traceback = true
+disallow_any_decorated = true
+disallow_any_unimported = true
+ignore_missing_imports = true
+local_partial_types = true
+no_implicit_optional = true
+strict = true
+warn_unreachable = true

pyproject.toml

@@ -35,6 +35,7 @@
         'Programming Language :: Python :: 3.12',
         'Programming Language :: Python :: Implementation :: CPython',
         'Programming Language :: Python :: Implementation :: PyPy',
+        'Typing :: Typed',
     ],
     python_requires=">=3.8",
     keywords='websocket client server trio',

setup.py

@@ -39,6 +39,7 @@
 from functools import partial, wraps
 from typing import TYPE_CHECKING, TypeVar, cast
 from unittest.mock import Mock, patch
+from importlib.metadata import version
 
 import attr
 import pytest
@@ -89,9 +90,11 @@
     from collections.abc import Awaitable, Callable
     from wsproto.events import Event
 
-    from typing_extensions import ParamSpec
+    from typing_extensions import ParamSpec, TypeAlias
     PS = ParamSpec("PS")
 
+    StapledMemoryStream: TypeAlias = trio.StapledStream[trio.testing.MemorySendStream, trio.testing.MemoryReceiveStream]
+
 WS_PROTO_VERSION = tuple(map(int, wsproto.__version__.split('.')))
 
 HOST = '127.0.0.1'
@@ -116,6 +119,8 @@ async def echo_server(nursery: trio.Nursery) -> AsyncGenerator[WebSocketServer,
     serve_fn = partial(serve_websocket, echo_request_handler, HOST, 0,
         ssl_context=None)
     server = await nursery.start(serve_fn)
+    # Cast needed because currently `nursery.start` has typing issues
+    # blocked by https://github.com/python/mypy/pull/17512
     yield cast(WebSocketServer, server)
 
 
@@ -147,37 +152,28 @@ def __init__(self, seconds: int) -> None:
         self._seconds = seconds
 
     def __call__(self, fn: Callable[PS, Awaitable[T]]) -> Callable[PS, Awaitable[T | None]]:
+        # Type of decorated function contains type `Any`
         @wraps(fn)
-        async def wrapper(*args: PS.args, **kwargs: PS.kwargs) -> T | None:
-            result: T | None = None
+        async def wrapper(  # type: ignore[misc]
+            *args: PS.args,
+            **kwargs: PS.kwargs,
+        ) -> T:
             with trio.move_on_after(self._seconds) as cancel_scope:
-                result = await fn(*args, **kwargs)
+                return await fn(*args, **kwargs)
             if cancel_scope.cancelled_caught:
                 pytest.fail(f'Test runtime exceeded the maximum {self._seconds} seconds')
-            return result
+            raise AssertionError("Should be unreachable")
         return wrapper
 
 
 @attr.s(hash=False, eq=False)
-class MemoryListener(
-    trio.abc.Listener[
-        "trio.StapledStream[trio.testing.MemorySendStream, trio.testing.MemoryReceiveStream]"
-    ]
-):
+class MemoryListener(trio.abc.Listener["StapledMemoryStream"]):
     closed: bool = attr.ib(default=False)
-    accepted_streams: list[
-        trio.StapledStream[trio.testing.MemorySendStream, trio.testing.MemoryReceiveStream]
-    ] = attr.ib(factory=list)
+    accepted_streams: list[StapledMemoryStream] = attr.ib(factory=list)
     queued_streams: tuple[
-        trio.MemorySendChannel[
-            trio.StapledStream[trio.testing.MemorySendStream, trio.testing.MemoryReceiveStream]
-        ],
-        trio.MemoryReceiveChannel[
-            trio.StapledStream[trio.testing.MemorySendStream, trio.testing.MemoryReceiveStream]
-        ],
-    ] = attr.ib(factory=lambda: trio.open_memory_channel[
-        "trio.StapledStream[trio.testing.MemorySendStream, trio.testing.MemoryReceiveStream]"
-    ](1))
+        trio.MemorySendChannel[StapledMemoryStream],
+        trio.MemoryReceiveChannel[StapledMemoryStream],
+    ] = attr.ib(factory=lambda: trio.open_memory_channel["StapledMemoryStream"](1))
     accept_hook: Callable[[], Awaitable[object]] | None = attr.ib(default=None)
 
     async def connect(self) -> trio.StapledStream[
@@ -385,8 +381,11 @@ async def test_ascii_encoded_path_is_ok(echo_server: WebSocketServer) -> None:
         assert conn.path == RESOURCE + '/' + path
 
 
+# Type ignore because @patch contains `Any`
 @patch('trio_websocket._impl.open_websocket')
-def test_client_open_url_options(open_websocket_mock: Mock) -> None:
+def test_client_open_url_options(  # type: ignore[misc]
+    open_websocket_mock: Mock,
+) -> None:
     """open_websocket_url() must pass its options on to open_websocket()"""
     port = 1234
     url = f'ws://{HOST}:{port}{RESOURCE}'
@@ -618,7 +617,7 @@ async def handler(request: WebSocketRequest) -> None:
     assert exc_info.value.__context__ is user_cancelled_context
 
 def _trio_default_non_strict_exception_groups() -> bool:
-    version = trio.__version__  # type: ignore[attr-defined]
+    version = version("trio")
     assert re.match(r'^0\.\d\d\.', version), "unexpected trio versioning scheme"
     return int(version[2:4]) < 25
 

tests/test_connection.py

@@ -12,6 +12,7 @@
 import struct
 import urllib.parse
 from typing import Any, List, NoReturn, Optional, Union, TypeVar, TYPE_CHECKING, Generic, cast
+from importlib.metadata import version
 
 import outcome
 import trio
@@ -38,22 +39,21 @@
 
 if TYPE_CHECKING:
     from types import TracebackType
+    from typing_extensions import Final
     from collections.abc import AsyncGenerator, Awaitable, Callable, Iterable, Coroutine, Sequence
 
-_IS_TRIO_MULTI_ERROR = tuple(
-    map(int, trio.__version__.split(".")[:2])  # type: ignore[attr-defined]
-) < (0, 22)
+_IS_TRIO_MULTI_ERROR: Final = tuple(map(int, version("trio").split(".")[:2])) < (0, 22)
 
 if _IS_TRIO_MULTI_ERROR:
     _TRIO_EXC_GROUP_TYPE = trio.MultiError  # type: ignore[attr-defined] # pylint: disable=no-member
 else:
     _TRIO_EXC_GROUP_TYPE = BaseExceptionGroup  # pylint: disable=possibly-used-before-assignment
 
-CONN_TIMEOUT = 60 # default connect & disconnect timeout, in seconds
-MESSAGE_QUEUE_SIZE = 1
-MAX_MESSAGE_SIZE = 2 ** 20 # 1 MiB
-RECEIVE_BYTES = 4 * 2 ** 10 # 4 KiB
-logger = logging.getLogger('trio-websocket')
+CONN_TIMEOUT: Final = 60 # default connect & disconnect timeout, in seconds
+MESSAGE_QUEUE_SIZE: Final = 1
+MAX_MESSAGE_SIZE: Final = 2 ** 20 # 1 MiB
+RECEIVE_BYTES: Final = 4 * 2 ** 10 # 4 KiB
+logger: Final = logging.getLogger('trio-websocket')
 
 T = TypeVar("T")
 E = TypeVar("E", bound=BaseException)
@@ -770,11 +770,16 @@ def __repr__(self) -> str:
                f'<code={self.code}, name={self.name}, reason={self.reason}>'
 
 
+NULL: Final = object()
+
+
 class Future(Generic[T]):
     ''' Represents a value that will be available in the future. '''
     def __init__(self) -> None:
         ''' Constructor. '''
-        self._value: T | None = None
+        # We do some type shenanigins
+        # Would do `T | Literal[NULL]` but that's not right apparently.
+        self._value: T = cast(T, NULL)
         self._value_event = trio.Event()
 
     def set_value(self, value: T) -> None:
@@ -793,7 +798,8 @@ async def wait_value(self) -> T:
         :returns: The value set by ``set_value()``.
         '''
         await self._value_event.wait()
-        return cast(T, self._value)
+        assert self._value is not NULL
+        return self._value
 
 
 class WebSocketRequest:
@@ -1509,6 +1515,9 @@ async def _reader_task(self) -> None:
                         handler = handlers[event_type]
                         logger.debug('%s received event: %s', self,
                             event_type)
+                        # Type checkers don't understand looking up type in handlers.
+                        # If we wanted to fix, best I can figure is we'd need a huge
+                        # if-else or case block for every type individually.
                         await handler(event)  # type: ignore[operator]
                     except KeyError:
                         logger.warning('%s received unknown event type: "%s"', self,