From 3980342f3164a62ba7036711c16cc8af20d06418 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Tue, 3 Dec 2019 22:19:18 -0800
Subject: [PATCH 1/2] Use structured suggestion for disambiguating method calls

Fix #65635.
---
 src/librustc/ty/mod.rs                        |  11 ++
 src/librustc_typeck/check/expr.rs             |   3 +-
 src/librustc_typeck/check/method/suggest.rs   | 127 +++++++++++++-----
 .../associated-const-ambiguity-report.stderr  |  10 +-
 src/test/ui/error-codes/E0034.stderr          |  10 +-
 .../inference_unstable_featured.stderr        |  10 +-
 src/test/ui/issues/issue-18446.stderr         |   6 +-
 src/test/ui/issues/issue-3702-2.stderr        |  10 +-
 .../issue-65634-raw-ident-suggestion.stderr   |  10 +-
 ...method-ambig-two-traits-cross-crate.stderr |  10 +-
 ...method-ambig-two-traits-from-bounds.stderr |  10 +-
 .../method-ambig-two-traits-from-impls.stderr |  10 +-
 ...method-ambig-two-traits-from-impls2.stderr |  10 +-
 ...mbig-two-traits-with-default-method.stderr |  10 +-
 ...e-trait-object-with-separate-params.stderr |  15 ++-
 src/test/ui/span/issue-37767.stderr           |  30 ++++-
 src/test/ui/span/issue-7575.stderr            |  20 ++-
 .../ui/traits/trait-alias-ambiguous.stderr    |  10 +-
 18 files changed, 248 insertions(+), 74 deletions(-)

diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs
index 78a31f4e54466..15bbfa7860fa7 100644
--- a/src/librustc/ty/mod.rs
+++ b/src/librustc/ty/mod.rs
@@ -212,6 +212,17 @@ pub enum AssocKind {
     Type
 }
 
+impl AssocKind {
+    pub fn suggestion_descr(&self) -> &'static str {
+        match self {
+            ty::AssocKind::Method => "method call",
+            ty::AssocKind::Type |
+            ty::AssocKind::OpaqueTy => "associated type",
+            ty::AssocKind::Const => "associated constant",
+        }
+    }
+}
+
 impl AssocItem {
     pub fn def_kind(&self) -> DefKind {
         match self.kind {
diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs
index 5bfc60c754067..b11a8a7ab5336 100644
--- a/src/librustc_typeck/check/expr.rs
+++ b/src/librustc_typeck/check/expr.rs
@@ -1422,8 +1422,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         field: ast::Ident,
     ) -> Ty<'tcx> {
         let expr_t = self.check_expr_with_needs(base, needs);
-        let expr_t = self.structurally_resolved_type(base.span,
-                                                     expr_t);
+        let expr_t = self.structurally_resolved_type(base.span, expr_t);
         let mut private_candidate = None;
         let mut autoderef = self.autoderef(expr.span, expr_t);
         while let Some((base_t, _)) = autoderef.next() {
diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs
index f4b53b4d10604..cd26e6f237c7d 100644
--- a/src/librustc_typeck/check/method/suggest.rs
+++ b/src/librustc_typeck/check/method/suggest.rs
@@ -82,34 +82,63 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let print_disambiguation_help = |
             err: &mut DiagnosticBuilder<'_>,
             trait_name: String,
+            rcvr_ty: Ty<'_>,
+            kind: ty::AssocKind,
+            span: Span,
+            candidate: Option<usize>,
         | {
-            err.help(&format!(
-                "to disambiguate the method call, write `{}::{}({}{})` instead",
-                trait_name,
-                item_name,
-                if rcvr_ty.is_region_ptr() && args.is_some() {
-                    if rcvr_ty.is_mutable_ptr() {
-                        "&mut "
+            let mut applicability = Applicability::MachineApplicable;
+            let sugg_args = if let ty::AssocKind::Method = kind {
+                format!(
+                    "({}{})",
+                    if rcvr_ty.is_region_ptr() && args.is_some() {
+                        if rcvr_ty.is_mutable_ptr() {
+                            "&mut "
+                        } else {
+                            "&"
+                        }
                     } else {
-                        "&"
-                    }
-                } else {
-                    ""
-                },
-                args.map(|arg| arg
-                    .iter()
-                    .map(|arg| self.tcx.sess.source_map().span_to_snippet(arg.span)
-                        .unwrap_or_else(|_| "...".to_owned()))
-                    .collect::<Vec<_>>()
-                    .join(", ")
-                ).unwrap_or_else(|| "...".to_owned())
-            ));
+                        ""
+                    },
+                    args.map(|arg| arg
+                        .iter()
+                        .map(|arg| self.tcx.sess.source_map().span_to_snippet(arg.span)
+                            .unwrap_or_else(|_| {
+                                applicability = Applicability::HasPlaceholders;
+                                "...".to_owned()
+                            }))
+                        .collect::<Vec<_>>()
+                        .join(", ")
+                    ).unwrap_or_else(|| {
+                        applicability = Applicability::HasPlaceholders;
+                        "...".to_owned()
+                    }),
+                )
+            } else {
+                String::new()
+            };
+            let sugg = format!("{}::{}{}", trait_name, item_name, sugg_args);
+            err.span_suggestion(
+                span,
+                &format!(
+                    "disambiguate the {} for {}",
+                    kind.suggestion_descr(),
+                    if let Some(candidate) = candidate {
+                        format!("candidate #{}", candidate)
+                    } else {
+                        "the candidate".to_string()
+                    },
+                ),
+                sugg,
+                applicability,
+            );
         };
 
         let report_candidates = |
             span: Span,
             err: &mut DiagnosticBuilder<'_>,
             mut sources: Vec<CandidateSource>,
+            sugg_span: Span,
         | {
             sources.sort();
             sources.dedup();
@@ -150,15 +179,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                             }
                         };
 
-                        let note_str = if sources.len() > 1 {
-                            format!("candidate #{} is defined in an impl{} for the type `{}`",
-                                    idx + 1,
-                                    insertion,
-                                    impl_ty)
+                        let (note_str, idx) = if sources.len() > 1 {
+                            (format!(
+                                "candidate #{} is defined in an impl{} for the type `{}`",
+                                idx + 1,
+                                insertion,
+                                impl_ty,
+                            ), Some(idx + 1))
                         } else {
-                            format!("the candidate is defined in an impl{} for the type `{}`",
-                                    insertion,
-                                    impl_ty)
+                            (format!(
+                                "the candidate is defined in an impl{} for the type `{}`",
+                                insertion,
+                                impl_ty,
+                            ), None)
                         };
                         if let Some(note_span) = note_span {
                             // We have a span pointing to the method. Show note with snippet.
@@ -168,7 +201,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                             err.note(&note_str);
                         }
                         if let Some(trait_ref) = self.tcx.impl_trait_ref(impl_did) {
-                            print_disambiguation_help(err, self.tcx.def_path_str(trait_ref.def_id));
+                            let path = self.tcx.def_path_str(trait_ref.def_id);
+
+                            let ty = match item.kind {
+                                ty::AssocKind::Const |
+                                ty::AssocKind::Type |
+                                ty::AssocKind::OpaqueTy => rcvr_ty,
+                                ty::AssocKind::Method => self.tcx.fn_sig(item.def_id)
+                                    .inputs()
+                                    .skip_binder()
+                                    .get(0)
+                                    .filter(|ty| ty.is_region_ptr() && !rcvr_ty.is_region_ptr())
+                                    .map(|ty| *ty)
+                                    .unwrap_or(rcvr_ty),
+                            };
+                            print_disambiguation_help(err, path, ty, item.kind, sugg_span, idx);
                         }
                     }
                     CandidateSource::TraitSource(trait_did) => {
@@ -182,19 +229,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         };
                         let item_span = self.tcx.sess.source_map()
                             .def_span(self.tcx.def_span(item.def_id));
-                        if sources.len() > 1 {
+                        let idx = if sources.len() > 1 {
                             span_note!(err,
                                        item_span,
                                        "candidate #{} is defined in the trait `{}`",
                                        idx + 1,
                                        self.tcx.def_path_str(trait_did));
+                            Some(idx + 1)
                         } else {
                             span_note!(err,
                                        item_span,
                                        "the candidate is defined in the trait `{}`",
                                        self.tcx.def_path_str(trait_did));
-                        }
-                        print_disambiguation_help(err, self.tcx.def_path_str(trait_did));
+                            None
+                        };
+                        let path = self.tcx.def_path_str(trait_did);
+                        print_disambiguation_help(err, path, rcvr_ty, item.kind, sugg_span, idx);
                     }
                 }
             }
@@ -203,6 +253,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             }
         };
 
+        let sugg_span = if let SelfSource::MethodCall(expr) = source {
+            // Given `foo.bar(baz)`, `expr` is `bar`, but we want to point to the whole thing.
+            self.tcx.hir().expect_expr(self.tcx.hir().get_parent_node(expr.hir_id)).span
+        } else {
+            span
+        };
+
         match error {
             MethodError::NoMatch(NoMatchData {
                 static_candidates: static_sources,
@@ -495,9 +552,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         ));
                     }
 
-                    report_candidates(span, &mut err, static_sources);
+                    report_candidates(span, &mut err, static_sources, sugg_span);
                 } else if static_sources.len() > 1 {
-                    report_candidates(span, &mut err, static_sources);
+                    report_candidates(span, &mut err, static_sources, sugg_span);
                 }
 
                 if !unsatisfied_predicates.is_empty() {
@@ -584,7 +641,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                                "multiple applicable items in scope");
                 err.span_label(span, format!("multiple `{}` found", item_name));
 
-                report_candidates(span, &mut err, sources);
+                report_candidates(span, &mut err, sources, sugg_span);
                 err.emit();
             }
 
diff --git a/src/test/ui/associated-const/associated-const-ambiguity-report.stderr b/src/test/ui/associated-const/associated-const-ambiguity-report.stderr
index bb217bd182db6..92a8d19021a2c 100644
--- a/src/test/ui/associated-const/associated-const-ambiguity-report.stderr
+++ b/src/test/ui/associated-const/associated-const-ambiguity-report.stderr
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `Foo` for the type `i32`
    |
 LL |     const ID: i32 = 1;
    |     ^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `Foo::ID(...)` instead
 note: candidate #2 is defined in an impl of the trait `Bar` for the type `i32`
   --> $DIR/associated-const-ambiguity-report.rs:14:5
    |
 LL |     const ID: i32 = 3;
    |     ^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `Bar::ID(...)` instead
+help: disambiguate the associated constant for candidate #1
+   |
+LL | const X: i32 = Foo::ID;
+   |                ^^^^^^^
+help: disambiguate the associated constant for candidate #2
+   |
+LL | const X: i32 = Bar::ID;
+   |                ^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/error-codes/E0034.stderr b/src/test/ui/error-codes/E0034.stderr
index a58d16bfafb59..30b44fd402bb8 100644
--- a/src/test/ui/error-codes/E0034.stderr
+++ b/src/test/ui/error-codes/E0034.stderr
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `Trait1` for the type `Tes
    |
 LL |     fn foo() {}
    |     ^^^^^^^^
-   = help: to disambiguate the method call, write `Trait1::foo(...)` instead
 note: candidate #2 is defined in an impl of the trait `Trait2` for the type `Test`
   --> $DIR/E0034.rs:16:5
    |
 LL |     fn foo() {}
    |     ^^^^^^^^
-   = help: to disambiguate the method call, write `Trait2::foo(...)` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     Trait1::foo(...)()
+   |     ^^^^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     Trait2::foo(...)()
+   |     ^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/inference/inference_unstable_featured.stderr b/src/test/ui/inference/inference_unstable_featured.stderr
index b06a6298a571c..fa908440e41ea 100644
--- a/src/test/ui/inference/inference_unstable_featured.stderr
+++ b/src/test/ui/inference/inference_unstable_featured.stderr
@@ -5,9 +5,15 @@ LL |     assert_eq!('x'.ipu_flatten(), 0);
    |                    ^^^^^^^^^^^ multiple `ipu_flatten` found
    |
    = note: candidate #1 is defined in an impl of the trait `inference_unstable_iterator::IpuIterator` for the type `char`
-   = help: to disambiguate the method call, write `inference_unstable_iterator::IpuIterator::ipu_flatten('x')` instead
    = note: candidate #2 is defined in an impl of the trait `inference_unstable_itertools::IpuItertools` for the type `char`
-   = help: to disambiguate the method call, write `inference_unstable_itertools::IpuItertools::ipu_flatten('x')` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     assert_eq!(inference_unstable_iterator::IpuIterator::ipu_flatten(&'x'), 0);
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     assert_eq!(inference_unstable_itertools::IpuItertools::ipu_flatten(&'x'), 0);
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-18446.stderr b/src/test/ui/issues/issue-18446.stderr
index e6afc4c13a912..3422add9dd96b 100644
--- a/src/test/ui/issues/issue-18446.stderr
+++ b/src/test/ui/issues/issue-18446.stderr
@@ -2,7 +2,10 @@ error[E0034]: multiple applicable items in scope
   --> $DIR/issue-18446.rs:18:7
    |
 LL |     x.foo();
-   |       ^^^ multiple `foo` found
+   |     --^^^--
+   |     | |
+   |     | multiple `foo` found
+   |     help: disambiguate the method call for candidate #2: `T::foo(&x)`
    |
 note: candidate #1 is defined in an impl for the type `dyn T`
   --> $DIR/issue-18446.rs:9:5
@@ -14,7 +17,6 @@ note: candidate #2 is defined in the trait `T`
    |
 LL |     fn foo(&self);
    |     ^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `T::foo(&x)` instead
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-3702-2.stderr b/src/test/ui/issues/issue-3702-2.stderr
index 4d0ff750c254c..b18e407c3d464 100644
--- a/src/test/ui/issues/issue-3702-2.stderr
+++ b/src/test/ui/issues/issue-3702-2.stderr
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `ToPrimitive` for the type
    |
 LL |     fn to_int(&self) -> isize { 0 }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `ToPrimitive::to_int(&self)` instead
 note: candidate #2 is defined in an impl of the trait `Add` for the type `isize`
   --> $DIR/issue-3702-2.rs:14:5
    |
 LL |     fn to_int(&self) -> isize { *self }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `Add::to_int(&self)` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |         ToPrimitive::to_int(&self) + other.to_int()
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |         Add::to_int(&self) + other.to_int()
+   |         ^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-65634-raw-ident-suggestion.stderr b/src/test/ui/issues/issue-65634-raw-ident-suggestion.stderr
index c7bb653dc1f14..feaf3dc753ffb 100644
--- a/src/test/ui/issues/issue-65634-raw-ident-suggestion.stderr
+++ b/src/test/ui/issues/issue-65634-raw-ident-suggestion.stderr
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `async` for the type `r#fn
    |
 LL |     fn r#struct(&self) {
    |     ^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `async::r#struct(r#fn {})` instead
 note: candidate #2 is defined in an impl of the trait `await` for the type `r#fn`
   --> $DIR/issue-65634-raw-ident-suggestion.rs:10:5
    |
 LL |     fn r#struct(&self) {
    |     ^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `await::r#struct(r#fn {})` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     async::r#struct(&r#fn {});
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     await::r#struct(&r#fn {});
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/methods/method-ambig-two-traits-cross-crate.stderr b/src/test/ui/methods/method-ambig-two-traits-cross-crate.stderr
index 9f46a722a508e..fa3add81a28f5 100644
--- a/src/test/ui/methods/method-ambig-two-traits-cross-crate.stderr
+++ b/src/test/ui/methods/method-ambig-two-traits-cross-crate.stderr
@@ -9,9 +9,15 @@ note: candidate #1 is defined in an impl of the trait `Me2` for the type `usize`
    |
 LL | impl Me2 for usize { fn me(&self) -> usize { *self } }
    |                      ^^^^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `Me2::me(1_usize)` instead
    = note: candidate #2 is defined in an impl of the trait `ambig_impl_2_lib::Me` for the type `usize`
-   = help: to disambiguate the method call, write `ambig_impl_2_lib::Me::me(1_usize)` instead
+help: disambiguate the method call for candidate #1
+   |
+LL | fn main() { Me2::me(&1_usize); }
+   |             ^^^^^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL | fn main() { ambig_impl_2_lib::Me::me(&1_usize); }
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/methods/method-ambig-two-traits-from-bounds.stderr b/src/test/ui/methods/method-ambig-two-traits-from-bounds.stderr
index 6c493c67e29d9..b6c81c2377ee4 100644
--- a/src/test/ui/methods/method-ambig-two-traits-from-bounds.stderr
+++ b/src/test/ui/methods/method-ambig-two-traits-from-bounds.stderr
@@ -9,13 +9,19 @@ note: candidate #1 is defined in the trait `A`
    |
 LL | trait A { fn foo(&self); }
    |           ^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `A::foo(t)` instead
 note: candidate #2 is defined in the trait `B`
   --> $DIR/method-ambig-two-traits-from-bounds.rs:2:11
    |
 LL | trait B { fn foo(&self); }
    |           ^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `B::foo(t)` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     A::foo(t);
+   |     ^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     B::foo(t);
+   |     ^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/methods/method-ambig-two-traits-from-impls.stderr b/src/test/ui/methods/method-ambig-two-traits-from-impls.stderr
index 0b3724e030fa4..71c65f7ccc68d 100644
--- a/src/test/ui/methods/method-ambig-two-traits-from-impls.stderr
+++ b/src/test/ui/methods/method-ambig-two-traits-from-impls.stderr
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `A` for the type `AB`
    |
 LL |     fn foo(self) {}
    |     ^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `A::foo(AB {})` instead
 note: candidate #2 is defined in an impl of the trait `B` for the type `AB`
   --> $DIR/method-ambig-two-traits-from-impls.rs:11:5
    |
 LL |     fn foo(self) {}
    |     ^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `B::foo(AB {})` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     A::foo(AB {});
+   |     ^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     B::foo(AB {});
+   |     ^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/methods/method-ambig-two-traits-from-impls2.stderr b/src/test/ui/methods/method-ambig-two-traits-from-impls2.stderr
index 81c99b33c813e..55499215799d7 100644
--- a/src/test/ui/methods/method-ambig-two-traits-from-impls2.stderr
+++ b/src/test/ui/methods/method-ambig-two-traits-from-impls2.stderr
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `A` for the type `AB`
    |
 LL |     fn foo() {}
    |     ^^^^^^^^
-   = help: to disambiguate the method call, write `A::foo(...)` instead
 note: candidate #2 is defined in an impl of the trait `B` for the type `AB`
   --> $DIR/method-ambig-two-traits-from-impls2.rs:11:5
    |
 LL |     fn foo() {}
    |     ^^^^^^^^
-   = help: to disambiguate the method call, write `B::foo(...)` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     A::foo(...)();
+   |     ^^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     B::foo(...)();
+   |     ^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/methods/method-ambig-two-traits-with-default-method.stderr b/src/test/ui/methods/method-ambig-two-traits-with-default-method.stderr
index dc8aef2503739..3dbb17371004a 100644
--- a/src/test/ui/methods/method-ambig-two-traits-with-default-method.stderr
+++ b/src/test/ui/methods/method-ambig-two-traits-with-default-method.stderr
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `Foo` for the type `usize`
    |
 LL | trait Foo { fn method(&self) {} }
    |             ^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `Foo::method(1_usize)` instead
 note: candidate #2 is defined in an impl of the trait `Bar` for the type `usize`
   --> $DIR/method-ambig-two-traits-with-default-method.rs:6:13
    |
 LL | trait Bar { fn method(&self) {} }
    |             ^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `Bar::method(1_usize)` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     Foo::method(&1_usize);
+   |     ^^^^^^^^^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     Bar::method(&1_usize);
+   |     ^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/methods/method-deref-to-same-trait-object-with-separate-params.stderr b/src/test/ui/methods/method-deref-to-same-trait-object-with-separate-params.stderr
index c9d7da84e09f4..e7f295df8c482 100644
--- a/src/test/ui/methods/method-deref-to-same-trait-object-with-separate-params.stderr
+++ b/src/test/ui/methods/method-deref-to-same-trait-object-with-separate-params.stderr
@@ -25,19 +25,28 @@ note: candidate #1 is defined in an impl of the trait `internal::X` for the type
    |
 LL |         fn foo(self: Smaht<Self, u64>) -> u64 {
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `internal::X::foo(x)` instead
 note: candidate #2 is defined in an impl of the trait `nuisance_foo::NuisanceFoo` for the type `_`
   --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:70:9
    |
 LL |         fn foo(self) {}
    |         ^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `nuisance_foo::NuisanceFoo::foo(x)` instead
 note: candidate #3 is defined in the trait `FinalFoo`
   --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:57:5
    |
 LL |     fn foo(&self) -> u8;
    |     ^^^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `FinalFoo::foo(x)` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     let z = internal::X::foo(x);
+   |             ^^^^^^^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     let z = nuisance_foo::NuisanceFoo::foo(x);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #3
+   |
+LL |     let z = FinalFoo::foo(x);
+   |             ^^^^^^^^^^^^^^^^
 
 error[E0308]: mismatched types
   --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:137:24
diff --git a/src/test/ui/span/issue-37767.stderr b/src/test/ui/span/issue-37767.stderr
index 0bbff45436c23..9ed6c8b826f79 100644
--- a/src/test/ui/span/issue-37767.stderr
+++ b/src/test/ui/span/issue-37767.stderr
@@ -9,13 +9,19 @@ note: candidate #1 is defined in the trait `A`
    |
 LL |     fn foo(&mut self) {}
    |     ^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `A::foo(&a)` instead
 note: candidate #2 is defined in the trait `B`
   --> $DIR/issue-37767.rs:6:5
    |
 LL |     fn foo(&mut self) {}
    |     ^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `B::foo(&a)` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     A::foo(&a)
+   |     ^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     B::foo(&a)
+   |     ^^^^^^^^^^
 
 error[E0034]: multiple applicable items in scope
   --> $DIR/issue-37767.rs:22:7
@@ -28,13 +34,19 @@ note: candidate #1 is defined in the trait `C`
    |
 LL |     fn foo(&self) {}
    |     ^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `C::foo(&a)` instead
 note: candidate #2 is defined in the trait `D`
   --> $DIR/issue-37767.rs:18:5
    |
 LL |     fn foo(&self) {}
    |     ^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `D::foo(&a)` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     C::foo(&a)
+   |     ^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     D::foo(&a)
+   |     ^^^^^^^^^^
 
 error[E0034]: multiple applicable items in scope
   --> $DIR/issue-37767.rs:34:7
@@ -47,13 +59,19 @@ note: candidate #1 is defined in the trait `E`
    |
 LL |     fn foo(self) {}
    |     ^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `E::foo(a)` instead
 note: candidate #2 is defined in the trait `F`
   --> $DIR/issue-37767.rs:30:5
    |
 LL |     fn foo(self) {}
    |     ^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `F::foo(a)` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     E::foo(a)
+   |     ^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     F::foo(a)
+   |     ^^^^^^^^^
 
 error: aborting due to 3 previous errors
 
diff --git a/src/test/ui/span/issue-7575.stderr b/src/test/ui/span/issue-7575.stderr
index 36db5bea86294..53a6238422b57 100644
--- a/src/test/ui/span/issue-7575.stderr
+++ b/src/test/ui/span/issue-7575.stderr
@@ -10,24 +10,33 @@ note: candidate #1 is defined in the trait `CtxtFn`
    |
 LL |     fn f9(_: usize) -> usize;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `CtxtFn::f9(u, 342)` instead
 note: candidate #2 is defined in the trait `OtherTrait`
   --> $DIR/issue-7575.rs:8:5
    |
 LL |     fn f9(_: usize) -> usize;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `OtherTrait::f9(u, 342)` instead
 note: candidate #3 is defined in the trait `UnusedTrait`
   --> $DIR/issue-7575.rs:17:5
    |
 LL |     fn f9(_: usize) -> usize;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `UnusedTrait::f9(u, 342)` instead
    = help: items from traits can only be used if the trait is implemented and in scope
    = note: the following traits define an item `f9`, perhaps you need to implement one of them:
            candidate #1: `CtxtFn`
            candidate #2: `OtherTrait`
            candidate #3: `UnusedTrait`
+help: disambiguate the method call for candidate #1
+   |
+LL |     u.f8(42) + CtxtFn::f9(u, 342) + m.fff(42)
+   |                ^^^^^^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     u.f8(42) + OtherTrait::f9(u, 342) + m.fff(42)
+   |                ^^^^^^^^^^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #3
+   |
+LL |     u.f8(42) + UnusedTrait::f9(u, 342) + m.fff(42)
+   |                ^^^^^^^^^^^^^^^^^^^^^^^
 
 error[E0599]: no method named `fff` found for type `Myisize` in the current scope
   --> $DIR/issue-7575.rs:62:30
@@ -60,8 +69,11 @@ note: the candidate is defined in the trait `ManyImplTrait`
    |
 LL |     fn is_str() -> bool {
    |     ^^^^^^^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `ManyImplTrait::is_str(t)` instead
    = help: items from traits can only be used if the type parameter is bounded by the trait
+help: disambiguate the method call for the candidate
+   |
+LL |     ManyImplTrait::is_str(t)
+   |
 help: the following trait defines an item `is_str`, perhaps you need to restrict type parameter `T` with it:
    |
 LL | fn param_bound<T: ManyImplTrait + ManyImplTrait>(t: T) -> bool {
diff --git a/src/test/ui/traits/trait-alias-ambiguous.stderr b/src/test/ui/traits/trait-alias-ambiguous.stderr
index cde7dd0824924..48a029104aeca 100644
--- a/src/test/ui/traits/trait-alias-ambiguous.stderr
+++ b/src/test/ui/traits/trait-alias-ambiguous.stderr
@@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `inner::A` for the type `u
    |
 LL |         fn foo(&self) {}
    |         ^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `inner::A::foo(t)` instead
 note: candidate #2 is defined in an impl of the trait `inner::B` for the type `u8`
   --> $DIR/trait-alias-ambiguous.rs:11:9
    |
 LL |         fn foo(&self) {}
    |         ^^^^^^^^^^^^^
-   = help: to disambiguate the method call, write `inner::B::foo(t)` instead
+help: disambiguate the method call for candidate #1
+   |
+LL |     inner::A::foo(&t);
+   |     ^^^^^^^^^^^^^^^^^
+help: disambiguate the method call for candidate #2
+   |
+LL |     inner::B::foo(&t);
+   |     ^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 

From 8c4f1d5f875e29e9a6e063744b71dc0a3c7deb6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Wed, 11 Dec 2019 18:20:29 -0800
Subject: [PATCH 2/2] review comments

---
 src/librustc_typeck/check/method/suggest.rs   | 134 ++++++++++--------
 src/test/ui/error-codes/E0034.stderr          |   8 +-
 ...method-ambig-two-traits-from-impls2.stderr |   8 +-
 3 files changed, 84 insertions(+), 66 deletions(-)

diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs
index cd26e6f237c7d..9cd8c9abfd783 100644
--- a/src/librustc_typeck/check/method/suggest.rs
+++ b/src/librustc_typeck/check/method/suggest.rs
@@ -15,7 +15,7 @@ use rustc::traits::Obligation;
 use rustc::ty::{self, Ty, TyCtxt, ToPolyTraitRef, ToPredicate, TypeFoldable};
 use rustc::ty::print::with_crate_prefix;
 use syntax_pos::{Span, FileName};
-use syntax::ast;
+use syntax::{ast, source_map};
 use syntax::util::lev_distance;
 
 use rustc_error_codes::*;
@@ -79,61 +79,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             return None;
         }
 
-        let print_disambiguation_help = |
-            err: &mut DiagnosticBuilder<'_>,
-            trait_name: String,
-            rcvr_ty: Ty<'_>,
-            kind: ty::AssocKind,
-            span: Span,
-            candidate: Option<usize>,
-        | {
-            let mut applicability = Applicability::MachineApplicable;
-            let sugg_args = if let ty::AssocKind::Method = kind {
-                format!(
-                    "({}{})",
-                    if rcvr_ty.is_region_ptr() && args.is_some() {
-                        if rcvr_ty.is_mutable_ptr() {
-                            "&mut "
-                        } else {
-                            "&"
-                        }
-                    } else {
-                        ""
-                    },
-                    args.map(|arg| arg
-                        .iter()
-                        .map(|arg| self.tcx.sess.source_map().span_to_snippet(arg.span)
-                            .unwrap_or_else(|_| {
-                                applicability = Applicability::HasPlaceholders;
-                                "...".to_owned()
-                            }))
-                        .collect::<Vec<_>>()
-                        .join(", ")
-                    ).unwrap_or_else(|| {
-                        applicability = Applicability::HasPlaceholders;
-                        "...".to_owned()
-                    }),
-                )
-            } else {
-                String::new()
-            };
-            let sugg = format!("{}::{}{}", trait_name, item_name, sugg_args);
-            err.span_suggestion(
-                span,
-                &format!(
-                    "disambiguate the {} for {}",
-                    kind.suggestion_descr(),
-                    if let Some(candidate) = candidate {
-                        format!("candidate #{}", candidate)
-                    } else {
-                        "the candidate".to_string()
-                    },
-                ),
-                sugg,
-                applicability,
-            );
-        };
-
         let report_candidates = |
             span: Span,
             err: &mut DiagnosticBuilder<'_>,
@@ -215,7 +160,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                     .map(|ty| *ty)
                                     .unwrap_or(rcvr_ty),
                             };
-                            print_disambiguation_help(err, path, ty, item.kind, sugg_span, idx);
+                            print_disambiguation_help(
+                                item_name,
+                                args,
+                                err,
+                                path,
+                                ty,
+                                item.kind,
+                                sugg_span,
+                                idx,
+                                self.tcx.sess.source_map(),
+                            );
                         }
                     }
                     CandidateSource::TraitSource(trait_did) => {
@@ -244,7 +199,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                             None
                         };
                         let path = self.tcx.def_path_str(trait_did);
-                        print_disambiguation_help(err, path, rcvr_ty, item.kind, sugg_span, idx);
+                        print_disambiguation_help(
+                            item_name,
+                            args,
+                            err,
+                            path,
+                            rcvr_ty,
+                            item.kind,
+                            sugg_span,
+                            idx,
+                            self.tcx.sess.source_map(),
+                        );
                     }
                 }
             }
@@ -1180,3 +1145,56 @@ impl hir::intravisit::Visitor<'tcx> for UsePlacementFinder<'tcx> {
         hir::intravisit::NestedVisitorMap::None
     }
 }
+
+fn print_disambiguation_help(
+    item_name: ast::Ident,
+    args: Option<&'tcx [hir::Expr]>,
+    err: &mut DiagnosticBuilder<'_>,
+    trait_name: String,
+    rcvr_ty: Ty<'_>,
+    kind: ty::AssocKind,
+    span: Span,
+    candidate: Option<usize>,
+    source_map: &source_map::SourceMap,
+) {
+    let mut applicability = Applicability::MachineApplicable;
+    let sugg_args = if let (ty::AssocKind::Method, Some(args)) = (kind, args) {
+        format!(
+            "({}{})",
+            if rcvr_ty.is_region_ptr() {
+                if rcvr_ty.is_mutable_ptr() {
+                    "&mut "
+                } else {
+                    "&"
+                }
+            } else {
+                ""
+            },
+            args.iter()
+                .map(|arg| source_map.span_to_snippet(arg.span)
+                    .unwrap_or_else(|_| {
+                        applicability = Applicability::HasPlaceholders;
+                        "_".to_owned()
+                    }))
+                .collect::<Vec<_>>()
+                .join(", "),
+        )
+    } else {
+        String::new()
+    };
+    let sugg = format!("{}::{}{}", trait_name, item_name, sugg_args);
+    err.span_suggestion(
+        span,
+        &format!(
+            "disambiguate the {} for {}",
+            kind.suggestion_descr(),
+            if let Some(candidate) = candidate {
+                format!("candidate #{}", candidate)
+            } else {
+                "the candidate".to_string()
+            },
+        ),
+        sugg,
+        applicability,
+    );
+}
diff --git a/src/test/ui/error-codes/E0034.stderr b/src/test/ui/error-codes/E0034.stderr
index 30b44fd402bb8..6db2ef5051d83 100644
--- a/src/test/ui/error-codes/E0034.stderr
+++ b/src/test/ui/error-codes/E0034.stderr
@@ -16,12 +16,12 @@ LL |     fn foo() {}
    |     ^^^^^^^^
 help: disambiguate the method call for candidate #1
    |
-LL |     Trait1::foo(...)()
-   |     ^^^^^^^^^^^^^^^^
+LL |     Trait1::foo()
+   |     ^^^^^^^^^^^
 help: disambiguate the method call for candidate #2
    |
-LL |     Trait2::foo(...)()
-   |     ^^^^^^^^^^^^^^^^
+LL |     Trait2::foo()
+   |     ^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/methods/method-ambig-two-traits-from-impls2.stderr b/src/test/ui/methods/method-ambig-two-traits-from-impls2.stderr
index 55499215799d7..44f85071505d2 100644
--- a/src/test/ui/methods/method-ambig-two-traits-from-impls2.stderr
+++ b/src/test/ui/methods/method-ambig-two-traits-from-impls2.stderr
@@ -16,12 +16,12 @@ LL |     fn foo() {}
    |     ^^^^^^^^
 help: disambiguate the method call for candidate #1
    |
-LL |     A::foo(...)();
-   |     ^^^^^^^^^^^
+LL |     A::foo();
+   |     ^^^^^^
 help: disambiguate the method call for candidate #2
    |
-LL |     B::foo(...)();
-   |     ^^^^^^^^^^^
+LL |     B::foo();
+   |     ^^^^^^
 
 error: aborting due to previous error