Auto merge of #32756 - nikomatsakis:borrowck-snippet, r=nrc

Overhaul borrowck error messages and compiler error formatting generally

This is a major overhaul of how the compiler reports errors. The primary goal is to be able to give many spans within the same overall context, such as this:

```
./borrow-errors.rs:73:17: 73:20: error: cannot borrow `*vec` as immutable because previous closure requires unique access [E0501]
70     let append = |e| {
                    ~~~ closure construction occurs here
71         vec.push(e)
           ~~~ previous borrow occurs due to use of `vec` in closure
72     };
73     let data = &vec[3];
                   ~~~ borrow occurs here
74 }
   ~ borrow from closure ends here
```

However, in the process we made a number of other changes:

- Removed the repetitive filenames from snippets and just give the line number.
- Color the line numbers blue so they "fade away"
- Remove the file name and line number from the error code suggestions since they don't seem to fit anymore. (This should probably happen in more places, like existing notes.)
- Newlines in between errors to help group them better.

This PR is not quite ready to land, but we thought it made sense to stop here and get some feedback from people at large. It'd be great if people can check out the branch and play with it. We'd be especially interested in hearing about cases that don't look good with the new formatting (I suspect they exist).

Here is a checklist of some pending work items for this PR. Some of them may be best left for follow-up PRs:

- [x] Accommodate multiple files in a `MultiSpan` (this should be easy)
  - In this case, we want to print filenames though.
- [x] Remove duplicate E0500 code.
- [x] Make the header message bold, rather than current hack that makes all errors/warnings bold
- [x] Update warning text color (yellow is hard to read w/ a white background)

Moved numerous follow-ups to: https://github.com/rust-lang/rust/issues/33240

Joint work with @jonathandturner.

Fixes https://github.com/rust-lang/rust/issues/3533

1 files changed, 83 insertions, 121 deletions

diff --git a/src/libsyntax/errors/json.rs b/src/libsyntax/errors/json.rs
index 821617bfe89..93c6268ccae 100644
--- a/src/libsyntax/errors/json.rs
+++ b/src/libsyntax/errors/json.rs
@@ -20,7 +20,7 @@
 // FIXME spec the JSON output properly.
 
 
-use codemap::{self, Span, MacroBacktrace, MultiSpan, CodeMap};
+use codemap::{self, MacroBacktrace, Span, SpanLabel, MultiSpan, CodeMap};
 use diagnostics::registry::Registry;
 use errors::{Level, DiagnosticBuilder, SubDiagnostic, RenderSpan, CodeSuggestion};
 use errors::emitter::Emitter;
@@ -53,20 +53,13 @@ impl JsonEmitter {
 }
 
 impl Emitter for JsonEmitter {
-    fn emit(&mut self, span: Option<&MultiSpan>, msg: &str, code: Option<&str>, level: Level) {
+    fn emit(&mut self, span: &MultiSpan, msg: &str, code: Option<&str>, level: Level) {
         let data = Diagnostic::new(span, msg, code, level, self);
         if let Err(e) = writeln!(&mut self.dst, "{}", as_json(&data)) {
             panic!("failed to print diagnostics: {:?}", e);
         }
     }
 
-    fn custom_emit(&mut self, sp: &RenderSpan, msg: &str, level: Level) {
-        let data = Diagnostic::from_render_span(sp, msg, level, self);
-        if let Err(e) = writeln!(&mut self.dst, "{}", as_json(&data)) {
-            panic!("failed to print diagnostics: {:?}", e);
-        }
-    }
-
     fn emit_struct(&mut self, db: &DiagnosticBuilder) {
         let data = Diagnostic::from_diagnostic_builder(db, self);
         if let Err(e) = writeln!(&mut self.dst, "{}", as_json(&data)) {
@@ -104,8 +97,13 @@ struct DiagnosticSpan {
     /// 1-based, character offset.
     column_start: usize,
     column_end: usize,
+    /// Is this a "primary" span -- meaning the point, or one of the points,
+    /// where the error occurred?
+    is_primary: bool,
     /// Source text from the start of line_start to the end of line_end.
     text: Vec<DiagnosticSpanLine>,
+    /// Label that should be placed at this location (if any)
+    label: Option<String>,
     /// If we are suggesting a replacement, this will contain text
     /// that should be sliced in atop this span. You may prefer to
     /// load the fully rendered version from the parent `Diagnostic`,
@@ -148,7 +146,7 @@ struct DiagnosticCode {
 }
 
 impl<'a> Diagnostic<'a> {
-    fn new(msp: Option<&MultiSpan>,
+    fn new(msp: &MultiSpan,
            msg: &'a str,
            code: Option<&str>,
            level: Level,
@@ -158,27 +156,12 @@ impl<'a> Diagnostic<'a> {
             message: msg,
             code: DiagnosticCode::map_opt_string(code.map(|c| c.to_owned()), je),
             level: level.to_str(),
-            spans: msp.map_or(vec![], |msp| DiagnosticSpan::from_multispan(msp, je)),
+            spans: DiagnosticSpan::from_multispan(msp, je),
             children: vec![],
             rendered: None,
         }
     }
 
-    fn from_render_span(span: &RenderSpan,
-                        msg: &'a str,
-                        level: Level,
-                        je: &JsonEmitter)
-                        -> Diagnostic<'a> {
-        Diagnostic {
-            message: msg,
-            code: None,
-            level: level.to_str(),
-            spans: DiagnosticSpan::from_render_span(span, je),
-            children: vec![],
-            rendered: je.render(span),
-        }
-    }
-
     fn from_diagnostic_builder<'c>(db: &'c DiagnosticBuilder,
                                    je: &JsonEmitter)
                                    -> Diagnostic<'c> {
@@ -186,7 +169,7 @@ impl<'a> Diagnostic<'a> {
             message: &db.message,
             code: DiagnosticCode::map_opt_string(db.code.clone(), je),
             level: db.level.to_str(),
-            spans: db.span.as_ref().map_or(vec![], |sp| DiagnosticSpan::from_multispan(sp, je)),
+            spans: DiagnosticSpan::from_multispan(&db.span, je),
             children: db.children.iter().map(|c| {
                 Diagnostic::from_sub_diagnostic(c, je)
             }).collect(),
@@ -201,8 +184,7 @@ impl<'a> Diagnostic<'a> {
             level: db.level.to_str(),
             spans: db.render_span.as_ref()
                      .map(|sp| DiagnosticSpan::from_render_span(sp, je))
-                     .or_else(|| db.span.as_ref().map(|s| DiagnosticSpan::from_multispan(s, je)))
-                     .unwrap_or(vec![]),
+                     .unwrap_or_else(|| DiagnosticSpan::from_multispan(&db.span, je)),
             children: vec![],
             rendered: db.render_span.as_ref()
                                     .and_then(|rsp| je.render(rsp)),
@@ -211,44 +193,68 @@ impl<'a> Diagnostic<'a> {
 }
 
 impl DiagnosticSpan {
-    fn from_span(span: Span, suggestion: Option<&String>, je: &JsonEmitter)
-                 -> DiagnosticSpan {
+    fn from_span_label(span: SpanLabel,
+                       suggestion: Option<&String>,
+                       je: &JsonEmitter)
+                       -> DiagnosticSpan {
+        Self::from_span_etc(span.span,
+                            span.is_primary,
+                            span.label,
+                            suggestion,
+                            je)
+    }
+
+    fn from_span_etc(span: Span,
+                     is_primary: bool,
+                     label: Option<String>,
+                     suggestion: Option<&String>,
+                     je: &JsonEmitter)
+                     -> DiagnosticSpan {
         // obtain the full backtrace from the `macro_backtrace`
         // helper; in some ways, it'd be better to expand the
         // backtrace ourselves, but the `macro_backtrace` helper makes
         // some decision, such as dropping some frames, and I don't
         // want to duplicate that logic here.
         let backtrace = je.cm.macro_backtrace(span).into_iter();
-        DiagnosticSpan::from_span_and_backtrace(span, suggestion, backtrace, je)
+        DiagnosticSpan::from_span_full(span,
+                                       is_primary,
+                                       label,
+                                       suggestion,
+                                       backtrace,
+                                       je)
     }
 
-    fn from_span_and_backtrace(span: Span,
-                               suggestion: Option<&String>,
-                               mut backtrace: vec::IntoIter<MacroBacktrace>,
-                               je: &JsonEmitter)
-                               -> DiagnosticSpan {
+    fn from_span_full(span: Span,
+                      is_primary: bool,
+                      label: Option<String>,
+                      suggestion: Option<&String>,
+                      mut backtrace: vec::IntoIter<MacroBacktrace>,
+                      je: &JsonEmitter)
+                      -> DiagnosticSpan {
         let start = je.cm.lookup_char_pos(span.lo);
         let end = je.cm.lookup_char_pos(span.hi);
-        let backtrace_step =
-            backtrace.next()
-                     .map(|bt| {
-                         let call_site =
-                             Self::from_span_and_backtrace(bt.call_site,
-                                                           None,
-                                                           backtrace,
-                                                           je);
-                         let def_site_span = bt.def_site_span.map(|sp| {
-                             Self::from_span_and_backtrace(sp,
-                                                           None,
-                                                           vec![].into_iter(),
-                                                           je)
-                         });
-                         Box::new(DiagnosticSpanMacroExpansion {
-                             span: call_site,
-                             macro_decl_name: bt.macro_decl_name,
-                             def_site_span: def_site_span,
-                         })
-                     });
+        let backtrace_step = backtrace.next().map(|bt| {
+            let call_site =
+                Self::from_span_full(bt.call_site,
+                                     false,
+                                     None,
+                                     None,
+                                     backtrace,
+                                     je);
+            let def_site_span = bt.def_site_span.map(|sp| {
+                Self::from_span_full(sp,
+                                     false,
+                                     None,
+                                     None,
+                                     vec![].into_iter(),
+                                     je)
+            });
+            Box::new(DiagnosticSpanMacroExpansion {
+                span: call_site,
+                macro_decl_name: bt.macro_decl_name,
+                def_site_span: def_site_span,
+            })
+        });
         DiagnosticSpan {
             file_name: start.file.name.clone(),
             byte_start: span.lo.0,
@@ -257,53 +263,41 @@ impl DiagnosticSpan {
             line_end: end.line,
             column_start: start.col.0 + 1,
             column_end: end.col.0 + 1,
+            is_primary: is_primary,
             text: DiagnosticSpanLine::from_span(span, je),
             suggested_replacement: suggestion.cloned(),
             expansion: backtrace_step,
+            label: label,
         }
     }
 
     fn from_multispan(msp: &MultiSpan, je: &JsonEmitter) -> Vec<DiagnosticSpan> {
-        msp.spans.iter().map(|&span| Self::from_span(span, None, je)).collect()
+        msp.span_labels()
+           .into_iter()
+           .map(|span_str| Self::from_span_label(span_str, None, je))
+           .collect()
     }
 
     fn from_suggestion(suggestion: &CodeSuggestion, je: &JsonEmitter)
                        -> Vec<DiagnosticSpan> {
-        assert_eq!(suggestion.msp.spans.len(), suggestion.substitutes.len());
-        suggestion.msp.spans.iter()
-                            .zip(&suggestion.substitutes)
-                            .map(|(&span, suggestion)| {
-                                DiagnosticSpan::from_span(span, Some(suggestion), je)
-                            })
-                            .collect()
+        assert_eq!(suggestion.msp.span_labels().len(), suggestion.substitutes.len());
+        suggestion.msp.span_labels()
+                      .into_iter()
+                      .zip(&suggestion.substitutes)
+                      .map(|(span_label, suggestion)| {
+                          DiagnosticSpan::from_span_label(span_label,
+                                                          Some(suggestion),
+                                                          je)
+                      })
+                      .collect()
     }
 
     fn from_render_span(rsp: &RenderSpan, je: &JsonEmitter) -> Vec<DiagnosticSpan> {
         match *rsp {
-            RenderSpan::FileLine(ref msp) |
-            RenderSpan::FullSpan(ref msp) => {
-                DiagnosticSpan::from_multispan(msp, je)
-            }
-            RenderSpan::Suggestion(ref suggestion) => {
-                DiagnosticSpan::from_suggestion(suggestion, je)
-            }
-            RenderSpan::EndSpan(ref msp) => {
-                msp.spans.iter().map(|&span| {
-                    let end = je.cm.lookup_char_pos(span.hi);
-                    DiagnosticSpan {
-                        file_name: end.file.name.clone(),
-                        byte_start: span.hi.0,
-                        byte_end: span.hi.0,
-                        line_start: end.line,
-                        line_end: end.line,
-                        column_start: end.col.0 + 1,
-                        column_end: end.col.0 + 1,
-                        text: DiagnosticSpanLine::from_span_end(span, je),
-                        suggested_replacement: None,
-                        expansion: None,
-                    }
-                }).collect()
-            }
+            RenderSpan::FullSpan(ref msp) =>
+                DiagnosticSpan::from_multispan(msp, je),
+            RenderSpan::Suggestion(ref suggestion) =>
+                DiagnosticSpan::from_suggestion(suggestion, je),
         }
     }
 }
@@ -340,34 +334,6 @@ impl DiagnosticSpanLine {
              })
             .unwrap_or(vec![])
     }
-
-    /// Create a list of DiagnosticSpanLines from span - the result covers all
-    /// of `span`, but the highlight is zero-length and at the end of `span`.
-    fn from_span_end(span: Span, je: &JsonEmitter) -> Vec<DiagnosticSpanLine> {
-        je.cm.span_to_lines(span)
-             .map(|lines| {
-                 let fm = &*lines.file;
-                 lines.lines.iter()
-                            .enumerate()
-                            .map(|(i, line)| {
-                                // Invariant - CodeMap::span_to_lines
-                                // will not return extra context lines
-                                // - the last line returned is the last
-                                // line of `span`.
-                                let highlight = if i == lines.lines.len() - 1 {
-                                    (line.end_col.0 + 1, line.end_col.0 + 1)
-                                } else {
-                                    (0, 0)
-                                };
-                                DiagnosticSpanLine::line_from_filemap(fm,
-                                                                      line.line_index,
-                                                                      highlight.0,
-                                                                      highlight.1)
-                            })
-                            .collect()
-             })
-            .unwrap_or(vec![])
-    }
 }
 
 impl DiagnosticCode {
@@ -389,16 +355,12 @@ impl DiagnosticCode {
 impl JsonEmitter {
     fn render(&self, render_span: &RenderSpan) -> Option<String> {
         match *render_span {
-            RenderSpan::FileLine(_) |
             RenderSpan::FullSpan(_) => {
                 None
             }
             RenderSpan::Suggestion(ref suggestion) => {
                 Some(suggestion.splice_lines(&self.cm))
             }
-            RenderSpan::EndSpan(_) => {
-                None
-            }
         }
     }
 }