Add typing for _wait_for_object.py (#2755)

* Add typing for `_wait_for_object.py`

* Fix CData type

* Future annotations

* Use explicit type alias

* Maybe saying it's `cffi.api.FFI` will fix it?

* Fix this type alias

* Handle None in raise_winerror()

* Fix copy-paste error

Interestingly not caught by tests, maybe we need specific tests for this.

* Update `pyproject.toml`

* Add a test for raise_winerror()

* Change names

* Fix mypy CI failure on linux and macos

* Remove `_wait_for_object` and `_core._windows_cffi` fully typed ignores

* Import sys for new mypy CI fix

* Entirely skip type checking test_windows on other platforms

---------

Co-authored-by: Spencer Brown <spencerb21@live.com>
Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>

pyproject.toml

@@ -57,15 +57,11 @@ disallow_untyped_defs = true
 check_untyped_defs = false
 disallow_untyped_calls = false
 
-
 # files not yet fully typed
 [[tool.mypy.overrides]]
 module = [
 # 2745
 "trio/_ssl",
-# 2755
-"trio/_core/_windows_cffi",
-"trio/_wait_for_object",
 # 2761
 "trio/_core/_generated_io_windows",
 "trio/_core/_io_windows",

trio/_core/_tests/test_windows.py

@@ -1,13 +1,18 @@
 import os
+import sys
 import tempfile
 from contextlib import contextmanager
+from typing import TYPE_CHECKING
+from unittest.mock import create_autospec
 
 import pytest
 
 on_windows = os.name == "nt"
 # Mark all the tests in this file as being windows-only
 pytestmark = pytest.mark.skipif(not on_windows, reason="windows only")
 
+assert sys.platform == "win32" or not TYPE_CHECKING  # Skip type checking on Windows
+
 from ... import _core, sleep
 from ...testing import wait_all_tasks_blocked
 from .tutil import gc_collect_harder, restore_unraisablehook, slow
@@ -22,6 +27,43 @@
     )
 
 
+def test_winerror(monkeypatch) -> None:
+    mock = create_autospec(ffi.getwinerror)
+    monkeypatch.setattr(ffi, "getwinerror", mock)
+
+    # Returning none = no error, should not happen.
+    mock.return_value = None
+    with pytest.raises(RuntimeError, match="No error set"):
+        raise_winerror()
+    mock.assert_called_once_with()
+    mock.reset_mock()
+
+    with pytest.raises(RuntimeError, match="No error set"):
+        raise_winerror(38)
+    mock.assert_called_once_with(38)
+    mock.reset_mock()
+
+    mock.return_value = (12, "test error")
+    with pytest.raises(OSError) as exc:
+        raise_winerror(filename="file_1", filename2="file_2")
+    mock.assert_called_once_with()
+    mock.reset_mock()
+    assert exc.value.winerror == 12
+    assert exc.value.strerror == "test error"
+    assert exc.value.filename == "file_1"
+    assert exc.value.filename2 == "file_2"
+
+    # With an explicit number passed in, it overrides what getwinerror() returns.
+    with pytest.raises(OSError) as exc:
+        raise_winerror(18, filename="a/file", filename2="b/file")
+    mock.assert_called_once_with(18)
+    mock.reset_mock()
+    assert exc.value.winerror == 18
+    assert exc.value.strerror == "test error"
+    assert exc.value.filename == "a/file"
+    assert exc.value.filename2 == "b/file"
+
+
 # The undocumented API that this is testing should be changed to stop using
 # UnboundedQueue (or just removed until we have time to redo it), but until
 # then we filter out the warning.

trio/_core/_windows_cffi.py

@@ -1,5 +1,11 @@
+from __future__ import annotations
+
 import enum
 import re
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from typing_extensions import NoReturn, TypeAlias
 
 import cffi
 
@@ -215,7 +221,8 @@
 #   being _MSC_VER >= 800)
 LIB = re.sub(r"\bPASCAL\b", "__stdcall", LIB)
 
-ffi = cffi.FFI()
+ffi = cffi.api.FFI()
+CData: TypeAlias = cffi.api.FFI.CData
 ffi.cdef(LIB)
 
 kernel32 = ffi.dlopen("kernel32.dll")
@@ -302,23 +309,33 @@ class IoControlCodes(enum.IntEnum):
 ################################################################
 
 
-def _handle(obj):
+def _handle(obj: int | CData) -> CData:
     # For now, represent handles as either cffi HANDLEs or as ints.  If you
     # try to pass in a file descriptor instead, it's not going to work
     # out. (For that msvcrt.get_osfhandle does the trick, but I don't know if
     # we'll actually need that for anything...) For sockets this doesn't
     # matter, Python never allocates an fd. So let's wait until we actually
     # encounter the problem before worrying about it.
-    if type(obj) is int:
+    if isinstance(obj, int):
         return ffi.cast("HANDLE", obj)
-    else:
-        return obj
+    return obj
 
 
-def raise_winerror(winerror=None, *, filename=None, filename2=None):
+def raise_winerror(
+    winerror: int | None = None,
+    *,
+    filename: str | None = None,
+    filename2: str | None = None,
+) -> NoReturn:
     if winerror is None:
-        winerror, msg = ffi.getwinerror()
+        err = ffi.getwinerror()
+        if err is None:
+            raise RuntimeError("No error set?")
+        winerror, msg = err
     else:
-        _, msg = ffi.getwinerror(winerror)
+        err = ffi.getwinerror(winerror)
+        if err is None:
+            raise RuntimeError("No error set?")
+        _, msg = err
     # https://docs.python.org/3/library/exceptions.html#OSError
     raise OSError(0, msg, filename, winerror, filename2)

trio/_wait_for_object.py

@@ -1,11 +1,20 @@
+from __future__ import annotations
+
 import math
 
 import trio
 
-from ._core._windows_cffi import ErrorCodes, _handle, ffi, kernel32, raise_winerror
+from ._core._windows_cffi import (
+    CData,
+    ErrorCodes,
+    _handle,
+    ffi,
+    kernel32,
+    raise_winerror,
+)
 
 
-async def WaitForSingleObject(obj):
+async def WaitForSingleObject(obj: int | CData) -> None:
     """Async and cancellable variant of WaitForSingleObject. Windows only.
 
     Args:
@@ -45,7 +54,7 @@ async def WaitForSingleObject(obj):
         kernel32.CloseHandle(cancel_handle)
 
 
-def WaitForMultipleObjects_sync(*handles):
+def WaitForMultipleObjects_sync(*handles: int | CData) -> None:
     """Wait for any of the given Windows handles to be signaled."""
     n = len(handles)
     handle_arr = ffi.new(f"HANDLE[{n}]")