Add a lint about deriving Hash and implementing PartialEq

3 files changed, 102 insertions, 0 deletions

diff --git a/src/derive.rs b/src/derive.rs
new file mode 100644
index 00000000000..6306bb8f09d
--- /dev/null
+++ b/src/derive.rs
@@ -0,0 +1,98 @@
+use rustc::lint::*;
+use rustc_front::hir::*;
+use syntax::ast::{Attribute, MetaItem_};
+use utils::{match_path, span_lint_and_then};
+use utils::HASH_PATH;
+
+use rustc::middle::ty::fast_reject::simplify_type;
+
+/// **What it does:** This lint warns about deriving `Hash` but implementing `PartialEq`
+/// explicitely.
+///
+/// **Why is this bad?** The implementation of these traits must agree (for example for use with
+/// `HashMap`) so it’s probably a bad idea to use a default-generated `Hash` implementation  with
+/// an explicitely defined `PartialEq`. In particular, the following must hold for any type:
+///
+/// ```rust
+/// k1 == k2 -> hash(k1) == hash(k2)
+/// ```
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// #[derive(Hash)]
+/// struct Foo;
+///
+/// impl PartialEq for Foo {
+///     ..
+/// }
+declare_lint! {
+    pub DERIVE_HASH_NOT_EQ,
+    Warn,
+    "deriving `Hash` but implementing `PartialEq` explicitly"
+}
+
+pub struct Derive;
+
+impl LintPass for Derive {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(DERIVE_HASH_NOT_EQ)
+    }
+}
+
+impl LateLintPass for Derive {
+    fn check_item(&mut self, cx: &LateContext, item: &Item) {
+        /// A `#[derive]`d implementation has a `#[automatically_derived]` attribute.
+        fn is_automatically_derived(attr: &Attribute) -> bool {
+            if let MetaItem_::MetaWord(ref word) = attr.node.value.node {
+                word == &"automatically_derived"
+            }
+            else {
+                false
+            }
+        }
+
+        // If `item` is an automatically derived `Hash` implementation
+        if_let_chain! {[
+            let ItemImpl(_, _, _, Some(ref trait_ref), ref ast_ty, _) = item.node,
+            match_path(&trait_ref.path, &HASH_PATH),
+            item.attrs.iter().any(is_automatically_derived),
+            let Some(peq_trait_def_id) = cx.tcx.lang_items.eq_trait()
+        ], {
+            let peq_trait_def = cx.tcx.lookup_trait_def(peq_trait_def_id);
+
+            cx.tcx.populate_implementations_for_trait_if_necessary(peq_trait_def.trait_ref.def_id);
+            let peq_impls = peq_trait_def.borrow_impl_lists(cx.tcx).1;
+            let ast_ty_to_ty_cache = cx.tcx.ast_ty_to_ty_cache.borrow();
+
+
+            // Look for the PartialEq implementations for `ty`
+            if_let_chain! {[
+                let Some(ty) = ast_ty_to_ty_cache.get(&ast_ty.id),
+                let Some(simpl_ty) = simplify_type(cx.tcx, ty, false),
+                let Some(impl_ids) = peq_impls.get(&simpl_ty)
+            ], {
+                for &impl_id in impl_ids {
+                    let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation");
+
+                    // Only care about `impl PartialEq<Foo> for Foo`
+                    if trait_ref.input_types()[0] == *ty &&
+                      !cx.tcx.get_attrs(impl_id).iter().any(is_automatically_derived) {
+                        span_lint_and_then(
+                            cx, DERIVE_HASH_NOT_EQ, item.span,
+                            &format!("you are deriving `Hash` but have implemented \
+                                      `PartialEq` explicitely"), |db| {
+                            if let Some(node_id) = cx.tcx.map.as_local_node_id(impl_id) {
+                                db.span_note(
+                                    cx.tcx.map.span(node_id),
+                                    "`PartialEq` implemented here"
+                                );
+                            }
+                        });
+                    }
+                }
+            }}
+        }}
+    }
+}
diff --git a/src/lib.rs b/src/lib.rs
index 1a4501ffce6..4a832cc7b89 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -75,6 +75,7 @@ pub mod entry;
 pub mod misc_early;
 pub mod array_indexing;
 pub mod panic;
+pub mod derive;
 
 mod reexport {
     pub use syntax::ast::{Name, NodeId};
@@ -136,6 +137,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box array_indexing::ArrayIndexing);
     reg.register_late_lint_pass(box panic::PanicPass);
     reg.register_late_lint_pass(box strings::StringLitAsBytes);
+    reg.register_late_lint_pass(box derive::Derive);
 
 
     reg.register_lint_group("clippy_pedantic", vec![
@@ -168,6 +170,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT,
         collapsible_if::COLLAPSIBLE_IF,
         cyclomatic_complexity::CYCLOMATIC_COMPLEXITY,
+        derive::DERIVE_HASH_NOT_EQ,
         entry::MAP_ENTRY,
         eq_op::EQ_OP,
         escape::BOXED_LOCAL,
diff --git a/src/utils.rs b/src/utils.rs
index 97d0d2ecf11..c5483997f90 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -27,6 +27,7 @@ pub const COW_PATH: [&'static str; 3] = ["collections", "borrow", "Cow"];
 pub const DEFAULT_TRAIT_PATH: [&'static str; 3] = ["core", "default", "Default"];
 pub const HASHMAP_ENTRY_PATH: [&'static str; 5] = ["std", "collections", "hash", "map", "Entry"];
 pub const HASHMAP_PATH: [&'static str; 5] = ["std", "collections", "hash", "map", "HashMap"];
+pub const HASH_PATH: [&'static str; 2] = ["hash", "Hash"];
 pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedList"];
 pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"];
 pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"];