gh-113320: Reduce the number of dangerous getattr() calls when constructing protocol classes (#113401)

- Only attempt to figure out whether protocol members are "method members" or not if the class is marked as a runtime protocol. This information is irrelevant for non-runtime protocols; we can safely skip the risky introspection for them.
- Only do the risky getattr() calls in one place (the runtime_checkable class decorator), rather than in three places (_ProtocolMeta.__init__, _ProtocolMeta.__instancecheck__ and _ProtocolMeta.__subclasscheck__). This reduces the number of locations in typing.py where the risky introspection could go wrong.
- For runtime protocols, if determining whether a protocol member is callable or not fails, give a better error message. I think it's reasonable for us to reject runtime protocols that have members which raise strange exceptions when you try to access them. PEP-544 clearly states that all protocol member must be callable for issubclass() calls against the protocol to be valid -- and if a member raises when we try to access it, there's no way for us to figure out whether it's a callable member or not!
This commit is contained in:
Alex Waygood 2024-01-05 01:01:48 +00:00 committed by GitHub
parent fcb3c2a444
commit ed6ea3ea79
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 70 additions and 22 deletions

View file

@ -3448,8 +3448,8 @@ class ProtocolTests(BaseTestCase):
self.assertNotIn("__protocol_attrs__", vars(NonP))
self.assertNotIn("__protocol_attrs__", vars(NonPR))
self.assertNotIn("__callable_proto_members_only__", vars(NonP))
self.assertNotIn("__callable_proto_members_only__", vars(NonPR))
self.assertNotIn("__non_callable_proto_members__", vars(NonP))
self.assertNotIn("__non_callable_proto_members__", vars(NonPR))
self.assertEqual(get_protocol_members(P), {"x"})
self.assertEqual(get_protocol_members(PR), {"meth"})
@ -4105,6 +4105,7 @@ class ProtocolTests(BaseTestCase):
self.assertNotIsInstance(42, ProtocolWithMixedMembers)
def test_protocol_issubclass_error_message(self):
@runtime_checkable
class Vec2D(Protocol):
x: float
y: float
@ -4120,6 +4121,39 @@ class ProtocolTests(BaseTestCase):
with self.assertRaisesRegex(TypeError, re.escape(expected_error_message)):
issubclass(int, Vec2D)
def test_nonruntime_protocol_interaction_with_evil_classproperty(self):
class classproperty:
def __get__(self, instance, type):
raise RuntimeError("NO")
class Commentable(Protocol):
evil = classproperty()
# recognised as a protocol attr,
# but not actually accessed by the protocol metaclass
# (which would raise RuntimeError) for non-runtime protocols.
# See gh-113320
self.assertEqual(get_protocol_members(Commentable), {"evil"})
def test_runtime_protocol_interaction_with_evil_classproperty(self):
class CustomError(Exception): pass
class classproperty:
def __get__(self, instance, type):
raise CustomError
with self.assertRaises(TypeError) as cm:
@runtime_checkable
class Commentable(Protocol):
evil = classproperty()
exc = cm.exception
self.assertEqual(
exc.args[0],
"Failed to determine whether protocol member 'evil' is a method member"
)
self.assertIs(type(exc.__cause__), CustomError)
class GenericTests(BaseTestCase):