Compare where predicates to trait bounds.

- only compare where predicates to trait bounds when generating where
  clause specific message to fix #9151
- use comparable_trait_ref to account for trait bound generics to fix #8757

Author: aldhsu

clippy_lints/src/trait_bounds.rs

@@ -4,7 +4,7 @@ use clippy_utils::{SpanlessEq, SpanlessHash};
 use core::hash::{Hash, Hasher};
 use if_chain::if_chain;
 use itertools::Itertools;
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::unhash::UnhashMap;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
@@ -238,31 +238,58 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
         return;
     }
 
-    let mut map = FxHashMap::<_, Vec<_>>::default();
-    for predicate in gen.predicates {
+    // Explanation:
+    // fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
+    // where T: Clone + Default, { unimplemented!(); }
+    //       ^^^^^^^^^^^^^^^^^^
+    //       |
+    // collects each of these where clauses into a set keyed by generic name and comparable trait
+    // eg. (T, Clone)
+    let where_predicates = gen
+        .predicates
+        .iter()
+        .filter_map(|pred| {
+            if_chain! {
+                if pred.in_where_clause();
+                if let WherePredicate::BoundPredicate(bound_predicate) = pred;
+                if let TyKind::Path(QPath::Resolved(_, path)) =  bound_predicate.bounded_ty.kind;
+                then {
+                    return Some(bound_predicate.bounds.iter().filter_map(|t| {
+                        Some((path.res, into_comparable_trait_ref(t.trait_ref()?)))
+                    }))
+                }
+            }
+            None
+        })
+        .flatten()
+        .collect::<FxHashSet<_>>();
+
+    // Explanation:
+    // fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z) ...
+    //            ^^^^^^^^^^^^^^^^^^  ^^^^^^^
+    //            |
+    // compare trait bounds keyed by generic name and comparable trait to collected where
+    // predicates eg. (T, Clone)
+    for predicate in gen.predicates.iter().filter(|pred| !pred.in_where_clause()) {
         if_chain! {
-            if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate;
+            if let WherePredicate::BoundPredicate(bound_predicate) = predicate;
             if bound_predicate.origin != PredicateOrigin::ImplTrait;
             if !bound_predicate.span.from_expansion();
-            if let TyKind::Path(QPath::Resolved(_, Path { segments, .. })) = bound_predicate.bounded_ty.kind;
-            if let Some(segment) = segments.first();
+            if let TyKind::Path(QPath::Resolved(_, path)) =  bound_predicate.bounded_ty.kind;
             then {
-                for (res_where, _, span_where) in bound_predicate.bounds.iter().filter_map(get_trait_info_from_bound) {
-                    let trait_resolutions_direct = map.entry(segment.ident).or_default();
-                    if let Some((_, span_direct)) = trait_resolutions_direct
-                                                .iter()
-                                                .find(|(res_direct, _)| *res_direct == res_where) {
-                        span_lint_and_help(
-                            cx,
-                            TRAIT_DUPLICATION_IN_BOUNDS,
-                            *span_direct,
-                            "this trait bound is already specified in the where clause",
-                            None,
-                            "consider removing this trait bound",
-                        );
-                    }
-                    else {
-                        trait_resolutions_direct.push((res_where, span_where));
+                for t in bound_predicate.bounds {
+                    if let Some(trait_ref) = t.trait_ref() {
+                        let key = (path.res, into_comparable_trait_ref(trait_ref));
+                        if where_predicates.contains(&key) {
+                            span_lint_and_help(
+                                cx,
+                                TRAIT_DUPLICATION_IN_BOUNDS,
+                                t.span(),
+                                "this trait bound is already specified in the where clause",
+                                None,
+                                "consider removing this trait bound",
+                                );
+                        }
                     }
                 }
             }

tests/ui/trait_duplication_in_bounds.stderr

@@ -67,28 +67,12 @@ LL |         Self: Iterator<Item = Foo>,
    |
    = help: consider removing this trait bound
 
-error: this trait bound is already specified in the where clause
-  --> $DIR/trait_duplication_in_bounds.rs:103:19
-   |
-LL |     fn bad_foo<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
-   |                   ^^^^^
-   |
-   = help: consider removing this trait bound
-
 error: these bounds contain repeated elements
   --> $DIR/trait_duplication_in_bounds.rs:103:19
    |
 LL |     fn bad_foo<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
 
-error: this trait bound is already specified in the where clause
-  --> $DIR/trait_duplication_in_bounds.rs:109:12
-   |
-LL |         T: Clone + Clone + Clone + Copy,
-   |            ^^^^^
-   |
-   = help: consider removing this trait bound
-
 error: these where clauses contain repeated elements
   --> $DIR/trait_duplication_in_bounds.rs:109:12
    |
@@ -107,14 +91,6 @@ error: these where clauses contain repeated elements
 LL |             Self: Clone + Clone + Clone;
    |                   ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone`
 
-error: this trait bound is already specified in the where clause
-  --> $DIR/trait_duplication_in_bounds.rs:158:28
-   |
-LL |     trait BadTraitBound<T: Clone + Clone + Clone + Copy, U: Clone + Copy> {
-   |                            ^^^^^
-   |
-   = help: consider removing this trait bound
-
 error: these bounds contain repeated elements
   --> $DIR/trait_duplication_in_bounds.rs:158:28
    |
@@ -127,41 +103,17 @@ error: these where clauses contain repeated elements
 LL |             T: Clone + Clone + Clone + Copy,
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
 
-error: this trait bound is already specified in the where clause
-  --> $DIR/trait_duplication_in_bounds.rs:195:24
-   |
-LL |     fn good_generic<T: GenericTrait<u64> + GenericTrait<u32>>(arg0: T) {
-   |                        ^^^^^^^^^^^^^^^^^
-   |
-   = help: consider removing this trait bound
-
-error: this trait bound is already specified in the where clause
-  --> $DIR/trait_duplication_in_bounds.rs:199:23
-   |
-LL |     fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
-   |                       ^^^^^^^^^^^^^^^^^
-   |
-   = help: consider removing this trait bound
-
 error: these bounds contain repeated elements
   --> $DIR/trait_duplication_in_bounds.rs:199:23
    |
 LL |     fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
    |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait<u32> + GenericTrait<u64>`
 
-error: this trait bound is already specified in the where clause
-  --> $DIR/trait_duplication_in_bounds.rs:207:26
-   |
-LL |     fn qualified_path<T: std::clone::Clone + Clone + foo::Clone>(arg0: T) {
-   |                          ^^^^^^^^^^^^^^^^^
-   |
-   = help: consider removing this trait bound
-
 error: these bounds contain repeated elements
   --> $DIR/trait_duplication_in_bounds.rs:207:26
    |
 LL |     fn qualified_path<T: std::clone::Clone + Clone + foo::Clone>(arg0: T) {
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + foo::Clone`
 
-error: aborting due to 22 previous errors
+error: aborting due to 16 previous errors