diff options
| -rw-r--r-- | README.md | 3 | ||||
| -rw-r--r-- | src/derive.rs | 98 | ||||
| -rw-r--r-- | src/lib.rs | 3 | ||||
| -rw-r--r-- | src/utils.rs | 1 | ||||
| -rwxr-xr-x | tests/compile-fail/derive.rs | 29 |
5 files changed, 133 insertions, 1 deletions
diff --git a/README.md b/README.md index ba257ce43a1..c45acf4af77 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 95 lints included in this crate: +There are 96 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -26,6 +26,7 @@ name [collapsible_if](https://github.com/Manishearth/rust-clippy/wiki#collapsible_if) | warn | two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }` [cyclomatic_complexity](https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity) | warn | finds functions that should be split up into multiple functions [deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver) | warn | `Warn` on `#[deprecated(since = "x")]` where x is not semver +[derive_hash_not_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_not_eq) | warn | deriving `Hash` but implementing `PartialEq` explicitly [duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | Function arguments having names which only differ by an underscore [empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected [eq_op](https://github.com/Manishearth/rust-clippy/wiki#eq_op) | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`) 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"]; diff --git a/tests/compile-fail/derive.rs b/tests/compile-fail/derive.rs new file mode 100755 index 00000000000..a879be28292 --- /dev/null +++ b/tests/compile-fail/derive.rs @@ -0,0 +1,29 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![deny(warnings)] + +#[derive(PartialEq, Hash)] +struct Foo; + +impl PartialEq<u64> for Foo { + fn eq(&self, _: &u64) -> bool { true } +} + +#[derive(Hash)] +//~^ ERROR you are deriving `Hash` but have implemented `PartialEq` explicitely +struct Bar; + +impl PartialEq for Bar { + fn eq(&self, _: &Bar) -> bool { true } +} + +#[derive(Hash)] +//~^ ERROR you are deriving `Hash` but have implemented `PartialEq` explicitely +struct Baz; + +impl PartialEq<Baz> for Baz { + fn eq(&self, _: &Baz) -> bool { true } +} + +fn main() {} |