Repository: python-attrs/attrs
Pull request #909 (2022-01-14 22:01:57)

Fix hashing for custom `eq` objects
# Summary

Thanks for making attrs btw! It's an amazing library. In the documentation for `attr.ib` (sorry, still speaking in 2021 attr here), we see:

> hash (Optional[bool]) – Include this attribute in the generated __hash__ method. If None (default), **mirror eq’s value**. This is the correct behavior according the Python spec. Setting this value to anything else than None is discouraged.

Currently this does not happen - if we passed in a `callable` to transform the field's value, we would see `_x_key(self.x)` in the generated eq code, but we don't see the same thing happening for the generated hash function.

Currently, I'd like to compare equality of a non-hashable object. I was surprised to see that the hash function did not respect the `eq` override and get it for free (which is an awesome feature from attr!).

# Pull Request Check List


- [x] Added **tests** for changed code.
  Our CI fails if coverage is not 100%.
- [x] New features have been added to our [Hypothesis testing strategy](https://github.com/python-attrs/attrs/blob/main/tests/strategies.py).
- [x] Changes or additions to public APIs are reflected in our type stubs (files ending in ``.pyi``).
    - [x] ...and used in the stub test file `tests/typing_example.py`.
    - [x] If they've been added to `attr/__init__.pyi`, they've *also* been re-imported in `attrs/__init__.pyi`.
- [x] Updated **documentation** for changed code.
    - [x] New functions/classes have to be added to `docs/api.rst` by hand.
    - [x] Changes to the signature of `@attr.s()` have to be added by hand too.
    - [x] Changed/added classes/methods/functions have appropriate `versionadded`, `versionchanged`, or `deprecated` [directives](http://www.sphinx-doc.org/en/stable/markup/para.html#directive-versionadded).
          Find the appropriate next version in our [``__init__.py``](https://github.com/python-attrs/attrs/blob/main/src/attr/__init__.py) file.
- [x] Documentation in `.rst` files is written using [semantic newlines](https://rhodesmill.org/brandon/2012/one-sentence-per-line/).
- [ ] Changes (and possible deprecations) have news fragments in [`changelog.d`](https://github.com/python-attrs/attrs/blob/main/changelog.d).

As the change is only meant to make the code be compliant with the docs, I have not updated any existing documentation. If this is worth adding into the changelog let me know and I'll add it asap. Thank you!

OneRaynyDay: Hi @hynek , thanks for getting back to me! I've not used this coverage tool, and I was wondering what I needed to do to bring the coverage back up to 100%. I updated the changelog.d with the update :)
hynek: Since I don't have push permissions against your branch, please apply the following patch and try again:

```patch
diff --git src/attr/_make.py src/attr/_make.py
index 962e560..86ced77 100644
--- src/attr/_make.py
+++ src/attr/_make.py
@@ -325,13 +325,11 @@ def _compile_and_eval(script, globs, locs=None, filename=""):
     eval(bytecode, globs, locs)
 
 
-def _make_method(name, script, filename, globs=None):
+def _make_method(name, script, filename, globs):
     """
     Create the method with the script given and return the method object.
     """
     locs = {}
-    if globs is None:
-        globs = {}
 
     # In order of debuggers like PDB being able to step through the code,
     # we add a fake linecache entry.
@@ -1955,7 +1953,7 @@ if HAS_F_STRINGS:
         ]
 
         return _make_method(
-            "__repr__", "\n".join(lines), unique_filename, globs=globs
+            "__repr__", "\n".join(lines), unique_filename, globs
         )
 
 else:
```

Patch:
diff --git a/changelog.d/909.change.rst b/changelog.d/909.change.rst
new file mode 100644
index 0000000..359d120
--- /dev/null
+++ b/changelog.d/909.change.rst
@@ -0,0 +1,1 @@
+If an eq key is defined, it is also used before hashing the attribute.
diff --git a/src/attr/_make.py b/src/attr/_make.py
index 915c5e6..19a3542 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -325,13 +325,11 @@ def _compile_and_eval(script, globs, locs=None, filename=""):
     eval(bytecode, globs, locs)
 
 
-def _make_method(name, script, filename, globs=None):
+def _make_method(name, script, filename, globs):
     """
     Create the method with the script given and return the method object.
     """
     locs = {}
-    if globs is None:
-        globs = {}
 
     # In order of debuggers like PDB being able to step through the code,
     # we add a fake linecache entry.
@@ -1680,6 +1678,8 @@ def _make_hash(cls, attrs, frozen, cache_hash):
 
     unique_filename = _generate_unique_filename(cls, "hash")
     type_hash = hash(unique_filename)
+    # If eq is custom generated, we need to include the functions in globs
+    globs = {}
 
     hash_def = "def __hash__(self"
     hash_func = "hash(("
@@ -1714,7 +1714,14 @@ def _make_hash(cls, attrs, frozen, cache_hash):
         )
 
         for a in attrs:
-            method_lines.append(indent + "        self.%s," % a.name)
+            if a.eq_key:
+                cmp_name = "_%s_key" % (a.name,)
+                globs[cmp_name] = a.eq_key
+                method_lines.append(
+                    indent + "        %s(self.%s)," % (cmp_name, a.name)
+                )
+            else:
+                method_lines.append(indent + "        self.%s," % a.name)
 
         method_lines.append(indent + "    " + closing_braces)
 
@@ -1734,7 +1741,7 @@ def _make_hash(cls, attrs, frozen, cache_hash):
         append_hash_computation_lines("return ", tab)
 
     script = "\n".join(method_lines)
-    return _make_method("__hash__", script, unique_filename)
+    return _make_method("__hash__", script, unique_filename, globs)
 
 
 def _add_hash(cls, attrs):


Test patch:
diff --git a/tests/test_make.py b/tests/test_make.py
index 6cc4f05..cdca30d 100644
--- a/tests/test_make.py
+++ b/tests/test_make.py
@@ -2057,6 +2057,27 @@ class TestAutoDetect:
 
         assert "hi" == repr(C(42))
 
+    @pytest.mark.parametrize("slots", [True, False])
+    @pytest.mark.parametrize("frozen", [True, False])
+    def test_hash_uses_eq(self, slots, frozen):
+        """
+        If eq is passed in, then __hash__ should use the eq callable
+        to generate the hash code.
+        """
+
+        @attr.s(slots=slots, frozen=frozen, hash=True)
+        class C(object):
+            x = attr.ib(eq=str)
+
+        @attr.s(slots=slots, frozen=frozen, hash=True)
+        class D(object):
+            x = attr.ib()
+
+        # These hashes should be the same because 1 is turned into
+        # string before hashing.
+        assert hash(C("1")) == hash(C(1))
+        assert hash(D("1")) != hash(D(1))
+
     @pytest.mark.parametrize("slots", [True, False])
     @pytest.mark.parametrize("frozen", [True, False])
     def test_detect_auto_hash(self, slots, frozen):