Issue #1785: Fix inspect and pydoc with misbehaving descriptors.

Also fixes issue #13581: `help(type)` wouldn't display anything.

3 files changed, 156 insertions, 32 deletions

diff --git a/Lib/inspect.py b/Lib/inspect.py
index c727530..cb07b9d 100644
--- a/Lib/inspect.py
+++ b/Lib/inspect.py
@@ -247,12 +247,23 @@ def isabstract(object):
 def getmembers(object, predicate=None):
     """Return all members of an object as (name, value) pairs sorted by name.
     Optionally, only return members that satisfy a given predicate."""
+    if isclass(object):
+        mro = (object,) + getmro(object)
+    else:
+        mro = ()
     results = []
     for key in dir(object):
-        try:
-            value = getattr(object, key)
-        except AttributeError:
-            continue
+        # First try to get the value via __dict__. Some descriptors don't
+        # like calling their __get__ (see bug #1785).
+        for base in mro:
+            if key in base.__dict__:
+                value = base.__dict__[key]
+                break
+        else:
+            try:
+                value = getattr(object, key)
+            except AttributeError:
+                continue
         if not predicate or predicate(value):
             results.append((key, value))
     results.sort()
@@ -288,30 +299,21 @@ def classify_class_attrs(cls):
     names = dir(cls)
     result = []
     for name in names:
-        # Get the object associated with the name.
+        # Get the object associated with the name, and where it was defined.
         # Getting an obj from the __dict__ sometimes reveals more than
         # using getattr.  Static and class methods are dramatic examples.
-        if name in cls.__dict__:
-            obj = cls.__dict__[name]
+        # Furthermore, some objects may raise an Exception when fetched with
+        # getattr(). This is the case with some descriptors (bug #1785).
+        # Thus, we only use getattr() as a last resort.
+        homecls = None
+        for base in (cls,) + mro:
+            if name in base.__dict__:
+                obj = base.__dict__[name]
+                homecls = base
+                break
         else:
             obj = getattr(cls, name)
-
-        # Figure out where it was defined.
-        homecls = getattr(obj, "__objclass__", None)
-        if homecls is None:
-            # search the dicts.
-            for base in mro:
-                if name in base.__dict__:
-                    homecls = base
-                    break
-
-        # Get the object again, in order to get it from the defining
-        # __dict__ instead of via getattr (if possible).
-        if homecls is not None and name in homecls.__dict__:
-            obj = homecls.__dict__[name]
-
-        # Also get the object via getattr.
-        obj_via_getattr = getattr(cls, name)
+            homecls = getattr(obj, "__objclass__", homecls)
 
         # Classify the object.
         if isinstance(obj, staticmethod):
@@ -320,11 +322,18 @@ def classify_class_attrs(cls):
             kind = "class method"
         elif isinstance(obj, property):
             kind = "property"
-        elif (ismethod(obj_via_getattr) or
-              ismethoddescriptor(obj_via_getattr)):
+        elif ismethoddescriptor(obj):
             kind = "method"
-        else:
+        elif isdatadescriptor(obj):
             kind = "data"
+        else:
+            obj_via_getattr = getattr(cls, name)
+            if (ismethod(obj_via_getattr) or
+                ismethoddescriptor(obj_via_getattr)):
+                kind = "method"
+            else:
+                kind = "data"
+            obj = obj_via_getattr
 
         result.append(Attribute(name, kind, homecls, obj))
 
diff --git a/Lib/pydoc.py b/Lib/pydoc.py
index d557790..19a71d8 100755
--- a/Lib/pydoc.py
+++ b/Lib/pydoc.py
@@ -740,8 +740,15 @@ class HTMLDoc(Doc):
                 hr.maybe()
                 push(msg)
                 for name, kind, homecls, value in ok:
-                    push(self.document(getattr(object, name), name, mod,
-                                       funcs, classes, mdict, object))
+                    try:
+                        value = getattr(object, name)
+                    except Exception:
+                        # Some descriptors may meet a failure in their __get__.
+                        # (bug #1785)
+                        push(self._docdescriptor(name, value, mod))
+                    else:
+                        push(self.document(value, name, mod,
+                                        funcs, classes, mdict, object))
                     push('\n')
             return attrs
 
@@ -781,7 +788,12 @@ class HTMLDoc(Doc):
         mdict = {}
         for key, kind, homecls, value in attrs:
             mdict[key] = anchor = '#' + name + '-' + key
-            value = getattr(object, key)
+            try:
+                value = getattr(object, name)
+            except Exception:
+                # Some descriptors may meet a failure in their __get__.
+                # (bug #1785)
+                pass
             try:
                 # The value may not be hashable (e.g., a data attr with
                 # a dict or list value).
@@ -1161,8 +1173,15 @@ class TextDoc(Doc):
                 hr.maybe()
                 push(msg)
                 for name, kind, homecls, value in ok:
-                    push(self.document(getattr(object, name),
-                                       name, mod, object))
+                    try:
+                        value = getattr(object, name)
+                    except Exception:
+                        # Some descriptors may meet a failure in their __get__.
+                        # (bug #1785)
+                        push(self._docdescriptor(name, value, mod))
+                    else:
+                        push(self.document(value,
+                                        name, mod, object))
             return attrs
 
         def spilldescriptors(msg, attrs, predicate):
diff --git a/Lib/test/test_inspect.py b/Lib/test/test_inspect.py
index b8c9a18..97de7a0 100644
--- a/Lib/test/test_inspect.py
+++ b/Lib/test/test_inspect.py
@@ -404,10 +404,37 @@ class TestBuggyCases(GetSourceBase):
         self.assertEqual(inspect.findsource(co), (lines,0))
         self.assertEqual(inspect.getsource(co), lines[0])
 
+
+class _BrokenDataDescriptor(object):
+    """
+    A broken data descriptor. See bug #1785.
+    """
+    def __get__(*args):
+        raise AssertionError("should not __get__ data descriptors")
+
+    def __set__(*args):
+        raise RuntimeError
+
+    def __getattr__(*args):
+        raise AssertionError("should not __getattr__ data descriptors")
+
+
+class _BrokenMethodDescriptor(object):
+    """
+    A broken method descriptor. See bug #1785.
+    """
+    def __get__(*args):
+        raise AssertionError("should not __get__ method descriptors")
+
+    def __getattr__(*args):
+        raise AssertionError("should not __getattr__ method descriptors")
+
+
 # Helper for testing classify_class_attrs.
 def attrs_wo_objs(cls):
     return [t[:3] for t in inspect.classify_class_attrs(cls)]
 
+
 class TestClassesAndFunctions(unittest.TestCase):
     def test_classic_mro(self):
         # Test classic-class method resolution order.
@@ -494,6 +521,9 @@ class TestClassesAndFunctions(unittest.TestCase):
 
             datablob = '1'
 
+            dd = _BrokenDataDescriptor()
+            md = _BrokenMethodDescriptor()
+
         attrs = attrs_wo_objs(A)
         self.assertIn(('s', 'static method', A), attrs, 'missing static method')
         self.assertIn(('c', 'class method', A), attrs, 'missing class method')
@@ -501,6 +531,8 @@ class TestClassesAndFunctions(unittest.TestCase):
         self.assertIn(('m', 'method', A), attrs, 'missing plain method')
         self.assertIn(('m1', 'method', A), attrs, 'missing plain method')
         self.assertIn(('datablob', 'data', A), attrs, 'missing data')
+        self.assertIn(('md', 'method', A), attrs, 'missing method descriptor')
+        self.assertIn(('dd', 'data', A), attrs, 'missing data descriptor')
 
         class B(A):
             def m(self): pass
@@ -512,6 +544,8 @@ class TestClassesAndFunctions(unittest.TestCase):
         self.assertIn(('m', 'method', B), attrs, 'missing plain method')
         self.assertIn(('m1', 'method', A), attrs, 'missing plain method')
         self.assertIn(('datablob', 'data', A), attrs, 'missing data')
+        self.assertIn(('md', 'method', A), attrs, 'missing method descriptor')
+        self.assertIn(('dd', 'data', A), attrs, 'missing data descriptor')
 
 
         class C(A):
@@ -525,6 +559,8 @@ class TestClassesAndFunctions(unittest.TestCase):
         self.assertIn(('m', 'method', C), attrs, 'missing plain method')
         self.assertIn(('m1', 'method', A), attrs, 'missing plain method')
         self.assertIn(('datablob', 'data', A), attrs, 'missing data')
+        self.assertIn(('md', 'method', A), attrs, 'missing method descriptor')
+        self.assertIn(('dd', 'data', A), attrs, 'missing data descriptor')
 
         class D(B, C):
             def m1(self): pass
@@ -539,6 +575,8 @@ class TestClassesAndFunctions(unittest.TestCase):
         self.assertIn(('m', 'method', B), attrs, 'missing plain method')
         self.assertIn(('m1', 'method', D), attrs, 'missing plain method')
         self.assertIn(('datablob', 'data', A), attrs, 'missing data')
+        self.assertIn(('md', 'method', A), attrs, 'missing method descriptor')
+        self.assertIn(('dd', 'data', A), attrs, 'missing data descriptor')
 
 
     def test_classify_oldstyle(self):
@@ -554,6 +592,64 @@ class TestClassesAndFunctions(unittest.TestCase):
         """
         self._classify_test(True)
 
+    def test_classify_builtin_types(self):
+        # Simple sanity check that all built-in types can have their
+        # attributes classified.
+        for name in dir(__builtin__):
+            builtin = getattr(__builtin__, name)
+            if isinstance(builtin, type):
+                inspect.classify_class_attrs(builtin)
+
+    def test_getmembers_descriptors(self):
+        # Old-style classes
+        class A:
+            dd = _BrokenDataDescriptor()
+            md = _BrokenMethodDescriptor()
+
+        self.assertEqual(inspect.getmembers(A, inspect.ismethoddescriptor),
+            [('md', A.__dict__['md'])])
+        self.assertEqual(inspect.getmembers(A, inspect.isdatadescriptor),
+            [('dd', A.__dict__['dd'])])
+
+        class B(A):
+            pass
+
+        self.assertEqual(inspect.getmembers(B, inspect.ismethoddescriptor),
+            [('md', A.__dict__['md'])])
+        self.assertEqual(inspect.getmembers(B, inspect.isdatadescriptor),
+            [('dd', A.__dict__['dd'])])
+
+        # New-style classes
+        class A(object):
+            dd = _BrokenDataDescriptor()
+            md = _BrokenMethodDescriptor()
+
+        def pred_wrapper(pred):
+            # A quick'n'dirty way to discard standard attributes of new-style
+            # classes.
+            class Empty(object):
+                pass
+            def wrapped(x):
+                if hasattr(x, '__name__') and hasattr(Empty, x.__name__):
+                    return False
+                return pred(x)
+            return wrapped
+
+        ismethoddescriptor = pred_wrapper(inspect.ismethoddescriptor)
+        isdatadescriptor = pred_wrapper(inspect.isdatadescriptor)
+
+        self.assertEqual(inspect.getmembers(A, ismethoddescriptor),
+            [('md', A.__dict__['md'])])
+        self.assertEqual(inspect.getmembers(A, isdatadescriptor),
+            [('dd', A.__dict__['dd'])])
+
+        class B(A):
+            pass
+
+        self.assertEqual(inspect.getmembers(B, ismethoddescriptor),
+            [('md', A.__dict__['md'])])
+        self.assertEqual(inspect.getmembers(B, isdatadescriptor),
+            [('dd', A.__dict__['dd'])])
 
 
 class TestGetcallargsFunctions(unittest.TestCase):