From a6b11b06f99a403573bec1bb6f131024d814bdd6 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 2 Jun 2023 14:28:03 +0100 Subject: [PATCH 1/6] Allow calling `issubclass(X, Protocol)` --- Lib/test/test_typing.py | 14 ++++++++++++-- Lib/typing.py | 4 +--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index f7114eb1fdbdd9..472a77a380a93f 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -2758,6 +2758,16 @@ def x(self): ... with self.assertRaisesRegex(TypeError, only_classes_allowed): issubclass(1, BadPG) + for proto in P, PG, BadP, BadPG: + with self.subTest(proto=proto.__name__): + self.assertIsSubclass(proto, Protocol) + + # TODO: the behaviour of some of these is questionable! + # For now, just check that these don't raise any Exceptions + issubclass(object, Protocol) + issubclass(str, Protocol) + issubclass(C, Protocol) + def test_protocols_issubclass_non_callable(self): class C: x = 1 @@ -3505,8 +3515,8 @@ def meth(self): self.assertTrue(P._is_protocol) self.assertTrue(PR._is_protocol) self.assertTrue(PG._is_protocol) - self.assertFalse(P._is_runtime_protocol) - self.assertTrue(PR._is_runtime_protocol) + self.assertFalse(getattr(P, "_is_runtime_protocol", False)) + self.assertTrue(getattr(PR, "_is_runtime_protocol", False)) self.assertTrue(PG[int]._is_protocol) self.assertEqual(typing._get_protocol_attrs(P), {'meth'}) self.assertEqual(typing._get_protocol_attrs(PR), {'x'}) diff --git a/Lib/typing.py b/Lib/typing.py index f589be7295c755..956114c0c2cd33 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1862,8 +1862,6 @@ def meth(self) -> T: ... """ __slots__ = () - _is_protocol = True - _is_runtime_protocol = False def __init_subclass__(cls, *args, **kwargs): super().__init_subclass__(*args, **kwargs) @@ -1904,7 +1902,7 @@ def _proto_hook(other): # ... otherwise check consistency of bases, and prohibit instantiation. for base in cls.__bases__: - if not (base in (object, Generic) or + if not (base in {object, Generic, Protocol} or base.__module__ in _PROTO_ALLOWLIST and base.__name__ in _PROTO_ALLOWLIST[base.__module__] or issubclass(base, Generic) and getattr(base, '_is_protocol', False)): From 36b2ac937effd3c5c7b531754908e66c0d9226a4 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 2 Jun 2023 14:42:55 +0100 Subject: [PATCH 2/6] Add back some implementation details so we don't unnecessarily break people --- Lib/typing.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/typing.py b/Lib/typing.py index 956114c0c2cd33..a0d8ed02281f41 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1862,6 +1862,8 @@ def meth(self) -> T: ... """ __slots__ = () + _is_protocol = False + _is_runtime_protocol = False def __init_subclass__(cls, *args, **kwargs): super().__init_subclass__(*args, **kwargs) From db51c8d6a5f72011cdc286932fcd8d1f97371387 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 2 Jun 2023 14:47:15 +0100 Subject: [PATCH 3/6] Revert unnecessary changes to tests --- Lib/test/test_typing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 472a77a380a93f..1101d1bc0665b6 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -3515,8 +3515,8 @@ def meth(self): self.assertTrue(P._is_protocol) self.assertTrue(PR._is_protocol) self.assertTrue(PG._is_protocol) - self.assertFalse(getattr(P, "_is_runtime_protocol", False)) - self.assertTrue(getattr(PR, "_is_runtime_protocol", False)) + self.assertFalse(P._is_runtime_protocol) + self.assertTrue(PR._is_runtime_protocol) self.assertTrue(PG[int]._is_protocol) self.assertEqual(typing._get_protocol_attrs(P), {'meth'}) self.assertEqual(typing._get_protocol_attrs(PR), {'x'}) From 776bf2bdbed6c01cc48bdb34446a41cac78bacae Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 2 Jun 2023 15:07:46 +0100 Subject: [PATCH 4/6] Fix longstanding bug and add many tests --- Lib/test/test_typing.py | 52 ++++++++++++++++--- Lib/typing.py | 2 + ...-06-02-14-57-11.gh-issue-105239.SAmuuj.rst | 2 + 3 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-06-02-14-57-11.gh-issue-105239.SAmuuj.rst diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 1101d1bc0665b6..29aa2d53c797a8 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -2758,15 +2758,55 @@ def x(self): ... with self.assertRaisesRegex(TypeError, only_classes_allowed): issubclass(1, BadPG) - for proto in P, PG, BadP, BadPG: + def test_issubclass_and_isinstance_on_Protocol_itself(self): + class C: + def x(self): pass + + self.assertNotIsSubclass(object, Protocol) + self.assertNotIsInstance(object(), Protocol) + + self.assertNotIsSubclass(str, Protocol) + self.assertNotIsInstance('foo', Protocol) + + self.assertNotIsSubclass(C, Protocol) + self.assertNotIsInstance(C(), Protocol) + + T = TypeVar('T') + + @runtime_checkable + class EmptyProtocol(Protocol): pass + + @runtime_checkable + class SupportsStartsWith(Protocol): + def startswith(self, x: str) -> bool: ... + + @runtime_checkable + class SupportsX(Protocol[T]): + def x(self): ... + + for proto in EmptyProtocol, SupportsStartsWith, SupportsX: with self.subTest(proto=proto.__name__): self.assertIsSubclass(proto, Protocol) - # TODO: the behaviour of some of these is questionable! - # For now, just check that these don't raise any Exceptions - issubclass(object, Protocol) - issubclass(str, Protocol) - issubclass(C, Protocol) + # gh-105237 / PR #105239: + # check that the presence of Protocol subclasses + # where `issubclass(X, )` evaluates to True + # doesn't influence the result of `issubclass(X, Protocol)` + + self.assertIsSubclass(object, EmptyProtocol) + self.assertIsInstance(object(), EmptyProtocol) + self.assertNotIsSubclass(object, Protocol) + self.assertNotIsInstance(object(), Protocol) + + self.assertIsSubclass(str, SupportsStartsWith) + self.assertIsInstance('foo', SupportsStartsWith) + self.assertNotIsSubclass(str, Protocol) + self.assertNotIsInstance('foo', Protocol) + + self.assertIsSubclass(C, SupportsX) + self.assertIsInstance(C(), SupportsX) + self.assertNotIsSubclass(C, Protocol) + self.assertNotIsInstance(C(), Protocol) def test_protocols_issubclass_non_callable(self): class C: diff --git a/Lib/typing.py b/Lib/typing.py index a0d8ed02281f41..a8573760e60b12 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1785,6 +1785,8 @@ def __subclasscheck__(cls, other): if not isinstance(other, type): # Same error message as for issubclass(1, int). raise TypeError('issubclass() arg 1 must be a class') + if cls is Protocol: + return type.__subclasscheck__(cls, other) if ( getattr(cls, '_is_protocol', False) and not _allow_reckless_class_checks() diff --git a/Misc/NEWS.d/next/Library/2023-06-02-14-57-11.gh-issue-105239.SAmuuj.rst b/Misc/NEWS.d/next/Library/2023-06-02-14-57-11.gh-issue-105239.SAmuuj.rst new file mode 100644 index 00000000000000..35e1b1a217b3a4 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-06-02-14-57-11.gh-issue-105239.SAmuuj.rst @@ -0,0 +1,2 @@ +Fix longstanding bug where ``issubclass(object, typing.Protocol)`` would +evaluate to ``True`` in some edge cases. Patch by Alex Waygood. From ffb7f1ccddcb0dafb9221c615e17add7ba746243 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 2 Jun 2023 15:25:36 +0100 Subject: [PATCH 5/6] Small micro-optimisation --- Lib/test/test_typing.py | 9 +++++++++ Lib/typing.py | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 29aa2d53c797a8..f6e0d203b5be86 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -2771,6 +2771,15 @@ def x(self): pass self.assertNotIsSubclass(C, Protocol) self.assertNotIsInstance(C(), Protocol) + only_classes_allowed = r"issubclass\(\) arg 1 must be a class" + + with self.assertRaisesRegex(TypeError, only_classes_allowed): + issubclass(1, Protocol) + with self.assertRaisesRegex(TypeError, only_classes_allowed): + issubclass('foo', Protocol) + with self.assertRaisesRegex(TypeError, only_classes_allowed): + issubclass(C(), Protocol) + T = TypeVar('T') @runtime_checkable diff --git a/Lib/typing.py b/Lib/typing.py index a8573760e60b12..84436318a6c952 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1782,11 +1782,11 @@ def __init__(cls, *args, **kwargs): ) def __subclasscheck__(cls, other): + if cls is Protocol: + return type.__subclasscheck__(cls, other) if not isinstance(other, type): # Same error message as for issubclass(1, int). raise TypeError('issubclass() arg 1 must be a class') - if cls is Protocol: - return type.__subclasscheck__(cls, other) if ( getattr(cls, '_is_protocol', False) and not _allow_reckless_class_checks() From b85a3205c4b22942c9cb4ce0f8619d7b04d2ffe0 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 2 Jun 2023 16:46:04 +0100 Subject: [PATCH 6/6] Cleaner solution --- Lib/typing.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/typing.py b/Lib/typing.py index 84436318a6c952..c831ba847fc384 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1805,6 +1805,8 @@ def __subclasscheck__(cls, other): def __instancecheck__(cls, instance): # We need this method for situations where attributes are # assigned in __init__. + if cls is Protocol: + return type.__instancecheck__(cls, instance) if not getattr(cls, "_is_protocol", False): # i.e., it's a concrete subclass of a protocol return super().__instancecheck__(instance) @@ -1864,7 +1866,7 @@ def meth(self) -> T: ... """ __slots__ = () - _is_protocol = False + _is_protocol = True _is_runtime_protocol = False def __init_subclass__(cls, *args, **kwargs): @@ -1906,7 +1908,7 @@ def _proto_hook(other): # ... otherwise check consistency of bases, and prohibit instantiation. for base in cls.__bases__: - if not (base in {object, Generic, Protocol} or + if not (base in (object, Generic) or base.__module__ in _PROTO_ALLOWLIST and base.__name__ in _PROTO_ALLOWLIST[base.__module__] or issubclass(base, Generic) and getattr(base, '_is_protocol', False)):