Repository: python-attrs/attrs (python)

Issue:
Add a convenience flag for upcalling on Exception subclasses
Currently, these tests both fail on 2.7 and only the latter fails on 3.6.

```python
@attr.s
class E1(Exception):
    x = attr.ib()
    y = attr.ib()


def test_exception_init_called():
    e = E1(1, 2)
    assert e.args == (e.x, e.y) == (1, 2)


def test_exception_init_called_even_with_kw():
    e = E1(y=3, x=4)
    assert e.args == (e.x, e.y) == (4, 3)
```

If attrs is overriding `__init__` then this makes sense; `args` is filled in by `__init__` in 2.x and `__new__` in 3.x, but only pulling from the `*args` and never `**kwargs`. I discussed this a bit with @markrwilliams but I'm not sure what the best way to handle an attrs-ified exception would be. Maybe it's okay to consider `args` vestigal.

Mostly I'm raising this on the issue tracker because there are some references to using attrs for exceptions, but only indirectly. It'd be nice to have some documentation on a suggested approach either way.

Pull request:
Implement first class exception support
Fixes #368

This adds first class exception support to `attrs`. While on the first sight it might make more sense to have an argument like `exc=True` or even have a separate decorator, I think in the ligtht of `import attrs` this approach is better, because – of course – `auto_exc` would be True by default in `@attrs.define`.

Let me know what you think or whether I've missed some subtleties.

~Open Questions:~

~1. Is it a good idea to put attributes with defaults into args too?~
~2. Should we add `init=False, default=NOTHING` too?~

~The more I look at the code I feel like it should be all (including default=NOTHING) or nothing (only non-default).~

Patch:
diff --git a/changelog.d/500.change.rst b/changelog.d/500.change.rst
new file mode 100644
index 0000000..c1a6402
--- /dev/null
+++ b/changelog.d/500.change.rst
@@ -0,0 +1,3 @@
+``attrs`` now has first class support for defining exception classes.
+
+If you define a class using ``@attr.s(auto_exc=True)`` and subclass an exception, the class will behave like a well-behaved exception class including an appropriate ``__str__`` method, and all attributes additionally available in an ``args`` attribute.
diff --git a/docs/api.rst b/docs/api.rst
index 3cd01a1..98ea845 100644
--- a/docs/api.rst
+++ b/docs/api.rst
@@ -18,7 +18,7 @@ What follows is the API explanation, if you'd like a more hands-on introduction,
 Core
 ----
 
-.. autofunction:: attr.s(these=None, repr_ns=None, repr=True, cmp=True, hash=None, init=True, slots=False, frozen=False, weakref_slot=True, str=False, auto_attribs=False, kw_only=False, cache_hash=False)
+.. autofunction:: attr.s(these=None, repr_ns=None, repr=True, cmp=True, hash=None, init=True, slots=False, frozen=False, weakref_slot=True, str=False, auto_attribs=False, kw_only=False, cache_hash=False, auto_exc=False)
 
    .. note::
 
@@ -42,6 +42,20 @@ Core
       >>> D = attr.s(these={"x": attr.ib()}, init=False)(D)
       >>> D(1)
       D(x=1)
+      >>> @attr.s(auto_exc=True)
+      ... class Error(Exception):
+      ...     x = attr.ib()
+      ...     y = attr.ib(default=42, init=False)
+      >>> Error("foo")
+      Error(x='foo', y=42)
+      >>> raise Error("foo")
+      Traceback (most recent call last):
+         ...
+      Error: ('foo', 42)
+      >>> raise ValueError("foo", 42)   # for comparison
+      Traceback (most recent call last):
+         ...
+      ValueError: ('foo', 42)
 
 
 .. autofunction:: attr.ib
diff --git a/src/attr/__init__.pyi b/src/attr/__init__.pyi
index ea788ea..fcb93b1 100644
--- a/src/attr/__init__.pyi
+++ b/src/attr/__init__.pyi
@@ -167,6 +167,7 @@ def attrs(
     auto_attribs: bool = ...,
     kw_only: bool = ...,
     cache_hash: bool = ...,
+    auto_exc: bool = ...,
 ) -> _C: ...
 @overload
 def attrs(
@@ -184,6 +185,7 @@ def attrs(
     auto_attribs: bool = ...,
     kw_only: bool = ...,
     cache_hash: bool = ...,
+    auto_exc: bool = ...,
 ) -> Callable[[_C], _C]: ...
 
 # TODO: add support for returning NamedTuple from the mypy plugin
@@ -212,6 +214,7 @@ def make_class(
     auto_attribs: bool = ...,
     kw_only: bool = ...,
     cache_hash: bool = ...,
+    auto_exc: bool = ...,
 ) -> type: ...
 
 # _funcs --
diff --git a/src/attr/_make.py b/src/attr/_make.py
index 843c76b..043edef 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -453,6 +453,7 @@ class _ClassBuilder(object):
         "_has_post_init",
         "_delete_attribs",
         "_base_attr_map",
+        "_is_exc",
     )
 
     def __init__(
@@ -465,6 +466,7 @@ class _ClassBuilder(object):
         auto_attribs,
         kw_only,
         cache_hash,
+        is_exc,
     ):
         attrs, base_attrs, base_map = _transform_attrs(
             cls, these, auto_attribs, kw_only
@@ -482,6 +484,7 @@ class _ClassBuilder(object):
         self._cache_hash = cache_hash
         self._has_post_init = bool(getattr(cls, "__attrs_post_init__", False))
         self._delete_attribs = not bool(these)
+        self._is_exc = is_exc
 
         self._cls_dict["__attrs_attrs__"] = self._attrs
 
@@ -688,6 +691,7 @@ class _ClassBuilder(object):
                 self._slots,
                 self._cache_hash,
                 self._base_attr_map,
+                self._is_exc,
             )
         )
 
@@ -738,6 +742,7 @@ def attrs(
     auto_attribs=False,
     kw_only=False,
     cache_hash=False,
+    auto_exc=False,
 ):
     r"""
     A class decorator that adds `dunder
@@ -847,6 +852,19 @@ def attrs(
         fields involved in hash code computation or mutations of the objects
         those fields point to after object creation.  If such changes occur,
         the behavior of the object's hash code is undefined.
+    :param bool auto_exc: If the class subclasses :class:`BaseException`
+        (which implicitly includes any subclass of any exception), the
+        following happens to behave like a well-behaved Python exceptions
+        class:
+
+        - the values for *cmp* and *hash* are ignored and the instances compare
+          and hash by the instance's ids (N.B. ``attrs`` will *not* remove
+          existing implementations of ``__hash__`` or the equality methods. It
+          just won't add own ones.),
+        - all attributes that are either passed into ``__init__`` or have a
+          default value are additionally available as a tuple in the ``args``
+          attribute,
+        - the value of *str* is ignored leaving ``__str__`` to base classes.
 
     .. versionadded:: 16.0.0 *slots*
     .. versionadded:: 16.1.0 *frozen*
@@ -866,12 +884,16 @@ def attrs(
        to each other.
     .. versionadded:: 18.2.0 *kw_only*
     .. versionadded:: 18.2.0 *cache_hash*
+    .. versionadded:: 19.1.0 *auto_exc*
     """
 
     def wrap(cls):
+
         if getattr(cls, "__class__", None) is None:
             raise TypeError("attrs only works with new-style classes.")
 
+        is_exc = auto_exc is True and issubclass(cls, BaseException)
+
         builder = _ClassBuilder(
             cls,
             these,
@@ -881,13 +903,14 @@ def attrs(
             auto_attribs,
             kw_only,
             cache_hash,
+            is_exc,
         )
 
         if repr is True:
             builder.add_repr(repr_ns)
         if str is True:
             builder.add_str()
-        if cmp is True:
+        if cmp is True and not is_exc:
             builder.add_cmp()
 
         if hash is not True and hash is not False and hash is not None:
@@ -902,7 +925,11 @@ def attrs(
                     " hashing must be either explicitly or implicitly "
                     "enabled."
                 )
-        elif hash is True or (hash is None and cmp is True and frozen is True):
+        elif (
+            hash is True
+            or (hash is None and cmp is True and frozen is True)
+            and is_exc is False
+        ):
             builder.add_hash()
         else:
             if cache_hash:
@@ -1241,7 +1268,9 @@ def _add_repr(cls, ns=None, attrs=None):
     return cls
 
 
-def _make_init(attrs, post_init, frozen, slots, cache_hash, base_attr_map):
+def _make_init(
+    attrs, post_init, frozen, slots, cache_hash, base_attr_map, is_exc
+):
     attrs = [a for a in attrs if a.init or a.default is not NOTHING]
 
     # We cache the generated init methods for the same kinds of attributes.
@@ -1250,16 +1279,18 @@ def _make_init(attrs, post_init, frozen, slots, cache_hash, base_attr_map):
     unique_filename = "<attrs generated init {0}>".format(sha1.hexdigest())
 
     script, globs, annotations = _attrs_to_init_script(
-        attrs, frozen, slots, post_init, cache_hash, base_attr_map
+        attrs, frozen, slots, post_init, cache_hash, base_attr_map, is_exc
     )
     locs = {}
     bytecode = compile(script, unique_filename, "exec")
     attr_dict = dict((a.name, a) for a in attrs)
     globs.update({"NOTHING": NOTHING, "attr_dict": attr_dict})
+
     if frozen is True:
         # Save the lookup overhead in __init__ if we need to circumvent
         # immutability.
         globs["_cached_setattr"] = _obj_setattr
+
     eval(bytecode, globs, locs)
 
     # In order of debuggers like PDB being able to step through the code,
@@ -1273,6 +1304,7 @@ def _make_init(attrs, post_init, frozen, slots, cache_hash, base_attr_map):
 
     __init__ = locs["__init__"]
     __init__.__annotations__ = annotations
+
     return __init__
 
 
@@ -1287,6 +1319,7 @@ def _add_init(cls, frozen):
         _is_slot_cls(cls),
         cache_hash=False,
         base_attr_map={},
+        is_exc=False,
     )
     return cls
 
@@ -1376,7 +1409,7 @@ def _is_slot_attr(a_name, base_attr_map):
 
 
 def _attrs_to_init_script(
-    attrs, frozen, slots, post_init, cache_hash, base_attr_map
+    attrs, frozen, slots, post_init, cache_hash, base_attr_map, is_exc
 ):
     """
     Return a script of an initializer for *attrs* and a dict of globals.
@@ -1625,6 +1658,13 @@ def _attrs_to_init_script(
             init_hash_cache = "self.%s = %s"
         lines.append(init_hash_cache % (_hash_cache_field, "None"))
 
+    # For exceptions we rely on BaseException.__init__ for proper
+    # initialization.
+    if is_exc:
+        vals = ",".join("self." + a.name for a in attrs if a.init)
+
+        lines.append("BaseException.__init__(self, %s)" % (vals,))
+
     args = ", ".join(args)
     if kw_only_args:
         if PY2:


Test patch:
diff --git a/tests/test_dark_magic.py b/tests/test_dark_magic.py
index e40fe93..bac2a01 100644
--- a/tests/test_dark_magic.py
+++ b/tests/test_dark_magic.py
@@ -6,6 +6,8 @@ from __future__ import absolute_import, division, print_function
 
 import pickle
 
+from copy import deepcopy
+
 import pytest
 import six
 
@@ -508,3 +510,68 @@ class TestDarkMagic(object):
         assert "property" == attr.fields(C).property.name
         assert "itemgetter" == attr.fields(C).itemgetter.name
         assert "x" == attr.fields(C).x.name
+
+    @pytest.mark.parametrize("slots", [True, False])
+    @pytest.mark.parametrize("frozen", [True, False])
+    def test_auto_exc(self, slots, frozen):
+        """
+        Classes with auto_exc=True have a Exception-style __str__, are neither
+        comparable nor hashable, and store the fields additionally in
+        self.args.
+        """
+
+        @attr.s(auto_exc=True, slots=slots, frozen=frozen)
+        class FooError(Exception):
+            x = attr.ib()
+            y = attr.ib(init=False, default=42)
+            z = attr.ib(init=False)
+            a = attr.ib()
+
+        FooErrorMade = attr.make_class(
+            "FooErrorMade",
+            bases=(Exception,),
+            attrs={
+                "x": attr.ib(),
+                "y": attr.ib(init=False, default=42),
+                "z": attr.ib(init=False),
+                "a": attr.ib(),
+            },
+            auto_exc=True,
+            slots=slots,
+            frozen=frozen,
+        )
+
+        assert FooError(1, "foo") != FooError(1, "foo")
+        assert FooErrorMade(1, "foo") != FooErrorMade(1, "foo")
+
+        for cls in (FooError, FooErrorMade):
+            with pytest.raises(cls) as ei:
+                raise cls(1, "foo")
+
+            e = ei.value
+
+            assert e is e
+            assert e == e
+            assert "(1, 'foo')" == str(e)
+            assert (1, "foo") == e.args
+
+            with pytest.raises(TypeError):
+                hash(e)
+
+            if not frozen:
+                deepcopy(e)
+
+    @pytest.mark.parametrize("slots", [True, False])
+    @pytest.mark.parametrize("frozen", [True, False])
+    def test_auto_exc_one_attrib(self, slots, frozen):
+        """
+        Having one attribute works with auto_exc=True.
+
+        Easy to get wrong with tuple literals.
+        """
+
+        @attr.s(auto_exc=True, slots=slots, frozen=frozen)
+        class FooError(Exception):
+            x = attr.ib()
+
+        FooError(1)
diff --git a/tests/test_make.py b/tests/test_make.py
index a0597f1..61b4f63 100644
--- a/tests/test_make.py
+++ b/tests/test_make.py
@@ -1425,7 +1425,9 @@ class TestClassBuilder(object):
         class C(object):
             pass
 
-        b = _ClassBuilder(C, None, True, True, False, False, False, False)
+        b = _ClassBuilder(
+            C, None, True, True, False, False, False, False, False
+        )
 
         assert "<_ClassBuilder(cls=C)>" == repr(b)
 
@@ -1437,7 +1439,9 @@ class TestClassBuilder(object):
         class C(object):
             x = attr.ib()
 
-        b = _ClassBuilder(C, None, True, True, False, False, False, False)
+        b = _ClassBuilder(
+            C, None, True, True, False, False, False, False, False
+        )
 
         cls = (
             b.add_cmp()
@@ -1500,6 +1504,7 @@ class TestClassBuilder(object):
             frozen=False,
             weakref_slot=True,
             auto_attribs=False,
+            is_exc=False,
             kw_only=False,
             cache_hash=False,
         )
diff --git a/tests/typing_example.py b/tests/typing_example.py
index 527216f..b68ce6e 100644
--- a/tests/typing_example.py
+++ b/tests/typing_example.py
@@ -80,6 +80,20 @@ HH(x=[1], y=[], z=1.1)
 c == cc
 
 
+# Exceptions
+@attr.s(auto_exc=True)
+class Error(Exception):
+    x = attr.ib()
+
+
+try:
+    raise Error(1)
+except Error as e:
+    e.x
+    e.args
+    str(e)
+
+
 # Converters
 # XXX: Currently converters can only be functions so none of this works
 # although the stubs should be correct.