Repository: PythonCharmers/python-future

Issue:
raise_ fails to reraise exception which requires arguments
When the `future.utils.raise_` function is used with all its three parameters, it is in some cases *not* able to reraise exceptions which need arguments for their initialization. For example, this code:

```python
import sys

from future.utils import raise_

class CustomException(Exception):
    def __init__(self, severity: int, message: str) -> None:
        super().__init__(f"custom message of severity {severity}: {message}")

try:
    raise CustomException(1, "hello")
except CustomException:
    raise_(*sys.exc_info())
```
fails with a `TypeError`:
```
Traceback (most recent call last):
  File "./future_test.py", line 11, in <module>
    raise CustomException(1, "hello")
__main__.CustomException: custom message of severity 1: hello

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./future_test.py", line 13, in <module>
    raise_(*sys.exc_info())
  File "../python3.6/site-packages/future/utils/__init__.py", line 409, in raise_
    exc = tp(value)
TypeError: __init__() missing 1 required positional argument: 'message'
```

The problem is caused by the fact that the `raise_` function tries to initialize the exception *class* (i.e. its first argument) using exception *value* (i.e. its second argument):

https://github.com/PythonCharmers/python-future/blob/3c44a1e4486eb74679ff68a9f75c84ce767dd62a/src/future/utils/__init__.py#L410

That is not always possible for exception types with custom initializers.

Discussion:

Patch:
diff --git a/src/future/utils/__init__.py b/src/future/utils/__init__.py
index 628b8f9..5992007 100644
--- a/src/future/utils/__init__.py
+++ b/src/future/utils/__init__.py
@@ -406,12 +406,34 @@ if PY3:
         allows re-raising exceptions with the cls value and traceback on
         Python 2 and 3.
         """
-        if value is not None and isinstance(tp, Exception):
-            raise TypeError("instance exception may not have a separate value")
-        if value is not None:
-            exc = tp(value)
-        else:
+        if isinstance(tp, Exception):
+            # If the first object is an instance, the type of the exception
+            # is the class of the instance, the instance itself is the value,
+            # and the second object must be None.
+            if value is not None:
+                raise TypeError("instance exception may not have a separate value")
             exc = tp
+        elif not issubclass(tp, Exception):
+            # If the first object is a class, it becomes the type of the
+            # exception.
+            raise TypeError("class must derive from Exception")
+        else:
+            # The second object is used to determine the exception value: If it
+            # is an instance of the class, the instance becomes the exception
+            # value. If the second object is a tuple, it is used as the argument
+            # list for the class constructor; if it is None, an empty argument
+            # list is used, and any other object is treated as a single argument
+            # to the constructor. The instance so created by calling the
+            # constructor is used as the exception value.
+            if isinstance(value, tp):
+                exc = value
+            elif isinstance(value, tuple):
+                exc = tp(*value)
+            elif value is None:
+                exc = tp()
+            else:
+                exc = tp(value)
+
         if exc.__traceback__ is not tb:
             raise exc.with_traceback(tb)
         raise exc
diff --git a/src/libfuturize/fixes/fix_raise.py b/src/libfuturize/fixes/fix_raise.py
index 3e8323d..f751841 100644
--- a/src/libfuturize/fixes/fix_raise.py
+++ b/src/libfuturize/fixes/fix_raise.py
@@ -4,33 +4,39 @@ From Armin Ronacher's ``python-modernize``.
 
 raise         -> raise
 raise E       -> raise E
-raise E, V    -> raise E(V)
+raise E, 5    -> raise E(5)
+raise E, 5, T -> raise E(5).with_traceback(T)
+raise E, None, T -> raise E.with_traceback(T)
 
-raise (((E, E'), E''), E'''), V -> raise E(V)
+raise (((E, E'), E''), E'''), 5 -> raise E(5)
+raise "foo", V, T               -> warns about string exceptions
 
+raise E, (V1, V2) -> raise E(V1, V2)
+raise E, (V1, V2), T -> raise E(V1, V2).with_traceback(T)
 
-CAVEATS:
-1) "raise E, V" will be incorrectly translated if V is an exception
-   instance. The correct Python 3 idiom is
 
-        raise E from V
+CAVEATS:
+1) "raise E, V, T" cannot be translated safely in general. If V
+   is not a tuple or a (number, string, None) literal, then:
 
-   but since we can't detect instance-hood by syntax alone and since
-   any client code would have to be changed as well, we don't automate
-   this.
+   raise E, V, T -> from future.utils import raise_
+                    raise_(E, V, T)
 """
-# Author: Collin Winter, Armin Ronacher
+# Author: Collin Winter, Armin Ronacher, Mark Huang
 
 # Local imports
 from lib2to3 import pytree, fixer_base
 from lib2to3.pgen2 import token
-from lib2to3.fixer_util import Name, Call, is_tuple
+from lib2to3.fixer_util import Name, Call, is_tuple, Comma, Attr, ArgList
+
+from libfuturize.fixer_util import touch_import_top
+
 
 class FixRaise(fixer_base.BaseFix):
 
     BM_compatible = True
     PATTERN = """
-    raise_stmt< 'raise' exc=any [',' val=any] >
+    raise_stmt< 'raise' exc=any [',' val=any [',' tb=any]] >
     """
 
     def transform(self, node, results):
@@ -55,19 +61,47 @@ class FixRaise(fixer_base.BaseFix):
                 exc = exc.children[1].children[0].clone()
             exc.prefix = u" "
 
-        if "val" not in results:
-            # One-argument raise
-            new = pytree.Node(syms.raise_stmt, [Name(u"raise"), exc])
-            new.prefix = node.prefix
-            return new
-
-        val = results["val"].clone()
-        if is_tuple(val):
-            args = [c.clone() for c in val.children[1:-1]]
+        if "tb" in results:
+            tb = results["tb"].clone()
+        else:
+            tb = None
+
+        if "val" in results:
+            val = results["val"].clone()
+            if is_tuple(val):
+                # Assume that exc is a subclass of Exception and call exc(*val).
+                args = [c.clone() for c in val.children[1:-1]]
+                exc = Call(exc, args)
+            elif val.type in (token.NUMBER, token.STRING):
+                # Handle numeric and string literals specially, e.g.
+                # "raise Exception, 5" -> "raise Exception(5)".
+                val.prefix = u""
+                exc = Call(exc, [val])
+            elif val.type == token.NAME and val.value == u"None":
+                # Handle None specially, e.g.
+                # "raise Exception, None" -> "raise Exception".
+                pass
+            else:
+                # val is some other expression. If val evaluates to an instance
+                # of exc, it should just be raised. If val evaluates to None,
+                # a default instance of exc should be raised (as above). If val
+                # evaluates to a tuple, exc(*val) should be called (as
+                # above). Otherwise, exc(val) should be called. We can only
+                # tell what to do at runtime, so defer to future.utils.raise_(),
+                # which handles all of these cases.
+                touch_import_top(u"future.utils", u"raise_", node)
+                exc.prefix = u""
+                args = [exc, Comma(), val]
+                if tb is not None:
+                    args += [Comma(), tb]
+                return Call(Name(u"raise_"), args)
+
+        if tb is not None:
+            tb.prefix = ""
+            exc_list = Attr(exc, Name('with_traceback')) + [ArgList([tb])]
         else:
-            val.prefix = u""
-            args = [val]
+            exc_list = [exc]
 
         return pytree.Node(syms.raise_stmt,
-                           [Name(u"raise"), Call(exc, args)],
+                           [Name(u"raise")] + exc_list,
                            prefix=node.prefix)


Test patch:
diff --git a/tests/test_future/test_libfuturize_fixers.py b/tests/test_future/test_libfuturize_fixers.py
index 8c4a9a3..4ac0b7e 100644
--- a/tests/test_future/test_libfuturize_fixers.py
+++ b/tests/test_future/test_libfuturize_fixers.py
@@ -702,133 +702,140 @@ class Test_print(FixerTestCase):
 #             except (Exception, SystemExit):
 #                 pass"""
 #         self.unchanged(s)
-#
-# class Test_raise(FixerTestCase):
-#     fixer = "raise"
-#
-#     def test_basic(self):
-#         b = """raise Exception, 5"""
-#         a = """raise Exception(5)"""
-#         self.check(b, a)
-#
-#     def test_prefix_preservation(self):
-#         b = """raise Exception,5"""
-#         a = """raise Exception(5)"""
-#         self.check(b, a)
-#
-#         b = """raise   Exception,    5"""
-#         a = """raise   Exception(5)"""
-#         self.check(b, a)
-#
-#     def test_with_comments(self):
-#         b = """raise Exception, 5 # foo"""
-#         a = """raise Exception(5) # foo"""
-#         self.check(b, a)
-#
-#         b = """raise E, (5, 6) % (a, b) # foo"""
-#         a = """raise E((5, 6) % (a, b)) # foo"""
-#         self.check(b, a)
-#
-#         b = """def foo():
-#                     raise Exception, 5, 6 # foo"""
-#         a = """def foo():
-#                     raise Exception(5).with_traceback(6) # foo"""
-#         self.check(b, a)
-#
-#     def test_None_value(self):
-#         b = """raise Exception(5), None, tb"""
-#         a = """raise Exception(5).with_traceback(tb)"""
-#         self.check(b, a)
-#
-#     def test_tuple_value(self):
-#         b = """raise Exception, (5, 6, 7)"""
-#         a = """raise Exception(5, 6, 7)"""
-#         self.check(b, a)
-#
-#     def test_tuple_detection(self):
-#         b = """raise E, (5, 6) % (a, b)"""
-#         a = """raise E((5, 6) % (a, b))"""
-#         self.check(b, a)
-#
-#     def test_tuple_exc_1(self):
-#         b = """raise (((E1, E2), E3), E4), V"""
-#         a = """raise E1(V)"""
-#         self.check(b, a)
-#
-#     def test_tuple_exc_2(self):
-#         b = """raise (E1, (E2, E3), E4), V"""
-#         a = """raise E1(V)"""
-#         self.check(b, a)
-#
-#     # These should produce a warning
-#
-#     def test_string_exc(self):
-#         s = """raise 'foo'"""
-#         self.warns_unchanged(s, "Python 3 does not support string exceptions")
-#
-#     def test_string_exc_val(self):
-#         s = """raise "foo", 5"""
-#         self.warns_unchanged(s, "Python 3 does not support string exceptions")
-#
-#     def test_string_exc_val_tb(self):
-#         s = """raise "foo", 5, 6"""
-#         self.warns_unchanged(s, "Python 3 does not support string exceptions")
-#
-#     # These should result in traceback-assignment
-#
-#     def test_tb_1(self):
-#         b = """def foo():
-#                     raise Exception, 5, 6"""
-#         a = """def foo():
-#                     raise Exception(5).with_traceback(6)"""
-#         self.check(b, a)
-#
-#     def test_tb_2(self):
-#         b = """def foo():
-#                     a = 5
-#                     raise Exception, 5, 6
-#                     b = 6"""
-#         a = """def foo():
-#                     a = 5
-#                     raise Exception(5).with_traceback(6)
-#                     b = 6"""
-#         self.check(b, a)
-#
-#     def test_tb_3(self):
-#         b = """def foo():
-#                     raise Exception,5,6"""
-#         a = """def foo():
-#                     raise Exception(5).with_traceback(6)"""
-#         self.check(b, a)
-#
-#     def test_tb_4(self):
-#         b = """def foo():
-#                     a = 5
-#                     raise Exception,5,6
-#                     b = 6"""
-#         a = """def foo():
-#                     a = 5
-#                     raise Exception(5).with_traceback(6)
-#                     b = 6"""
-#         self.check(b, a)
-#
-#     def test_tb_5(self):
-#         b = """def foo():
-#                     raise Exception, (5, 6, 7), 6"""
-#         a = """def foo():
-#                     raise Exception(5, 6, 7).with_traceback(6)"""
-#         self.check(b, a)
-#
-#     def test_tb_6(self):
-#         b = """def foo():
-#                     a = 5
-#                     raise Exception, (5, 6, 7), 6
-#                     b = 6"""
-#         a = """def foo():
-#                     a = 5
-#                     raise Exception(5, 6, 7).with_traceback(6)
-#                     b = 6"""
-#         self.check(b, a)
+
+class Test_raise(FixerTestCase):
+    fixer = "raise"
+
+    def test_basic(self):
+        b = """raise Exception, 5"""
+        a = """raise Exception(5)"""
+        self.check(b, a)
+
+    def test_prefix_preservation(self):
+        b = """raise Exception,5"""
+        a = """raise Exception(5)"""
+        self.check(b, a)
+
+        b = """raise   Exception,    5"""
+        a = """raise   Exception(5)"""
+        self.check(b, a)
+
+    def test_with_comments(self):
+        b = """raise Exception, 5 # foo"""
+        a = """raise Exception(5) # foo"""
+        self.check(b, a)
+
+        b = """def foo():
+                    raise Exception, 5, 6 # foo"""
+        a = """def foo():
+                    raise Exception(5).with_traceback(6) # foo"""
+        self.check(b, a)
+
+    def test_None_value(self):
+        b = """raise Exception(5), None, tb"""
+        a = """raise Exception(5).with_traceback(tb)"""
+        self.check(b, a)
+
+    def test_tuple_value(self):
+        b = """raise Exception, (5, 6, 7)"""
+        a = """raise Exception(5, 6, 7)"""
+        self.check(b, a)
+
+    def test_tuple_exc_1(self):
+        b = """raise (((E1, E2), E3), E4), 5"""
+        a = """raise E1(5)"""
+        self.check(b, a)
+
+    def test_tuple_exc_2(self):
+        b = """raise (E1, (E2, E3), E4), 5"""
+        a = """raise E1(5)"""
+        self.check(b, a)
+
+    def test_unknown_value(self):
+        b = """
+        raise E, V"""
+        a = """
+        from future.utils import raise_
+        raise_(E, V)"""
+        self.check(b, a)
+
+    def test_unknown_value_with_traceback_with_comments(self):
+        b = """
+        raise E, Func(arg1, arg2, arg3), tb # foo"""
+        a = """
+        from future.utils import raise_
+        raise_(E, Func(arg1, arg2, arg3), tb) # foo"""
+        self.check(b, a)
+
+    # These should produce a warning
+
+    def test_string_exc(self):
+        s = """raise 'foo'"""
+        self.warns_unchanged(s, "Python 3 does not support string exceptions")
+
+    def test_string_exc_val(self):
+        s = """raise "foo", 5"""
+        self.warns_unchanged(s, "Python 3 does not support string exceptions")
+
+    def test_string_exc_val_tb(self):
+        s = """raise "foo", 5, 6"""
+        self.warns_unchanged(s, "Python 3 does not support string exceptions")
+
+    # These should result in traceback-assignment
+
+    def test_tb_1(self):
+        b = """def foo():
+                    raise Exception, 5, 6"""
+        a = """def foo():
+                    raise Exception(5).with_traceback(6)"""
+        self.check(b, a)
+
+    def test_tb_2(self):
+        b = """def foo():
+                    a = 5
+                    raise Exception, 5, 6
+                    b = 6"""
+        a = """def foo():
+                    a = 5
+                    raise Exception(5).with_traceback(6)
+                    b = 6"""
+        self.check(b, a)
+
+    def test_tb_3(self):
+        b = """def foo():
+                    raise Exception,5,6"""
+        a = """def foo():
+                    raise Exception(5).with_traceback(6)"""
+        self.check(b, a)
+
+    def test_tb_4(self):
+        b = """def foo():
+                    a = 5
+                    raise Exception,5,6
+                    b = 6"""
+        a = """def foo():
+                    a = 5
+                    raise Exception(5).with_traceback(6)
+                    b = 6"""
+        self.check(b, a)
+
+    def test_tb_5(self):
+        b = """def foo():
+                    raise Exception, (5, 6, 7), 6"""
+        a = """def foo():
+                    raise Exception(5, 6, 7).with_traceback(6)"""
+        self.check(b, a)
+
+    def test_tb_6(self):
+        b = """def foo():
+                    a = 5
+                    raise Exception, (5, 6, 7), 6
+                    b = 6"""
+        a = """def foo():
+                    a = 5
+                    raise Exception(5, 6, 7).with_traceback(6)
+                    b = 6"""
+        self.check(b, a)
 #
 # class Test_throw(FixerTestCase):
 #     fixer = "throw"
diff --git a/tests/test_future/test_utils.py b/tests/test_future/test_utils.py
index b80e8c1..488c006 100644
--- a/tests/test_future/test_utils.py
+++ b/tests/test_future/test_utils.py
@@ -110,11 +110,7 @@ class TestUtils(unittest.TestCase):
         self.assertFalse(isbytes(self.s))
         self.assertFalse(isbytes(self.s2))
 
-    @unittest.skipIf(PY3, 'test_raise_ currently fails on Py3')
     def test_raise_(self):
-        """
-        The with_value() test currently fails on Py3
-        """
         def valerror():
             try:
                 raise ValueError("Apples!")
@@ -173,6 +169,23 @@ class TestUtils(unittest.TestCase):
             pass
         # incorrectly raises a TypeError on Py3 as of v0.15.2.
 
+    def test_raise_custom_exception(self):
+        """
+        Test issue #387.
+        """
+        class CustomException(Exception):
+            def __init__(self, severity, message):
+                super().__init__("custom message of severity %d: %s" % (
+                    severity, message))
+
+        def raise_custom_exception():
+            try:
+                raise CustomException(1, "hello")
+            except CustomException:
+                raise_(*sys.exc_info())
+
+        self.assertRaises(CustomException, raise_custom_exception)
+
     @skip26
     def test_as_native_str(self):
         """