review comments

7 files changed, 81 insertions, 41 deletions

diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs
index 3f1b91256c4..811a48a39f0 100644
--- a/src/librustc_errors/diagnostic.rs
+++ b/src/librustc_errors/diagnostic.rs
@@ -298,9 +298,13 @@ impl Diagnostic {
     /// * may contain a name of a function, variable, or type, but not whole expressions
     ///
     /// See `CodeSuggestion` for more information.
-    pub fn span_suggestion(&mut self, sp: Span, msg: &str,
-                                       suggestion: String,
-                                       applicability: Applicability) -> &mut Self {
+    pub fn span_suggestion(
+        &mut self,
+        sp: Span,
+        msg: &str,
+        suggestion: String,
+        applicability: Applicability,
+    ) -> &mut Self {
         self.suggestions.push(CodeSuggestion {
             substitutions: vec![Substitution {
                 parts: vec![SubstitutionPart {
@@ -315,10 +319,35 @@ impl Diagnostic {
         self
     }
 
+    pub fn span_suggestion_verbose(
+        &mut self,
+        sp: Span,
+        msg: &str,
+        suggestion: String,
+        applicability: Applicability,
+    ) -> &mut Self {
+        self.suggestions.push(CodeSuggestion {
+            substitutions: vec![Substitution {
+                parts: vec![SubstitutionPart {
+                    snippet: suggestion,
+                    span: sp,
+                }],
+            }],
+            msg: msg.to_owned(),
+            style: SuggestionStyle::ShowAlways,
+            applicability,
+        });
+        self
+    }
+
     /// Prints out a message with multiple suggested edits of the code.
-    pub fn span_suggestions(&mut self, sp: Span, msg: &str,
-        suggestions: impl Iterator<Item = String>, applicability: Applicability) -> &mut Self
-    {
+    pub fn span_suggestions(
+        &mut self,
+        sp: Span,
+        msg: &str,
+        suggestions: impl Iterator<Item = String>,
+        applicability: Applicability,
+    ) -> &mut Self {
         self.suggestions.push(CodeSuggestion {
             substitutions: suggestions.map(|snippet| Substitution {
                 parts: vec![SubstitutionPart {
diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs
index 0c7aa3582ac..bd8191065ee 100644
--- a/src/librustc_errors/emitter.rs
+++ b/src/librustc_errors/emitter.rs
@@ -221,7 +221,9 @@ pub trait Emitter {
                // when this style is set we want the suggestion to be a message, not inline
                sugg.style != SuggestionStyle::HideCodeAlways &&
                // trivial suggestion for tooling's sake, never shown
-               sugg.style != SuggestionStyle::CompletelyHidden
+               sugg.style != SuggestionStyle::CompletelyHidden &&
+               // subtle suggestion, never shown inline
+               sugg.style != SuggestionStyle::ShowAlways
             {
                 let substitution = &sugg.substitutions[0].parts[0].snippet.trim();
                 let msg = if substitution.len() == 0 || sugg.style.hide_inline() {
diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs
index f9dc13ce97e..2fae584c153 100644
--- a/src/librustc_errors/lib.rs
+++ b/src/librustc_errors/lib.rs
@@ -81,6 +81,8 @@ pub enum SuggestionStyle {
     /// This will *not* show the code if the suggestion is inline *and* the suggested code is
     /// empty.
     ShowCode,
+    /// Always show the suggested code independently.
+    ShowAlways,
 }
 
 impl SuggestionStyle {
diff --git a/src/libsyntax/parse/diagnostics.rs b/src/libsyntax/parse/diagnostics.rs
index 72206ffb28d..e3abf8ffc6c 100644
--- a/src/libsyntax/parse/diagnostics.rs
+++ b/src/libsyntax/parse/diagnostics.rs
@@ -17,8 +17,7 @@ use syntax_pos::{Span, DUMMY_SP, MultiSpan, SpanSnippetError};
 use log::{debug, trace};
 use std::mem;
 
-const TURBOFISH: &'static str = "use the \"turbofish\" `::<...>` instead of `<...>` to specify \
-                                 type arguments";
+const TURBOFISH: &'static str = "use `::<...>` instead of `<...>` to specify type arguments";
 /// Creates a placeholder argument.
 crate fn dummy_arg(ident: Ident) -> Param {
     let pat = P(Pat {
@@ -585,7 +584,7 @@ impl<'a> Parser<'a> {
                 );
 
                 let suggest = |err: &mut DiagnosticBuilder<'_>| {
-                    err.span_suggestion(
+                    err.span_suggestion_verbose(
                         op_span.shrink_to_lo(),
                         TURBOFISH,
                         "::".to_string(),
@@ -647,29 +646,16 @@ impl<'a> Parser<'a> {
                         // We have high certainty that this was a bad turbofish at this point.
                         // `foo< bar >(`
                         suggest(&mut err);
-
-                        let snapshot = self.clone();
-                        self.bump(); // `(`
-
                         // Consume the fn call arguments.
-                        let modifiers = [
-                            (token::OpenDelim(token::Paren), 1),
-                            (token::CloseDelim(token::Paren), -1),
-                        ];
-                        self.consume_tts(1, &modifiers[..]);
-
-                        if self.token.kind == token::Eof {
-                            // Not entirely sure now, but we bubble the error up with the
-                            // suggestion.
-                            mem::replace(self, snapshot);
-                            Err(err)
-                        } else {
-                            // 99% certain that the suggestion is correct, continue parsing.
-                            err.emit();
-                            // FIXME: actually check that the two expressions in the binop are
-                            // paths and resynthesize new fn call expression instead of using
-                            // `ExprKind::Err` placeholder.
-                            mk_err_expr(self, lhs.span.to(self.prev_span))
+                        match self.consume_fn_args() {
+                            Err(()) => Err(err),
+                            Ok(()) => {
+                                err.emit();
+                                // FIXME: actually check that the two expressions in the binop are
+                                // paths and resynthesize new fn call expression instead of using
+                                // `ExprKind::Err` placeholder.
+                                mk_err_expr(self, lhs.span.to(self.prev_span))
+                            }
                         }
                     } else {
                         // All we know is that this is `foo < bar >` and *nothing* else. Try to
@@ -687,6 +673,27 @@ impl<'a> Parser<'a> {
         Ok(None)
     }
 
+    fn consume_fn_args(&mut self) -> Result<(), ()> {
+        let snapshot = self.clone();
+        self.bump(); // `(`
+
+        // Consume the fn call arguments.
+        let modifiers = [
+            (token::OpenDelim(token::Paren), 1),
+            (token::CloseDelim(token::Paren), -1),
+        ];
+        self.consume_tts(1, &modifiers[..]);
+
+        if self.token.kind == token::Eof {
+            // Not entirely sure that what we consumed were fn arguments, rollback.
+            mem::replace(self, snapshot);
+            Err(())
+        } else {
+            // 99% certain that the suggestion is correct, continue parsing.
+            Ok(())
+        }
+    }
+
     crate fn maybe_report_ambiguous_plus(
         &mut self,
         allow_plus: bool,
diff --git a/src/test/ui/did_you_mean/issue-40396.stderr b/src/test/ui/did_you_mean/issue-40396.stderr
index 9757f8258c1..7fc7c2628c4 100644
--- a/src/test/ui/did_you_mean/issue-40396.stderr
+++ b/src/test/ui/did_you_mean/issue-40396.stderr
@@ -3,7 +3,7 @@ error: chained comparison operators require parentheses
    |
 LL |     (0..13).collect<Vec<i32>>();
    |                    ^^^^^
-help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
+help: use `::<...>` instead of `<...>` to specify type arguments
    |
 LL |     (0..13).collect::<Vec<i32>>();
    |                    ^^
@@ -13,7 +13,7 @@ error: chained comparison operators require parentheses
    |
 LL |     Vec<i32>::new();
    |        ^^^^^
-help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
+help: use `::<...>` instead of `<...>` to specify type arguments
    |
 LL |     Vec::<i32>::new();
    |        ^^
@@ -23,7 +23,7 @@ error: chained comparison operators require parentheses
    |
 LL |     (0..13).collect<Vec<i32>();
    |                    ^^^^^
-help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
+help: use `::<...>` instead of `<...>` to specify type arguments
    |
 LL |     (0..13).collect::<Vec<i32>();
    |                    ^^
diff --git a/src/test/ui/parser/require-parens-for-chained-comparison.rs b/src/test/ui/parser/require-parens-for-chained-comparison.rs
index f3bfe2d482f..9c7a25d589a 100644
--- a/src/test/ui/parser/require-parens-for-chained-comparison.rs
+++ b/src/test/ui/parser/require-parens-for-chained-comparison.rs
@@ -12,15 +12,15 @@ fn main() {
 
     f<X>();
     //~^ ERROR chained comparison operators require parentheses
-    //~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
+    //~| HELP use `::<...>` instead of `<...>` to specify type arguments
 
     f<Result<Option<X>, Option<Option<X>>>(1, 2);
     //~^ ERROR chained comparison operators require parentheses
-    //~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
+    //~| HELP use `::<...>` instead of `<...>` to specify type arguments
 
     use std::convert::identity;
     let _ = identity<u8>;
     //~^ ERROR chained comparison operators require parentheses
-    //~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
+    //~| HELP use `::<...>` instead of `<...>` to specify type arguments
     //~| HELP or use `(...)` if you meant to specify fn arguments
 }
diff --git a/src/test/ui/parser/require-parens-for-chained-comparison.stderr b/src/test/ui/parser/require-parens-for-chained-comparison.stderr
index 4b108e1db87..5aa37a40cbd 100644
--- a/src/test/ui/parser/require-parens-for-chained-comparison.stderr
+++ b/src/test/ui/parser/require-parens-for-chained-comparison.stderr
@@ -15,7 +15,7 @@ error: chained comparison operators require parentheses
    |
 LL |     f<X>();
    |      ^^^
-help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
+help: use `::<...>` instead of `<...>` to specify type arguments
    |
 LL |     f::<X>();
    |      ^^
@@ -25,7 +25,7 @@ error: chained comparison operators require parentheses
    |
 LL |     f<Result<Option<X>, Option<Option<X>>>(1, 2);
    |      ^^^^^^^^
-help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
+help: use `::<...>` instead of `<...>` to specify type arguments
    |
 LL |     f::<Result<Option<X>, Option<Option<X>>>(1, 2);
    |      ^^
@@ -36,7 +36,7 @@ error: chained comparison operators require parentheses
 LL |     let _ = identity<u8>;
    |                     ^^^^
    |
-   = help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
+   = help: use `::<...>` instead of `<...>` to specify type arguments
    = help: or use `(...)` if you meant to specify fn arguments
 
 error[E0308]: mismatched types