ncoghlan · Jun 18, 2024 · Jun 12, 2024 · Jun 12, 2024 · Jun 12, 2024 · Jun 12, 2024
diff --git 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

diff --git a/Lib/inspect.py b/Lib/inspect.py
@@ -301,9 +301,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
@@ -313,7 +314,9 @@ def ismethoddescriptor(object):
         # mutual exclusion
         return False
     tp = type(object)
-    return hasattr(tp, "__get__") and not hasattr(tp, "__set__")
+    if hasattr(tp, "__set__") or hasattr(tp, "__delete__"):
+        return False
+    return hasattr(tp, "__get__")
-    if hasattr(tp, "__set__") or hasattr(tp, "__delete__"):
-        return False
-    return hasattr(tp, "__get__")
+    return hasattr(tp, "__get__") and not (hasattr(tp, "__set__") or hasattr(tp, "__delete__"))
-    if hasattr(tp, "__set__") or hasattr(tp, "__delete__"):
-        return False
-    return hasattr(tp, "__get__")
+    return hasattr(tp, "__get__") and not (hasattr(tp, "__set__") or 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
@@ -53,9 +53,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.
@@ -177,6 +176,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')
@@ -1700,6 +1700,110 @@ 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,
+                                MethodDescriptor):
+            pass
+        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')
+        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')
+        # Note: descriptor classes themselves are *not* descriptors.
+        self.assertFalse(inspect.ismethoddescriptor(MethodDescriptor))
+        self.assertFalse(inspect.ismethoddescriptor(MethodDescriptorSub))
+
+    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
+        def function():
+            pass
+        a_lambda = lambda: None
+        class Slotted:
+            __slots__ = 'foo',
+        # 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(Slotted.foo),
+            'a slot is not a method descriptor')
+        self.assertFalse(
+            inspect.ismethoddescriptor(Owner.a_property),
+            'a property 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().static_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(Owner().static_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
@@ -0,0 +1,2 @@
+Correct :func:`inspect.ismethoddescriptor` to check also for the lack of
+:meth:`~object.__delete__`.  Patch by Jan Kaliszewski.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		Correct :func:`inspect.ismethoddescriptor` to check also for the lack of
		:meth:`~object.__delete__`. Patch by Jan Kaliszewski.