Repository: python-attrs/attrs (python)

Issue:
auto_exc's hashing behavior and its documentation do not match
The [documentation](https://github.com/python-attrs/attrs/blob/25a02bbc7b5309acebda20101125372d42c978db/src/attr/_make.py#L846) for `auto_exc` says:
```
  - 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.),
```

However, the test for `auto_exc` [says](https://github.com/python-attrs/attrs/blob/25a02bbc7b5309acebda20101125372d42c978db/tests/test_dark_magic.py#L519):
```
    Classes with auto_exc=True have a Exception-style __str__, are neither
        comparable nor hashable, and store the fields additionally in
        self.args.
```

and [tests](https://github.com/python-attrs/attrs/blob/25a02bbc7b5309acebda20101125372d42c978db/tests/test_dark_magic.py#L558):

```
            with pytest.raises(TypeError):
                hash(e)
```

The documentation and the code should be made consistent.  I noticed this because `unittest` complained that the exception type was unhashable when thrown from a test and I see some [references to `logging` expecting hashable exceptions](https://github.com/schematics/schematics/issues/452), so the documented behavior is probably better than the implemented behavior.

Pull request:
Make auto_exc=True classes hashable by ID
They now behave as the docs claim.

Fixes #543

Patch:
diff --git a/changelog.d/543.change.rst b/changelog.d/543.change.rst
new file mode 100644
index 0000000..906275e
--- /dev/null
+++ b/changelog.d/543.change.rst
@@ -0,0 +1,1 @@
+``@attr.s(auto_exc=True)`` now generates classes that are hashable by ID, as the documentation always claimed it would.
diff --git a/changelog.d/563.change.rst b/changelog.d/563.change.rst
new file mode 100644
index 0000000..906275e
--- /dev/null
+++ b/changelog.d/563.change.rst
@@ -0,0 +1,1 @@
+``@attr.s(auto_exc=True)`` now generates classes that are hashable by ID, as the documentation always claimed it would.
diff --git a/src/attr/_make.py b/src/attr/_make.py
index cd3ce97..40acf43 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -912,20 +912,20 @@ def attrs(
             raise TypeError(
                 "Invalid value for hash.  Must be True, False, or None."
             )
-        elif hash is False or (hash is None and cmp is False):
+        elif hash is False or (hash is None and cmp is False) or is_exc:
+            # Don't do anything. Should fall back to __object__'s __hash__
+            # which is by id.
             if cache_hash:
                 raise TypeError(
                     "Invalid value for cache_hash.  To use hash caching,"
                     " hashing must be either explicitly or implicitly "
                     "enabled."
                 )
-        elif (
-            hash is True
-            or (hash is None and cmp is True and frozen is True)
-            and is_exc is False
-        ):
+        elif hash is True or (hash is None and cmp is True and frozen is True):
+            # Build a __hash__ if told so, or if it's safe.
             builder.add_hash()
         else:
+            # Raise TypeError on attempts to hash.
             if cache_hash:
                 raise TypeError(
                     "Invalid value for cache_hash.  To use hash caching,"


Test patch:
diff --git a/tests/test_dark_magic.py b/tests/test_dark_magic.py
index 49477a9..778fc9f 100644
--- a/tests/test_dark_magic.py
+++ b/tests/test_dark_magic.py
@@ -515,9 +515,8 @@ class TestDarkMagic(object):
     @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.
+        Classes with auto_exc=True have a Exception-style __str__, compare and
+        hash by id, and store the fields additionally in self.args.
         """
 
         @attr.s(auto_exc=True, slots=slots, frozen=frozen)
@@ -545,21 +544,28 @@ class TestDarkMagic(object):
         assert FooErrorMade(1, "foo") != FooErrorMade(1, "foo")
 
         for cls in (FooError, FooErrorMade):
-            with pytest.raises(cls) as ei:
+            with pytest.raises(cls) as ei1:
                 raise cls(1, "foo")
 
-            e = ei.value
+            with pytest.raises(cls) as ei2:
+                raise cls(1, "foo")
+
+            e1 = ei1.value
+            e2 = ei2.value
 
-            assert e is e
-            assert e == e
-            assert "(1, 'foo')" == str(e)
-            assert (1, "foo") == e.args
+            assert e1 is e1
+            assert e1 == e1
+            assert e2 == e2
+            assert e1 != e2
+            assert "(1, 'foo')" == str(e1) == str(e2)
+            assert (1, "foo") == e1.args == e2.args
 
-            with pytest.raises(TypeError):
-                hash(e)
+            hash(e1) == hash(e1)
+            hash(e2) == hash(e2)
 
             if not frozen:
-                deepcopy(e)
+                deepcopy(e1)
+                deepcopy(e2)
 
     @pytest.mark.parametrize("slots", [True, False])
     @pytest.mark.parametrize("frozen", [True, False])