[3.13] gh-120381: Fix inspect.ismethoddescriptor() (GH-120684)

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.

(cherry picked from commit dacc5ac71a8e546f9ef76805827cb50d4d40cabf)

Co-authored-by: Jan Kaliszewski <zuo@kaliszewski.net>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com>

diff --git a/Doc/library/inspect.rst b/Doc/library/inspect.rst
index 7130faa4b5b6967d860b599e34aa3701d0375ff7..0ec7d7cecb89e0ecfbc5a0015ca610a492951b28 100644 (file)
--- 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 1eb2b35bd9a2d3740cbdae32c865d8be7524e483..2c82ad53cb77461d9a6133d3edcad8261bcc8a47 100644 (file)
--- a/Lib/inspect.py
+++ b/Lib/inspect.py
@@ -313,9 +313,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
@@ -325,7 +326,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 fff0e5463b8266f03792da932c5e9268997aa014..34739b6f5db2f57f9c9a3d372a6e9f51c381c8b7 100644 (file)
--- a/Lib/test/test_inspect/test_inspect.py
+++ b/Lib/test/test_inspect/test_inspect.py
@@ -55,9 +55,8 @@ from test.test_inspect import inspect_stringized_annotations_pep695
 # 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 @@ class TestPredicates(IsTestBase):
         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 @@ class TestFormatAnnotation(unittest.TestCase):
         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 (file)
index 0000000..44f49bc
--- /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.