Auto merge of #40851 - oli-obk:multisugg, r=jonathandturner

Minimize single span suggestions into a label

changes

```
14 |     println!("☃{}", tup[0]);
   |                     ^^^^^^
   |
help: to access tuple elements, use tuple indexing syntax as shown
   |     println!("☃{}", tup.0);
```

into

```
14 |     println!("☃{}", tup[0]);
   |                     ^^^^^^ to access tuple elements, use `tup.0`
```

Also makes suggestions explicit in the backend in preparation of adding multiple suggestions to a single diagnostic. Currently that's already possible, but results in a full help message + modified code snippet per suggestion, and has no rate limit (might show 100+ suggestions).

2 files changed, 19 insertions, 12 deletions

diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs
index 47b60f0e080..0271ddbccbf 100644
--- a/src/libsyntax/json.rs
+++ b/src/libsyntax/json.rs
@@ -22,8 +22,9 @@
 use codemap::{CodeMap, FilePathMapping};
 use syntax_pos::{self, MacroBacktrace, Span, SpanLabel, MultiSpan};
 use errors::registry::Registry;
-use errors::{DiagnosticBuilder, SubDiagnostic, RenderSpan, CodeSuggestion, CodeMapper};
+use errors::{Level, DiagnosticBuilder, SubDiagnostic, RenderSpan, CodeSuggestion, CodeMapper};
 use errors::emitter::Emitter;
+use errors::snippet::Style;
 
 use std::rc::Rc;
 use std::io::{self, Write};
@@ -153,12 +154,21 @@ impl Diagnostic {
     fn from_diagnostic_builder(db: &DiagnosticBuilder,
                                je: &JsonEmitter)
                                -> Diagnostic {
+        let sugg = db.suggestion.as_ref().map(|sugg| {
+            SubDiagnostic {
+                level: Level::Help,
+                message: vec![(sugg.msg.clone(), Style::NoStyle)],
+                span: MultiSpan::new(),
+                render_span: Some(RenderSpan::Suggestion(sugg.clone())),
+            }
+        });
+        let sugg = sugg.as_ref();
         Diagnostic {
             message: db.message(),
             code: DiagnosticCode::map_opt_string(db.code.clone(), je),
             level: db.level.to_str(),
             spans: DiagnosticSpan::from_multispan(&db.span, je),
-            children: db.children.iter().map(|c| {
+            children: db.children.iter().chain(sugg).map(|c| {
                 Diagnostic::from_sub_diagnostic(c, je)
             }).collect(),
             rendered: None,
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 84d53dbef7c..d252963274e 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -1492,9 +1492,8 @@ impl<'a> Parser<'a> {
         let bounds = self.parse_ty_param_bounds()?;
         let sum_span = ty.span.to(self.prev_span);
 
-        let mut err = struct_span_err!(self.sess.span_diagnostic, ty.span, E0178,
+        let mut err = struct_span_err!(self.sess.span_diagnostic, sum_span, E0178,
             "expected a path on the left-hand side of `+`, not `{}`", pprust::ty_to_string(&ty));
-        err.span_label(ty.span, &format!("expected a path"));
 
         match ty.node {
             TyKind::Rptr(ref lifetime, ref mut_ty) => {
@@ -1513,9 +1512,11 @@ impl<'a> Parser<'a> {
                 err.span_suggestion(sum_span, "try adding parentheses:", sum_with_parens);
             }
             TyKind::Ptr(..) | TyKind::BareFn(..) => {
-                help!(&mut err, "perhaps you forgot parentheses?");
+                err.span_label(sum_span, &"perhaps you forgot parentheses?");
             }
-            _ => {}
+            _ => {
+                err.span_label(sum_span, &"expected a path");
+            },
         }
         err.emit();
         Ok(())
@@ -5131,7 +5132,6 @@ impl<'a> Parser<'a> {
         }
 
         if self.check(&token::OpenDelim(token::Paren)) {
-            let start_span = self.span;
             // We don't `self.bump()` the `(` yet because this might be a struct definition where
             // `()` or a tuple might be allowed. For example, `struct Struct(pub (), pub (usize));`.
             // Because of this, we only `bump` the `(` if we're assured it is appropriate to do so
@@ -5170,12 +5170,9 @@ impl<'a> Parser<'a> {
 `pub(in path::to::module)`: visible only on the specified path"##;
                 let path = self.parse_path(PathStyle::Mod)?;
                 let path_span = self.prev_span;
-                let help_msg = format!("to make this visible only to module `{}`, add `in` before \
-                                       the path:",
-                                       path);
+                let help_msg = format!("make this visible only to module `{}` with `in`:", path);
                 self.expect(&token::CloseDelim(token::Paren))?;  // `)`
-                let sp = start_span.to(self.prev_span);
-                let mut err = self.span_fatal_help(sp, &msg, &suggestion);
+                let mut err = self.span_fatal_help(path_span, &msg, &suggestion);
                 err.span_suggestion(path_span, &help_msg, format!("in {}", path));
                 err.emit();  // emit diagnostic, but continue with public visibility
             }