Auto merge of #10727 - john-h-k:lint/dup-auto-traits, r=Manishearth

Extend `trait_duplication_in_bounds` to cover trait objects

This PR extends `trait_duplication_in_bounds` to cover trait objects.

Currently,
```rs
fn foo(_a: &(dyn Any + Send + Send)) {}
```

generates no warnings. With this PR, it will complain about a duplicate trait and can remove it

Moved from rust-lang/rust#110991

changelog: [`trait_duplication_in_bounds`]: warn on duplicate trait object constraints

Author: bors

clippy_lints/src/trait_bounds.rs

@@ -37,12 +37,12 @@ declare_clippy_lint! {
     #[clippy::version = "1.38.0"]
     pub TYPE_REPETITION_IN_BOUNDS,
     nursery,
-    "types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`"
+    "types are repeated unnecessarily in trait bounds, use `+` instead of using `T: _, T: _`"
 }
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for cases where generics are being used and multiple
+    /// Checks for cases where generics or trait objects are being used and multiple
     /// syntax specifications for trait bounds are used simultaneously.
     ///
     /// ### Why is this bad?
@@ -167,6 +167,61 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
             }
         }
     }
+
+    fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) {
+        if_chain! {
+            if let TyKind::Ref(.., mut_ty) = &ty.kind;
+            if let TyKind::TraitObject(bounds, ..) = mut_ty.ty.kind;
+            if bounds.len() > 2;
+            then {
+
+                // Build up a hash of every trait we've seen
+                // When we see a trait for the first time, add it to unique_traits
+                // so we can later use it to build a string of all traits exactly once, without duplicates
+
+                let mut seen_def_ids = FxHashSet::default();
+                let mut unique_traits = Vec::new();
+
+                // Iterate the bounds and add them to our seen hash
+                // If we haven't yet seen it, add it to the fixed traits
+                for bound in bounds.iter() {
+                    let Some(def_id) = bound.trait_ref.trait_def_id() else { continue; };
+
+                    let new_trait = seen_def_ids.insert(def_id);
+
+                    if new_trait {
+                        unique_traits.push(bound);
+                    }
+                }
+
+                // If the number of unique traits isn't the same as the number of traits in the bounds,
+                // there must be 1 or more duplicates
+                if bounds.len() != unique_traits.len() {
+                    let mut bounds_span = bounds[0].span;
+
+                    for bound in bounds.iter().skip(1) {
+                        bounds_span = bounds_span.to(bound.span);
+                    }
+
+                    let fixed_trait_snippet = unique_traits
+                        .iter()
+                        .filter_map(|b| snippet_opt(cx, b.span))
+                        .collect::<Vec<_>>()
+                        .join(" + ");
+
+                    span_lint_and_sugg(
+                        cx,
+                        TRAIT_DUPLICATION_IN_BOUNDS,
+                        bounds_span,
+                        "this trait bound is already specified in trait declaration",
+                        "try",
+                        fixed_trait_snippet,
+                        Applicability::MaybeIncorrect,
+                    );
+                }
+            }
+        }
+    }
 }
 
 impl TraitBounds {

tests/ui/trait_duplication_in_bounds.fixed

@@ -2,6 +2,8 @@
 #![deny(clippy::trait_duplication_in_bounds)]
 #![allow(unused)]
 
+use std::any::Any;
+
 fn bad_foo<T: Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
     unimplemented!();
 }
@@ -109,4 +111,12 @@ fn qualified_path<T: std::clone::Clone + foo::Clone>(arg0: T) {
     unimplemented!();
 }
 
+fn good_trait_object(arg0: &(dyn Any + Send)) {
+    unimplemented!();
+}
+
+fn bad_trait_object(arg0: &(dyn Any + Send)) {
+    unimplemented!();
+}
+
 fn main() {}

tests/ui/trait_duplication_in_bounds.rs

@@ -2,6 +2,8 @@
 #![deny(clippy::trait_duplication_in_bounds)]
 #![allow(unused)]
 
+use std::any::Any;
+
 fn bad_foo<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
     unimplemented!();
 }
@@ -109,4 +111,12 @@ fn qualified_path<T: std::clone::Clone + Clone + foo::Clone>(arg0: T) {
     unimplemented!();
 }
 
+fn good_trait_object(arg0: &(dyn Any + Send)) {
+    unimplemented!();
+}
+
+fn bad_trait_object(arg0: &(dyn Any + Send + Send)) {
+    unimplemented!();
+}
+
 fn main() {}

tests/ui/trait_duplication_in_bounds.stderr

@@ -1,5 +1,5 @@
 error: these bounds contain repeated elements
-  --> $DIR/trait_duplication_in_bounds.rs:5:15
+  --> $DIR/trait_duplication_in_bounds.rs:7:15
    |
 LL | fn bad_foo<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) {
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
@@ -11,46 +11,52 @@ LL | #![deny(clippy::trait_duplication_in_bounds)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: these where clauses contain repeated elements
-  --> $DIR/trait_duplication_in_bounds.rs:11:8
+  --> $DIR/trait_duplication_in_bounds.rs:13:8
    |
 LL |     T: Clone + Clone + Clone + Copy,
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
 
 error: these bounds contain repeated elements
-  --> $DIR/trait_duplication_in_bounds.rs:39:26
+  --> $DIR/trait_duplication_in_bounds.rs:41:26
    |
 LL | trait BadSelfTraitBound: Clone + Clone + Clone {
    |                          ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone`
 
 error: these where clauses contain repeated elements
-  --> $DIR/trait_duplication_in_bounds.rs:46:15
+  --> $DIR/trait_duplication_in_bounds.rs:48:15
    |
 LL |         Self: Clone + Clone + Clone;
    |               ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone`
 
 error: these bounds contain repeated elements
-  --> $DIR/trait_duplication_in_bounds.rs:60:24
+  --> $DIR/trait_duplication_in_bounds.rs:62:24
    |
 LL | trait BadTraitBound<T: Clone + Clone + Clone + Copy, U: Clone + Copy> {
    |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
 
 error: these where clauses contain repeated elements
-  --> $DIR/trait_duplication_in_bounds.rs:67:12
+  --> $DIR/trait_duplication_in_bounds.rs:69:12
    |
 LL |         T: Clone + Clone + Clone + Copy,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy`
 
 error: these bounds contain repeated elements
-  --> $DIR/trait_duplication_in_bounds.rs:100:19
+  --> $DIR/trait_duplication_in_bounds.rs:102:19
    |
 LL | fn bad_generic<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) {
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait<u64> + GenericTrait<u32>`
 
 error: these bounds contain repeated elements
-  --> $DIR/trait_duplication_in_bounds.rs:108:22
+  --> $DIR/trait_duplication_in_bounds.rs:110:22
    |
 LL | fn qualified_path<T: std::clone::Clone + Clone + foo::Clone>(arg0: T) {
    |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::clone::Clone + foo::Clone`
 
-error: aborting due to 8 previous errors
+error: this trait bound is already specified in trait declaration
+  --> $DIR/trait_duplication_in_bounds.rs:118:33
+   |
+LL | fn bad_trait_object(arg0: &(dyn Any + Send + Send)) {
+   |                                 ^^^^^^^^^^^^^^^^^ help: try: `Any + Send`
+
+error: aborting due to 9 previous errors