Refactor repeated where clause or trait bound.

Author: Allen Hsu

clippy_lints/src/trait_bounds.rs

@@ -1,5 +1,5 @@
-use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_help};
-use clippy_utils::source::{snippet, snippet_with_applicability, snippet_opt};
+use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
+use clippy_utils::source::{snippet, snippet_opt, snippet_with_applicability};
 use clippy_utils::{SpanlessEq, SpanlessHash};
 use core::hash::{Hash, Hasher};
 use if_chain::if_chain;
@@ -272,34 +272,22 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
 }
 
 fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
-    if gen.span.from_expansion() {
-        return;
-    }
-
-    for param in gen.params {
+    fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) {
         let mut map = FxHashMap::default();
         let mut repeated_spans = false;
-        if let ParamName::Plain(name) = param.name { // other alternatives are errors and elided which won't have duplicates
-            for bound in param.bounds.iter().filter_map(get_trait_info_from_bound) {
-                let (definition, _, span_direct) = bound;
-                if let Some(_) = map.insert(definition, span_direct) {
-                    repeated_spans = true;
-                }
+        for bound in bounds.iter().filter_map(get_trait_info_from_bound) {
+            let (definition, _, span_direct) = bound;
+            if map.insert(definition, span_direct).is_some() {
+                repeated_spans = true;
             }
+        }
 
-            if repeated_spans {
-                let all_trait_span = param
-                    .bounds
-                    .get(0)
-                    .unwrap()
-                    .span()
-                    .to(
-                        param
-                        .bounds
-                        .iter()
-                        .last()
-                        .unwrap()
-                        .span());
+        if_chain! {
+            if repeated_spans;
+            if let Some(first_trait) = bounds.get(0);
+            if let Some(last_trait) = bounds.iter().last();
+            then {
+                let all_trait_span = first_trait.span().to(last_trait.span());
 
                 let mut traits = map.values()
                     .filter_map(|span| snippet_opt(cx, *span))
@@ -311,7 +299,7 @@ fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>
                     cx,
                     REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND,
                     all_trait_span,
-                    "this trait bound contains repeated elements",
+                    msg,
                     "try",
                     traits,
                     Applicability::MachineApplicable
@@ -320,50 +308,24 @@ fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>
         }
     }
 
-    for predicate in gen.where_clause.predicates {
-        if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate {
-            let mut where_clauses = FxHashMap::default();
-            let mut repeated_spans = false;
-
-            for (definition, _, span_direct) in bound_predicate
-                .bounds
-                .iter()
-                .filter_map(get_trait_info_from_bound)
-            {
-                if let Some(_) = where_clauses.insert(definition, span_direct) {
-                    repeated_spans = true;
-                }
-            }
+    if gen.span.from_expansion() || (gen.params.is_empty() && gen.where_clause.predicates.is_empty()) {
+        return;
+    }
 
-            if repeated_spans {
-                let all_trait_span = bound_predicate
-                    .bounds
-                    .get(0)
-                    .unwrap()
-                    .span()
-                    .to(
-                        bound_predicate
-                        .bounds
-                        .iter()
-                        .last()
-                        .unwrap()
-                        .span());
+    for param in gen.params {
+        if let ParamName::Plain(_) = param.name {
+            // other alternatives are errors and elided which won't have duplicates
+            rollup_traits(cx, param.bounds, "this trait bound contains repeated elements");
+        }
+    }
 
-                let mut traits = where_clauses.values()
-                    .filter_map(|span| snippet_opt(cx, *span))
-                    .collect::<Vec<_>>();
-                traits.sort_unstable();
-                let traits = traits.join(" + ");
-                span_lint_and_sugg(
-                    cx,
-                    REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND,
-                    all_trait_span,
-                    "this where clause has already been specified",
-                    "try",
-                    traits,
-                    Applicability::MachineApplicable
-                    );
-            }
+    for predicate in gen.where_clause.predicates {
+        if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate {
+            rollup_traits(
+                cx,
+                bound_predicate.bounds,
+                "this where clause contains repeated elements",
+            );
         }
     }
 }

tests/ui/repeated_where_clause_or_trait_bound.fixed

@@ -1,8 +1,6 @@
 // run-rustfix
 //
-#![allow(
-    unused,
-)]
+#![allow(unused)]
 #![deny(clippy::repeated_where_clause_or_trait_bound)]
 
 fn bad_foo<T: Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {

tests/ui/repeated_where_clause_or_trait_bound.rs

@@ -1,8 +1,6 @@
 // run-rustfix
 //
-#![allow(
-    unused,
-)]
+#![allow(unused)]
 #![deny(clippy::repeated_where_clause_or_trait_bound)]
 
 fn bad_foo<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {

tests/ui/repeated_where_clause_or_trait_bound.stderr

@@ -1,35 +1,35 @@
 error: this trait bound contains repeated elements
-  --> $DIR/repeated_where_clause_or_trait_bound.rs:8:15
+  --> $DIR/repeated_where_clause_or_trait_bound.rs:6:15
    |
 LL | fn bad_foo<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
    |
 note: the lint level is defined here
-  --> $DIR/repeated_where_clause_or_trait_bound.rs:6:9
+  --> $DIR/repeated_where_clause_or_trait_bound.rs:4:9
    |
 LL | #![deny(clippy::repeated_where_clause_or_trait_bound)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: this where clause has already been specified
-  --> $DIR/repeated_where_clause_or_trait_bound.rs:14:8
+error: this where clause contains repeated elements
+  --> $DIR/repeated_where_clause_or_trait_bound.rs:12:8
    |
 LL |     T: Clone + Clone + Clone + Copy,
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
 
-error: this where clause has already been specified
-  --> $DIR/repeated_where_clause_or_trait_bound.rs:49:15
+error: this where clause contains repeated elements
+  --> $DIR/repeated_where_clause_or_trait_bound.rs:47:15
    |
 LL |         Self: Clone + Clone + Clone;
    |               ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone`
 
 error: this trait bound contains repeated elements
-  --> $DIR/repeated_where_clause_or_trait_bound.rs:63:24
+  --> $DIR/repeated_where_clause_or_trait_bound.rs:61:24
    |
 LL | trait BadTraitBound<T: Clone + Clone + Clone + Copy, U: Clone + Copy> {
    |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
 
-error: this where clause has already been specified
-  --> $DIR/repeated_where_clause_or_trait_bound.rs:70:12
+error: this where clause contains repeated elements
+  --> $DIR/repeated_where_clause_or_trait_bound.rs:68:12
    |
 LL |         T: Clone + Clone + Clone + Copy,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`