diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index b5f11b4acae02..4ccda15068bbb 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/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::>() + .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 { diff --git a/tests/ui/trait_duplication_in_bounds.fixed b/tests/ui/trait_duplication_in_bounds.fixed index eef8024b131cb..fdac0e4cb1e83 100644 --- a/tests/ui/trait_duplication_in_bounds.fixed +++ b/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(arg0: T, argo1: U) { unimplemented!(); } @@ -109,4 +111,12 @@ fn qualified_path(arg0: T) { unimplemented!(); } +fn good_trait_object(arg0: &(dyn Any + Send)) { + unimplemented!(); +} + +fn bad_trait_object(arg0: &(dyn Any + Send)) { + unimplemented!(); +} + fn main() {} diff --git a/tests/ui/trait_duplication_in_bounds.rs b/tests/ui/trait_duplication_in_bounds.rs index a7a1caf2880d3..a0300da555588 100644 --- a/tests/ui/trait_duplication_in_bounds.rs +++ b/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(arg0: T, argo1: U) { unimplemented!(); } @@ -109,4 +111,12 @@ fn qualified_path(arg0: T) { unimplemented!(); } +fn good_trait_object(arg0: &(dyn Any + Send)) { + unimplemented!(); +} + +fn bad_trait_object(arg0: &(dyn Any + Send + Send)) { + unimplemented!(); +} + fn main() {} diff --git a/tests/ui/trait_duplication_in_bounds.stderr b/tests/ui/trait_duplication_in_bounds.stderr index af800ba78880c..539b6114ca3ae 100644 --- a/tests/ui/trait_duplication_in_bounds.stderr +++ b/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(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 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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 + GenericTrait + GenericTrait>(arg0: T) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait + GenericTrait` 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(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