Treat different generic arguments as different types in `multiple_inh…

…erent_impl`
rust-lang · Apr 25, 2021 · d962f37 · d962f37
1 parent b1c675f
commit d962f37
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 15 deletions.
diff --git a/clippy_lints/src/inherent_impl.rs b/clippy_lints/src/inherent_impl.rs
@@ -2,11 +2,12 @@
 
 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 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;
+use indexmap::map::Entry;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for multiple inherent implementations of a struct
@@ -43,7 +44,7 @@ declare_clippy_lint! {
 #[allow(clippy::module_name_repetitions)]
 #[derive(Default)]
 pub struct MultipleInherentImpl {
-    impls: DefIdMap<Span>,
+    impls: Vec<(LocalDefId, Span)>,
 }
 
 impl_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]);
@@ -56,33 +57,39 @@ 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
@@ -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
@@ -33,4 +33,16 @@ impl fmt::Debug for MyStruct {
     }
 }
 
+// issue #5772
+struct WithArgs<T>(T);
+impl WithArgs<u32> {
+    fn f1() {}
+}
+impl WithArgs<u64> {
+    fn f2() {}
+}
+impl WithArgs<u64> {
+    fn f3() {}
+}
+
 fn main() {}
diff --git 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<u64> {
+LL | |     fn f3() {}
+LL | | }
+   | |_^
+   |
+note: first implementation here
+  --> $DIR/impl.rs:41:1
+   |
+LL | / impl WithArgs<u64> {
+LL | |     fn f2() {}
+LL | | }
+   | |_^
+
+error: aborting due to 3 previous errors