Visitar URL original
gh-105144: Runtime-checkable protocols: move all 'sanity checks' to `_ProtocolMeta.__subclasscheck__` by AlexWaygood · Pull Request #105152 · python/cpython · GitHub
Skip to content

gh-105144: Runtime-checkable protocols: move all 'sanity checks' to _ProtocolMeta.__subclasscheck__ #105152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AlexWaygood merged 4 commits into from
May 31, 2023
Merged

gh-105144: Runtime-checkable protocols: move all 'sanity checks' to _ProtocolMeta.__subclasscheck__ #105152

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
937e8dc
Runtime-checkable protocols: move all 'sanity checks' to `_ProtocolMe…
AlexWaygood May 31, 2023
3b56274
Simplify
AlexWaygood May 31, 2023
0b13ae0
Explicitly clear the cache
AlexWaygood May 31, 2023
3ba9647
off-by-one error in a comment!
AlexWaygood May 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 91 additions & 18 deletions Lib/test/test_typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import contextlib
import collections
import collections.abc
from collections import defaultdict
from functools import lru_cache, wraps
import inspect
Expand Down Expand Up @@ -2722,19 +2723,41 @@ def x(self): ...
self.assertIsSubclass(C, PG)
self.assertIsSubclass(BadP, PG)

with self.assertRaises(TypeError):
no_subscripted_generics = (
"Subscripted generics cannot be used with class and instance checks"
)

with self.assertRaisesRegex(TypeError, no_subscripted_generics):
issubclass(C, PG[T])
with self.assertRaises(TypeError):
with self.assertRaisesRegex(TypeError, no_subscripted_generics):
issubclass(C, PG[C])
with self.assertRaises(TypeError):

only_runtime_checkable_protocols = (
"Instance and class checks can only be used with "
"@runtime_checkable protocols"
)

with self.assertRaisesRegex(TypeError, only_runtime_checkable_protocols):
issubclass(C, BadP)
with self.assertRaises(TypeError):
with self.assertRaisesRegex(TypeError, only_runtime_checkable_protocols):
issubclass(C, BadPG)
with self.assertRaises(TypeError):

with self.assertRaisesRegex(TypeError, no_subscripted_generics):
issubclass(P, PG[T])
with self.assertRaises(TypeError):
with self.assertRaisesRegex(TypeError, no_subscripted_generics):
issubclass(PG, PG[int])

only_classes_allowed = r"issubclass\(\) arg 1 must be a class"

with self.assertRaisesRegex(TypeError, only_classes_allowed):
issubclass(1, P)
with self.assertRaisesRegex(TypeError, only_classes_allowed):
issubclass(1, PG)
with self.assertRaisesRegex(TypeError, only_classes_allowed):
issubclass(1, BadP)
with self.assertRaisesRegex(TypeError, only_classes_allowed):
issubclass(1, BadPG)

def test_protocols_issubclass_non_callable(self):
class C:
x = 1
Expand All @@ -2743,12 +2766,19 @@ class C:
class PNonCall(Protocol):
x = 1

with self.assertRaises(TypeError):
non_callable_members_illegal = (
"Protocols with non-method members don't support issubclass()"
)

with self.assertRaisesRegex(TypeError, non_callable_members_illegal):
issubclass(C, PNonCall)

self.assertIsInstance(C(), PNonCall)
PNonCall.register(C)
with self.assertRaises(TypeError):

with self.assertRaisesRegex(TypeError, non_callable_members_illegal):
issubclass(C, PNonCall)

self.assertIsInstance(C(), PNonCall)

# check that non-protocol subclasses are not affected
Expand All @@ -2759,7 +2789,8 @@ class D(PNonCall): ...
D.register(C)
self.assertIsSubclass(C, D)
self.assertIsInstance(C(), D)
with self.assertRaises(TypeError):

with self.assertRaisesRegex(TypeError, non_callable_members_illegal):
issubclass(D, PNonCall)

def test_no_weird_caching_with_issubclass_after_isinstance(self):
Expand All @@ -2778,7 +2809,10 @@ def __init__(self) -> None:
# 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):
with self.assertRaisesRegex(
TypeError,
"Protocols with non-method members don't support issubclass()"
):
issubclass(Eggs, Spam)

def test_no_weird_caching_with_issubclass_after_isinstance_2(self):
Expand All @@ -2795,7 +2829,10 @@ class Eggs: ...
# 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):
with self.assertRaisesRegex(
TypeError,
"Protocols with non-method members don't support issubclass()"
):
issubclass(Eggs, Spam)

def test_no_weird_caching_with_issubclass_after_isinstance_3(self):
Expand All @@ -2816,7 +2853,10 @@ def __getattr__(self, attr):
# 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):
with self.assertRaisesRegex(
TypeError,
"Protocols with non-method members don't support issubclass()"
):
issubclass(Eggs, Spam)

def test_no_weird_caching_with_issubclass_after_isinstance_pep695(self):
Expand All @@ -2835,7 +2875,10 @@ def __init__(self, x: T) -> None:
# 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):
with self.assertRaisesRegex(
TypeError,
"Protocols with non-method members don't support issubclass()"
):
issubclass(Eggs, Spam)

def test_protocols_isinstance(self):
Expand Down Expand Up @@ -2883,13 +2926,21 @@ def __init__(self):
with self.subTest(klass=klass.__name__, proto=proto.__name__):
self.assertIsInstance(klass(), proto)

with self.assertRaises(TypeError):
no_subscripted_generics = "Subscripted generics cannot be used with class and instance checks"

with self.assertRaisesRegex(TypeError, no_subscripted_generics):
isinstance(C(), PG[T])
with self.assertRaises(TypeError):
with self.assertRaisesRegex(TypeError, no_subscripted_generics):
isinstance(C(), PG[C])
with self.assertRaises(TypeError):

only_runtime_checkable_msg = (
"Instance and class checks can only be used "
"with @runtime_checkable protocols"
)

with self.assertRaisesRegex(TypeError, only_runtime_checkable_msg):
isinstance(C(), BadP)
with self.assertRaises(TypeError):
with self.assertRaisesRegex(TypeError, only_runtime_checkable_msg):
isinstance(C(), BadPG)

def test_protocols_isinstance_properties_and_descriptors(self):
Expand Down Expand Up @@ -3274,7 +3325,7 @@ class P(Protocol):

class C: pass

with self.assertRaises(TypeError):
with self.assertRaisesRegex(TypeError, r"issubclass\(\) arg 1 must be a class"):
issubclass(C(), P)

def test_defining_generic_protocols(self):
Expand Down Expand Up @@ -3654,6 +3705,28 @@ def __init__(self):

Foo() # Previously triggered RecursionError

def test_interaction_with_isinstance_checks_on_superclasses_with_ABCMeta(self):
Comment thread
JelleZijlstra marked this conversation as resolved.
# Ensure the cache is empty, or this test won't work correctly
collections.abc.Sized._abc_registry_clear()

class Foo(collections.abc.Sized, Protocol): pass

# gh-105144: this previously raised TypeError
# if a Protocol subclass of Sized had been created
# before any isinstance() checks against Sized
self.assertNotIsInstance(1, collections.abc.Sized)

def test_interaction_with_isinstance_checks_on_superclasses_with_ABCMeta_2(self):
# Ensure the cache is empty, or this test won't work correctly
collections.abc.Sized._abc_registry_clear()

class Foo(typing.Sized, Protocol): pass

# gh-105144: this previously raised TypeError
# if a Protocol subclass of Sized had been created
# before any isinstance() checks against Sized
self.assertNotIsInstance(1, typing.Sized)


class GenericTests(BaseTestCase):

Expand Down
33 changes: 15 additions & 18 deletions Lib/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1727,7 +1727,7 @@ def _caller(depth=1, default='__main__'):
pass
return None

def _allow_reckless_class_checks(depth=3):
def _allow_reckless_class_checks(depth=2):
"""Allow instance and class checks for special stdlib modules.

The abc and functools modules indiscriminately call isinstance() and
Expand Down Expand Up @@ -1782,14 +1782,22 @@ def __init__(cls, *args, **kwargs):
)

def __subclasscheck__(cls, other):
if not isinstance(other, type):
Comment thread
JelleZijlstra marked this conversation as resolved.
# Same error message as for issubclass(1, int).
raise TypeError('issubclass() arg 1 must be a class')
if (
getattr(cls, '_is_protocol', False)
and not cls.__callable_proto_members_only__
and not _allow_reckless_class_checks(depth=2)
and not _allow_reckless_class_checks()
):
raise TypeError(
"Protocols with non-method members don't support issubclass()"
)
if not cls.__callable_proto_members_only__:
raise TypeError(
"Protocols with non-method members don't support issubclass()"
)
if not getattr(cls, '_is_runtime_protocol', False):
raise TypeError(
"Instance and class checks can only be used with "
"@runtime_checkable protocols"
)
return super().__subclasscheck__(other)

def __instancecheck__(cls, instance):
Expand All @@ -1801,7 +1809,7 @@ def __instancecheck__(cls, instance):

if (
not getattr(cls, '_is_runtime_protocol', False) and
not _allow_reckless_class_checks(depth=2)
not _allow_reckless_class_checks()
):
raise TypeError("Instance and class checks can only be used with"
" @runtime_checkable protocols")
Expand Down Expand Up @@ -1869,17 +1877,6 @@ def _proto_hook(other):
if not cls.__dict__.get('_is_protocol', False):
return NotImplemented

# First, perform various sanity checks.
if not getattr(cls, '_is_runtime_protocol', False):
if _allow_reckless_class_checks():
return NotImplemented
raise TypeError("Instance and class checks can only be used with"
" @runtime_checkable protocols")

if not isinstance(other, type):
# Same error message as for issubclass(1, int).
raise TypeError('issubclass() arg 1 must be a class')

# Second, perform the actual structural compatibility check.
Comment thread
AlexWaygood marked this conversation as resolved.
Outdated
for attr in cls.__protocol_attrs__:
for base in other.__mro__:
Expand Down
5 changes: 5 additions & 0 deletions Misc/NEWS.d/next/Library/2023-05-31-16-58-42.gh-issue-105144.Oqfn0V.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix a recent regression in the :mod:`typing` module. The regression meant
that doing ``class Foo(X, typing.Protocol)``, where ``X`` was a class that
had :class:`abc.ABCMeta` as its metaclass, would then cause subsequent
``isinstance(1, X)`` calls to erroneously raise :exc:`TypeError`. Patch by
Alex Waygood.