Make sure that __getattr__ is always working with Interpreter

fixes #1378

jedi/inference/compiled/access.py

@@ -336,8 +336,23 @@ class DirectObjectAccess(object):
         except TypeError:
             return False
 
-    def is_allowed_getattr(self, name):
+    def is_allowed_getattr(self, name, unsafe=False):
         # TODO this API is ugly.
+        if unsafe:
+            # Unsafe is mostly used to check for __getattr__/__getattribute__.
+            # getattr_static works for properties, but the underscore methods
+            # are just ignored (because it's safer and avoids more code
+            # execution). See also GH #1378.
+
+            # Avoid warnings, see comment in the next function.
+            with warnings.catch_warnings(record=True):
+                warnings.simplefilter("always")
+                try:
+                    return hasattr(self._obj, name), False
+                except Exception:
+                    # Obviously has an attribute (propably a property) that
+                    # gets executed, so just avoid all exceptions here.
+                    return False, False
         try:
             attr, is_get_descriptor = getattr_static(self._obj, name)
         except AttributeError:
@@ -490,7 +505,6 @@ class DirectObjectAccess(object):
         Used to return a couple of infos that are needed when accessing the sub
         objects of an objects
         """
-        # TODO is_allowed_getattr might raise an AttributeError
         tuples = dict(
             (force_unicode(name), self.is_allowed_getattr(name))
             for name in self.dir()

jedi/inference/compiled/value.py

@@ -399,7 +399,7 @@ class CompiledObjectFilter(AbstractFilter):
     def get(self, name):
         return self._get(
             name,
-            lambda name: self.compiled_object.access_handle.is_allowed_getattr(name),
+            lambda name, unsafe: self.compiled_object.access_handle.is_allowed_getattr(name, unsafe),
             lambda name: name in self.compiled_object.access_handle.dir(),
             check_has_attribute=True
         )
@@ -411,12 +411,18 @@ class CompiledObjectFilter(AbstractFilter):
         # Always use unicode objects in Python 2 from here.
         name = force_unicode(name)
 
-        has_attribute, is_descriptor = allowed_getattr_callback(name)
+        if self._inference_state.allow_descriptor_getattr:
+            pass
+
+        has_attribute, is_descriptor = allowed_getattr_callback(
+            name,
+            unsafe=self._inference_state.allow_descriptor_getattr
+        )
         if check_has_attribute and not has_attribute:
             return []
 
-        if (is_descriptor and not self._inference_state.allow_descriptor_getattr) \
+        if (is_descriptor or not has_attribute) \
-                or not has_attribute:
+                and not self._inference_state.allow_descriptor_getattr:
             return [self._get_cached_name(name, is_empty=True)]
 
         if self.is_instance and not in_dir_callback(name):
@@ -434,10 +440,15 @@ class CompiledObjectFilter(AbstractFilter):
         from jedi.inference.compiled import builtin_from_name
         names = []
         needs_type_completions, dir_infos = self.compiled_object.access_handle.get_dir_infos()
+        # We could use `unsafe` here as well, especially as a parameter to
+        # get_dir_infos. But this would lead to a lot of property executions
+        # that are probably not wanted. The drawback for this is that we
+        # have a different name for `get` and `values`. For `get` we always
+        # execute.
         for name in dir_infos:
             names += self._get(
                 name,
-                lambda name: dir_infos[name],
+                lambda name, unsafe: dir_infos[name],
                 lambda name: name in dir_infos,
             )
 

test/test_api/test_interpreter.py

@@ -200,7 +200,7 @@ def test_getitem_side_effects():
 
 @pytest.mark.parametrize('stacklevel', [1, 2])
 @pytest.mark.filterwarnings("error")
-def test_property_warnings(stacklevel):
+def test_property_warnings(stacklevel, allow_unsafe_getattr):
     class Foo3:
         @property
         def prop(self):
@@ -209,16 +209,50 @@ def test_property_warnings(stacklevel):
             return ''
 
     foo = Foo3()
-    _assert_interpreter_complete('foo.prop.uppe', locals(), ['upper'])
+    expected = ['upper'] if allow_unsafe_getattr else []
+    _assert_interpreter_complete('foo.prop.uppe', locals(), expected)
+
+
+@pytest.mark.parametrize('class_is_findable', [False, True])
+def test__getattr__completions(allow_unsafe_getattr, class_is_findable):
+    class CompleteGetattr(object):
+        def __getattr__(self, name):
+            if name == 'foo':
+                return self
+            if name == 'fbar':
+                return ''
+            raise AttributeError(name)
+
+        def __dir__(self):
+            return ['foo', 'fbar'] + object.__dir__(self)
+
+    if not class_is_findable:
+        CompleteGetattr.__name__ = "something_somewhere"
+    namespace = {'c': CompleteGetattr()}
+    expected = ['foo', 'fbar']
+    _assert_interpreter_complete('c.f', namespace, expected)
+
+    # Completions don't work for class_is_findable, because __dir__ is checked
+    # for interpreter analysis, but if the static analysis part tries to help
+    # it will not work. However static analysis is pretty good and understands
+    # how gettatr works (even the ifs/comparisons).
+    if not allow_unsafe_getattr:
+        expected = []
+    _assert_interpreter_complete('c.foo.f', namespace, expected)
+    _assert_interpreter_complete('c.foo.foo.f', namespace, expected)
+    _assert_interpreter_complete('c.foo.uppe', namespace, [])
+
+    expected_int = ['upper'] if allow_unsafe_getattr or class_is_findable else []
+    _assert_interpreter_complete('c.foo.fbar.uppe', namespace, expected_int)
 
 
 @pytest.fixture(params=[False, True])
-def allow_descriptor_access_or_not(request, monkeypatch):
+def allow_unsafe_getattr(request, monkeypatch):
     monkeypatch.setattr(jedi.Interpreter, '_allow_descriptor_getattr_default', request.param)
     return request.param
 
 
-def test_property_error_oldstyle(allow_descriptor_access_or_not):
+def test_property_error_oldstyle(allow_unsafe_getattr):
     lst = []
     class Foo3:
         @property
@@ -230,14 +264,14 @@ def test_property_error_oldstyle(allow_descriptor_access_or_not):
     _assert_interpreter_complete('foo.bar', locals(), ['bar'])
     _assert_interpreter_complete('foo.bar.baz', locals(), [])
 
-    if allow_descriptor_access_or_not:
+    if allow_unsafe_getattr:
         assert lst == [1, 1]
     else:
         # There should not be side effects
         assert lst == []
 
 
-def test_property_error_newstyle(allow_descriptor_access_or_not):
+def test_property_error_newstyle(allow_unsafe_getattr):
     lst = []
     class Foo3(object):
         @property
@@ -249,7 +283,7 @@ def test_property_error_newstyle(allow_descriptor_access_or_not):
     _assert_interpreter_complete('foo.bar', locals(), ['bar'])
     _assert_interpreter_complete('foo.bar.baz', locals(), [])
 
-    if allow_descriptor_access_or_not:
+    if allow_unsafe_getattr:
         assert lst == [1, 1]
     else:
         # There should not be side effects
@@ -367,7 +401,7 @@ def test_repr_execution_issue():
     assert d.type == 'instance'
 
 
-def test_dir_magic_method():
+def test_dir_magic_method(allow_unsafe_getattr):
     class CompleteAttrs(object):
         def __getattr__(self, name):
             if name == 'foo':
@@ -392,7 +426,12 @@ def test_dir_magic_method():
     assert 'bar' in names
 
     foo = [c for c in completions if c.name == 'foo'][0]
-    assert foo.infer() == []
+    if allow_unsafe_getattr:
+        inst, = foo.infer()
+        assert inst.name == 'int'
+        assert inst.type == 'instance'
+    else:
+        assert foo.infer() == []
 
 
 def test_name_not_findable():