Type RaisesGroup properly (#3141)

* Improve RaisesGroup typing now that mypy supports narrowing by self type

* remove Sphinx workaround

---------

Co-authored-by: John Litborn <[email protected]>

Author: A5rocks

docs/source/conf.py

@@ -22,7 +22,6 @@
 import glob
 import os
 import sys
-import types
 from pathlib import Path
 from typing import TYPE_CHECKING, cast
 
@@ -152,16 +151,6 @@ def autodoc_process_signature(
     return_annotation: str,
 ) -> tuple[str, str]:
     """Modify found signatures to fix various issues."""
-    if name == "trio.testing._raises_group._ExceptionInfo.type":
-        # This has the type "type[E]", which gets resolved into the property itself.
-        # That means Sphinx can't resolve it. Fix the issue by overwriting with a fully-qualified
-        # name.
-        assert isinstance(obj, property), obj
-        assert isinstance(obj.fget, types.FunctionType), obj.fget
-        assert (
-            obj.fget.__annotations__["return"] == "type[MatchE]"
-        ), obj.fget.__annotations__
-        obj.fget.__annotations__["return"] = "type[~trio.testing._raises_group.MatchE]"
     if signature is not None:
         signature = signature.replace("~_contextvars.Context", "~contextvars.Context")
         if name == "trio.lowlevel.RunVar":  # Typevar is not useful here.
@@ -170,16 +159,6 @@ def autodoc_process_signature(
             # Strip the type from the union, make it look like = ...
             signature = signature.replace(" | type[trio._core._local._NoValue]", "")
             signature = signature.replace("<class 'trio._core._local._NoValue'>", "...")
-        if name in ("trio.testing.RaisesGroup", "trio.testing.Matcher") and (
-            "+E" in signature or "+MatchE" in signature
-        ):
-            # This typevar being covariant isn't handled correctly in some cases, strip the +
-            # and insert the fully-qualified name.
-            signature = signature.replace("+E", "~trio.testing._raises_group.E")
-            signature = signature.replace(
-                "+MatchE",
-                "~trio.testing._raises_group.MatchE",
-            )
         if "DTLS" in name:
             signature = signature.replace("SSL.Context", "OpenSSL.SSL.Context")
         # Don't specify PathLike[str] | PathLike[bytes], this is just for humans.

newsfragments/3141.bugfix.rst

@@ -0,0 +1 @@
+Fix `trio.testing.RaisesGroup`'s typing.

src/trio/_tests/test_testing_raisesgroup.py

@@ -24,7 +24,7 @@ def test_raises_group() -> None:
             f'Invalid argument "{TypeError()!r}" must be exception type, Matcher, or RaisesGroup.',
         ),
     ):
-        RaisesGroup(TypeError())
+        RaisesGroup(TypeError())  # type: ignore[call-overload]
 
     with RaisesGroup(ValueError):
         raise ExceptionGroup("foo", (ValueError(),))

src/trio/_tests/type_tests/path.py

@@ -54,7 +54,7 @@ def sync_attrs(path: trio.Path) -> None:
     assert_type(path.match("*.py"), bool)
     assert_type(path.relative_to("/usr"), trio.Path)
     if sys.version_info >= (3, 12):
-        assert_type(path.relative_to("/", walk_up=True), bool)
+        assert_type(path.relative_to("/", walk_up=True), trio.Path)
     assert_type(path.with_name("filename.txt"), trio.Path)
     assert_type(path.with_stem("readme"), trio.Path)
     assert_type(path.with_suffix(".log"), trio.Path)

src/trio/_tests/type_tests/raisesgroup.py

@@ -1,21 +1,7 @@
-"""The typing of RaisesGroup involves a lot of deception and lies, since AFAIK what we
-actually want to achieve is ~impossible. This is because we specify what we expect with
-instances of RaisesGroup and exception classes, but excinfo.value will be instances of
-[Base]ExceptionGroup and instances of exceptions. So we need to "translate" from
-RaisesGroup to ExceptionGroup.
-
-The way it currently works is that RaisesGroup[E] corresponds to
-ExceptionInfo[BaseExceptionGroup[E]], so the top-level group will be correct. But
-RaisesGroup[RaisesGroup[ValueError]] will become
-ExceptionInfo[BaseExceptionGroup[RaisesGroup[ValueError]]]. To get around that we specify
-RaisesGroup as a subclass of BaseExceptionGroup during type checking - which should mean
-that most static type checking for end users should be mostly correct.
-"""
-
 from __future__ import annotations
 
 import sys
-from typing import Union
+from typing import Callable, Union
 
 from trio.testing import Matcher, RaisesGroup
 from typing_extensions import assert_type
@@ -26,17 +12,6 @@
 # split into functions to isolate the different scopes
 
 
-def check_inheritance_and_assignments() -> None:
-    # Check inheritance
-    _: BaseExceptionGroup[ValueError] = RaisesGroup(ValueError)
-    _ = RaisesGroup(RaisesGroup(ValueError))  # type: ignore
-
-    a: BaseExceptionGroup[BaseExceptionGroup[ValueError]]
-    a = RaisesGroup(RaisesGroup(ValueError))
-    a = BaseExceptionGroup("", (BaseExceptionGroup("", (ValueError(),)),))
-    assert a
-
-
 def check_matcher_typevar_default(e: Matcher) -> object:
     assert e.exception_type is not None
     exc: type[BaseException] = e.exception_type
@@ -46,29 +21,32 @@ def check_matcher_typevar_default(e: Matcher) -> object:
 
 
 def check_basic_contextmanager() -> None:
-    # One level of Group is correctly translated - except it's a BaseExceptionGroup
-    # instead of an ExceptionGroup.
     with RaisesGroup(ValueError) as e:
         raise ExceptionGroup("foo", (ValueError(),))
-    assert_type(e.value, BaseExceptionGroup[ValueError])
+    assert_type(e.value, ExceptionGroup[ValueError])
 
 
 def check_basic_matches() -> None:
     # check that matches gets rid of the naked ValueError in the union
     exc: ExceptionGroup[ValueError] | ValueError = ExceptionGroup("", (ValueError(),))
     if RaisesGroup(ValueError).matches(exc):
-        assert_type(exc, BaseExceptionGroup[ValueError])
+        assert_type(exc, ExceptionGroup[ValueError])
+
+    # also check that BaseExceptionGroup shows up for BaseExceptions
+    if RaisesGroup(KeyboardInterrupt).matches(exc):
+        assert_type(exc, BaseExceptionGroup[KeyboardInterrupt])
 
 
 def check_matches_with_different_exception_type() -> None:
-    # This should probably raise some type error somewhere, since
-    # ValueError != KeyboardInterrupt
     e: BaseExceptionGroup[KeyboardInterrupt] = BaseExceptionGroup(
         "",
         (KeyboardInterrupt(),),
     )
+
+    # note: it might be tempting to have this warn.
+    # however, that isn't possible with current typing
     if RaisesGroup(ValueError).matches(e):
-        assert_type(e, BaseExceptionGroup[ValueError])
+        assert_type(e, ExceptionGroup[ValueError])
 
 
 def check_matcher_init() -> None:
@@ -134,59 +112,44 @@ def handle_value(e: BaseExceptionGroup[ValueError]) -> bool:
 
 
 def raisesgroup_narrow_baseexceptiongroup() -> None:
-    """Check type narrowing specifically for the container exceptiongroup.
-    This is not currently working, and after playing around with it for a bit
-    I think the only way is to introduce a subclass `NonBaseRaisesGroup`, and overload
-    `__new__` in Raisesgroup to return the subclass when exceptions are non-base.
-    (or make current class BaseRaisesGroup and introduce RaisesGroup for non-base)
-    I encountered problems trying to type this though, see
-    https://github.com/python/mypy/issues/17251
-    That is probably possible to work around by entirely using `__new__` instead of
-    `__init__`, but........ ugh.
-    """
+    """Check type narrowing specifically for the container exceptiongroup."""
 
     def handle_group(e: ExceptionGroup[Exception]) -> bool:
         return True
 
     def handle_group_value(e: ExceptionGroup[ValueError]) -> bool:
         return True
 
-    # should work, but BaseExceptionGroup does not get narrowed to ExceptionGroup
-    RaisesGroup(ValueError, check=handle_group_value)  # type: ignore
+    RaisesGroup(ValueError, check=handle_group_value)
 
-    # should work, but BaseExceptionGroup does not get narrowed to ExceptionGroup
-    RaisesGroup(Exception, check=handle_group)  # type: ignore
+    RaisesGroup(Exception, check=handle_group)
 
 
 def check_matcher_transparent() -> None:
     with RaisesGroup(Matcher(ValueError)) as e:
         ...
     _: BaseExceptionGroup[ValueError] = e.value
-    assert_type(e.value, BaseExceptionGroup[ValueError])
+    assert_type(e.value, ExceptionGroup[ValueError])
 
 
 def check_nested_raisesgroups_contextmanager() -> None:
     with RaisesGroup(RaisesGroup(ValueError)) as excinfo:
         raise ExceptionGroup("foo", (ValueError(),))
 
-    # thanks to inheritance this assignment works
     _: BaseExceptionGroup[BaseExceptionGroup[ValueError]] = excinfo.value
-    # and it can mostly be treated like an exceptiongroup
-    print(excinfo.value.exceptions[0].exceptions[0])
 
-    # but assert_type reveals the lies
-    print(type(excinfo.value))  # would print "ExceptionGroup"
-    # typing says it's a BaseExceptionGroup
     assert_type(
         excinfo.value,
-        BaseExceptionGroup[RaisesGroup[ValueError]],
+        ExceptionGroup[ExceptionGroup[ValueError]],
     )
 
-    print(type(excinfo.value.exceptions[0]))  # would print "ExceptionGroup"
-    # but type checkers are utterly confused
     assert_type(
         excinfo.value.exceptions[0],
-        Union[RaisesGroup[ValueError], BaseExceptionGroup[RaisesGroup[ValueError]]],
+        # this union is because of how typeshed defines .exceptions
+        Union[
+            ExceptionGroup[ValueError],
+            ExceptionGroup[ExceptionGroup[ValueError]],
+        ],
     )
 
 
@@ -196,17 +159,17 @@ def check_nested_raisesgroups_matches() -> None:
         "",
         (ExceptionGroup("", (ValueError(),)),),
     )
-    # has the same problems as check_nested_raisesgroups_contextmanager
+
     if RaisesGroup(RaisesGroup(ValueError)).matches(exc):
-        assert_type(exc, BaseExceptionGroup[RaisesGroup[ValueError]])
+        assert_type(exc, ExceptionGroup[ExceptionGroup[ValueError]])
 
 
 def check_multiple_exceptions_1() -> None:
     a = RaisesGroup(ValueError, ValueError)
     b = RaisesGroup(Matcher(ValueError), Matcher(ValueError))
     c = RaisesGroup(ValueError, Matcher(ValueError))
 
-    d: BaseExceptionGroup[ValueError]
+    d: RaisesGroup[ValueError]
     d = a
     d = b
     d = c
@@ -219,7 +182,7 @@ def check_multiple_exceptions_2() -> None:
     b = RaisesGroup(Matcher(ValueError), TypeError)
     c = RaisesGroup(ValueError, TypeError)
 
-    d: BaseExceptionGroup[Exception]
+    d: RaisesGroup[Exception]
     d = a
     d = b
     d = c
@@ -252,3 +215,25 @@ def check_raisesgroup_overloads() -> None:
 
     # if they're both false we can of course specify nested raisesgroup
     RaisesGroup(RaisesGroup(ValueError))
+
+
+def check_triple_nested_raisesgroup() -> None:
+    with RaisesGroup(RaisesGroup(RaisesGroup(ValueError))) as e:
+        assert_type(e.value, ExceptionGroup[ExceptionGroup[ExceptionGroup[ValueError]]])
+
+
+def check_check_typing() -> None:
+    # mypy issue is https://github.com/python/mypy/issues/18185
+
+    # fmt: off
+    # mypy raises an error on `assert_type`
+    # pyright raises an error on `RaisesGroup(ValueError).check`
+    # to satisfy both, need to disable formatting and put it on one line
+    assert_type(RaisesGroup(ValueError).check,  # type: ignore
+        Union[
+            Callable[[BaseExceptionGroup[ValueError]], None],
+            Callable[[ExceptionGroup[ValueError]], None],
+            None,
+        ],
+    )
+    # fmt: on