re-add AttrCleaner for one spot and add testing

bpython · thomasballinger · Aug 4, 2020 · Jul 27, 2020 · Jul 29, 2020 · Aug 3, 2020
commit 234b8ba4abc3b66f906ec9e8d02796f98f76713f
diff --git a/bpython/autocomplete.py b/bpython/autocomplete.py
@@ -357,14 +357,13 @@ def attr_matches(self, text, namespace):
         return matches
 
     def attr_lookup(self, obj, expr, attr):
-        """Second half of original attr_matches method factored out so it can
-        be wrapped in a safe try/finally block in case anything bad happens to
-        restore the original __getattribute__ method."""
+        """Second half of attr_matches."""
         words = self.list_attributes(obj)
         if inspection.hasattr_safe(obj, "__class__"):
             words.append("__class__")
-            words = words + rlcompleter.get_class_members(obj.__class__)
-            if not isinstance(obj.__class__, abc.ABCMeta):
+            klass = inspection.getattr_safe(obj, "__class__")
+            words = words + rlcompleter.get_class_members(klass)
+            if not isinstance(klass, abc.ABCMeta):
                 try:
                     words.remove("__abstractmethods__")
                 except ValueError:
@@ -384,21 +383,25 @@ def attr_lookup(self, obj, expr, attr):
     if py3:
 
         def list_attributes(self, obj):
-            return dir(obj)
+            # TODO: re-implement dir using getattr_static to avoid using
+            # AttrCleaner here?
+            with inspection.AttrCleaner(obj):
+                return dir(obj)
 
     else:
 
         def list_attributes(self, obj):
-            if isinstance(obj, InstanceType):
-                try:
+            with inspection.AttrCleaner(obj):
+                if isinstance(obj, InstanceType):
+                    try:
+                        return dir(obj)
+                    except Exception:
+                        # This is a case where we can not prevent user code from
+                        # running. We return a default list attributes on error
+                        # instead. (#536)
+                        return ["__doc__", "__module__"]
+                else:
                     return dir(obj)
-                except Exception:
-                    # This is a case where we can not prevent user code from
-                    # running. We return a default list attributes on error
-                    # instead. (#536)
-                    return ["__doc__", "__module__"]
-            else:
-                return dir(obj)
 
 
 class DictKeyCompletion(BaseCompletionType):

diff --git a/bpython/test/test_autocomplete.py b/bpython/test/test_autocomplete.py
@@ -251,6 +251,9 @@ def asserts_when_called(self):
 class Slots(object):
     __slots__ = ["a", "b"]
 
+class OverriddenGetattribute(Foo):
+    def __getattribute__(self,name):
+        raise AssertionError("custom get attribute invoked")
 
 class TestAttrCompletion(unittest.TestCase):
     @classmethod
@@ -298,12 +301,20 @@ def test_descriptor_attributes_not_run(self):
             com.matches(2, "a.", locals_={"a": Properties()}),
             set(["a.b", "a.a", "a.method", "a.asserts_when_called"]),
         )
+
+    def test_custom_get_attribute_not_invoked(self):
+        com = autocomplete.AttrCompletion()
+        self.assertSetEqual(
+            com.matches(2, "a.", locals_={"a": OverriddenGetattribute()}),
+            set(["a.b", "a.a", "a.method"]),
+        )
+
 
     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"]),
+            set(["A.b", "A.a"]),
         )
 
 

diff --git a/bpython/test/test_inspection.py b/bpython/test/test_inspection.py
@@ -155,73 +155,124 @@ def test_get_source_file(self):
         self.assertEqual(encoding, "utf-8")
 
 
-    class TestSafeGetAttribute(unittest.TestCase):
-        def test_lookup_on_object(self):
-            a = A()
-            a.x = 1
-            self.assertEqual(getattr_safe(a, "x"), 1)
-            self.assertEqual(getattr_safe(a, "a"), "a")
-            b = B()
-            b.y = 2
-            self.assertEqual(getattr_safe(b, "y"), 2)
-            self.assertEqual(getattr_safe(b, "a"), "a")
-            self.assertEqual(getattr_safe(b, "b"), "b")
-
-            self.assertEqual(hasattr_safe(b, "y"), True)
-            self.assertEqual(hasattr_safe(b, "b"), True)
-
-
-        def test_avoid_running_properties(self):
-            p = Property()
-            self.assertEqual(getattr_safe(p, "prop"), Property.prop)
-            self.assertEqual(hasattr_safe(p, "prop"), True)
-
-        def test_lookup_with_slots(self):
-            s = Slots()
-            s.s1 = "s1"
-            self.assertEqual(getattr_safe(s, "s1"), "s1")
-            with self.assertRaises(AttributeError):
-                getattr_safe(s, "s2")
-
-            self.assertEqual(hasattr_safe(s, "s2"), False)
-
-        def test_lookup_on_slots_classes(self):
-            sga = getattr_safe
-            s = SlotsSubclass()
-            self.assertIsInstance(sga(Slots, "s1"), member_descriptor)
-            self.assertIsInstance(sga(SlotsSubclass, "s1"), member_descriptor)
-            self.assertIsInstance(sga(SlotsSubclass, "s4"), property)
-            self.assertIsInstance(sga(s, "s4"), property)
-
-            self.assertEqual(hasattr_safe(s, "s1"), True)
-            self.assertEqual(hasattr_safe(s, "s4"), True)
-
-        @unittest.skipIf(py3, "Py 3 doesn't allow slots and prop in same class")
-        def test_lookup_with_property_and_slots(self):
-            sga = getattr_safe
-            s = SlotsSubclass()
-            self.assertIsInstance(sga(Slots, "s3"), property)
-            self.assertEqual(getattr_safe(s, "s3"), Slots.__dict__["s3"])
-            self.assertIsInstance(sga(SlotsSubclass, "s3"), property)
-
-        def test_lookup_on_overridden_methods(self):
-            sga = getattr_safe
-            self.assertEqual(sga(OverriddenGetattr(), "a"), 1)
-            self.assertEqual(sga(OverriddenGetattribute(), "a"), 1)
-            self.assertEqual(sga(OverriddenMRO(), "a"), 1)
-            with self.assertRaises(AttributeError):
-                sga(OverriddenGetattr(), "b")
-            with self.assertRaises(AttributeError):
-                sga(OverriddenGetattribute(), "b")
-            with self.assertRaises(AttributeError):
-                sga(OverriddenMRO(), "b")
-
-            self.assertEqual(hasattr_safe(OverriddenGetattr(), "b"), False)
-            self.assertEqual(hasattr_safe(OverriddenGetattribute(), "b"), False)
-            self.assertEqual(hasattr_safe(OverriddenMRO(), "b"), False)
-
+class A(object):
+    a = "a"
+
+
+class B(A):
+    b = "b"
+
+
+class Property(object):
+    @property
+    def prop(self):
+        raise AssertionError("Property __get__ executed")
+
+
+class Slots(object):
+    __slots__ = ["s1", "s2", "s3"]
+
+    if not py3:
+
+        @property
+        def s3(self):
+            raise AssertionError("Property __get__ executed")
+
+
+class SlotsSubclass(Slots):
+    @property
+    def s4(self):
+        raise AssertionError("Property __get__ executed")
+
+
+class OverriddenGetattr(object):
+    def __getattr__(self, attr):
+        raise AssertionError("custom __getattr__ executed")
+
+    a = 1
 
 
+class OverriddenGetattribute(object):
+    def __getattribute__(self, attr):
+        raise AssertionError("custom __getattribute__ executed")
+
+    a = 1
+
+
+class OverriddenMRO(object):
+    def __mro__(self):
+        raise AssertionError("custom mro executed")
+
+    a = 1
+
+member_descriptor = type(Slots.s1)
+
+class TestSafeGetAttribute(unittest.TestCase):
+    def test_lookup_on_object(self):
+        a = A()
+        a.x = 1
+        self.assertEqual(inspection.getattr_safe(a, "x"), 1)
+        self.assertEqual(inspection.getattr_safe(a, "a"), "a")
+        b = B()
+        b.y = 2
+        self.assertEqual(inspection.getattr_safe(b, "y"), 2)
+        self.assertEqual(inspection.getattr_safe(b, "a"), "a")
+        self.assertEqual(inspection.getattr_safe(b, "b"), "b")
+
+        self.assertEqual(inspection.hasattr_safe(b, "y"), True)
+        self.assertEqual(inspection.hasattr_safe(b, "b"), True)
+
+
+    def test_avoid_running_properties(self):
+        p = Property()
+        self.assertEqual(inspection.getattr_safe(p, "prop"), Property.prop)
+        self.assertEqual(inspection.hasattr_safe(p, "prop"), True)
+
+    def test_lookup_with_slots(self):
+        s = Slots()
+        s.s1 = "s1"
+        self.assertEqual(inspection.getattr_safe(s, "s1"), "s1")
+        with self.assertRaises(AttributeError):
+            inspection.getattr_safe(s, "s2")
+
+        self.assertEqual(inspection.hasattr_safe(s, "s1"), True)
+        self.assertEqual(inspection.hasattr_safe(s, "s2"), False)
+
+    def test_lookup_on_slots_classes(self):
+        sga = inspection.getattr_safe
+        s = SlotsSubclass()
+        self.assertIsInstance(sga(Slots, "s1"), member_descriptor)
+        self.assertIsInstance(sga(SlotsSubclass, "s1"), member_descriptor)
+        self.assertIsInstance(sga(SlotsSubclass, "s4"), property)
+        self.assertIsInstance(sga(s, "s4"), property)
+
+        self.assertEqual(inspection.hasattr_safe(s, "s1"), False)
+        self.assertEqual(inspection.hasattr_safe(s, "s4"), True)
+
+    @unittest.skipIf(py3, "Py 3 doesn't allow slots and prop in same class")
+    def test_lookup_with_property_and_slots(self):
+        sga = inspection.getattr_safe
+        s = SlotsSubclass()
+        self.assertIsInstance(sga(Slots, "s3"), property)
+        self.assertEqual(inspection.getattr_safe(s, "s3"), Slots.__dict__["s3"])
+        self.assertIsInstance(sga(SlotsSubclass, "s3"), property)
+
+    def test_lookup_on_overridden_methods(self):
+        sga = inspection.getattr_safe
+        self.assertEqual(sga(OverriddenGetattr(), "a"), 1)
+        self.assertEqual(sga(OverriddenGetattribute(), "a"), 1)
+        self.assertEqual(sga(OverriddenMRO(), "a"), 1)
+        with self.assertRaises(AttributeError):
+            sga(OverriddenGetattr(), "b")
+        with self.assertRaises(AttributeError):
+            sga(OverriddenGetattribute(), "b")
+        with self.assertRaises(AttributeError):
+            sga(OverriddenMRO(), "b")
+
+        self.assertEqual(inspection.hasattr_safe(OverriddenGetattr(), "b"), False)
+        self.assertEqual(inspection.hasattr_safe(OverriddenGetattribute(), "b"), False)
+        self.assertEqual(inspection.hasattr_safe(OverriddenMRO(), "b"), False)
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/bpython/test/test_simpleeval.py b/bpython/test/test_simpleeval.py
@@ -133,58 +133,6 @@ def test_with_namespace(self):
         self.assertCannotEval("a[1].a|bc", {})
 
 
-class A(object):
-    a = "a"
-
-
-class B(A):
-    b = "b"
-
-
-class Property(object):
-    @property
-    def prop(self):
-        raise AssertionError("Property __get__ executed")
-
-
-class Slots(object):
-    __slots__ = ["s1", "s2", "s3"]
-
-    if not py3:
-
-        @property
-        def s3(self):
-            raise AssertionError("Property __get__ executed")
-
-
-class SlotsSubclass(Slots):
-    @property
-    def s4(self):
-        raise AssertionError("Property __get__ executed")
-
-
-class OverriddenGetattr(object):
-    def __getattr__(self, attr):
-        raise AssertionError("custom __getattr__ executed")
-
-    a = 1
-
-
-class OverriddenGetattribute(object):
-    def __getattribute__(self, attr):
-        raise AssertionError("custom __getattribute__ executed")
-
-    a = 1
-
-
-class OverriddenMRO(object):
-    def __mro__(self):
-        raise AssertionError("custom mro executed")
-
-    a = 1
-
-
-member_descriptor = type(Slots.s1)
 
 
 if __name__ == "__main__":