trait_duplication_in_bounds: warn on duplicate trait bounds of the same syntax

ibraheemdev · ibraheemdev · commit 9bc25262f819 · 2021-08-16T21:59:44.000-04:00
diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs
@@ -57,6 +57,7 @@ use rustc_trait_selection::traits::misc::can_type_implement_copy;
 use crate::nonstandard_style::{method_context, MethodLateContext};
 
 use std::fmt::Write;
+use std::hash::{Hash, Hasher};
 use tracing::{debug, trace};
 
 // hardwired lints from librustc_middle
@@ -3174,61 +3175,82 @@ declare_lint_pass!(TraitDuplicationInBounds => [TRAIT_DUPLICATION_IN_BOUNDS]);
 
 impl<'tcx> LateLintPass<'tcx> for TraitDuplicationInBounds {
     fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx hir::Generics<'_>) {
-        fn get_trait_res_span_from_bound(bound: &hir::GenericBound<'_>) -> Option<(Res, Span)> {
-            if let hir::GenericBound::Trait(t, _) = bound {
-                Some((t.trait_ref.path.res, t.span))
-            } else {
-                None
+        struct TraitRes {
+            res: Res,
+            span: Span,
+        }
+
+        impl TraitRes {
+            fn from_bound(bound: &hir::GenericBound<'_>) -> Option<Self> {
+                if let hir::GenericBound::Trait(t, _) = bound {
+                    Some(Self { res: t.trait_ref.path.res, span: t.span })
+                } else {
+                    None
+                }
             }
         }
-        if gen.span.from_expansion()
-            || gen.params.is_empty()
-            || gen.where_clause.predicates.is_empty()
-        {
+
+        impl PartialEq for TraitRes {
+            fn eq(&self, other: &Self) -> bool {
+                self.res == other.res
+            }
+        }
+
+        impl Hash for TraitRes {
+            fn hash<H: Hasher>(&self, state: &mut H) {
+                self.res.hash(state)
+            }
+        }
+
+        impl Eq for TraitRes {}
+
+        if gen.span.from_expansion() {
             return;
         }
 
-        let mut map = FxHashMap::default();
+        let mut trait_resolutions = FxHashMap::default();
         for param in gen.params {
             if let hir::ParamName::Plain(ref ident) = param.name {
-                let res = param
-                    .bounds
-                    .iter()
-                    .filter_map(get_trait_res_span_from_bound)
-                    .collect::<Vec<_>>();
-                map.insert(*ident, res);
+                let mut uniq = FxHashSet::default();
+
+                for res in param.bounds.iter().filter_map(TraitRes::from_bound) {
+                    let span = res.span.clone();
+                    if !uniq.insert(res) {
+                        cx.struct_span_lint(TRAIT_DUPLICATION_IN_BOUNDS, span, |lint| {
+                            lint.build("this trait bound has already been specified")
+                                .help("consider removing this trait bound")
+                                .emit()
+                        });
+                    }
+                }
+
+                trait_resolutions.insert(*ident, uniq);
             }
         }
 
         for predicate in gen.where_clause.predicates {
             if let hir::WherePredicate::BoundPredicate(ref bound_predicate) = predicate {
-                if !bound_predicate.span.from_expansion() {
-                    if let hir::TyKind::Path(hir::QPath::Resolved(_, hir::Path { segments, .. })) =
-                        bound_predicate.bounded_ty.kind
-                    {
-                        if let Some(segment) = segments.first() {
-                            if let Some(trait_resolutions_direct) = map.get(&segment.ident) {
-                                for (res_where, _) in bound_predicate
-                                    .bounds
-                                    .iter()
-                                    .filter_map(get_trait_res_span_from_bound)
-                                {
-                                    if let Some((_, span_direct)) = trait_resolutions_direct
-                                        .iter()
-                                        .find(|(res_direct, _)| *res_direct == res_where)
-                                    {
-                                        cx.struct_span_lint(
-                                            TRAIT_DUPLICATION_IN_BOUNDS,
-                                            *span_direct,
-                                            |lint| {
-                                                lint.build(
-                                                "this trait bound is already specified in the where clause"
-                                                )
-                                                .help("consider removing this trait bound")
-                                                .emit()
-                                            },
-                                        );
-                                    }
+                if let hir::TyKind::Path(hir::QPath::Resolved(_, hir::Path { segments, .. })) =
+                    bound_predicate.bounded_ty.kind
+                {
+                    if let Some(segment) = segments.first() {
+                        if let Some(trait_resolutions) = trait_resolutions.get_mut(&segment.ident) {
+                            for res in
+                                bound_predicate.bounds.iter().filter_map(TraitRes::from_bound)
+                            {
+                                let span = res.span.clone();
+                                if !trait_resolutions.insert(res) {
+                                    cx.struct_span_lint(
+                                        TRAIT_DUPLICATION_IN_BOUNDS,
+                                        span,
+                                        |lint| {
+                                            lint.build(
+                                                "this trait bound has already been specified",
+                                            )
+                                            .help("consider removing this trait bound")
+                                            .emit()
+                                        },
+                                    );
                                 }
                             }
                         }
diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs
@@ -2434,6 +2434,7 @@ pub trait Iterator {
     /// assert!(result.is_err());
     /// ```
     #[inline]
+    #[cfg_attr(not(bootstrap), allow(trait_duplication_in_bounds))]
     #[unstable(feature = "try_find", reason = "new API", issue = "63178")]
     fn try_find<F, R, E>(&mut self, f: F) -> Result<Option<Self::Item>, E>
     where
diff --git a/library/proc_macro/src/lib.rs b/library/proc_macro/src/lib.rs
@@ -1233,6 +1233,7 @@ pub mod tracked_env {
     /// Besides the dependency tracking this function should be equivalent to `env::var` from the
     /// standard library, except that the argument must be UTF-8.
     #[unstable(feature = "proc_macro_tracked_env", issue = "74690")]
+    #[cfg_attr(not(bootstrap), allow(trait_duplication_in_bounds))]
     pub fn var<K: AsRef<OsStr> + AsRef<str>>(key: K) -> Result<String, VarError> {
         let key: &str = key.as_ref();
         let value = env::var(key);
diff --git a/src/test/ui/lint/trait_duplication_in_bounds.rs b/src/test/ui/lint/trait_duplication_in_bounds.rs
@@ -1,31 +1,47 @@
 #![deny(trait_duplication_in_bounds)]
 
-use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};
-
-fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
-//~^ ERROR this trait bound is already specified in the where clause
-//~| ERROR this trait bound is already specified in the where clause
+trait DupDirectAndWhere {}
+fn dup_direct_and_where<T: DupDirectAndWhere>(t: T)
 where
-    T: Clone,
-    T: Default,
+    T: DupDirectAndWhere,
+    //~^ ERROR this trait bound has already been specified
+    T: DupDirectAndWhere,
+    //~^ ERROR this trait bound has already been specified
 {
     unimplemented!();
 }
 
-fn good_bar<T: Clone + Default>(arg: T) {
+trait DupDirect {}
+fn dup_direct<T: DupDirect + DupDirect>(t: T) {
+    //~^ ERROR this trait bound has already been specified
     unimplemented!();
 }
 
-fn good_foo<T>(arg: T)
+trait DupWhere {}
+fn dup_where<T>(t: T)
 where
-    T: Clone + Default,
+    T: DupWhere + DupWhere,
+    //~^ ERROR this trait bound has already been specified
 {
     unimplemented!();
 }
 
-fn good_foobar<T: Default>(arg: T)
+trait NotDup {}
+fn not_dup<T: NotDup, U: NotDup>((t, u): (T, U)) {
+    unimplemented!();
+}
+
+trait Everything {}
+fn everything<T: Everything + Everything, U: Everything + Everything>((t, u): (T, U))
+//~^ ERROR this trait bound has already been specified
+//~| ERROR this trait bound has already been specified
 where
-    T: Clone,
+    T: Everything + Everything + Everything,
+    //~^ ERROR this trait bound has already been specified
+    //~| ERROR this trait bound has already been specified
+    //~| ERROR this trait bound has already been specified
+    U: Everything,
+    //~^ ERROR this trait bound has already been specified
 {
     unimplemented!();
 }
diff --git a/src/test/ui/lint/trait_duplication_in_bounds.stderr b/src/test/ui/lint/trait_duplication_in_bounds.stderr
@@ -1,8 +1,8 @@
-error: this trait bound is already specified in the where clause
-  --> $DIR/trait_duplication_in_bounds.rs:5:15
+error: this trait bound has already been specified
+  --> $DIR/trait_duplication_in_bounds.rs:6:8
    |
-LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
-   |               ^^^^^
+LL |     T: DupDirectAndWhere,
+   |        ^^^^^^^^^^^^^^^^^
    |
 note: the lint level is defined here
   --> $DIR/trait_duplication_in_bounds.rs:1:9
@@ -11,13 +11,77 @@ LL | #![deny(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
+error: this trait bound has already been specified
+  --> $DIR/trait_duplication_in_bounds.rs:8:8
    |
-LL | fn bad_foo<T: Clone + Default, Z: Copy>(arg0: T, arg1: Z)
-   |                       ^^^^^^^
+LL |     T: DupDirectAndWhere,
+   |        ^^^^^^^^^^^^^^^^^
    |
    = help: consider removing this trait bound
 
-error: aborting due to 2 previous errors
+error: this trait bound has already been specified
+  --> $DIR/trait_duplication_in_bounds.rs:15:30
+   |
+LL | fn dup_direct<T: DupDirect + DupDirect>(t: T) {
+   |                              ^^^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: this trait bound has already been specified
+  --> $DIR/trait_duplication_in_bounds.rs:23:19
+   |
+LL |     T: DupWhere + DupWhere,
+   |                   ^^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: this trait bound has already been specified
+  --> $DIR/trait_duplication_in_bounds.rs:35:31
+   |
+LL | fn everything<T: Everything + Everything, U: Everything + Everything>((t, u): (T, U))
+   |                               ^^^^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: this trait bound has already been specified
+  --> $DIR/trait_duplication_in_bounds.rs:35:59
+   |
+LL | fn everything<T: Everything + Everything, U: Everything + Everything>((t, u): (T, U))
+   |                                                           ^^^^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: this trait bound has already been specified
+  --> $DIR/trait_duplication_in_bounds.rs:39:8
+   |
+LL |     T: Everything + Everything + Everything,
+   |        ^^^^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: this trait bound has already been specified
+  --> $DIR/trait_duplication_in_bounds.rs:39:21
+   |
+LL |     T: Everything + Everything + Everything,
+   |                     ^^^^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: this trait bound has already been specified
+  --> $DIR/trait_duplication_in_bounds.rs:39:34
+   |
+LL |     T: Everything + Everything + Everything,
+   |                                  ^^^^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: this trait bound has already been specified
+  --> $DIR/trait_duplication_in_bounds.rs:43:8
+   |
+LL |     U: Everything,
+   |        ^^^^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: aborting due to 10 previous errors
 
