From 8664d03ec6ed7704d87f780d164c3b83ccada541 Mon Sep 17 00:00:00 2001 From: Sebastian Ullrich Date: Sat, 29 Oct 2016 21:33:57 -0400 Subject: [PATCH] implement 'Re-implementing `PartialEq::ne`' lint closes #86 --- CHANGELOG.md | 1 + README.md | 3 +- clippy_lints/src/lib.rs | 3 ++ clippy_lints/src/partialeq_ne_impl.rs | 55 +++++++++++++++++++++++++ tests/compile-fail/partialeq_ne_impl.rs | 15 +++++++ 5 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/partialeq_ne_impl.rs create mode 100644 tests/compile-fail/partialeq_ne_impl.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c08ab71c479..07276a28abd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -319,6 +319,7 @@ All notable changes to this project will be documented in this file. [`out_of_bounds_indexing`]: https://github.com/Manishearth/rust-clippy/wiki#out_of_bounds_indexing [`overflow_check_conditional`]: https://github.com/Manishearth/rust-clippy/wiki#overflow_check_conditional [`panic_params`]: https://github.com/Manishearth/rust-clippy/wiki#panic_params +[`partialeq_ne_impl`]: https://github.com/Manishearth/rust-clippy/wiki#partialeq_ne_impl [`precedence`]: https://github.com/Manishearth/rust-clippy/wiki#precedence [`print_stdout`]: https://github.com/Manishearth/rust-clippy/wiki#print_stdout [`print_with_newline`]: https://github.com/Manishearth/rust-clippy/wiki#print_with_newline diff --git a/README.md b/README.md index 4223ee2b017..61561167cee 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,7 @@ You can check out this great service at [clippy.bashy.io](https://clippy.bashy.i ## Lints -There are 175 lints included in this crate: +There are 176 lints included in this crate: name | default | triggers on -----------------------------------------------------------------------------------------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------- @@ -293,6 +293,7 @@ name [out_of_bounds_indexing](https://github.com/Manishearth/rust-clippy/wiki#out_of_bounds_indexing) | deny | out of bounds constant indexing [overflow_check_conditional](https://github.com/Manishearth/rust-clippy/wiki#overflow_check_conditional) | warn | overflow checks inspired by C which are likely to panic [panic_params](https://github.com/Manishearth/rust-clippy/wiki#panic_params) | warn | missing parameters in `panic!` calls +[partialeq_ne_impl](https://github.com/Manishearth/rust-clippy/wiki#partialeq_ne_impl) | warn | re-implementing `PartialEq::ne` [precedence](https://github.com/Manishearth/rust-clippy/wiki#precedence) | warn | operations where precedence may be unclear [print_stdout](https://github.com/Manishearth/rust-clippy/wiki#print_stdout) | allow | printing on stdout [print_with_newline](https://github.com/Manishearth/rust-clippy/wiki#print_with_newline) | warn | using `print!()` with a format string that ends in a newline diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e14815f5379..3f8385feb88 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -111,6 +111,7 @@ pub mod ok_if_let; pub mod open_options; pub mod overflow_check_conditional; pub mod panic; +pub mod partialeq_ne_impl; pub mod precedence; pub mod print; pub mod ptr; @@ -259,6 +260,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box missing_doc::MissingDoc::new()); reg.register_late_lint_pass(box ok_if_let::Pass); reg.register_late_lint_pass(box if_let_redundant_pattern_matching::Pass); + reg.register_late_lint_pass(box partialeq_ne_impl::Pass); reg.register_lint_group("clippy_restrictions", vec![ arithmetic::FLOAT_ARITHMETIC, @@ -415,6 +417,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { open_options::NONSENSICAL_OPEN_OPTIONS, overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, panic::PANIC_PARAMS, + partialeq_ne_impl::PARTIALEQ_NE_IMPL, precedence::PRECEDENCE, print::PRINT_WITH_NEWLINE, ptr::CMP_NULL, diff --git a/clippy_lints/src/partialeq_ne_impl.rs b/clippy_lints/src/partialeq_ne_impl.rs new file mode 100644 index 00000000000..fdee5c256e1 --- /dev/null +++ b/clippy_lints/src/partialeq_ne_impl.rs @@ -0,0 +1,55 @@ +use rustc::lint::*; +use rustc::hir::*; +use utils::{is_automatically_derived, span_lint}; + +/// **What it does:** Checks for manual re-implementations of `PartialEq::ne`. +/// +/// **Why is this bad?** `PartialEq::ne` is required to always return the +/// negated result of `PartialEq::eq`, which is exactly what the default +/// implementation does. Therefore, there should never be any need to +/// re-implement it. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// struct Foo; +/// +/// impl PartialEq for Foo { +/// fn eq(&self, other: &Foo) -> bool { ... } +/// fn ne(&self, other: &Foo) -> bool { !(self == other) } +/// } +/// ``` +declare_lint! { + pub PARTIALEQ_NE_IMPL, + Warn, + "re-implementing `PartialEq::ne`" +} + +#[derive(Clone, Copy)] +pub struct Pass; + +impl LintPass for Pass { + fn get_lints(&self) -> LintArray { + lint_array!(PARTIALEQ_NE_IMPL) + } +} + +impl LateLintPass for Pass { + fn check_item(&mut self, cx: &LateContext, item: &Item) { + if_let_chain! {[ + let ItemImpl(_, _, _, Some(ref trait_ref), _, ref impl_items) = item.node, + !is_automatically_derived(&*item.attrs), + cx.tcx.expect_def(trait_ref.ref_id).def_id() == cx.tcx.lang_items.eq_trait().unwrap(), + ], { + for impl_item in impl_items { + if &*impl_item.name.as_str() == "ne" { + span_lint(cx, + PARTIALEQ_NE_IMPL, + impl_item.span, + "re-implementing `PartialEq::ne` is unnecessary") + } + } + }}; + } +} diff --git a/tests/compile-fail/partialeq_ne_impl.rs b/tests/compile-fail/partialeq_ne_impl.rs new file mode 100644 index 00000000000..f4ecdd4261a --- /dev/null +++ b/tests/compile-fail/partialeq_ne_impl.rs @@ -0,0 +1,15 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![deny(warnings)] +#![allow(dead_code)] + +struct Foo; + +impl PartialEq for Foo { + fn eq(&self, _: &Foo) -> bool { true } + fn ne(&self, _: &Foo) -> bool { false } + //~^ ERROR re-implementing `PartialEq::ne` is unnecessary +} + +fn main() {}