From 6d59c9e32ebb6d8468c7eb26e7cf3efd458c7a73 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 2 Apr 2023 14:22:19 +0100 Subject: gh-102433: Use `inspect.getattr_static` in `typing._ProtocolMeta.__instancecheck__` (#103034) --- Doc/library/typing.rst | 9 +++ Doc/whatsnew/3.12.rst | 11 +++ Lib/test/test_typing.py | 93 +++++++++++++++++++++- Lib/typing.py | 26 ++++-- .../2023-03-25-16-57-18.gh-issue-102433.L-7x2Q.rst | 10 +++ 5 files changed, 142 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-03-25-16-57-18.gh-issue-102433.L-7x2Q.rst diff --git a/Doc/library/typing.rst b/Doc/library/typing.rst index 384458d..8728ca7 100644 --- a/Doc/library/typing.rst +++ b/Doc/library/typing.rst @@ -1598,6 +1598,15 @@ These are not used in annotations. They are building blocks for creating generic import threading assert isinstance(threading.Thread(name='Bob'), Named) + .. versionchanged:: 3.12 + The internal implementation of :func:`isinstance` checks against + runtime-checkable protocols now uses :func:`inspect.getattr_static` + to look up attributes (previously, :func:`hasattr` was used). + As a result, some objects which used to be considered instances + of a runtime-checkable protocol may no longer be considered instances + of that protocol on Python 3.12+, and vice versa. + Most users are unlikely to be affected by this change. + .. note:: :func:`!runtime_checkable` will check only the presence of the required diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 8ab9613..93543cb 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -391,6 +391,17 @@ typing same name on a base class, as per :pep:`698`. (Contributed by Steven Troxler in :gh:`101564`.) +* :func:`isinstance` checks against + :func:`runtime-checkable protocols ` now use + :func:`inspect.getattr_static` rather than :func:`hasattr` to lookup whether + attributes exist. This means that descriptors and :meth:`~object.__getattr__` + methods are no longer unexpectedly evaluated during ``isinstance()`` checks + against runtime-checkable protocols. However, it may also mean that some + objects which used to be considered instances of a runtime-checkable protocol + may no longer be considered instances of that protocol on Python 3.12+, and + vice versa. Most users are unlikely to be affected by this change. + (Contributed by Alex Waygood in :gh:`102433`.) + sys --- diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 23bf2b5..7d2e6a6 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -2637,7 +2637,15 @@ class ProtocolTests(BaseTestCase): class PG1(Protocol[T]): attr: T - for protocol_class in P, P1, PG, PG1: + @runtime_checkable + class MethodP(Protocol): + def attr(self): ... + + @runtime_checkable + class MethodPG(Protocol[T]): + def attr(self) -> T: ... + + for protocol_class in P, P1, PG, PG1, MethodP, MethodPG: for klass in C, D, E, F: with self.subTest( klass=klass.__name__, @@ -2662,7 +2670,12 @@ class ProtocolTests(BaseTestCase): class BadPG1(Protocol[T]): attr: T - for obj in PG[T], PG[C], PG1[T], PG1[C], BadP, BadP1, BadPG, BadPG1: + cases = ( + PG[T], PG[C], PG1[T], PG1[C], MethodPG[T], + MethodPG[C], BadP, BadP1, BadPG, BadPG1 + ) + + for obj in cases: for klass in C, D, E, F, Empty: with self.subTest(klass=klass.__name__, obj=obj): with self.assertRaises(TypeError): @@ -2685,6 +2698,82 @@ class ProtocolTests(BaseTestCase): self.assertIsInstance(CustomDirWithX(), HasX) self.assertNotIsInstance(CustomDirWithoutX(), HasX) + def test_protocols_isinstance_attribute_access_with_side_effects(self): + class C: + @property + def attr(self): + raise AttributeError('no') + + class CustomDescriptor: + def __get__(self, obj, objtype=None): + raise RuntimeError("NO") + + class D: + attr = CustomDescriptor() + + # Check that properties set on superclasses + # are still found by the isinstance() logic + class E(C): ... + class F(D): ... + + class WhyWouldYouDoThis: + def __getattr__(self, name): + raise RuntimeError("wut") + + T = TypeVar('T') + + @runtime_checkable + class P(Protocol): + @property + def attr(self): ... + + @runtime_checkable + class P1(Protocol): + attr: int + + @runtime_checkable + class PG(Protocol[T]): + @property + def attr(self): ... + + @runtime_checkable + class PG1(Protocol[T]): + attr: T + + @runtime_checkable + class MethodP(Protocol): + def attr(self): ... + + @runtime_checkable + class MethodPG(Protocol[T]): + def attr(self) -> T: ... + + for protocol_class in P, P1, PG, PG1, MethodP, MethodPG: + for klass in C, D, E, F: + with self.subTest( + klass=klass.__name__, + protocol_class=protocol_class.__name__ + ): + self.assertIsInstance(klass(), protocol_class) + + with self.subTest( + klass="WhyWouldYouDoThis", + protocol_class=protocol_class.__name__ + ): + self.assertNotIsInstance(WhyWouldYouDoThis(), protocol_class) + + def test_protocols_isinstance___slots__(self): + # As per the consensus in https://github.com/python/typing/issues/1367, + # this is desirable behaviour + @runtime_checkable + class HasX(Protocol): + x: int + + class HasNothingButSlots: + __slots__ = ("x",) + + self.assertIsInstance(HasNothingButSlots(), HasX) + def test_protocols_isinstance_py36(self): class APoint: def __init__(self, x, y, label): diff --git a/Lib/typing.py b/Lib/typing.py index a88542c..442d684 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1998,6 +1998,17 @@ _PROTO_ALLOWLIST = { } +@functools.cache +def _lazy_load_getattr_static(): + # Import getattr_static lazily so as not to slow down the import of typing.py + # Cache the result so we don't slow down _ProtocolMeta.__instancecheck__ unnecessarily + from inspect import getattr_static + return getattr_static + + +_cleanups.append(_lazy_load_getattr_static.cache_clear) + + class _ProtocolMeta(ABCMeta): # This metaclass is really unfortunate and exists only because of # the lack of __instancehook__. @@ -2025,12 +2036,17 @@ class _ProtocolMeta(ABCMeta): return True if is_protocol_cls: - if all(hasattr(instance, attr) and - # All *methods* can be blocked by setting them to None. - (not callable(getattr(cls, attr, None)) or - getattr(instance, attr) is not None) - for attr in protocol_attrs): + getattr_static = _lazy_load_getattr_static() + for attr in protocol_attrs: + try: + val = getattr_static(instance, attr) + except AttributeError: + break + if callable(getattr(cls, attr, None)) and val is None: + break + else: return True + return super().__instancecheck__(instance) diff --git a/Misc/NEWS.d/next/Library/2023-03-25-16-57-18.gh-issue-102433.L-7x2Q.rst b/Misc/NEWS.d/next/Library/2023-03-25-16-57-18.gh-issue-102433.L-7x2Q.rst new file mode 100644 index 0000000..a4de4b4 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-03-25-16-57-18.gh-issue-102433.L-7x2Q.rst @@ -0,0 +1,10 @@ +:func:`isinstance` checks against :func:`runtime-checkable protocols +` now use :func:`inspect.getattr_static` rather +than :func:`hasattr` to lookup whether attributes exist. This means that +descriptors and :meth:`~object.__getattr__` methods are no longer +unexpectedly evaluated during ``isinstance()`` checks against +runtime-checkable protocols. However, it may also mean that some objects +which used to be considered instances of a runtime-checkable protocol may no +longer be considered instances of that protocol on Python 3.12+, and vice +versa. Most users are unlikely to be affected by this change. Patch by Alex +Waygood. -- cgit v0.12