Fix multiple_inherent_impl false negatives for generic impl blocks (#16284)

lint now correctly detects multiple impl blocks for generic types with
identical bounds. Fixes:
#16283

changelog: [`multiple_inherent_impl`]: fix false negatives for generic
impl blocks

Author: dswij

clippy_lints/src/inherent_impl.rs

@@ -101,7 +101,21 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
                     InherentImplLintScope::Crate => Criterion::Crate,
                 };
                 let is_test = is_cfg_test(cx.tcx, hir_id) || is_in_cfg_test(cx.tcx, hir_id);
-                match type_map.entry((impl_ty, criterion, is_test)) {
+                let predicates = {
+                    // Gets the predicates (bounds) for the given impl block,
+                    // sorted for consistent comparison to allow distinguishing between impl blocks
+                    // with different generic bounds.
+                    let mut predicates = cx
+                        .tcx
+                        .predicates_of(impl_id)
+                        .predicates
+                        .iter()
+                        .map(|(clause, _)| *clause)
+                        .collect::<Vec<_>>();
+                    predicates.sort_by_key(|c| format!("{c:?}"));
+                    predicates
+                };
+                match type_map.entry((impl_ty, predicates, criterion, is_test)) {
                     Entry::Vacant(e) => {
                         // Store the id for the first impl block of this type. The span is retrieved lazily.
                         e.insert(IdOrSpan::Id(impl_id));
@@ -152,15 +166,12 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
 fn get_impl_span(cx: &LateContext<'_>, id: LocalDefId) -> Option<Span> {
     let id = cx.tcx.local_def_id_to_hir_id(id);
     if let Node::Item(&Item {
-        kind: ItemKind::Impl(impl_item),
+        kind: ItemKind::Impl(_),
         span,
         ..
     }) = cx.tcx.hir_node(id)
     {
-        (!span.from_expansion()
-            && impl_item.generics.params.is_empty()
-            && !fulfill_or_allowed(cx, MULTIPLE_INHERENT_IMPL, [id]))
-        .then_some(span)
+        (!span.from_expansion() && !fulfill_or_allowed(cx, MULTIPLE_INHERENT_IMPL, [id])).then_some(span)
     } else {
         None
     }

tests/ui/impl.rs

@@ -14,6 +14,7 @@ impl MyStruct {
 }
 
 impl<'a> MyStruct {
+    //~^ multiple_inherent_impl
     fn lifetimed() {}
 }
 
@@ -90,10 +91,12 @@ struct Lifetime<'s> {
 }
 
 impl Lifetime<'_> {}
-impl Lifetime<'_> {} // false negative
+impl Lifetime<'_> {}
+//~^ multiple_inherent_impl
 
 impl<'a> Lifetime<'a> {}
-impl<'a> Lifetime<'a> {} // false negative
+impl<'a> Lifetime<'a> {}
+//~^ multiple_inherent_impl
 
 impl<'b> Lifetime<'b> {} // false negative?
 
@@ -104,6 +107,39 @@ struct Generic<G> {
 }
 
 impl<G> Generic<G> {}
-impl<G> Generic<G> {} // false negative
+impl<G> Generic<G> {}
+//~^ multiple_inherent_impl
+
+use std::fmt::Debug;
+
+#[derive(Debug)]
+struct GenericWithBounds<T: Debug>(T);
+
+impl<T: Debug> GenericWithBounds<T> {
+    fn make_one(_one: T) -> Self {
+        todo!()
+    }
+}
+
+impl<T: Debug> GenericWithBounds<T> {
+    //~^ multiple_inherent_impl
+    fn make_two(_two: T) -> Self {
+        todo!()
+    }
+}
+
+struct MultipleTraitBounds<T>(T);
+
+impl<T: Debug> MultipleTraitBounds<T> {
+    fn debug_fn() {}
+}
+
+impl<T: Clone> MultipleTraitBounds<T> {
+    fn clone_fn() {}
+}
+
+impl<T: Debug + Clone> MultipleTraitBounds<T> {
+    fn debug_clone_fn() {}
+}
 
 fn main() {}

tests/ui/impl.stderr

@@ -19,7 +19,24 @@ LL | | }
    = help: to override `-D warnings` add `#[allow(clippy::multiple_inherent_impl)]`
 
 error: multiple implementations of this structure
-  --> tests/ui/impl.rs:26:5
+  --> tests/ui/impl.rs:16:1
+   |
+LL | / impl<'a> MyStruct {
+LL | |
+LL | |     fn lifetimed() {}
+LL | | }
+   | |_^
+   |
+note: first implementation here
+  --> tests/ui/impl.rs:6:1
+   |
+LL | / impl MyStruct {
+LL | |     fn first() {}
+LL | | }
+   | |_^
+
+error: multiple implementations of this structure
+  --> tests/ui/impl.rs:27:5
    |
 LL | /     impl super::MyStruct {
 LL | |
@@ -37,7 +54,7 @@ LL | | }
    | |_^
 
 error: multiple implementations of this structure
-  --> tests/ui/impl.rs:48:1
+  --> tests/ui/impl.rs:49:1
    |
 LL | / impl WithArgs<u64> {
 LL | |
@@ -47,36 +64,93 @@ LL | | }
    | |_^
    |
 note: first implementation here
-  --> tests/ui/impl.rs:45:1
+  --> tests/ui/impl.rs:46:1
    |
 LL | / impl WithArgs<u64> {
 LL | |     fn f2() {}
 LL | | }
    | |_^
 
 error: multiple implementations of this structure
-  --> tests/ui/impl.rs:71:1
+  --> tests/ui/impl.rs:72:1
    |
 LL | impl OneAllowedImpl {}
    | ^^^^^^^^^^^^^^^^^^^^^^
    |
 note: first implementation here
-  --> tests/ui/impl.rs:68:1
+  --> tests/ui/impl.rs:69:1
    |
 LL | impl OneAllowedImpl {}
    | ^^^^^^^^^^^^^^^^^^^^^^
 
 error: multiple implementations of this structure
-  --> tests/ui/impl.rs:84:1
+  --> tests/ui/impl.rs:85:1
    |
 LL | impl OneExpected {}
    | ^^^^^^^^^^^^^^^^^^^
    |
 note: first implementation here
-  --> tests/ui/impl.rs:81:1
+  --> tests/ui/impl.rs:82:1
    |
 LL | impl OneExpected {}
    | ^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 5 previous errors
+error: multiple implementations of this structure
+  --> tests/ui/impl.rs:94:1
+   |
+LL | impl Lifetime<'_> {}
+   | ^^^^^^^^^^^^^^^^^^^^
+   |
+note: first implementation here
+  --> tests/ui/impl.rs:93:1
+   |
+LL | impl Lifetime<'_> {}
+   | ^^^^^^^^^^^^^^^^^^^^
+
+error: multiple implementations of this structure
+  --> tests/ui/impl.rs:98:1
+   |
+LL | impl<'a> Lifetime<'a> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: first implementation here
+  --> tests/ui/impl.rs:97:1
+   |
+LL | impl<'a> Lifetime<'a> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: multiple implementations of this structure
+  --> tests/ui/impl.rs:110:1
+   |
+LL | impl<G> Generic<G> {}
+   | ^^^^^^^^^^^^^^^^^^^^^
+   |
+note: first implementation here
+  --> tests/ui/impl.rs:109:1
+   |
+LL | impl<G> Generic<G> {}
+   | ^^^^^^^^^^^^^^^^^^^^^
+
+error: multiple implementations of this structure
+  --> tests/ui/impl.rs:124:1
+   |
+LL | / impl<T: Debug> GenericWithBounds<T> {
+LL | |
+LL | |     fn make_two(_two: T) -> Self {
+LL | |         todo!()
+LL | |     }
+LL | | }
+   | |_^
+   |
+note: first implementation here
+  --> tests/ui/impl.rs:118:1
+   |
+LL | / impl<T: Debug> GenericWithBounds<T> {
+LL | |     fn make_one(_one: T) -> Self {
+LL | |         todo!()
+LL | |     }
+LL | | }
+   | |_^
+
+error: aborting due to 10 previous errors