property deal with slots in safe attr lookup

thomasballinger · thomasballinger · commit d76cc10952b1 · 2015-12-11T08:43:03.000-05:00
diff --git a/bpython/autocomplete.py b/bpython/autocomplete.py
@@ -707,8 +707,12 @@ def attr_lookup(obj, expr, attr):
     n = len(attr)
     for word in words:
         if method_match(word, n, attr) and word != "__builtins__":
-            attr_obj = inspection.safe_get_attribute(obj, word)
-            matches.append(_callable_postfix(attr_obj, "%s.%s" % (expr, word)))
+            try:
+                attr_obj = inspection.safe_get_attribute(obj, word)
+            except AttributeError:
+                matches.append("%s.%s" % (expr, word))
+            else:
+                matches.append(_callable_postfix(attr_obj, "%s.%s" % (expr, word)))
     return matches
 
 
diff --git a/bpython/inspection.py b/bpython/inspection.py
@@ -282,39 +282,49 @@ def is_callable(obj):
     return callable(obj)
 
 
-class AttributeIsEmptySlot(object):
-    pass
-
-
 def safe_get_attribute(obj, attr):
-    """Gets attributes without triggering descriptors on new-style clases
-
-    Returns AttributeIsEmptySlot if requested attribute does not have a value,
-    but is a slot entry.
-    """
+    """Gets attributes without triggering descriptors on new-style clases"""
     if is_new_style(obj):
-        return safe_get_attribute_new_style(obj, attr)
+        result = safe_get_attribute_new_style(obj, attr)
+        if isinstance(result, member_descriptor):
+            # will either be the same slot descriptor or the value
+            return getattr(obj, attr)
+        return result
     return getattr(obj, attr)
 
 
+class _ClassWithSlots(object):
+    __slots__ = ['a']
+member_descriptor = type(_ClassWithSlots.a)
+
+
 def safe_get_attribute_new_style(obj, attr):
+    """Returns approximately the attribute returned by getattr(obj, attr)
+
+    The object returned ought to be callable if getattr(obj, attr) was.
+    Fake callable objects may be returned instead, in order to avoid executing
+    arbitrary code in descriptors.
+
+    If the object is an instance of a class that uses __slots__, will return
+    the member_descriptor object instead of the value.
+    """
     if not is_new_style(obj):
         raise ValueError("%r is not a new-style class or object" % obj)
     to_look_through = (obj.mro()
                        if hasattr(obj, 'mro')
                        else [obj] + type(obj).mro())
 
+    found_in_slots = hasattr(obj, '__slots__') and attr in obj.__slots__
     for cls in to_look_through:
         if hasattr(cls, '__dict__') and attr in cls.__dict__:
             return cls.__dict__[attr]
-        elif hasattr(cls, '__slots__') and attr in cls.__slots__:
-            try:
-                return getattr(cls, attr)
-            except AttributeError:
-                return AttributeIsEmptySlot
+
+    if found_in_slots:
+        return AttributeIsEmptySlot
 
     raise AttributeError()
 
+
 def get_attribute(obj, attr):
     cls = type(obj)
     for cls in [obj] + cls.mro():
diff --git a/bpython/test/test_autocomplete.py b/bpython/test/test_autocomplete.py
@@ -242,6 +242,10 @@ def asserts_when_called(self):
         raise AssertionError("getter method called")
 
 
+class Slots(object):
+    __slots__ = ['a', 'b']
+
+
 class TestAttrCompletion(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
@@ -280,6 +284,11 @@ def test_descriptor_attributes_not_run(self):
                             set(['a.b', 'a.a', 'a.method(',
                                  'a.asserts_when_called']))
 
+    def test_slots_not_crash(self):
+        com = autocomplete.AttrCompletion()
+        self.assertSetEqual(com.matches(2, 'A.', locals_={'A': Slots}),
+                            set(['A.b', 'A.a', 'A.mro']))
+
 
 class TestArrayItemCompletion(unittest.TestCase):
     @classmethod
diff --git a/bpython/test/test_inspection.py b/bpython/test/test_inspection.py
@@ -161,9 +161,19 @@ def prop(self):
 
 
 class Slots(object):
-    __slots__ = ['s1', 's2']
+    __slots__ = ['s1', 's2', 's3']
+
+    @property
+    def s3(self):
+        raise AssertionError('Property __get__ executed')
+
+
+class SlotsSubclass(Slots):
+    pass
 
 
+member_descriptor = type(Slots.s1)
+
 class TestSafeGetAttribute(unittest.TestCase):
 
     def test_lookup_on_object(self):
@@ -191,10 +201,26 @@ class Old: pass
     def test_lookup_with_slots(self):
         s = Slots()
         s.s1 = 's1'
-        self.assertEquals(inspection.safe_get_attribute_new_style(s, 's1'), 's1')
-        self.assertEquals(inspection.safe_get_attribute_new_style(s, 's2'),
-                          inspection.AttributeIsEmptySlot)
-
+        self.assertEquals(inspection.safe_get_attribute(s, 's1'), 's1')
+        self.assertIsInstance(inspection.safe_get_attribute_new_style(s, 's1'),
+                              member_descriptor)
+        with self.assertRaises(AttributeError):
+            inspection.safe_get_attribute(s, 's2')
+        self.assertIsInstance(inspection.safe_get_attribute_new_style(s, 's2'),
+                              member_descriptor)
+        self.assertEquals(inspection.safe_get_attribute(s, 's3'),
+                          Slots.__dict__['s3'])
+
+    def test_lookup_on_slots_classes(self):
+        sga = inspection.safe_get_attribute
+        self.assertIsInstance(inspection.safe_get_attribute(Slots, 's1'),
+                              member_descriptor)
+        self.assertIsInstance(inspection.safe_get_attribute(Slots, 's3'),
+                              property)
+        self.assertIsInstance(inspection.safe_get_attribute(SlotsSubclass, 's1'),
+                              member_descriptor)
+        self.assertIsInstance(inspection.safe_get_attribute(SlotsSubclass, 's3'),
+                              property)
 
 if __name__ == '__main__':
     unittest.main()
