Auto merge of #8315 - dswij:8306, r=giraffate

`trait_duplication_in_bounds` checks path segments for trait items

closes #8306

changelog: [`trait_duplication_in_bounds`] Fix FP when path segments exists for trait items

3 files changed, 68 insertions, 36 deletions

diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs
index 6369aafe3f9..5257f5302cd 100644
--- a/clippy_lints/src/trait_bounds.rs
+++ b/clippy_lints/src/trait_bounds.rs
@@ -3,7 +3,7 @@ use clippy_utils::source::{snippet, snippet_with_applicability};
 use clippy_utils::{SpanlessEq, SpanlessHash};
 use core::hash::{Hash, Hasher};
 use if_chain::if_chain;
-use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::unhash::UnhashMap;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
@@ -91,7 +91,7 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
 
     fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'tcx>) {
         let Generics { where_clause, .. } = &item.generics;
-        let mut self_bounds_set = FxHashSet::default();
+        let mut self_bounds_map = FxHashMap::default();
 
         for predicate in where_clause.predicates {
             if_chain! {
@@ -108,27 +108,29 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
                         )
                     ) = cx.tcx.hir().get_if_local(*def_id);
                 then {
-                    if self_bounds_set.is_empty() {
+                    if self_bounds_map.is_empty() {
                         for bound in self_bounds.iter() {
-                            let Some((self_res, _)) = get_trait_res_span_from_bound(bound) else { continue };
-                            self_bounds_set.insert(self_res);
+                            let Some((self_res, self_segments, _)) = get_trait_info_from_bound(bound) else { continue };
+                            self_bounds_map.insert(self_res, self_segments);
                         }
                     }
 
                     bound_predicate
                         .bounds
                         .iter()
-                        .filter_map(get_trait_res_span_from_bound)
-                        .for_each(|(trait_item_res, span)| {
-                            if self_bounds_set.get(&trait_item_res).is_some() {
-                                span_lint_and_help(
-                                    cx,
-                                    TRAIT_DUPLICATION_IN_BOUNDS,
-                                    span,
-                                    "this trait bound is already specified in trait declaration",
-                                    None,
-                                    "consider removing this trait bound",
-                                );
+                        .filter_map(get_trait_info_from_bound)
+                        .for_each(|(trait_item_res, trait_item_segments, span)| {
+                            if let Some(self_segments) = self_bounds_map.get(&trait_item_res) {
+                                if SpanlessEq::new(cx).eq_path_segments(self_segments, trait_item_segments) {
+                                    span_lint_and_help(
+                                        cx,
+                                        TRAIT_DUPLICATION_IN_BOUNDS,
+                                        span,
+                                        "this trait bound is already specified in trait declaration",
+                                        None,
+                                        "consider removing this trait bound",
+                                    );
+                                }
                             }
                         });
                 }
@@ -137,14 +139,6 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
     }
 }
 
-fn get_trait_res_span_from_bound(bound: &GenericBound<'_>) -> Option<(Res, Span)> {
-    if let GenericBound::Trait(t, _) = bound {
-        Some((t.trait_ref.path.res, t.span))
-    } else {
-        None
-    }
-}
-
 impl TraitBounds {
     fn check_type_repetition<'tcx>(self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) {
         struct SpanlessTy<'cx, 'tcx> {
@@ -231,7 +225,7 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
             let res = param
                 .bounds
                 .iter()
-                .filter_map(get_trait_res_span_from_bound)
+                .filter_map(get_trait_info_from_bound)
                 .collect::<Vec<_>>();
             map.insert(*ident, res);
         }
@@ -245,10 +239,10 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
             if let Some(segment) = segments.first();
             if let Some(trait_resolutions_direct) = map.get(&segment.ident);
             then {
-                for (res_where, _) in bound_predicate.bounds.iter().filter_map(get_trait_res_span_from_bound) {
-                    if let Some((_, span_direct)) = trait_resolutions_direct
+                for (res_where, _,  _) in bound_predicate.bounds.iter().filter_map(get_trait_info_from_bound) {
+                    if let Some((_, _, span_direct)) = trait_resolutions_direct
                                                 .iter()
-                                                .find(|(res_direct, _)| *res_direct == res_where) {
+                                                .find(|(res_direct, _, _)| *res_direct == res_where) {
                         span_lint_and_help(
                             cx,
                             TRAIT_DUPLICATION_IN_BOUNDS,
@@ -263,3 +257,11 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
         }
     }
 }
+
+fn get_trait_info_from_bound<'a>(bound: &'a GenericBound<'_>) -> Option<(Res, &'a [PathSegment<'a>], Span)> {
+    if let GenericBound::Trait(t, _) = bound {
+        Some((t.trait_ref.path.res, t.trait_ref.path.segments, t.span))
+    } else {
+        None
+    }
+}
diff --git a/tests/ui/trait_duplication_in_bounds.rs b/tests/ui/trait_duplication_in_bounds.rs
index 2edb202892a..21de19a2601 100644
--- a/tests/ui/trait_duplication_in_bounds.rs
+++ b/tests/ui/trait_duplication_in_bounds.rs
@@ -1,5 +1,6 @@
 #![deny(clippy::trait_duplication_in_bounds)]
 
+use std::collections::BTreeMap;
 use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};
 
 fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
@@ -73,4 +74,25 @@ impl U for Life {
     fn f() {}
 }
 
+// should not warn
+trait Iter: Iterator {
+    fn into_group_btreemap<K, V>(self) -> BTreeMap<K, Vec<V>>
+    where
+        Self: Iterator<Item = (K, V)> + Sized,
+        K: Ord + Eq,
+    {
+        unimplemented!();
+    }
+}
+
+struct Foo {}
+
+trait FooIter: Iterator<Item = Foo> {
+    fn bar()
+    where
+        Self: Iterator<Item = Foo>,
+    {
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/trait_duplication_in_bounds.stderr b/tests/ui/trait_duplication_in_bounds.stderr
index e0c7a7ec618..6f8c8e47dfb 100644
--- a/tests/ui/trait_duplication_in_bounds.stderr
+++ b/tests/ui/trait_duplication_in_bounds.stderr
@@ -1,5 +1,5 @@
 error: this trait bound is already specified in the where clause
-  --> $DIR/trait_duplication_in_bounds.rs:5:15
+  --> $DIR/trait_duplication_in_bounds.rs:6:15
    |
 LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
    |               ^^^^^
@@ -12,7 +12,7 @@ LL | #![deny(clippy::trait_duplication_in_bounds)]
    = help: consider removing this trait bound
 
 error: this trait bound is already specified in the where clause
-  --> $DIR/trait_duplication_in_bounds.rs:5:23
+  --> $DIR/trait_duplication_in_bounds.rs:6:23
    |
 LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
    |                       ^^^^^^^
@@ -20,7 +20,7 @@ LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
    = help: consider removing this trait bound
 
 error: this trait bound is already specified in trait declaration
-  --> $DIR/trait_duplication_in_bounds.rs:34:15
+  --> $DIR/trait_duplication_in_bounds.rs:35:15
    |
 LL |         Self: Default;
    |               ^^^^^^^
@@ -28,7 +28,7 @@ LL |         Self: Default;
    = help: consider removing this trait bound
 
 error: this trait bound is already specified in trait declaration
-  --> $DIR/trait_duplication_in_bounds.rs:48:15
+  --> $DIR/trait_duplication_in_bounds.rs:49:15
    |
 LL |         Self: Default + Clone;
    |               ^^^^^^^
@@ -36,7 +36,7 @@ LL |         Self: Default + Clone;
    = help: consider removing this trait bound
 
 error: this trait bound is already specified in trait declaration
-  --> $DIR/trait_duplication_in_bounds.rs:54:15
+  --> $DIR/trait_duplication_in_bounds.rs:55:15
    |
 LL |         Self: Default + Clone;
    |               ^^^^^^^
@@ -44,7 +44,7 @@ LL |         Self: Default + Clone;
    = help: consider removing this trait bound
 
 error: this trait bound is already specified in trait declaration
-  --> $DIR/trait_duplication_in_bounds.rs:54:25
+  --> $DIR/trait_duplication_in_bounds.rs:55:25
    |
 LL |         Self: Default + Clone;
    |                         ^^^^^
@@ -52,12 +52,20 @@ LL |         Self: Default + Clone;
    = help: consider removing this trait bound
 
 error: this trait bound is already specified in trait declaration
-  --> $DIR/trait_duplication_in_bounds.rs:57:15
+  --> $DIR/trait_duplication_in_bounds.rs:58:15
    |
 LL |         Self: Default;
    |               ^^^^^^^
    |
    = help: consider removing this trait bound
 
-error: aborting due to 7 previous errors
+error: this trait bound is already specified in trait declaration
+  --> $DIR/trait_duplication_in_bounds.rs:93:15
+   |
+LL |         Self: Iterator<Item = Foo>,
+   |               ^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: aborting due to 8 previous errors