From dacc5ac71a8e546f9ef76805827cb50d4d40cabf Mon Sep 17 00:00:00 2001 From: Jan Kaliszewski Date: Tue, 18 Jun 2024 14:19:43 +0200 Subject: [PATCH] gh-120381: Fix inspect.ismethoddescriptor() (#120383) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `inspect.ismethoddescriptor()` function did not check for the lack of `__delete__()` and, consequently, erroneously returned True when applied to *data* descriptors with only `__get__()` and `__delete__()` defined. Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> Co-authored-by: Alyssa Coghlan --- Doc/library/inspect.rst | 11 +- Lib/inspect.py | 11 +- Lib/test/test_inspect/test_inspect.py | 121 +++++++++++++++++- ...-06-12-11-54-05.gh-issue-120381.O-BNLs.rst | 2 + 4 files changed, 135 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-06-12-11-54-05.gh-issue-120381.O-BNLs.rst diff --git a/Doc/library/inspect.rst b/Doc/library/inspect.rst index 7130faa4b5b..0ec7d7cecb8 100644 --- a/Doc/library/inspect.rst +++ b/Doc/library/inspect.rst @@ -504,9 +504,9 @@ attributes (see :ref:`import-mod-attrs` for module attributes): are true. This, for example, is true of ``int.__add__``. An object passing this test - has a :meth:`~object.__get__` method but not a :meth:`~object.__set__` - method, but beyond that the set of attributes varies. A - :attr:`~definition.__name__` attribute is usually + has a :meth:`~object.__get__` method, but not a :meth:`~object.__set__` + method or a :meth:`~object.__delete__` method. Beyond that, the set of + attributes varies. A :attr:`~definition.__name__` attribute is usually sensible, and :attr:`!__doc__` often is. Methods implemented via descriptors that also pass one of the other tests @@ -515,6 +515,11 @@ attributes (see :ref:`import-mod-attrs` for module attributes): :attr:`~method.__func__` attribute (etc) when an object passes :func:`ismethod`. + .. versionchanged:: 3.13 + This function no longer incorrectly reports objects with :meth:`~object.__get__` + and :meth:`~object.__delete__`, but not :meth:`~object.__set__`, as being method + descriptors (such objects are data descriptors, not method descriptors). + .. function:: isdatadescriptor(object) diff --git a/Lib/inspect.py b/Lib/inspect.py index 11544b8d0d4..6e403772d03 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -307,9 +307,10 @@ def ismethoddescriptor(object): But not if ismethod() or isclass() or isfunction() are true. This is new in Python 2.2, and, for example, is true of int.__add__. - An object passing this test has a __get__ attribute but not a __set__ - attribute, but beyond that the set of attributes varies. __name__ is - usually sensible, and __doc__ often is. + An object passing this test has a __get__ attribute, but not a + __set__ attribute or a __delete__ attribute. Beyond that, the set + of attributes varies; __name__ is usually sensible, and __doc__ + often is. Methods implemented via descriptors that also pass one of the other tests return false from the ismethoddescriptor() test, simply because @@ -319,7 +320,9 @@ def ismethoddescriptor(object): # mutual exclusion return False tp = type(object) - return hasattr(tp, "__get__") and not hasattr(tp, "__set__") + return (hasattr(tp, "__get__") + and not hasattr(tp, "__set__") + and not hasattr(tp, "__delete__")) def isdatadescriptor(object): """Return true if the object is a data descriptor. diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index 9ebab2f81fa..88835cd79aa 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -55,9 +55,8 @@ # ismodule, isclass, ismethod, isfunction, istraceback, isframe, iscode, # isbuiltin, isroutine, isgenerator, isgeneratorfunction, getmembers, # getdoc, getfile, getmodule, getsourcefile, getcomments, getsource, -# getclasstree, getargvalues, formatargvalues, -# currentframe, stack, trace, isdatadescriptor, -# ismethodwrapper +# getclasstree, getargvalues, formatargvalues, currentframe, +# stack, trace, ismethoddescriptor, isdatadescriptor, ismethodwrapper # NOTE: There are some additional tests relating to interaction with # zipimport in the test_zipimport_support test module. @@ -179,6 +178,7 @@ def test_excluding_predicates(self): self.istest(inspect.ismethod, 'git.argue') self.istest(inspect.ismethod, 'mod.custom_method') self.istest(inspect.ismodule, 'mod') + self.istest(inspect.ismethoddescriptor, 'int.__add__') self.istest(inspect.isdatadescriptor, 'collections.defaultdict.default_factory') self.istest(inspect.isgenerator, '(x for x in range(2))') self.istest(inspect.isgeneratorfunction, 'generator_function_example') @@ -1813,6 +1813,121 @@ def test_typing_replacement(self): self.assertEqual(inspect.formatannotation(ann1), 'Union[List[testModule.typing.A], int]') +class TestIsMethodDescriptor(unittest.TestCase): + + def test_custom_descriptors(self): + class MethodDescriptor: + def __get__(self, *_): pass + class MethodDescriptorSub(MethodDescriptor): + pass + class DataDescriptorWithNoGet: + def __set__(self, *_): pass + class DataDescriptorWithGetSet: + def __get__(self, *_): pass + def __set__(self, *_): pass + class DataDescriptorWithGetDelete: + def __get__(self, *_): pass + def __delete__(self, *_): pass + class DataDescriptorSub(DataDescriptorWithNoGet, + DataDescriptorWithGetDelete): + pass + + # Custom method descriptors: + self.assertTrue( + inspect.ismethoddescriptor(MethodDescriptor()), + '__get__ and no __set__/__delete__ => method descriptor') + self.assertTrue( + inspect.ismethoddescriptor(MethodDescriptorSub()), + '__get__ (inherited) and no __set__/__delete__' + ' => method descriptor') + + # Custom data descriptors: + self.assertFalse( + inspect.ismethoddescriptor(DataDescriptorWithNoGet()), + '__set__ (and no __get__) => not a method descriptor') + self.assertFalse( + inspect.ismethoddescriptor(DataDescriptorWithGetSet()), + '__get__ and __set__ => not a method descriptor') + self.assertFalse( + inspect.ismethoddescriptor(DataDescriptorWithGetDelete()), + '__get__ and __delete__ => not a method descriptor') + self.assertFalse( + inspect.ismethoddescriptor(DataDescriptorSub()), + '__get__, __set__ and __delete__ => not a method descriptor') + + # Classes of descriptors (are *not* descriptors themselves): + self.assertFalse(inspect.ismethoddescriptor(MethodDescriptor)) + self.assertFalse(inspect.ismethoddescriptor(MethodDescriptorSub)) + self.assertFalse(inspect.ismethoddescriptor(DataDescriptorSub)) + + def test_builtin_descriptors(self): + builtin_slot_wrapper = int.__add__ # This one is mentioned in docs. + class Owner: + def instance_method(self): pass + @classmethod + def class_method(cls): pass + @staticmethod + def static_method(): pass + @property + def a_property(self): pass + class Slotermeyer: + __slots__ = 'a_slot', + def function(): + pass + a_lambda = lambda: None + + # Example builtin method descriptors: + self.assertTrue( + inspect.ismethoddescriptor(builtin_slot_wrapper), + 'a builtin slot wrapper is a method descriptor') + self.assertTrue( + inspect.ismethoddescriptor(Owner.__dict__['class_method']), + 'a classmethod object is a method descriptor') + self.assertTrue( + inspect.ismethoddescriptor(Owner.__dict__['static_method']), + 'a staticmethod object is a method descriptor') + + # Example builtin data descriptors: + self.assertFalse( + inspect.ismethoddescriptor(Owner.__dict__['a_property']), + 'a property is not a method descriptor') + self.assertFalse( + inspect.ismethoddescriptor(Slotermeyer.__dict__['a_slot']), + 'a slot is not a method descriptor') + + # `types.MethodType`/`types.FunctionType` instances (they *are* + # method descriptors, but `ismethoddescriptor()` explicitly + # excludes them): + self.assertFalse(inspect.ismethoddescriptor(Owner().instance_method)) + self.assertFalse(inspect.ismethoddescriptor(Owner().class_method)) + self.assertFalse(inspect.ismethoddescriptor(Owner().static_method)) + self.assertFalse(inspect.ismethoddescriptor(Owner.instance_method)) + self.assertFalse(inspect.ismethoddescriptor(Owner.class_method)) + self.assertFalse(inspect.ismethoddescriptor(Owner.static_method)) + self.assertFalse(inspect.ismethoddescriptor(function)) + self.assertFalse(inspect.ismethoddescriptor(a_lambda)) + + def test_descriptor_being_a_class(self): + class MethodDescriptorMeta(type): + def __get__(self, *_): pass + class ClassBeingMethodDescriptor(metaclass=MethodDescriptorMeta): + pass + # `ClassBeingMethodDescriptor` itself *is* a method descriptor, + # but it is *also* a class, and `ismethoddescriptor()` explicitly + # excludes classes. + self.assertFalse( + inspect.ismethoddescriptor(ClassBeingMethodDescriptor), + 'classes (instances of type) are explicitly excluded') + + def test_non_descriptors(self): + class Test: + pass + self.assertFalse(inspect.ismethoddescriptor(Test())) + self.assertFalse(inspect.ismethoddescriptor(Test)) + self.assertFalse(inspect.ismethoddescriptor([42])) + self.assertFalse(inspect.ismethoddescriptor(42)) + + class TestIsDataDescriptor(unittest.TestCase): def test_custom_descriptors(self): diff --git a/Misc/NEWS.d/next/Library/2024-06-12-11-54-05.gh-issue-120381.O-BNLs.rst b/Misc/NEWS.d/next/Library/2024-06-12-11-54-05.gh-issue-120381.O-BNLs.rst new file mode 100644 index 00000000000..44f49bc19a4 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-06-12-11-54-05.gh-issue-120381.O-BNLs.rst @@ -0,0 +1,2 @@ +Correct :func:`inspect.ismethoddescriptor` to check also for the lack of +:meth:`~object.__delete__`. Patch by Jan Kaliszewski.