Tweak suggest_constraining_type_param

Some of the bound restriction structured suggestions were incorrect
while others had subpar output.

Author: estebank

src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs

@@ -222,8 +222,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                             &mut err,
                             &param.name.as_str(),
                             "Copy",
-                            tcx.sess.source_map(),
-                            span,
                             None,
                         );
                     }

src/librustc_trait_selection/traits/error_reporting/mod.rs

@@ -25,8 +25,7 @@ use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder}
 use rustc_hir as hir;
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
 use rustc_hir::{Node, QPath, TyKind, WhereBoundPredicate, WherePredicate};
-use rustc_span::source_map::SourceMap;
-use rustc_span::{ExpnKind, Span, DUMMY_SP};
+use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP};
 use std::fmt;
 
 use crate::traits::query::evaluate_obligation::InferCtxtExt as _;
@@ -1680,14 +1679,8 @@ pub fn suggest_constraining_type_param(
     err: &mut DiagnosticBuilder<'_>,
     param_name: &str,
     constraint: &str,
-    source_map: &SourceMap,
-    span: Span,
     def_id: Option<DefId>,
 ) -> bool {
-    const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound with";
-    const MSG_RESTRICT_TYPE: &str = "consider restricting this type parameter with";
-    const MSG_RESTRICT_TYPE_FURTHER: &str = "consider further restricting this type parameter with";
-
     let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name);
 
     let param = if let Some(param) = param {
@@ -1696,6 +1689,11 @@ pub fn suggest_constraining_type_param(
         return false;
     };
 
+    const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound";
+    let msg_restrict_type = format!("consider restricting type parameter `{}`", param_name);
+    let msg_restrict_type_further =
+        format!("consider further restricting type parameter `{}`", param_name);
+
     if def_id == tcx.lang_items().sized_trait() {
         // Type parameters are already `Sized` by default.
         err.span_label(param.span, &format!("this type parameter needs to be `{}`", constraint));
@@ -1718,19 +1716,21 @@ pub fn suggest_constraining_type_param(
         //             |
         //             replace with: `impl Foo + Bar`
 
-        err.span_help(param.span, &format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint));
-
-        err.tool_only_span_suggestion(
-            param.span,
+        err.span_suggestion_verbose(
+            param.span.shrink_to_hi(),
             MSG_RESTRICT_BOUND_FURTHER,
-            format!("{} + {}", param_name, constraint),
+            format!(" + {}", constraint),
             Applicability::MachineApplicable,
         );
 
         return true;
     }
 
-    if generics.where_clause.predicates.is_empty() {
+    if generics.where_clause.predicates.is_empty()
+        // Given `trait Base<T = String>: Super<T>` where `T: Copy`, suggest restricting in the
+        // `where` clause in stead of `trait Base<T: Copy = String>: Super<T>`.
+        && !matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. })
+    {
         if let Some(bounds_span) = param.bounds_span() {
             // If user has provided some bounds, suggest restricting them:
             //
@@ -1746,37 +1746,21 @@ pub fn suggest_constraining_type_param(
             //          |
             //          replace with: `T: Bar +`
 
-            err.span_help(
-                bounds_span,
-                &format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
+            err.span_suggestion_verbose(
+                bounds_span.shrink_to_hi(),
+                MSG_RESTRICT_BOUND_FURTHER,
+                format!(" + {}", constraint),
+                Applicability::MachineApplicable,
             );
-
-            let span_hi = param.span.with_hi(span.hi());
-            let span_with_colon = source_map.span_through_char(span_hi, ':');
-
-            if span_hi != param.span && span_with_colon != span_hi {
-                err.tool_only_span_suggestion(
-                    span_with_colon,
-                    MSG_RESTRICT_BOUND_FURTHER,
-                    format!("{}: {} + ", param_name, constraint),
-                    Applicability::MachineApplicable,
-                );
-            }
         } else {
             // If user hasn't provided any bounds, suggest adding a new one:
             //
             //   fn foo<T>(t: T) { ... }
             //          - help: consider restricting this type parameter with `T: Foo`
-
-            err.span_help(
-                param.span,
-                &format!("{} `{}: {}`", MSG_RESTRICT_TYPE, param_name, constraint),
-            );
-
-            err.tool_only_span_suggestion(
-                param.span,
-                MSG_RESTRICT_TYPE,
-                format!("{}: {}", param_name, constraint),
+            err.span_suggestion_verbose(
+                param.span.shrink_to_hi(),
+                &msg_restrict_type,
+                format!(": {}", constraint),
                 Applicability::MachineApplicable,
             );
         }
@@ -1840,55 +1824,44 @@ pub fn suggest_constraining_type_param(
             }
         }
 
-        let where_clause_span =
-            generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi();
+        let where_clause_span = generics.where_clause.span_for_predicates_or_empty_place();
+        // Account for `fn foo<T>(t: T) where T: Foo,` so we don't suggest two trailing commas.
+        let mut trailing_comma = false;
+        if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(where_clause_span) {
+            if snippet.ends_with(",") {
+                trailing_comma = true;
+            }
+        }
+        let where_clause_span = if trailing_comma {
+            let hi = where_clause_span.hi();
+            Span::new(hi - BytePos(1), hi, where_clause_span.ctxt())
+        } else {
+            where_clause_span.shrink_to_hi()
+        };
 
         match &param_spans[..] {
             &[] => {
-                err.span_help(
-                    param.span,
-                    &format!("{} `where {}: {}`", MSG_RESTRICT_TYPE, param_name, constraint),
-                );
-
-                err.tool_only_span_suggestion(
+                err.span_suggestion_verbose(
                     where_clause_span,
-                    MSG_RESTRICT_TYPE,
+                    &msg_restrict_type_further,
                     format!(", {}: {}", param_name, constraint),
                     Applicability::MachineApplicable,
                 );
             }
 
             &[&param_span] => {
-                err.span_help(
-                    param_span,
-                    &format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
+                err.span_suggestion_verbose(
+                    param_span.shrink_to_hi(),
+                    MSG_RESTRICT_BOUND_FURTHER,
+                    format!(" + {}", constraint),
+                    Applicability::MachineApplicable,
                 );
-
-                let span_hi = param_span.with_hi(span.hi());
-                let span_with_colon = source_map.span_through_char(span_hi, ':');
-
-                if span_hi != param_span && span_with_colon != span_hi {
-                    err.tool_only_span_suggestion(
-                        span_with_colon,
-                        MSG_RESTRICT_BOUND_FURTHER,
-                        format!("{}: {} +", param_name, constraint),
-                        Applicability::MachineApplicable,
-                    );
-                }
             }
 
             _ => {
-                err.span_help(
-                    param.span,
-                    &format!(
-                        "{} `where {}: {}`",
-                        MSG_RESTRICT_TYPE_FURTHER, param_name, constraint,
-                    ),
-                );
-
-                err.tool_only_span_suggestion(
+                err.span_suggestion_verbose(
                     where_clause_span,
-                    MSG_RESTRICT_BOUND_FURTHER,
+                    &msg_restrict_type_further,
                     format!(", {}: {}", param_name, constraint),
                     Applicability::MachineApplicable,
                 );