Refactor repeated where clause or trait bound.

rust-lang · Apr 15, 2022 · 92d2b01 · 92d2b01
1 parent 0517a6c
commit 92d2b01
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 97 deletions.
diff --git a/clippy_lints/src/trait_bounds.rs b/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;
@@ -290,98 +290,60 @@ 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 repeated_spans {
+            if_chain! {
+                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))
-                    .collect::<Vec<_>>();
-                traits.sort_unstable();
-                let traits = traits.join(" + ");
+                    let mut traits = map.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 trait bound contains repeated elements",
-                    "try",
-                    traits,
-                    Applicability::MachineApplicable
-                    );
+                    span_lint_and_sugg(
+                        cx,
+                        REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND,
+                        all_trait_span,
+                        msg,
+                        "try",
+                        traits,
+                        Applicability::MachineApplicable
+                        );
+                }
             }
         }
     }
+    if gen.span.from_expansion() || (gen.params.is_empty() && gen.where_clause.predicates.is_empty()) {
+        return;
+    }
+
+    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");
+        }
+    }
 
     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 repeated_spans {
-                let all_trait_span = bound_predicate
-                    .bounds
-                    .get(0)
-                    .unwrap()
-                    .span()
-                    .to(
-                        bound_predicate
-                        .bounds
-                        .iter()
-                        .last()
-                        .unwrap()
-                        .span());
-
-                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
-                    );
-            }
+            rollup_traits(
+                cx,
+                bound_predicate.bounds,
+                "this where clause contains repeated elements",
+            );
         }
     }
 }

diff --git a/tests/ui/repeated_where_clause_or_trait_bound.fixed b/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) {

diff --git a/tests/ui/repeated_where_clause_or_trait_bound.rs b/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) {

diff --git a/tests/ui/repeated_where_clause_or_trait_bound.stderr b/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`