Lint: Warn if only one of {Hash, PartialEq} is derived, the other manually implemented

[untitaker]

This is a continuation of rust-lang/rust#29758

The derived Hash and PartialEq implementations for structs rely on the struct fields' implementations of those traits for comparison and hashing. If I manually implement one of those traits by myself, the following rule might be broken:

k1 == k2 -> hash(k1) == hash(k2)

(found in the docs of Hash)

See the previous issue on top for an example.

This seems like a potential pitfall. In my particular case, I had a struct with derive(Eq, PartialEq, Hash), and wanted to change the way equality works. So I manually implemented PartialEq, but forgot to do the same for Hash as well. Luckily @eefriedman reminded me of the incorrect Hash impl during code review, but the code compiled perfectly fine.

I think rustc could detect this kind of situation. IMO the compiler should warn if one of {Hash, PartialEq} is derived, and the other trait manually implemented, because:

1. It's unlikely that the developer intentionally derives only one of those traits.
2. It's unlikely (if not impossible) that the struct's hashing and equivalency behavior satisfy the rule above AND differ in behavior from impls you get with derive

[pnkfelix]

I think I have seen lots of code that derives PartialEq without any impl of Hash (derived or not)

I wouldn't want warnings on all those cases

[untitaker]

I think I have seen lots of code that derives PartialEq without any impl of Hash (derived or not)

The proposal in bold doesn't concern those types. I've now updated the title as well.

To be clear:

• #[derive(PartialEq, Hash)] is fine
• #[derive(PartialEq)] is fine
• #[derive(PartialEq)] and manually impl Hash should yield a warning
• #[derive(Hash)] and manually impl PartialEq should yield a warning

[sfackler]

This seems like something that can be handled in e.g. clippy.

[oli-obk]

This seems like something that can be handled in e.g. clippy.

One direction is in clippy:

https://github.com/Manishearth/rust-clippy/wiki#derive_hash_not_eq

[untitaker]

This is awesome!

On Mon, Feb 15, 2016 at 01:39:17AM -0800, Oliver Schneider wrote:

This seems like something that can be handled in e.g. clippy.

One direction is in clippy:

https://github.com/Manishearth/rust-clippy/wiki#derive_hash_not_eq

---

Reply to this email directly or view it on GitHub:
#1499 (comment)

[oli-obk]

Clippy now requires either both derived or both not derived: https://github.com/Manishearth/rust-clippy/pull/676

[untitaker]

Let's close this issue for now and see how the lint in Clippy works out. If it works well enough I'd still like to see it in rustc itself, but it's probably too early to tell.