Auto merge of #9132 - hellow554:maybe_trait_bound_on_type_repetition, r=Manishearth

Maybe trait bound on type repetition

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: fix maybe trait on [`type_repetition_in_bounds`] lint

I simplified the two for loops, which did exactly the same. Only downside is, that I need a `copied`, but that's to convert from `&&` to `&`, to that should be a noop?

One more thing: I only handle [`TraitBoundModifier::Maybe`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/enum.TraitBoundModifier.html#variant.Maybe). Can anyone give me an example (and testcase) for [`TraitBoundModifier::MaybeConst`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/enum.TraitBoundModifier.html#variant.MaybeConst)?

closes #8770

3 files changed, 55 insertions, 30 deletions

diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs
index 71957572f2e..2741179074b 100644
--- a/clippy_lints/src/trait_bounds.rs
+++ b/clippy_lints/src/trait_bounds.rs
@@ -3,18 +3,18 @@ use clippy_utils::source::{snippet, snippet_with_applicability};
 use clippy_utils::{SpanlessEq, SpanlessHash};
 use core::hash::{Hash, Hasher};
 use if_chain::if_chain;
+use itertools::Itertools;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::unhash::UnhashMap;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
 use rustc_hir::{
-    GenericBound, Generics, Item, ItemKind, Node, Path, PathSegment, PredicateOrigin, QPath, TraitItem, Ty, TyKind,
-    WherePredicate,
+    GenericBound, Generics, Item, ItemKind, Node, Path, PathSegment, PredicateOrigin, QPath, TraitBoundModifier,
+    TraitItem, Ty, TyKind, WherePredicate,
 };
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
-use rustc_span::Span;
-use std::fmt::Write as _;
+use rustc_span::{BytePos, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -178,30 +178,18 @@ impl TraitBounds {
                 );
 
                 then {
-                    let mut hint_string = format!(
-                        "consider combining the bounds: `{}:",
-                        snippet(cx, p.bounded_ty.span, "_")
+                    let trait_bounds = v
+                        .iter()
+                        .copied()
+                        .chain(p.bounds.iter())
+                        .filter_map(get_trait_info_from_bound)
+                        .map(|(_, _, span)| snippet_with_applicability(cx, span, "..", &mut applicability))
+                        .join(" + ");
+                    let hint_string = format!(
+                        "consider combining the bounds: `{}: {}`",
+                        snippet(cx, p.bounded_ty.span, "_"),
+                        trait_bounds,
                     );
-                    for b in v.iter() {
-                        if let GenericBound::Trait(ref poly_trait_ref, _) = b {
-                            let path = &poly_trait_ref.trait_ref.path;
-                            let _ = write!(hint_string,
-                                " {} +",
-                                snippet_with_applicability(cx, path.span, "..", &mut applicability)
-                            );
-                        }
-                    }
-                    for b in p.bounds.iter() {
-                        if let GenericBound::Trait(ref poly_trait_ref, _) = b {
-                            let path = &poly_trait_ref.trait_ref.path;
-                            let _ = write!(hint_string,
-                                " {} +",
-                                snippet_with_applicability(cx, path.span, "..", &mut applicability)
-                            );
-                        }
-                    }
-                    hint_string.truncate(hint_string.len() - 2);
-                    hint_string.push('`');
                     span_lint_and_help(
                         cx,
                         TYPE_REPETITION_IN_BOUNDS,
@@ -254,8 +242,17 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
 }
 
 fn get_trait_info_from_bound<'a>(bound: &'a GenericBound<'_>) -> Option<(Res, &'a [PathSegment<'a>], Span)> {
-    if let GenericBound::Trait(t, _) = bound {
-        Some((t.trait_ref.path.res, t.trait_ref.path.segments, t.span))
+    if let GenericBound::Trait(t, tbm) = bound {
+        let trait_path = t.trait_ref.path;
+        let trait_span = {
+            let path_span = trait_path.span;
+            if let TraitBoundModifier::Maybe = tbm {
+                path_span.with_lo(path_span.lo() - BytePos(1)) // include the `?`
+            } else {
+                path_span
+            }
+        };
+        Some((trait_path.res, trait_path.segments, trait_span))
     } else {
         None
     }
diff --git a/tests/ui/type_repetition_in_bounds.rs b/tests/ui/type_repetition_in_bounds.rs
index d11432f9046..2eca1f4701c 100644
--- a/tests/ui/type_repetition_in_bounds.rs
+++ b/tests/ui/type_repetition_in_bounds.rs
@@ -79,6 +79,18 @@ where
     u: U,
 }
 
+// Check for the `?` in `?Sized`
+pub fn f<T: ?Sized>()
+where
+    T: Clone,
+{
+}
+pub fn g<T: Clone>()
+where
+    T: ?Sized,
+{
+}
+
 // This should not lint
 fn impl_trait(_: impl AsRef<str>, _: impl AsRef<str>) {}
 
diff --git a/tests/ui/type_repetition_in_bounds.stderr b/tests/ui/type_repetition_in_bounds.stderr
index 148c19c7d07..1d88714814d 100644
--- a/tests/ui/type_repetition_in_bounds.stderr
+++ b/tests/ui/type_repetition_in_bounds.stderr
@@ -19,5 +19,21 @@ LL |     Self: Copy + Default + Ord,
    |
    = help: consider combining the bounds: `Self: Clone + Copy + Default + Ord`
 
-error: aborting due to 2 previous errors
+error: this type has already been used as a bound predicate
+  --> $DIR/type_repetition_in_bounds.rs:85:5
+   |
+LL |     T: Clone,
+   |     ^^^^^^^^
+   |
+   = help: consider combining the bounds: `T: ?Sized + Clone`
+
+error: this type has already been used as a bound predicate
+  --> $DIR/type_repetition_in_bounds.rs:90:5
+   |
+LL |     T: ?Sized,
+   |     ^^^^^^^^^
+   |
+   = help: consider combining the bounds: `T: Clone + ?Sized`
+
+error: aborting due to 4 previous errors