gh-104555: Runtime-checkable protocols: Don't let previous calls to isinstance() influence whether issubclass() raises an exception (#104559)

AlexWaygood · carljm · web-flow · commit b27fe67f3c64 · 2023-05-17T23:43:12.000Z
Co-authored-by: Carl Meyer &lt;carl@oddbird.net&gt;
diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py
@@ -2695,6 +2695,82 @@ class D(PNonCall): ...
         with self.assertRaises(TypeError):
             issubclass(D, PNonCall)
 
+    def test_no_weird_caching_with_issubclass_after_isinstance(self):
+        @runtime_checkable
+        class Spam(Protocol):
+            x: int
+
+        class Eggs:
+            def __init__(self) -> None:
+                self.x = 42
+
+        self.assertIsInstance(Eggs(), Spam)
+
+        # gh-104555: If we didn't override ABCMeta.__subclasscheck__ in _ProtocolMeta,
+        # TypeError wouldn't be raised here,
+        # as the cached result of the isinstance() check immediately above
+        # would mean the issubclass() call would short-circuit
+        # before we got to the "raise TypeError" line
+        with self.assertRaises(TypeError):
+            issubclass(Eggs, Spam)
+
+    def test_no_weird_caching_with_issubclass_after_isinstance_2(self):
+        @runtime_checkable
+        class Spam(Protocol):
+            x: int
+
+        class Eggs: ...
+
+        self.assertNotIsInstance(Eggs(), Spam)
+
+        # gh-104555: If we didn't override ABCMeta.__subclasscheck__ in _ProtocolMeta,
+        # TypeError wouldn't be raised here,
+        # as the cached result of the isinstance() check immediately above
+        # would mean the issubclass() call would short-circuit
+        # before we got to the "raise TypeError" line
+        with self.assertRaises(TypeError):
+            issubclass(Eggs, Spam)
+
+    def test_no_weird_caching_with_issubclass_after_isinstance_3(self):
+        @runtime_checkable
+        class Spam(Protocol):
+            x: int
+
+        class Eggs:
+            def __getattr__(self, attr):
+                if attr == "x":
+                    return 42
+                raise AttributeError(attr)
+
+        self.assertNotIsInstance(Eggs(), Spam)
+
+        # gh-104555: If we didn't override ABCMeta.__subclasscheck__ in _ProtocolMeta,
+        # TypeError wouldn't be raised here,
+        # as the cached result of the isinstance() check immediately above
+        # would mean the issubclass() call would short-circuit
+        # before we got to the "raise TypeError" line
+        with self.assertRaises(TypeError):
+            issubclass(Eggs, Spam)
+
+    def test_no_weird_caching_with_issubclass_after_isinstance_pep695(self):
+        @runtime_checkable
+        class Spam[T](Protocol):
+            x: T
+
+        class Eggs[T]:
+            def __init__(self, x: T) -> None:
+                self.x = x
+
+        self.assertIsInstance(Eggs(42), Spam)
+
+        # gh-104555: If we didn't override ABCMeta.__subclasscheck__ in _ProtocolMeta,
+        # TypeError wouldn't be raised here,
+        # as the cached result of the isinstance() check immediately above
+        # would mean the issubclass() call would short-circuit
+        # before we got to the "raise TypeError" line
+        with self.assertRaises(TypeError):
+            issubclass(Eggs, Spam)
+
     def test_protocols_isinstance(self):
         T = TypeVar('T')
 
diff --git a/Lib/typing.py b/Lib/typing.py
@@ -1775,8 +1775,8 @@ def _pickle_pskwargs(pskwargs):
 
 
 class _ProtocolMeta(ABCMeta):
-    # This metaclass is really unfortunate and exists only because of
-    # the lack of __instancehook__.
+    # This metaclass is somewhat unfortunate,
+    # but is necessary for several reasons...
     def __init__(cls, *args, **kwargs):
         super().__init__(*args, **kwargs)
         cls.__protocol_attrs__ = _get_protocol_attrs(cls)
@@ -1786,6 +1786,17 @@ def __init__(cls, *args, **kwargs):
             callable(getattr(cls, attr, None)) for attr in cls.__protocol_attrs__
         )
 
+    def __subclasscheck__(cls, other):
+        if (
+            getattr(cls, '_is_protocol', False)
+            and not cls.__callable_proto_members_only__
+            and not _allow_reckless_class_checks(depth=2)
+        ):
+            raise TypeError(
+                "Protocols with non-method members don't support issubclass()"
+            )
+        return super().__subclasscheck__(other)
+
     def __instancecheck__(cls, instance):
         # We need this method for situations where attributes are
         # assigned in __init__.
@@ -1869,11 +1880,6 @@ def _proto_hook(other):
                 raise TypeError("Instance and class checks can only be used with"
                                 " @runtime_checkable protocols")
 
-            if not cls.__callable_proto_members_only__ :
-                if _allow_reckless_class_checks():
-                    return NotImplemented
-                raise TypeError("Protocols with non-method members"
-                                " don't support issubclass()")
             if not isinstance(other, type):
                 # Same error message as for issubclass(1, int).
                 raise TypeError('issubclass() arg 1 must be a class')
diff --git a/Misc/NEWS.d/next/Library/2023-05-17-16-58-23.gh-issue-104555.5rb5oM.rst b/Misc/NEWS.d/next/Library/2023-05-17-16-58-23.gh-issue-104555.5rb5oM.rst
@@ -0,0 +1,7 @@
+Fix issue where an :func:`issubclass` check comparing a class ``X`` against a
+:func:`runtime-checkable protocol <typing.runtime_checkable>` ``Y`` with
+non-callable members would not cause :exc:`TypeError` to be raised if an
+:func:`isinstance` call had previously been made comparing an instance of ``X``
+to ``Y``. This issue was present in edge cases on Python 3.11, but became more
+prominent in 3.12 due to some unrelated changes that were made to
+runtime-checkable protocols. Patch by Alex Waygood.
