Skip to content

Commit

Permalink
gh-102433: Use inspect.getattr_static in `typing._ProtocolMeta.__in…
Browse files Browse the repository at this point in the history
…stancecheck__` (#103034)
  • Loading branch information
AlexWaygood authored Apr 2, 2023
1 parent d828b35 commit 6d59c9e
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 7 deletions.
9 changes: 9 additions & 0 deletions Doc/library/typing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions Doc/whatsnew/3.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typing.runtime_checkable>` 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
---

Expand Down
93 changes: 91 additions & 2 deletions Lib/test/test_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2637,7 +2637,15 @@ def attr(self): ...
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__,
Expand All @@ -2662,7 +2670,12 @@ def attr(self): ...
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):
Expand All @@ -2685,6 +2698,82 @@ def __dir__(self):
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):
Expand Down
26 changes: 21 additions & 5 deletions Lib/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1998,6 +1998,17 @@ def _allow_reckless_class_checks(depth=3):
}


@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__.
Expand Down Expand Up @@ -2025,12 +2036,17 @@ def __instancecheck__(cls, instance):
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)


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
:func:`isinstance` checks against :func:`runtime-checkable protocols
<typing.runtime_checkable>` 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.

0 comments on commit 6d59c9e

Please sign in to comment.