Merge pull request #166 from tkf/rethrow_uncaught

Better way of re-raising AttributeError

jedi/_compatibility.py

@@ -81,6 +81,25 @@ else:
     eval(compile("""def exec_function(source, global_map):
                         exec source in global_map """, 'blub', 'exec'))
 
+# re-raise function
+if is_py3k:
+    def reraise(exception, traceback):
+        raise exception.with_traceback(traceback)
+else:
+    eval(compile("""
+def reraise(exception, traceback):
+    raise exception, None, traceback
+""", 'blub', 'exec'))
+
+reraise.__doc__ = """
+Re-raise `exception` with a `traceback` object.
+
+Usage::
+
+    reraise(Exception, sys.exc_info()[2])
+
+"""
+
 # StringIO (Python 2.5 has no io module), so use io only for py3k
 try:
     from StringIO import StringIO

jedi/common.py

@@ -1,8 +1,10 @@
 """ A universal module with functions / classes without dependencies. """
+import sys
 import contextlib
+import functools
 import tokenize
 
-from _compatibility import next
+from _compatibility import next, reraise
 import debug
 import settings
 
@@ -14,23 +16,47 @@ class MultiLevelStopIteration(Exception):
     pass
 
 
-class MultiLevelAttributeError(Exception):
+class UncaughtAttributeError(Exception):
     """
     Important, because `__getattr__` and `hasattr` catch AttributeErrors
     implicitly. This is really evil (mainly because of `__getattr__`).
     `hasattr` in Python 2 is even more evil, because it catches ALL exceptions.
-    Therefore this class has to be a `BaseException` and not an `Exception`.
+    Therefore this class originally had to be derived from `BaseException`
-    But because I rewrote hasattr, we can now switch back to `Exception`.
+    instead of `Exception`.  But because I removed relevant `hasattr` from
+    the code base, we can now switch back to `Exception`.
 
     :param base: return values of sys.exc_info().
     """
-    def __init__(self, base=None):
-        self.base = base
 
-    def __str__(self):
+
-        import traceback
+def rethrow_uncaught(func):
-        tb = traceback.format_exception(*self.base)
+    """
-        return 'Original:\n\n' + ''.join(tb)
+    Re-throw uncaught `AttributeError`.
+
+    Usage:  Put ``@rethrow_uncaught`` in front of the function
+    which does **not** suppose to raise `AttributeError`.
+
+    AttributeError is easily get caught by `hasattr` and another
+    ``except AttributeError`` clause.  This becomes problem when you use
+    a lot of "dynamic" attributes (e.g., using ``@property``) because you
+    can't distinguish if the property does not exist for real or some code
+    inside of the "dynamic" attribute through that error.  In a well
+    written code, such error should not exist but getting there is very
+    difficult.  This decorator is to help us getting there by changing
+    `AttributeError` to `UncaughtAttributeError` to avoid unexpected catch.
+    This helps us noticing bugs earlier and facilitates debugging.
+
+    .. note:: Treating StopIteration here is easy.
+              Add that feature when needed.
+    """
+    @functools.wraps(func)
+    def wrapper(*args, **kwds):
+        try:
+            return func(*args, **kwds)
+        except AttributeError:
+            exc_info = sys.exc_info()
+            reraise(UncaughtAttributeError(exc_info[1]), exc_info[2])
+    return wrapper
 
 
 class PushBackIterator(object):

jedi/evaluate.py

@@ -68,7 +68,7 @@ backtracking algorithm.
 
 .. todo:: nonlocal statement, needed or can be ignored? (py3k)
 """
-from _compatibility import next, hasattr, is_py3k, unicode, utf8
+from _compatibility import next, hasattr, is_py3k, unicode, reraise
 
 import sys
 import itertools
@@ -175,7 +175,7 @@ def get_names_of_scope(scope, position=None, star_search=True,
                     yield scope, get_defined_names_for_position(scope,
                                                     position, in_func_scope)
             except StopIteration:
-                raise common.MultiLevelStopIteration('StopIteration raised')
+                reraise(common.MultiLevelStopIteration, sys.exc_info()[2])
         if scope.isinstance(pr.ForFlow) and scope.is_list_comp:
             # is a list comprehension
             yield scope, scope.get_set_vars(is_internal_call=True)
@@ -581,12 +581,7 @@ def follow_statement(stmt, seek_name=None):
     commands = stmt.get_commands()
     debug.dbg('calls: %s' % commands)
 
-    try:
+    result = follow_call_list(commands)
-        result = follow_call_list(commands)
-    except AttributeError:
-        # This is so evil! But necessary to propagate errors. The attribute
-        # errors here must not be catched, because they shouldn't exist.
-        raise common.MultiLevelAttributeError(sys.exc_info())
 
     # Assignment checking is only important if the statement defines multiple
     # variables.
@@ -598,6 +593,7 @@ def follow_statement(stmt, seek_name=None):
     return set(result)
 
 
+@common.rethrow_uncaught
 def follow_call_list(call_list, follow_array=False):
     """
     `call_list` can be either `pr.Array` or `list of list`.

jedi/evaluate_representation.py

@@ -9,7 +9,6 @@ instantiated. This class represents these cases.
 So, why is there also a ``Class`` class here? Well, there are decorators and
 they change classes in Python 3.
 """
-import sys
 import copy
 import itertools
 
@@ -668,6 +667,7 @@ class Execution(Executable):
         """
         return self.get_params() + pr.Scope.get_set_vars(self)
 
+    @common.rethrow_uncaught
     def copy_properties(self, prop):
         """
         Literally copies a property of a Function. Copying is very expensive,
@@ -675,22 +675,19 @@ class Execution(Executable):
         objects can be used for the executions, as if they were in the
         execution.
         """
-        try:
+        # Copy all these lists into this local function.
-            # Copy all these lists into this local function.
+        attr = getattr(self.base, prop)
-            attr = getattr(self.base, prop)
+        objects = []
-            objects = []
+        for element in attr:
-            for element in attr:
+            if element is None:
-                if element is None:
+                copied = element
-                    copied = element
+            else:
-                else:
+                copied = helpers.fast_parent_copy(element)
-                    copied = helpers.fast_parent_copy(element)
+                copied.parent = self._scope_copy(copied.parent)
-                    copied.parent = self._scope_copy(copied.parent)
+                if isinstance(copied, pr.Function):
-                    if isinstance(copied, pr.Function):
+                    copied = Function(copied)
-                        copied = Function(copied)
+            objects.append(copied)
-                objects.append(copied)
+        return objects
-            return objects
-        except AttributeError:
-            raise common.MultiLevelAttributeError(sys.exc_info())
 
     def __getattr__(self, name):
         if name not in ['start_pos', 'end_pos', 'imports', '_sub_module']:
@@ -698,21 +695,19 @@ class Execution(Executable):
         return getattr(self.base, name)
 
     @cache.memoize_default()
+    @common.rethrow_uncaught
     def _scope_copy(self, scope):
-        try:
+        """ Copies a scope (e.g. if) in an execution """
-            """ Copies a scope (e.g. if) in an execution """
+        # TODO method uses different scopes than the subscopes property.
-            # TODO method uses different scopes than the subscopes property.
 
-            # just check the start_pos, sometimes it's difficult with closures
+        # just check the start_pos, sometimes it's difficult with closures
-            # to compare the scopes directly.
+        # to compare the scopes directly.
-            if scope.start_pos == self.start_pos:
+        if scope.start_pos == self.start_pos:
-                return self
+            return self
-            else:
+        else:
-                copied = helpers.fast_parent_copy(scope)
+            copied = helpers.fast_parent_copy(scope)
-                copied.parent = self._scope_copy(copied.parent)
+            copied.parent = self._scope_copy(copied.parent)
-                return copied
+            return copied
-        except AttributeError:
-            raise common.MultiLevelAttributeError(sys.exc_info())
 
     @property
     @cache.memoize_default()