Deduplicate some code and apply review comments

14 files changed, 117 insertions, 125 deletions

diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs
index 91106d35a7e..abf265be7c5 100644
--- a/src/librustc/traits/error_reporting.rs
+++ b/src/librustc/traits/error_reporting.rs
@@ -984,84 +984,66 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             }
         };
 
+        let mut suggest_restriction = |generics: &hir::Generics, msg| {
+            err.span_suggestion(
+                generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(),
+                &format!("consider further restricting {}", msg),
+                format!(
+                    "{} {} ",
+                    if !generics.where_clause.predicates.is_empty() {
+                        ","
+                    } else {
+                        " where"
+                    },
+                    trait_ref.to_predicate(),
+                ),
+                Applicability::MachineApplicable,
+            );
+        };
+
+        // FIXME: Add check for trait bound that is already present, particularly `?Sized` so we
+        //        don't suggest `T: Sized + ?Sized`.
         let mut hir_id = body_id;
         while let Some(node) = self.tcx.hir().find(hir_id) {
-            debug!("suggest_restricting_param_bound node={:?}", node);
             match node {
                 hir::Node::Item(hir::Item {
-                    kind: hir::ItemKind::Fn(decl, _, generics, _), ..
+                    kind: hir::ItemKind::Fn(_, _, generics, _), ..
                 }) |
                 hir::Node::TraitItem(hir::TraitItem {
                     generics,
-                    kind: hir::TraitItemKind::Method(hir::MethodSig { decl, .. }, _), ..
+                    kind: hir::TraitItemKind::Method(..), ..
                 }) |
                 hir::Node::ImplItem(hir::ImplItem {
                     generics,
-                    kind: hir::ImplItemKind::Method(hir::MethodSig { decl, .. }, _), ..
-                }) if param_ty.map(|p| p.name.as_str() == "Self").unwrap_or(false) => {
-                    if !generics.where_clause.predicates.is_empty() {
-                        err.span_suggestion(
-                            generics.where_clause.span().unwrap().shrink_to_hi(),
-                            "consider further restricting `Self`",
-                            format!(", {}", trait_ref.to_predicate()),
-                            Applicability::MachineApplicable,
-                        );
-                    } else {
-                        err.span_suggestion(
-                            decl.output.span().shrink_to_hi(),
-                            "consider further restricting `Self`",
-                            format!(" where {}", trait_ref.to_predicate()),
-                            Applicability::MachineApplicable,
-                        );
-                    }
+                    kind: hir::ImplItemKind::Method(..), ..
+                }) if param_ty.map_or(false, |p| p.name.as_str() == "Self") => {
+                    // Restricting `Self` for a single method.
+                    suggest_restriction(&generics, "`Self`");
                     return;
                 }
+
                 hir::Node::Item(hir::Item {
-                    kind: hir::ItemKind::Fn(decl, _, generics, _), ..
+                    kind: hir::ItemKind::Fn(_, _, generics, _), ..
                 }) |
                 hir::Node::TraitItem(hir::TraitItem {
                     generics,
-                    kind: hir::TraitItemKind::Method(hir::MethodSig { decl, .. }, _), ..
+                    kind: hir::TraitItemKind::Method(..), ..
                 }) |
                 hir::Node::ImplItem(hir::ImplItem {
                     generics,
-                    kind: hir::ImplItemKind::Method(hir::MethodSig { decl, .. }, _), ..
-                }) if projection.is_some() => {
-                    if !generics.where_clause.predicates.is_empty() {
-                        err.span_suggestion(
-                            generics.where_clause.span().unwrap().shrink_to_hi(),
-                            "consider further restricting the associated type",
-                            format!(", {}", trait_ref.to_predicate()),
-                            Applicability::MachineApplicable,
-                        );
-                    } else {
-                        err.span_suggestion(
-                            decl.output.span().shrink_to_hi(),
-                            "consider further restricting the associated type",
-                            format!(" where {}", trait_ref.to_predicate()),
-                            Applicability::MachineApplicable,
-                        );
-                    }
-                    return;
-                }
+                    kind: hir::ImplItemKind::Method(..), ..
+                }) |
+                hir::Node::Item(hir::Item {
+                    kind: hir::ItemKind::Trait(_, _, generics, _, _), ..
+                }) |
                 hir::Node::Item(hir::Item {
                     kind: hir::ItemKind::Impl(_, _, _, generics, ..), ..
                 }) if projection.is_some() => {
-                    err.span_suggestion(
-                        generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(),
-                        "consider further restricting the associated type",
-                        format!(
-                            "{} {}", if generics.where_clause.predicates.is_empty() {
-                                " where"
-                            } else {
-                                " ,"
-                            },
-                            trait_ref.to_predicate(),
-                        ),
-                        Applicability::MachineApplicable,
-                    );
+                    // Missing associated type bound.
+                    suggest_restriction(&generics, "the associated type");
                     return;
                 }
+
                 hir::Node::Item(hir::Item { kind: hir::ItemKind::Struct(_, generics), span, .. }) |
                 hir::Node::Item(hir::Item { kind: hir::ItemKind::Enum(_, generics), span, .. }) |
                 hir::Node::Item(hir::Item { kind: hir::ItemKind::Union(_, generics), span, .. }) |
@@ -1086,73 +1068,82 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                 hir::Node::TraitItem(hir::TraitItem { generics, span, .. }) |
                 hir::Node::ImplItem(hir::ImplItem { generics, span, .. })
                 if param_ty.is_some() => {
+                    // Missing generic type parameter bound.
                     let restrict_msg = "consider further restricting this bound";
                     let param_name = param_ty.unwrap().name.as_str();
-                    for param in &generics.params {
-                        if param_name == param.name.ident().as_str() {
-                            if param_name.starts_with("impl ") {
+                    for param in generics.params.iter().filter(|p| {
+                        param_name == p.name.ident().as_str()
+                    }) {
+                        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.ident(), trait_ref),
+                                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,
-                                    restrict_msg,
-                                    // `impl CurrentTrait + MissingTrait`
-                                    format!("{} + {}", param.name.ident(), trait_ref),
+                                    "consider restricting this bound",
+                                    format!("{}", trait_ref.to_predicate()),
+                                    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, Trait {}`
+                                err.span_suggestion(
+                                    generics.where_clause.span().unwrap().shrink_to_hi(),
+                                    &format!(
+                                        "consider further restricting type parameter `{}`",
+                                        param_name,
+                                    ),
+                                    format!(", {}", trait_ref.to_predicate()),
                                     Applicability::MachineApplicable,
                                 );
                             } else {
-                                if generics.where_clause.predicates.is_empty() &&
-                                    param.bounds.is_empty()
-                                {
-                                    err.span_suggestion(
-                                        param.span,
-                                        "consider restricting this bound",
-                                        format!("{}", trait_ref.to_predicate()),
-                                        Applicability::MachineApplicable,
-                                    );
-                                } else if !generics.where_clause.predicates.is_empty() {
-                                    err.span_suggestion(
-                                        generics.where_clause.span().unwrap().shrink_to_hi(),
-                                        &format!(
-                                            "consider further restricting type parameter `{}`",
-                                            param_name,
-                                        ),
-                                        format!(", {}", trait_ref.to_predicate()),
-                                        Applicability::MachineApplicable,
-                                    );
+                                // 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 = self.tcx.sess.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!(
+                                        "{} + ",
+                                        trait_ref.to_predicate(),
+                                    ), Applicability::MachineApplicable);
                                 } else {
-                                    let sp = param.span.with_hi(span.hi());
-                                    let span = self.tcx.sess.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!(
-                                            "{} + ",
-                                            trait_ref.to_predicate(),
-                                        ), Applicability::MachineApplicable);
-                                    } else {
-                                        err.span_label(param.span, &format!(
-                                            "consider adding a `where {}` bound",
-                                            trait_ref.to_predicate(),
-                                        ));
-                                    }
+                                    err.span_label(param.span, &format!(
+                                        "consider adding a `where {}` bound",
+                                        trait_ref.to_predicate(),
+                                    ));
                                 }
                             }
-                            return;
                         }
+                        return;
                     }
                 }
+
                 hir::Node::Crate => return,
+
                 _ => {}
             }
 
             hir_id = self.tcx.hir().get_parent_item(hir_id);
         }
-        // FIXME: Add special check for `?Sized` so we don't suggest `T: Sized + ?Sized`.
-
-        // Fallback in case we didn't find the type argument. Can happen on associated types
-        // bounds and when `Self` needs to be restricted, like in the ui test
-        // `associated-types-projection-to-unrelated-trait-in-method-without-default.rs`.
-        err.help(&format!("consider adding a `where {}` bound", trait_ref.to_predicate()));
     }
 
     /// When encountering an assignment of an unsized trait, like `let x = ""[..];`, provide a
diff --git a/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr b/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr
index 1795fb5aceb..9f6a73cfe39 100644
--- a/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr
+++ b/src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr
@@ -10,7 +10,7 @@ error[E0277]: `<<T as Case1>::C as std::iter::Iterator>::Item` is not an iterato
   --> $DIR/bad-bounds-on-assoc-in-trait.rs:37:1
    |
 LL |   fn assume_case1<T: Case1>() {
-   |   ^                           - help: consider further restricting the associated type: `where <<T as Case1>::C as std::iter::Iterator>::Item: std::iter::Iterator`
+   |   ^                          - help: consider further restricting the associated type: `where <<T as Case1>::C as std::iter::Iterator>::Item: std::iter::Iterator`
    |  _|
    | |
 LL | |
@@ -30,7 +30,7 @@ LL |   trait Case1 {
    |   ----------- required by `Case1`
 ...
 LL |   fn assume_case1<T: Case1>() {
-   |   ^                           - help: consider further restricting the associated type: `where <<T as Case1>::C as std::iter::Iterator>::Item: std::marker::Send`
+   |   ^                          - help: consider further restricting the associated type: `where <<T as Case1>::C as std::iter::Iterator>::Item: std::marker::Send`
    |  _|
    | |
 LL | |
@@ -50,7 +50,7 @@ LL |   trait Case1 {
    |   ----------- required by `Case1`
 ...
 LL |   fn assume_case1<T: Case1>() {
-   |   ^                           - help: consider further restricting the associated type: `where <<T as Case1>::C as std::iter::Iterator>::Item: std::marker::Sync`
+   |   ^                          - help: consider further restricting the associated type: `where <<T as Case1>::C as std::iter::Iterator>::Item: std::marker::Sync`
    |  _|
    | |
 LL | |
diff --git a/src/test/ui/associated-types/associated-types-bound-failure.fixed b/src/test/ui/associated-types/associated-types-bound-failure.fixed
index 68ee38d16b3..cc47f31d004 100644
--- a/src/test/ui/associated-types/associated-types-bound-failure.fixed
+++ b/src/test/ui/associated-types/associated-types-bound-failure.fixed
@@ -14,7 +14,7 @@ pub trait GetToInt
 }
 
 fn foo<G>(g: G) -> isize
-    where G : GetToInt, <G as GetToInt>::R: ToInt
+    where G : GetToInt, <G as GetToInt>::R: ToInt 
 {
     ToInt::to_int(&g.get()) //~ ERROR E0277
 }
diff --git a/src/test/ui/associated-types/associated-types-for-unimpl-trait.fixed b/src/test/ui/associated-types/associated-types-for-unimpl-trait.fixed
index b27d58018c2..aa23326506f 100644
--- a/src/test/ui/associated-types/associated-types-for-unimpl-trait.fixed
+++ b/src/test/ui/associated-types/associated-types-for-unimpl-trait.fixed
@@ -7,7 +7,7 @@ trait Get {
 }
 
 trait Other {
-    fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value)  where Self: Get{}
+    fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) where Self: Get  {}
     //~^ ERROR the trait bound `Self: Get` is not satisfied
 }
 
diff --git a/src/test/ui/associated-types/associated-types-for-unimpl-trait.stderr b/src/test/ui/associated-types/associated-types-for-unimpl-trait.stderr
index f8fc2b37e73..83d5390417e 100644
--- a/src/test/ui/associated-types/associated-types-for-unimpl-trait.stderr
+++ b/src/test/ui/associated-types/associated-types-for-unimpl-trait.stderr
@@ -2,9 +2,9 @@ error[E0277]: the trait bound `Self: Get` is not satisfied
   --> $DIR/associated-types-for-unimpl-trait.rs:10:5
    |
 LL |     fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^
-   |     |                                                        |
-   |     |                                                        help: consider further restricting `Self`: `where Self: Get`
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^^
+   |     |                                                       |
+   |     |                                                       help: consider further restricting `Self`: `where Self: Get`
    |     the trait `Get` is not implemented for `Self`
 
 error: aborting due to previous error
diff --git a/src/test/ui/associated-types/associated-types-no-suitable-supertrait-2.stderr b/src/test/ui/associated-types/associated-types-no-suitable-supertrait-2.stderr
index 43950defc7c..6aa0403088d 100644
--- a/src/test/ui/associated-types/associated-types-no-suitable-supertrait-2.stderr
+++ b/src/test/ui/associated-types/associated-types-no-suitable-supertrait-2.stderr
@@ -2,9 +2,9 @@ error[E0277]: the trait bound `Self: Get` is not satisfied
   --> $DIR/associated-types-no-suitable-supertrait-2.rs:17:5
    |
 LL |     fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^
-   |     |                                                        |
-   |     |                                                        help: consider further restricting `Self`: `where Self: Get`
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^^
+   |     |                                                       |
+   |     |                                                       help: consider further restricting `Self`: `where Self: Get`
    |     the trait `Get` is not implemented for `Self`
 
 error: aborting due to previous error
diff --git a/src/test/ui/associated-types/associated-types-no-suitable-supertrait.stderr b/src/test/ui/associated-types/associated-types-no-suitable-supertrait.stderr
index 82e366dbea4..8c242be9796 100644
--- a/src/test/ui/associated-types/associated-types-no-suitable-supertrait.stderr
+++ b/src/test/ui/associated-types/associated-types-no-suitable-supertrait.stderr
@@ -2,9 +2,9 @@ error[E0277]: the trait bound `Self: Get` is not satisfied
   --> $DIR/associated-types-no-suitable-supertrait.rs:17:5
    |
 LL |     fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^
-   |     |                                                        |
-   |     |                                                        help: consider further restricting `Self`: `where Self: Get`
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-^^
+   |     |                                                       |
+   |     |                                                       help: consider further restricting `Self`: `where Self: Get`
    |     the trait `Get` is not implemented for `Self`
 
 error[E0277]: the trait bound `(T, U): Get` is not satisfied
diff --git a/src/test/ui/associated-types/associated-types-projection-to-unrelated-trait-in-method-without-default.fixed b/src/test/ui/associated-types/associated-types-projection-to-unrelated-trait-in-method-without-default.fixed
index 9bc308465eb..f357045a456 100644
--- a/src/test/ui/associated-types/associated-types-projection-to-unrelated-trait-in-method-without-default.fixed
+++ b/src/test/ui/associated-types/associated-types-projection-to-unrelated-trait-in-method-without-default.fixed
@@ -7,7 +7,7 @@ trait Get {
 }
 
 trait Other {
-    fn okay<U:Get>(&self, foo: U, bar: <Self as Get>::Value) where Self: Get;
+    fn okay<U:Get>(&self, foo: U, bar: <Self as Get>::Value) where Self: Get ;
     //~^ ERROR E0277
 }
 
diff --git a/src/test/ui/associated-types/associated-types-unsized.fixed b/src/test/ui/associated-types/associated-types-unsized.fixed
index 385de541e56..f780d171fee 100644
--- a/src/test/ui/associated-types/associated-types-unsized.fixed
+++ b/src/test/ui/associated-types/associated-types-unsized.fixed
@@ -6,7 +6,7 @@ trait Get {
     fn get(&self) -> <Self as Get>::Value;
 }
 
-fn foo<T:Get>(t: T)  where <T as Get>::Value: std::marker::Sized{
+fn foo<T:Get>(t: T) where <T as Get>::Value: std::marker::Sized  {
     let x = t.get(); //~ ERROR the size for values of type
 }
 
diff --git a/src/test/ui/associated-types/associated-types-unsized.stderr b/src/test/ui/associated-types/associated-types-unsized.stderr
index 18595721bce..2352ac4ad38 100644
--- a/src/test/ui/associated-types/associated-types-unsized.stderr
+++ b/src/test/ui/associated-types/associated-types-unsized.stderr
@@ -2,7 +2,7 @@ error[E0277]: the size for values of type `<T as Get>::Value` cannot be known at
   --> $DIR/associated-types-unsized.rs:10:9
    |
 LL | fn foo<T:Get>(t: T) {
-   |                     - help: consider further restricting the associated type: `where <T as Get>::Value: std::marker::Sized`
+   |                    - help: consider further restricting the associated type: `where <T as Get>::Value: std::marker::Sized`
 LL |     let x = t.get();
    |         ^ doesn't have a size known at compile-time
    |
diff --git a/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.fixed b/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.fixed
index 757d8b8a235..7a108d880be 100644
--- a/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.fixed
+++ b/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.fixed
@@ -7,7 +7,7 @@ trait Trait {
     type AssocType;
     fn dummy(&self) { }
 }
-fn bar<T:Trait+Send>()  where <T as Trait>::AssocType: std::marker::Send{
+fn bar<T:Trait+Send>() where <T as Trait>::AssocType: std::marker::Send  {
     is_send::<T::AssocType>(); //~ ERROR E0277
 }
 
diff --git a/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.stderr b/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.stderr
index dd6dcbe2ef5..2e54cdf0132 100644
--- a/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.stderr
+++ b/src/test/ui/typeck/typeck-default-trait-impl-assoc-type.stderr
@@ -2,7 +2,7 @@ error[E0277]: `<T as Trait>::AssocType` cannot be sent between threads safely
   --> $DIR/typeck-default-trait-impl-assoc-type.rs:11:5
    |
 LL | fn bar<T:Trait+Send>() {
-   |                        - help: consider further restricting the associated type: `where <T as Trait>::AssocType: std::marker::Send`
+   |                       - help: consider further restricting the associated type: `where <T as Trait>::AssocType: std::marker::Send`
 LL |     is_send::<T::AssocType>();
    |     ^^^^^^^^^^^^^^^^^^^^^^^ `<T as Trait>::AssocType` cannot be sent between threads safely
 ...
diff --git a/src/test/ui/wf/wf-trait-associated-type-trait.stderr b/src/test/ui/wf/wf-trait-associated-type-trait.stderr
index d8ab9550482..93cb948cdbf 100644
--- a/src/test/ui/wf/wf-trait-associated-type-trait.stderr
+++ b/src/test/ui/wf/wf-trait-associated-type-trait.stderr
@@ -3,11 +3,12 @@ error[E0277]: the trait bound `<Self as SomeTrait>::Type1: std::marker::Copy` is
    |
 LL | struct IsCopy<T:Copy> { x: T }
    | --------------------- required by `IsCopy`
-...
+LL | 
+LL | trait SomeTrait {
+   |                - help: consider further restricting the associated type: `where <Self as SomeTrait>::Type1: std::marker::Copy`
+LL |     type Type1;
 LL |     type Type2 = IsCopy<Self::Type1>;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `<Self as SomeTrait>::Type1`
-   |
-   = help: consider adding a `where <Self as SomeTrait>::Type1: std::marker::Copy` bound
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/wf/wf-trait-default-fn-arg.stderr b/src/test/ui/wf/wf-trait-default-fn-arg.stderr
index 3efcdf72eee..9f3545b9c6a 100644
--- a/src/test/ui/wf/wf-trait-default-fn-arg.stderr
+++ b/src/test/ui/wf/wf-trait-default-fn-arg.stderr
@@ -5,7 +5,7 @@ LL |   struct Bar<T:Eq+?Sized> { value: Box<T> }
    |   ----------------------- required by `Bar`
 ...
 LL |       fn bar(&self, x: &Bar<Self>) {
-   |       ^                            - help: consider further restricting `Self`: `where Self: std::cmp::Eq`
+   |       ^                           - help: consider further restricting `Self`: `where Self: std::cmp::Eq`
    |  _____|
    | |
 LL | |