diff --git a/clippy_lints/src/inherent_impl.rs b/clippy_lints/src/inherent_impl.rs index c31013e49be5..23d40c0e9ac1 100644 --- a/clippy_lints/src/inherent_impl.rs +++ b/clippy_lints/src/inherent_impl.rs @@ -2,8 +2,9 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::in_macro; -use rustc_hir::def_id::DefIdMap; -use rustc_hir::{def_id, Crate, Impl, Item, ItemKind}; +use indexmap::map::Entry; +use rustc_data_structures::fx::FxIndexMap; +use rustc_hir::{def_id::LocalDefId, Crate, Impl, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Span; @@ -43,7 +44,7 @@ declare_clippy_lint! { #[allow(clippy::module_name_repetitions)] #[derive(Default)] pub struct MultipleInherentImpl { - impls: DefIdMap, + impls: Vec<(LocalDefId, Span)>, } impl_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]); @@ -56,33 +57,41 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl { .. }) = item.kind { - // Remember for each inherent implementation encountered its span and generics + // Remember for each inherent implementation encountered its id and span, // but filter out implementations that have generic params (type or lifetime) // or are derived from a macro if !in_macro(item.span) && generics.params.is_empty() { - self.impls.insert(item.def_id.to_def_id(), item.span); + self.impls.push((item.def_id, item.span)); } } } fn check_crate_post(&mut self, cx: &LateContext<'tcx>, krate: &'tcx Crate<'_>) { if !krate.items.is_empty() { - // Retrieve all inherent implementations from the crate, grouped by type - for impls in cx.tcx.crate_inherent_impls(def_id::LOCAL_CRATE).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)); - if let Some(initial_span) = impl_spans.next() { - impl_spans.for_each(|additional_span| { + // Split the collected impls by type + let mut type_map = FxIndexMap::default(); + for (ty, span) in self.impls.iter().map(|&(did, span)| (cx.tcx.type_of(did), span)) { + match type_map.entry(ty) { + Entry::Vacant(e) => { + e.insert(vec![span]); + }, + Entry::Occupied(mut e) => e.get_mut().push(span), + } + } + + for spans in type_map.values() { + if let Some((&initial_span, spans)) = spans.split_first() { + for &additional_span in spans { span_lint_and_then( cx, MULTIPLE_INHERENT_IMPL, - *additional_span, + additional_span, "multiple implementations of this structure", |diag| { - diag.span_note(*initial_span, "first implementation here"); + diag.span_note(initial_span, "first implementation here"); }, ) - }) + } } } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 78a95b004034..b34885bb168f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -20,6 +20,7 @@ // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) +extern crate indexmap; extern crate rustc_ast; extern crate rustc_ast_pretty; extern crate rustc_data_structures; diff --git a/tests/ui/impl.rs b/tests/ui/impl.rs index 1c46e3a53378..db49af5c0c15 100644 --- a/tests/ui/impl.rs +++ b/tests/ui/impl.rs @@ -33,4 +33,16 @@ impl fmt::Debug for MyStruct { } } +// issue #5772 +struct WithArgs(T); +impl WithArgs { + fn f1() {} +} +impl WithArgs { + fn f2() {} +} +impl WithArgs { + fn f3() {} +} + fn main() {} diff --git a/tests/ui/impl.stderr b/tests/ui/impl.stderr index aab688cc2d8b..1ca50957ae6c 100644 --- a/tests/ui/impl.stderr +++ b/tests/ui/impl.stderr @@ -31,5 +31,21 @@ LL | | fn first() {} LL | | } | |_^ -error: aborting due to 2 previous errors +error: multiple implementations of this structure + --> $DIR/impl.rs:44:1 + | +LL | / impl WithArgs { +LL | | fn f3() {} +LL | | } + | |_^ + | +note: first implementation here + --> $DIR/impl.rs:41:1 + | +LL | / impl WithArgs { +LL | | fn f2() {} +LL | | } + | |_^ + +error: aborting due to 3 previous errors