Rollup merge of #107555 - edward-shen:edward-shen/dup-trait-suggestion, r=compiler-errors

Modify existing bounds if they exist

Fixes #107335.

This implementation is kinda gross but I don't really see a better way to do it.

This primarily does two things: Modifies `suggest_constraining_type_param` to accept a new parameter that indicates a span to be replaced instead of added, if presented, and limit the additive suggestions to either suggest a new bound on an existing bound (see newly added unit test) or add the generics argument if a generics argument wasn't found.

The former change is required to retain the capability to add an entirely new bounds if it was entirely omitted.

r? ``@compiler-errors``

Author: matthiaskrgr

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -803,6 +803,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 predicates
                     .iter()
                     .map(|(param, constraint)| (param.name.as_str(), &**constraint, None)),
+                None,
             );
         }
     }

compiler/rustc_const_eval/src/transform/check_consts/ops.rs

@@ -136,6 +136,7 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
                             &param_ty.name.as_str(),
                             &constraint,
                             None,
+                            None,
                         );
                     }
                 }

compiler/rustc_hir_analysis/src/coherence/builtin.rs

@@ -176,6 +176,7 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) {
                 bounds.iter().map(|(param, constraint, def_id)| {
                     (param.as_str(), constraint.as_str(), *def_id)
                 }),
+                None,
             );
             err.emit();
         }

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -1457,6 +1457,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     generics,
                     diag,
                     vec![(param.name.as_str(), "Clone", Some(clone_trait_did))].into_iter(),
+                    None,
                 );
             } else {
                 self.suggest_derive(diag, &[(trait_ref.to_predicate(self.tcx), None, None)]);

compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs

@@ -77,49 +77,86 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     (ty::Param(p), ty::Alias(ty::Projection, proj)) | (ty::Alias(ty::Projection, proj), ty::Param(p))
                         if tcx.def_kind(proj.def_id) != DefKind::ImplTraitPlaceholder =>
                     {
-                        let generics = tcx.generics_of(body_owner_def_id);
-                        let p_span = tcx.def_span(generics.type_param(p, tcx).def_id);
+                        let p_def_id = tcx
+                            .generics_of(body_owner_def_id)
+                            .type_param(p, tcx)
+                            .def_id;
+                        let p_span = tcx.def_span(p_def_id);
                         if !sp.contains(p_span) {
                             diag.span_label(p_span, "this type parameter");
                         }
                         let hir = tcx.hir();
                         let mut note = true;
-                        if let Some(generics) = generics
-                            .type_param(p, tcx)
-                            .def_id
+                        let parent = p_def_id
                             .as_local()
-                            .map(|id| hir.local_def_id_to_hir_id(id))
-                            .and_then(|id| tcx.hir().find_parent(id))
-                            .as_ref()
-                            .and_then(|node| node.generics())
+                            .and_then(|id| {
+                                let local_id = hir.local_def_id_to_hir_id(id);
+                                let generics = tcx.hir().find_parent(local_id)?.generics()?;
+                                Some((id, generics))
+                            });
+                        if let Some((local_id, generics)) = parent
                         {
                             // Synthesize the associated type restriction `Add<Output = Expected>`.
                             // FIXME: extract this logic for use in other diagnostics.
                             let (trait_ref, assoc_substs) = proj.trait_ref_and_own_substs(tcx);
-                            let path =
-                                tcx.def_path_str_with_substs(trait_ref.def_id, trait_ref.substs);
                             let item_name = tcx.item_name(proj.def_id);
                             let item_args = self.format_generic_args(assoc_substs);
 
-                            let path = if path.ends_with('>') {
-                                format!(
-                                    "{}, {}{} = {}>",
-                                    &path[..path.len() - 1],
-                                    item_name,
-                                    item_args,
-                                    p
-                                )
+                            // Here, we try to see if there's an existing
+                            // trait implementation that matches the one that
+                            // we're suggesting to restrict. If so, find the
+                            // "end", whether it be at the end of the trait
+                            // or the end of the generic arguments.
+                            let mut matching_span = None;
+                            let mut matched_end_of_args = false;
+                            for bound in generics.bounds_for_param(local_id) {
+                                let potential_spans = bound
+                                    .bounds
+                                    .iter()
+                                    .find_map(|bound| {
+                                        let bound_trait_path = bound.trait_ref()?.path;
+                                        let def_id = bound_trait_path.res.opt_def_id()?;
+                                        let generic_args = bound_trait_path.segments.iter().last().map(|path| path.args());
+                                        (def_id == trait_ref.def_id).then_some((bound_trait_path.span, generic_args))
+                                    });
+
+                                if let Some((end_of_trait, end_of_args)) = potential_spans {
+                                    let args_span = end_of_args.and_then(|args| args.span());
+                                    matched_end_of_args = args_span.is_some();
+                                    matching_span = args_span
+                                        .or_else(|| Some(end_of_trait))
+                                        .map(|span| span.shrink_to_hi());
+                                    break;
+                                }
+                            }
+
+                            if matched_end_of_args {
+                                // Append suggestion to the end of our args
+                                let path = format!(", {}{} = {}",item_name, item_args, p);
+                                note = !suggest_constraining_type_param(
+                                    tcx,
+                                    generics,
+                                    diag,
+                                    &format!("{}", proj.self_ty()),
+                                    &path,
+                                    None,
+                                    matching_span,
+                                );
                             } else {
-                                format!("{}<{}{} = {}>", path, item_name, item_args, p)
-                            };
-                            note = !suggest_constraining_type_param(
-                                tcx,
-                                generics,
-                                diag,
-                                &format!("{}", proj.self_ty()),
-                                &path,
-                                None,
-                            );
+                                // Suggest adding a bound to an existing trait
+                                // or if the trait doesn't exist, add the trait
+                                // and the suggested bounds.
+                                let path = format!("<{}{} = {}>", item_name, item_args, p);
+                                note = !suggest_constraining_type_param(
+                                    tcx,
+                                    generics,
+                                    diag,
+                                    &format!("{}", proj.self_ty()),
+                                    &path,
+                                    None,
+                                    matching_span,
+                                );
+                            }
                         }
                         if note {
                             diag.note("you might be missing a type parameter or trait bound");

compiler/rustc_middle/src/ty/diagnostics.rs

@@ -193,19 +193,24 @@ fn suggest_removing_unsized_bound(
 }
 
 /// Suggest restricting a type param with a new bound.
+///
+/// If `span_to_replace` is provided, then that span will be replaced with the
+/// `constraint`. If one wasn't provided, then the full bound will be suggested.
 pub fn suggest_constraining_type_param(
     tcx: TyCtxt<'_>,
     generics: &hir::Generics<'_>,
     err: &mut Diagnostic,
     param_name: &str,
     constraint: &str,
     def_id: Option<DefId>,
+    span_to_replace: Option<Span>,
 ) -> bool {
     suggest_constraining_type_params(
         tcx,
         generics,
         err,
         [(param_name, constraint, def_id)].into_iter(),
+        span_to_replace,
     )
 }
 
@@ -215,6 +220,7 @@ pub fn suggest_constraining_type_params<'a>(
     generics: &hir::Generics<'_>,
     err: &mut Diagnostic,
     param_names_and_constraints: impl Iterator<Item = (&'a str, &'a str, Option<DefId>)>,
+    span_to_replace: Option<Span>,
 ) -> bool {
     let mut grouped = FxHashMap::default();
     param_names_and_constraints.for_each(|(param_name, constraint, def_id)| {
@@ -253,7 +259,9 @@ pub fn suggest_constraining_type_params<'a>(
         let mut suggest_restrict = |span, bound_list_non_empty| {
             suggestions.push((
                 span,
-                if bound_list_non_empty {
+                if span_to_replace.is_some() {
+                    constraint.clone()
+                } else if bound_list_non_empty {
                     format!(" + {}", constraint)
                 } else {
                     format!(" {}", constraint)
@@ -262,6 +270,11 @@ pub fn suggest_constraining_type_params<'a>(
             ))
         };
 
+        if let Some(span) = span_to_replace {
+            suggest_restrict(span, true);
+            continue;
+        }
+
         // When the type parameter has been provided bounds
         //
         //    Message:

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -679,6 +679,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                         &param_name,
                         &constraint,
                         Some(trait_pred.def_id()),
+                        None,
                     ) {
                         return;
                     }
@@ -1087,6 +1088,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                     param.name.as_str(),
                     "Clone",
                     Some(clone_trait),
+                    None,
                 );
             }
             err.span_suggestion_verbose(

tests/ui/associated-types/hr-associated-type-projection-1.stderr

@@ -16,8 +16,8 @@ LL |     for<'b> <Self as UnsafeCopy<'b, T>>::Item: std::ops::Deref<Target = T>,
    |                                                                ^^^^^^^^^^ required by this bound in `UnsafeCopy`
 help: consider further restricting this bound
    |
-LL | impl<T: Copy + std::ops::Deref + Deref<Target = T>> UnsafeCopy<'_, T> for T {
-   |                                +++++++++++++++++++
+LL | impl<T: Copy + std::ops::Deref<Target = T>> UnsafeCopy<'_, T> for T {
+   |                               ++++++++++++
 
 error: aborting due to previous error
 

tests/ui/generic-associated-types/issue-68656-unsized-values.stderr

@@ -15,8 +15,8 @@ LL |     type Item<'a>: std::ops::Deref<Target = T>;
    |                                    ^^^^^^^^^^ required by this bound in `UnsafeCopy::Item`
 help: consider further restricting this bound
    |
-LL | impl<T: Copy + std::ops::Deref + Deref<Target = T>> UnsafeCopy<T> for T {
-   |                                +++++++++++++++++++
+LL | impl<T: Copy + std::ops::Deref<Target = T>> UnsafeCopy<T> for T {
+   |                               ++++++++++++
 
 error: aborting due to previous error
 

tests/ui/generic-associated-types/missing-bounds.fixed

@@ -4,7 +4,7 @@ use std::ops::Add;
 
 struct A<B>(B);
 
-impl<B> Add for A<B> where B: Add + Add<Output = B> {
+impl<B> Add for A<B> where B: Add<Output = B> {
     type Output = Self;
 
     fn add(self, rhs: Self) -> Self {
@@ -14,7 +14,7 @@ impl<B> Add for A<B> where B: Add + Add<Output = B> {
 
 struct C<B>(B);
 
-impl<B: Add + Add<Output = B>> Add for C<B> {
+impl<B: Add<Output = B>> Add for C<B> {
     type Output = Self;
 
     fn add(self, rhs: Self) -> Self {
@@ -34,7 +34,7 @@ impl<B: std::ops::Add<Output = B>> Add for D<B> {
 
 struct E<B>(B);
 
-impl<B: Add + Add<Output = B>> Add for E<B> where B: Add<Output = B> {
+impl<B: Add<Output = B>> Add for E<B> where B: Add<Output = B> {
     //~^ ERROR equality constraints are not yet supported in `where` clauses
     type Output = Self;
 

tests/ui/generic-associated-types/missing-bounds.stderr

@@ -37,8 +37,8 @@ LL | struct A<B>(B);
    |        ^
 help: consider further restricting this bound
    |
-LL | impl<B> Add for A<B> where B: Add + Add<Output = B> {
-   |                                   +++++++++++++++++
+LL | impl<B> Add for A<B> where B: Add<Output = B> {
+   |                                  ++++++++++++
 
 error[E0308]: mismatched types
   --> $DIR/missing-bounds.rs:21:14
@@ -60,8 +60,8 @@ LL | struct C<B>(B);
    |        ^
 help: consider further restricting this bound
    |
-LL | impl<B: Add + Add<Output = B>> Add for C<B> {
-   |             +++++++++++++++++
+LL | impl<B: Add<Output = B>> Add for C<B> {
+   |            ++++++++++++
 
 error[E0369]: cannot add `B` to `B`
   --> $DIR/missing-bounds.rs:31:21
@@ -96,8 +96,8 @@ LL | struct E<B>(B);
    |        ^
 help: consider further restricting this bound
    |
-LL | impl<B: Add + Add<Output = B>> Add for E<B> where <B as Add>::Output = B {
-   |             +++++++++++++++++
+LL | impl<B: Add<Output = B>> Add for E<B> where <B as Add>::Output = B {
+   |            ++++++++++++
 
 error: aborting due to 5 previous errors
 

tests/ui/suggestions/restrict-existing-type-bounds.rs

@@ -0,0 +1,30 @@
+pub trait TryAdd<Rhs = Self> {
+    type Error;
+    type Output;
+
+    fn try_add(self, rhs: Rhs) -> Result<Self::Output, Self::Error>;
+}
+
+impl<T: TryAdd> TryAdd for Option<T> {
+    type Error = <T as TryAdd>::Error;
+    type Output = Option<<T as TryAdd>::Output>;
+
+    fn try_add(self, rhs: Self) -> Result<Self::Output, Self::Error> {
+        Ok(self) //~ ERROR mismatched types
+    }
+}
+
+struct Other<A>(A);
+
+struct X;
+
+impl<T: TryAdd<Error = X>> TryAdd for Other<T> {
+    type Error = <T as TryAdd>::Error;
+    type Output = Other<<T as TryAdd>::Output>;
+
+    fn try_add(self, rhs: Self) -> Result<Self::Output, Self::Error> {
+        Ok(self) //~ ERROR mismatched types
+    }
+}
+
+fn main() {}

tests/ui/suggestions/restrict-existing-type-bounds.stderr

@@ -0,0 +1,57 @@
+error[E0308]: mismatched types
+  --> $DIR/restrict-existing-type-bounds.rs:13:12
+   |
+LL | impl<T: TryAdd> TryAdd for Option<T> {
+   |      - this type parameter
+...
+LL |         Ok(self)
+   |         -- ^^^^ expected `Option<<T as TryAdd>::Output>`, found `Option<T>`
+   |         |
+   |         arguments to this enum variant are incorrect
+   |
+   = note: expected enum `Option<<T as TryAdd>::Output>`
+              found enum `Option<T>`
+help: the type constructed contains `Option<T>` due to the type of the argument passed
+  --> $DIR/restrict-existing-type-bounds.rs:13:9
+   |
+LL |         Ok(self)
+   |         ^^^----^
+   |            |
+   |            this argument influences the type of `Ok`
+note: tuple variant defined here
+  --> $SRC_DIR/core/src/result.rs:LL:COL
+help: consider further restricting this bound
+   |
+LL | impl<T: TryAdd<Output = T>> TryAdd for Option<T> {
+   |               ++++++++++++
+
+error[E0308]: mismatched types
+  --> $DIR/restrict-existing-type-bounds.rs:26:12
+   |
+LL | impl<T: TryAdd<Error = X>> TryAdd for Other<T> {
+   |      - this type parameter
+...
+LL |         Ok(self)
+   |         -- ^^^^ expected `Other<<T as TryAdd>::Output>`, found `Other<T>`
+   |         |
+   |         arguments to this enum variant are incorrect
+   |
+   = note: expected struct `Other<<T as TryAdd>::Output>`
+              found struct `Other<T>`
+help: the type constructed contains `Other<T>` due to the type of the argument passed
+  --> $DIR/restrict-existing-type-bounds.rs:26:9
+   |
+LL |         Ok(self)
+   |         ^^^----^
+   |            |
+   |            this argument influences the type of `Ok`
+note: tuple variant defined here
+  --> $SRC_DIR/core/src/result.rs:LL:COL
+help: consider further restricting this bound
+   |
+LL | impl<T: TryAdd<Error = X, Output = T>> TryAdd for Other<T> {
+   |                         ++++++++++++
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0308`.