diff --git a/clippy_lints/src/inherent_impl.rs b/clippy_lints/src/inherent_impl.rs new file mode 100644 index 00000000000..fb01f46949d --- /dev/null +++ b/clippy_lints/src/inherent_impl.rs @@ -0,0 +1,95 @@ +//! lint on inherent implementations + +use rustc::hir::*; +use rustc::lint::*; +use std::collections::HashMap; +use std::default::Default; +use syntax_pos::Span; + +/// **What it does:** Checks for multiple inherent implementations of a struct +/// +/// **Why is this bad?** Splitting the implementation of a type makes the code harder to navigate. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// struct X; +/// impl X { +/// fn one() {} +/// } +/// impl X { +/// fn other() {} +/// } +/// ``` +/// +/// Could be written: +/// +/// ```rust +/// struct X; +/// impl X { +/// fn one() {} +/// fn other() {} +/// } +/// ``` +declare_clippy_lint! { + pub MULTIPLE_INHERENT_IMPL, + pedantic, + "Multiple inherent impl that could be grouped" +} + +pub struct Pass { + impls: HashMap, +} + +impl Default for Pass { + fn default() -> Self { + Pass { impls: HashMap::new() } + } +} + +impl LintPass for Pass { + fn get_lints(&self) -> LintArray { + lint_array!(MULTIPLE_INHERENT_IMPL) + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { + fn check_item(&mut self, _: &LateContext<'a, 'tcx>, item: &'tcx Item) { + if let Item_::ItemImpl(_, _, _, ref generics, None, _, _) = item.node { + // Remember for each inherent implementation encoutered its span and generics + self.impls + .insert(item.hir_id.owner_def_id(), (item.span, generics.clone())); + } + } + + fn check_crate_post(&mut self, cx: &LateContext<'a, 'tcx>, krate: &'tcx Crate) { + if let Some(item) = krate.items.values().nth(0) { + // Retrieve all inherent implementations from the crate, grouped by type + for impls in cx + .tcx + .crate_inherent_impls(item.hir_id.owner_def_id().krate) + .inherent_impls + .values() + { + // Filter out implementations that have generic params (type or lifetime) + let mut impl_spans = impls + .iter() + .filter_map(|impl_def| self.impls.get(impl_def)) + .filter(|(_, generics)| generics.params.len() == 0) + .map(|(span, _)| span); + if let Some(initial_span) = impl_spans.nth(0) { + impl_spans.for_each(|additional_span| { + cx.span_lint_note( + MULTIPLE_INHERENT_IMPL, + *additional_span, + "Multiple implementations of this structure", + *initial_span, + "First implementation here", + ) + }) + } + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 01bfdc28c95..3b00e4fba48 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -138,6 +138,7 @@ macro_rules! declare_clippy_lint { pub mod if_not_else; pub mod infallible_destructuring_match; pub mod infinite_iter; +pub mod inherent_impl; pub mod inline_fn_without_body; pub mod int_plus_one; pub mod invalid_ref; @@ -416,6 +417,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_early_lint_pass(box multiple_crate_versions::Pass); reg.register_late_lint_pass(box map_unit_fn::Pass); reg.register_late_lint_pass(box infallible_destructuring_match::Pass); + reg.register_late_lint_pass(box inherent_impl::Pass::default()); reg.register_lint_group("clippy_restriction", vec![ @@ -452,6 +454,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { enum_variants::STUTTER, if_not_else::IF_NOT_ELSE, infinite_iter::MAYBE_INFINITE_ITER, + inherent_impl::MULTIPLE_INHERENT_IMPL, items_after_statements::ITEMS_AFTER_STATEMENTS, matches::SINGLE_MATCH_ELSE, methods::FILTER_MAP, diff --git a/tests/ui/impl.rs b/tests/ui/impl.rs new file mode 100644 index 00000000000..9e10dbade4e --- /dev/null +++ b/tests/ui/impl.rs @@ -0,0 +1,36 @@ +#![allow(dead_code)] +#![warn(multiple_inherent_impl)] + +struct MyStruct; + +impl MyStruct { + fn first() {} +} + +impl MyStruct { + fn second() {} +} + +impl<'a> MyStruct { + fn lifetimed() {} +} + +mod submod { + struct MyStruct; + impl MyStruct { + fn other() {} + } + + impl super::MyStruct { + fn third() {} + } +} + +use std::fmt; +impl fmt::Debug for MyStruct { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "MyStruct {{ }}") + } +} + +fn main() {} diff --git a/tests/ui/impl.stderr b/tests/ui/impl.stderr new file mode 100644 index 00000000000..95e627cd509 --- /dev/null +++ b/tests/ui/impl.stderr @@ -0,0 +1,35 @@ +error: Multiple implementations of this structure + --> $DIR/impl.rs:10:1 + | +10 | / impl MyStruct { +11 | | fn second() {} +12 | | } + | |_^ + | + = note: `-D multiple-inherent-impl` implied by `-D warnings` +note: First implementation here + --> $DIR/impl.rs:6:1 + | +6 | / impl MyStruct { +7 | | fn first() {} +8 | | } + | |_^ + +error: Multiple implementations of this structure + --> $DIR/impl.rs:24:5 + | +24 | / impl super::MyStruct { +25 | | fn third() {} +26 | | } + | |_____^ + | +note: First implementation here + --> $DIR/impl.rs:6:1 + | +6 | / impl MyStruct { +7 | | fn first() {} +8 | | } + | |_^ + +error: aborting due to 2 previous errors +