Auto merge of rust-lang#10184 - robertbastian:master, r=xFrednet

Allow implementing `Hash` with derived `PartialEq` (`derive_hash_xor_eq`

This is a common pattern and is totally allowed by the `Hash` trait.

Fixes rust-lang#2627

changelog: Move: Renamed `derive_hash_xor_eq` to [`derived_hash_with_manual_eq`]
[rust-lang#10184](rust-lang/rust-clippy#10184)
changelog: Enhancement: [`derived_hash_with_manual_eq`]: Now allows `#[derive(PartialEq)]` with custom `Hash` implementations
[rust-lang#10184](rust-lang/rust-clippy#10184)
<!-- changelog_checked -->

CHANGELOG.md

@@ -4137,6 +4137,7 @@ Released 2018-09-13
 [`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq
 [`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
 [`derive_partial_eq_without_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_partial_eq_without_eq
+[`derived_hash_with_manual_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derived_hash_with_manual_eq
 [`disallowed_macros`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros
 [`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method
 [`disallowed_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods

clippy_lints/src/declared_lints.rs

@@ -111,7 +111,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::dereference::NEEDLESS_BORROW_INFO,
     crate::dereference::REF_BINDING_TO_REFERENCE_INFO,
     crate::derivable_impls::DERIVABLE_IMPLS_INFO,
-    crate::derive::DERIVE_HASH_XOR_EQ_INFO,
+    crate::derive::DERIVED_HASH_WITH_MANUAL_EQ_INFO,
     crate::derive::DERIVE_ORD_XOR_PARTIAL_ORD_INFO,
     crate::derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ_INFO,
     crate::derive::EXPL_IMPL_CLONE_ON_COPY_INFO,

clippy_lints/src/derive.rs

@@ -46,7 +46,7 @@ declare_clippy_lint! {
     /// }
     /// ```
     #[clippy::version = "pre 1.29.0"]
-    pub DERIVE_HASH_XOR_EQ,
+    pub DERIVED_HASH_WITH_MANUAL_EQ,
     correctness,
     "deriving `Hash` but implementing `PartialEq` explicitly"
 }
@@ -197,7 +197,7 @@ declare_clippy_lint! {
 
 declare_lint_pass!(Derive => [
     EXPL_IMPL_CLONE_ON_COPY,
-    DERIVE_HASH_XOR_EQ,
+    DERIVED_HASH_WITH_MANUAL_EQ,
     DERIVE_ORD_XOR_PARTIAL_ORD,
     UNSAFE_DERIVE_DESERIALIZE,
     DERIVE_PARTIAL_EQ_WITHOUT_EQ
@@ -226,7 +226,7 @@ impl<'tcx> LateLintPass<'tcx> for Derive {
     }
 }
 
-/// Implementation of the `DERIVE_HASH_XOR_EQ` lint.
+/// Implementation of the `DERIVED_HASH_WITH_MANUAL_EQ` lint.
 fn check_hash_peq<'tcx>(
     cx: &LateContext<'tcx>,
     span: Span,
@@ -243,7 +243,7 @@ fn check_hash_peq<'tcx>(
             cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| {
                 let peq_is_automatically_derived = cx.tcx.has_attr(impl_id, sym::automatically_derived);
 
-                if peq_is_automatically_derived == hash_is_automatically_derived {
+                if !hash_is_automatically_derived || peq_is_automatically_derived {
                     return;
                 }
 
@@ -252,17 +252,11 @@ fn check_hash_peq<'tcx>(
                 // Only care about `impl PartialEq<Foo> for Foo`
                 // For `impl PartialEq<B> for A, input_types is [A, B]
                 if trait_ref.substs.type_at(1) == ty {
-                    let mess = if peq_is_automatically_derived {
-                        "you are implementing `Hash` explicitly but have derived `PartialEq`"
-                    } else {
-                        "you are deriving `Hash` but have implemented `PartialEq` explicitly"
-                    };
-
                     span_lint_and_then(
                         cx,
-                        DERIVE_HASH_XOR_EQ,
+                        DERIVED_HASH_WITH_MANUAL_EQ,
                         span,
-                        mess,
+                        "you are deriving `Hash` but have implemented `PartialEq` explicitly",
                         |diag| {
                             if let Some(local_def_id) = impl_id.as_local() {
                                 let hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_def_id);

clippy_lints/src/renamed_lints.rs

@@ -9,6 +9,7 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
     ("clippy::box_vec", "clippy::box_collection"),
     ("clippy::const_static_lifetime", "clippy::redundant_static_lifetimes"),
     ("clippy::cyclomatic_complexity", "clippy::cognitive_complexity"),
+    ("clippy::derive_hash_xor_eq", "clippy::derived_hash_with_manual_eq"),
     ("clippy::disallowed_method", "clippy::disallowed_methods"),
     ("clippy::disallowed_type", "clippy::disallowed_types"),
     ("clippy::eval_order_dependence", "clippy::mixed_read_write_in_expression"),

tests/ui/derive_hash_xor_eq.rs → tests/ui/derived_hash_with_manual_eq.rs

@@ -27,30 +27,13 @@ impl PartialEq<Baz> for Baz {
     }
 }
 
+// Implementing `Hash` with a derived `PartialEq` is fine. See #2627
+
 #[derive(PartialEq)]
 struct Bah;
 
 impl std::hash::Hash for Bah {
     fn hash<H: std::hash::Hasher>(&self, _: &mut H) {}
 }
 
-#[derive(PartialEq)]
-struct Foo2;
-
-trait Hash {}
-
-// We don't want to lint on user-defined traits called `Hash`
-impl Hash for Foo2 {}
-
-mod use_hash {
-    use std::hash::{Hash, Hasher};
-
-    #[derive(PartialEq)]
-    struct Foo3;
-
-    impl Hash for Foo3 {
-        fn hash<H: std::hash::Hasher>(&self, _: &mut H) {}
-    }
-}
-
 fn main() {}

tests/ui/derived_hash_with_manual_eq.stderr

@@ -0,0 +1,29 @@
+error: you are deriving `Hash` but have implemented `PartialEq` explicitly
+  --> $DIR/derived_hash_with_manual_eq.rs:12:10
+   |
+LL | #[derive(Hash)]
+   |          ^^^^
+   |
+note: `PartialEq` implemented here
+  --> $DIR/derived_hash_with_manual_eq.rs:15:1
+   |
+LL | impl PartialEq for Bar {
+   | ^^^^^^^^^^^^^^^^^^^^^^
+   = note: `#[deny(clippy::derived_hash_with_manual_eq)]` on by default
+   = note: this error originates in the derive macro `Hash` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: you are deriving `Hash` but have implemented `PartialEq` explicitly
+  --> $DIR/derived_hash_with_manual_eq.rs:21:10
+   |
+LL | #[derive(Hash)]
+   |          ^^^^
+   |
+note: `PartialEq` implemented here
+  --> $DIR/derived_hash_with_manual_eq.rs:24:1
+   |
+LL | impl PartialEq<Baz> for Baz {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: this error originates in the derive macro `Hash` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: aborting due to 2 previous errors
+

tests/ui/rename.fixed

@@ -10,6 +10,7 @@
 #![allow(clippy::box_collection)]
 #![allow(clippy::redundant_static_lifetimes)]
 #![allow(clippy::cognitive_complexity)]
+#![allow(clippy::derived_hash_with_manual_eq)]
 #![allow(clippy::disallowed_methods)]
 #![allow(clippy::disallowed_types)]
 #![allow(clippy::mixed_read_write_in_expression)]
@@ -45,6 +46,7 @@
 #![warn(clippy::box_collection)]
 #![warn(clippy::redundant_static_lifetimes)]
 #![warn(clippy::cognitive_complexity)]
+#![warn(clippy::derived_hash_with_manual_eq)]
 #![warn(clippy::disallowed_methods)]
 #![warn(clippy::disallowed_types)]
 #![warn(clippy::mixed_read_write_in_expression)]

tests/ui/rename.rs

@@ -10,6 +10,7 @@
 #![allow(clippy::box_collection)]
 #![allow(clippy::redundant_static_lifetimes)]
 #![allow(clippy::cognitive_complexity)]
+#![allow(clippy::derived_hash_with_manual_eq)]
 #![allow(clippy::disallowed_methods)]
 #![allow(clippy::disallowed_types)]
 #![allow(clippy::mixed_read_write_in_expression)]
@@ -45,6 +46,7 @@
 #![warn(clippy::box_vec)]
 #![warn(clippy::const_static_lifetime)]
 #![warn(clippy::cyclomatic_complexity)]
+#![warn(clippy::derive_hash_xor_eq)]
 #![warn(clippy::disallowed_method)]
 #![warn(clippy::disallowed_type)]
 #![warn(clippy::eval_order_dependence)]