Improve reporting errors and suggestions for trait bounds

src/librustc/traits/error_reporting/mod.rs

@@ -27,6 +27,8 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
 use rustc_hir as hir;
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
+use rustc_hir::{QPath, TyKind, WhereBoundPredicate, WherePredicate};
+use rustc_span::source_map::SourceMap;
 use rustc_span::{ExpnKind, Span, DUMMY_SP};
 use std::fmt;
 use syntax::ast;
@@ -1426,3 +1428,229 @@ impl ArgKind {
         }
     }
 }
+
+/// Suggest restricting a type param with a new bound.
+pub fn suggest_constraining_type_param(
+    tcx: TyCtxt<'_>,
+    generics: &hir::Generics<'_>,
+    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().filter(|p| p.name.ident().as_str() == param_name).next();
+
+    let param = if let Some(param) = param {
+        param
+    } else {
+        return false;
+    };
+
+    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));
+        return true;
+    }
+
+    if param_name.starts_with("impl ") {
+        // If there's an `impl Trait` used in argument position, suggest
+        // restricting it:
+        //
+        //   fn foo(t: impl Foo) { ... }
+        //             --------
+        //             |
+        //             help: consider further restricting this bound with `+ Bar`
+        //
+        // Suggestion for tools in this case is:
+        //
+        //   fn foo(t: impl Foo) { ... }
+        //             --------
+        //             |
+        //             replace with: `impl Foo + Bar`
+
+        err.span_help(param.span, &format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint));
+
+        err.tool_only_span_suggestion(
+            param.span,
+            MSG_RESTRICT_BOUND_FURTHER,
+            format!("{} + {}", param_name, constraint),
+            Applicability::MachineApplicable,
+        );
+
+        return true;
+    }
+
+    if generics.where_clause.predicates.is_empty() {
+        if let Some(bounds_span) = param.bounds_span() {
+            // If user has provided some bounds, suggest restricting them:
+            //
+            //   fn foo<T: Foo>(t: T) { ... }
+            //             ---
+            //             |
+            //             help: consider further restricting this bound with `+ Bar`
+            //
+            // Suggestion for tools in this case is:
+            //
+            //   fn foo<T: Foo>(t: T) { ... }
+            //          --
+            //          |
+            //          replace with: `T: Bar +`
+
+            err.span_help(
+                bounds_span,
+                &format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
+            );
+
+            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),
+                Applicability::MachineApplicable,
+            );
+        }
+
+        true
+    } else {
+        // This part is a bit tricky, because using the `where` clause user can
+        // provide zero, one or many bounds for the same type parameter, so we
+        // have following cases to consider:
+        //
+        // 1) When the type parameter has been provided zero bounds
+        //
+        //    Message:
+        //      fn foo<X, Y>(x: X, y: Y) where Y: Foo { ... }
+        //             - help: consider restricting this type parameter with `where X: Bar`
+        //
+        //    Suggestion:
+        //      fn foo<X, Y>(x: X, y: Y) where Y: Foo { ... }
+        //                                           - insert: `, X: Bar`
+        //
+        //
+        // 2) When the type parameter has been provided one bound
+        //
+        //    Message:
+        //      fn foo<T>(t: T) where T: Foo { ... }
+        //                            ^^^^^^
+        //                            |
+        //                            help: consider further restricting this bound with `+ Bar`
+        //
+        //    Suggestion:
+        //      fn foo<T>(t: T) where T: Foo { ... }
+        //                            ^^
+        //                            |
+        //                            replace with: `T: Bar +`
+        //
+        //
+        // 3) When the type parameter has been provided many bounds
+        //
+        //    Message:
+        //      fn foo<T>(t: T) where T: Foo, T: Bar {... }
+        //             - help: consider further restricting this type parameter with `where T: Zar`
+        //
+        //    Suggestion:
+        //      fn foo<T>(t: T) where T: Foo, T: Bar {... }
+        //                                          - insert: `, T: Zar`
+
+        let mut param_spans = Vec::new();
+
+        for predicate in generics.where_clause.predicates {
+            if let WherePredicate::BoundPredicate(WhereBoundPredicate {
+                span, bounded_ty, ..
+            }) = predicate
+            {
+                if let TyKind::Path(QPath::Resolved(_, path)) = &bounded_ty.kind {
+                    if let Some(segment) = path.segments.first() {
+                        if segment.ident.to_string() == param_name {
+                            param_spans.push(span);
+                        }
+                    }
+                }
+            }
+        }
+
+        let where_clause_span =
+            generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi();
+
+        match &param_spans[..] {
+            &[] => {
+                err.span_help(
+                    param.span,
+                    &format!("{} `where {}: {}`", MSG_RESTRICT_TYPE, param_name, constraint),
+                );
+
+                err.tool_only_span_suggestion(
+                    where_clause_span,
+                    MSG_RESTRICT_TYPE,
+                    format!(", {}: {}", param_name, constraint),
+                    Applicability::MachineApplicable,
+                );
+            }
+
+            &[&param_span] => {
+                err.span_help(
+                    param_span,
+                    &format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
+                );
+
+                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(
+                    where_clause_span,
+                    MSG_RESTRICT_BOUND_FURTHER,
+                    format!(", {}: {}", param_name, constraint),
+                    Applicability::MachineApplicable,
+                );
+            }
+        }
+
+        true
+    }
+}

src/librustc/traits/error_reporting/suggestions.rs

@@ -4,10 +4,10 @@ use super::{
 };
 
 use crate::infer::InferCtxt;
+use crate::traits::error_reporting::suggest_constraining_type_param;
 use crate::traits::object_safety::object_safety_violations;
 use crate::ty::TypeckTables;
 use crate::ty::{self, AdtKind, DefIdTree, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness};
-
 use rustc_errors::{
     error_code, pluralize, struct_span_err, Applicability, DiagnosticBuilder, Style,
 };
@@ -16,7 +16,6 @@ use rustc_hir::def::DefKind;
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::Visitor;
 use rustc_hir::Node;
-use rustc_span::source_map::SourceMap;
 use rustc_span::symbol::{kw, sym};
 use rustc_span::{MultiSpan, Span, DUMMY_SP};
 use std::fmt;
@@ -430,12 +429,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                             .span_take_while(span, |c| c.is_whitespace() || *c == '&');
 
                         let remove_refs = refs_remaining + 1;
-                        let format_str =
-                            format!("consider removing {} leading `&`-references", remove_refs);
+
+                        let msg = if remove_refs == 1 {
+                            "consider removing the leading `&`-reference".to_string()
+                        } else {
+                            format!("consider removing {} leading `&`-references", remove_refs)
+                        };
 
                         err.span_suggestion_short(
                             sp,
-                            &format_str,
+                            &msg,
                             String::new(),
                             Applicability::MachineApplicable,
                         );
@@ -1652,85 +1655,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     }
 }
 
-/// Suggest restricting a type param with a new bound.
-pub fn suggest_constraining_type_param(
-    tcx: TyCtxt<'_>,
-    generics: &hir::Generics<'_>,
-    err: &mut DiagnosticBuilder<'_>,
-    param_name: &str,
-    constraint: &str,
-    source_map: &SourceMap,
-    span: Span,
-    def_id: Option<DefId>,
-) -> bool {
-    let restrict_msg = "consider further restricting this bound";
-    if let Some(param) =
-        generics.params.iter().filter(|p| p.name.ident().as_str() == param_name).next()
-    {
-        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),
-            );
-        } else if param_name.starts_with("impl ") {
-            // `impl Trait` in argument:
-            // `fn foo(x: impl Trait) {}` → `fn foo(t: impl Trait + Trait2) {}`
-            err.span_suggestion(
-                param.span,
-                restrict_msg,
-                // `impl CurrentTrait + MissingTrait`
-                format!("{} + {}", param_name, constraint),
-                Applicability::MachineApplicable,
-            );
-        } else if generics.where_clause.predicates.is_empty() && param.bounds.is_empty() {
-            // If there are no bounds whatsoever, suggest adding a constraint
-            // to the type parameter:
-            // `fn foo<T>(t: T) {}` → `fn foo<T: Trait>(t: T) {}`
-            err.span_suggestion(
-                param.span,
-                "consider restricting this bound",
-                format!("{}: {}", param_name, constraint),
-                Applicability::MachineApplicable,
-            );
-        } else if !generics.where_clause.predicates.is_empty() {
-            // There is a `where` clause, so suggest expanding it:
-            // `fn foo<T>(t: T) where T: Debug {}` →
-            // `fn foo<T>(t: T) where T: Debug, T: Trait {}`
-            err.span_suggestion(
-                generics.where_clause.span().unwrap().shrink_to_hi(),
-                &format!("consider further restricting type parameter `{}`", param_name),
-                format!(", {}: {}", param_name, constraint),
-                Applicability::MachineApplicable,
-            );
-        } else {
-            // If there is no `where` clause lean towards constraining to the
-            // type parameter:
-            // `fn foo<X: Bar, T>(t: T, x: X) {}` → `fn foo<T: Trait>(t: T) {}`
-            // `fn foo<T: Bar>(t: T) {}` → `fn foo<T: Bar + Trait>(t: T) {}`
-            let sp = param.span.with_hi(span.hi());
-            let span = source_map.span_through_char(sp, ':');
-            if sp != param.span && sp != span {
-                // Only suggest if we have high certainty that the span
-                // covers the colon in `foo<T: Trait>`.
-                err.span_suggestion(
-                    span,
-                    restrict_msg,
-                    format!("{}: {} + ", param_name, constraint),
-                    Applicability::MachineApplicable,
-                );
-            } else {
-                err.span_label(
-                    param.span,
-                    &format!("consider adding a `where {}: {}` bound", param_name, constraint),
-                );
-            }
-        }
-        return true;
-    }
-    false
-}
-
 /// Collect all the returned expressions within the input expression.
 /// Used to point at the return spans when we want to suggest some change to them.
 #[derive(Default)]

src/librustc_hir/hir.rs

@@ -441,6 +441,16 @@ pub struct GenericParam<'hir> {
     pub kind: GenericParamKind<'hir>,
 }
 
+impl GenericParam<'hir> {
+    pub fn bounds_span(&self) -> Option<Span> {
+        self.bounds.iter().fold(None, |span, bound| {
+            let span = span.map(|s| s.to(bound.span())).unwrap_or_else(|| bound.span());
+
+            Some(span)
+        })
+    }
+}
+
 #[derive(Default)]
 pub struct GenericParamCount {
     pub lifetimes: usize,
@@ -513,7 +523,7 @@ pub enum SyntheticTyParamKind {
 #[derive(RustcEncodable, RustcDecodable, Debug, HashStable_Generic)]
 pub struct WhereClause<'hir> {
     pub predicates: &'hir [WherePredicate<'hir>],
-    // Only valid if predicates isn't empty.
+    // Only valid if predicates aren't empty.
     pub span: Span,
 }
 

src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs

@@ -3,7 +3,7 @@ use rustc::mir::{
     FakeReadCause, Local, LocalDecl, LocalInfo, LocalKind, Location, Operand, Place, PlaceRef,
     ProjectionElem, Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm,
 };
-use rustc::traits::error_reporting::suggestions::suggest_constraining_type_param;
+use rustc::traits::error_reporting::suggest_constraining_type_param;
 use rustc::ty::{self, Ty};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::{Applicability, DiagnosticBuilder};