Auto merge of #33688 - jonathandturner:fix_old_school, r=nikomatsakis

Fix for old school error issues, improvements to new school

This PR:
* Fixes some old school error issues, specifically #33559, #33543, #33366
* Improves wording borrowck errors with match patterns
* De-emphasize multi-line spans, so we don't color the single source character when we're trying to say "span starts here"
* Rollup of #33392 (which should help fix #33390)

r? @nikomatsakis

13 files changed, 125 insertions, 68 deletions

diff --git a/src/librustc/infer/error_reporting.rs b/src/librustc/infer/error_reporting.rs
index 45982793a70..8afee54c4bc 100644
--- a/src/librustc/infer/error_reporting.rs
+++ b/src/librustc/infer/error_reporting.rs
@@ -90,7 +90,7 @@ use std::cell::{Cell, RefCell};
 use std::char::from_u32;
 use std::fmt;
 use syntax::ast;
-use syntax::errors::DiagnosticBuilder;
+use syntax::errors::{DiagnosticBuilder, check_old_skool};
 use syntax::codemap::{self, Pos, Span};
 use syntax::parse::token;
 use syntax::ptr::P;
@@ -481,7 +481,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                                        "{}",
                                        trace.origin);
 
-        if !is_simple_error {
+        if !is_simple_error || check_old_skool() {
             err.note_expected_found(&"type", &expected, &found);
         }
 
diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs
index fcfab96b9df..36f95f62d06 100644
--- a/src/librustc_borrowck/borrowck/check_loans.rs
+++ b/src/librustc_borrowck/borrowck/check_loans.rs
@@ -872,7 +872,7 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
                        &format!("borrow of `{}` occurs here",
                                self.bccx.loan_path_to_string(loan_path)))
             .span_label(span,
-                       &format!("assignment to `{}` occurs here",
+                       &format!("assignment to borrowed `{}` occurs here",
                                self.bccx.loan_path_to_string(loan_path)))
             .emit();
     }
diff --git a/src/librustc_borrowck/borrowck/gather_loans/move_error.rs b/src/librustc_borrowck/borrowck/gather_loans/move_error.rs
index cdc68edbf30..5ebb1ab32b8 100644
--- a/src/librustc_borrowck/borrowck/gather_loans/move_error.rs
+++ b/src/librustc_borrowck/borrowck/gather_loans/move_error.rs
@@ -126,7 +126,7 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
                              move_from.descriptive_string(bccx.tcx));
             err.span_label(
                 move_from.span,
-                &format!("move occurs here")
+                &format!("cannot move out of {}", move_from.descriptive_string(bccx.tcx))
                 );
             err
         }
@@ -138,7 +138,7 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
                                                "cannot move out of type `{}`, \
                                                a non-copy fixed-size array",
                                                b.ty);
-                err.span_label(move_from.span, &format!("can not move out of here"));
+                err.span_label(move_from.span, &format!("cannot move out of here"));
                 err
             } else {
                 span_bug!(move_from.span, "this path should not cause illegal move");
@@ -154,7 +154,7 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
                                                    "cannot move out of type `{}`, \
                                                    which defines the `Drop` trait",
                                                    b.ty);
-                    err.span_label(move_from.span, &format!("can not move out of here"));
+                    err.span_label(move_from.span, &format!("cannot move out of here"));
                     err
                 },
                 _ => {
@@ -175,16 +175,12 @@ fn note_move_destination(mut err: DiagnosticBuilder,
     if is_first_note {
         err.span_label(
             move_to_span,
-            &format!("attempting to move value to here"));
-        err.help(
-            &format!("to prevent the move, \
-                      use `ref {0}` or `ref mut {0}` to capture value by \
-                      reference",
+            &format!("hint: to prevent move, use `ref {0}` or `ref mut {0}`",
                      pat_name));
         err
     } else {
-        err.span_note(move_to_span,
-                      &format!("and here (use `ref {0}` or `ref mut {0}`)",
+        err.span_label(move_to_span,
+                      &format!("...and here (use `ref {0}` or `ref mut {0}`)",
                                pat_name));
         err
     }
diff --git a/src/libsyntax/errors/emitter.rs b/src/libsyntax/errors/emitter.rs
index e7007fb0568..8d0c93f21b2 100644
--- a/src/libsyntax/errors/emitter.rs
+++ b/src/libsyntax/errors/emitter.rs
@@ -367,7 +367,8 @@ impl EmitterWriter {
             let mut output_vec = vec![];
 
             for span_label in msp.span_labels() {
-                let mut snippet_data = snippet_data.clone();
+                let mut snippet_data = SnippetData::new(self.cm.clone(),
+                                                        Some(span_label.span));
 
                 snippet_data.push(span_label.span,
                                   span_label.is_primary,
@@ -524,6 +525,13 @@ impl Destination {
             }
             Style::Quotation => {
             }
+            Style::OldSkoolNote => {
+                self.start_attr(term::Attr::Bold)?;
+                self.start_attr(term::Attr::ForegroundColor(term::color::BRIGHT_GREEN))?;
+            }
+            Style::OldSkoolNoteText => {
+                self.start_attr(term::Attr::Bold)?;
+            }
             Style::UnderlinePrimary | Style::LabelPrimary => {
                 self.start_attr(term::Attr::Bold)?;
                 self.start_attr(term::Attr::ForegroundColor(lvl.color()))?;
diff --git a/src/libsyntax/errors/mod.rs b/src/libsyntax/errors/mod.rs
index 2ca61ba76d4..f06672fe111 100644
--- a/src/libsyntax/errors/mod.rs
+++ b/src/libsyntax/errors/mod.rs
@@ -699,13 +699,13 @@ pub fn expect<T, M>(diag: &Handler, opt: Option<T>, msg: M) -> T where
 ///
 /// FIXME(#33240)
 #[cfg(not(test))]
-fn check_old_skool() -> bool {
+pub fn check_old_skool() -> bool {
     use std::env;
     env::var("RUST_NEW_ERROR_FORMAT").is_err()
 }
 
 /// For unit tests, use the new format.
 #[cfg(test)]
-fn check_old_skool() -> bool {
+pub fn check_old_skool() -> bool {
     false
 }
diff --git a/src/libsyntax/errors/snippet/mod.rs b/src/libsyntax/errors/snippet/mod.rs
index 092effbb2f6..188e676e7df 100644
--- a/src/libsyntax/errors/snippet/mod.rs
+++ b/src/libsyntax/errors/snippet/mod.rs
@@ -58,6 +58,9 @@ struct Annotation {
     /// Is this annotation derived from primary span
     is_primary: bool,
 
+    /// Is this a large span minimized down to a smaller span
+    is_minimized: bool,
+
     /// Optional label to display adjacent to the annotation.
     label: Option<String>,
 }
@@ -90,6 +93,8 @@ pub enum Style {
     UnderlineSecondary,
     LabelPrimary,
     LabelSecondary,
+    OldSkoolNoteText,
+    OldSkoolNote,
     NoStyle,
 }
 
@@ -382,10 +387,10 @@ impl FileInfo {
         // Basically, although this loses information, multi-line spans just
         // never look good.
 
-        let (line, start_col, mut end_col) = if lines.len() == 1 {
-            (lines[0].line_index, lines[0].start_col, lines[0].end_col)
+        let (line, start_col, mut end_col, is_minimized) = if lines.len() == 1 {
+            (lines[0].line_index, lines[0].start_col, lines[0].end_col, false)
         } else {
-            (lines[0].line_index, lines[0].start_col, CharPos(lines[0].start_col.0 + 1))
+            (lines[0].line_index, lines[0].start_col, CharPos(lines[0].start_col.0 + 1), true)
         };
 
         // Watch out for "empty spans". If we get a span like 6..6, we
@@ -401,6 +406,7 @@ impl FileInfo {
         self.lines[index].push_annotation(start_col,
                                           end_col,
                                           is_primary,
+                                          is_minimized,
                                           label);
     }
 
@@ -497,6 +503,30 @@ impl FileInfo {
                     match self.primary_span {
                         Some(span) => {
                             let lo = codemap.lookup_char_pos(span.lo);
+                            let hi = codemap.lookup_char_pos(span.hi);
+                            //Before each secondary line in old skool-mode, print the label
+                            //as an old-style note
+                            if !line.annotations[0].is_primary {
+                                if let Some(ann) = line.annotations[0].label.clone() {
+                                    output.push(RenderedLine {
+                                        text: vec![StyledString {
+                                            text: lo.file.name.clone(),
+                                            style: Style::FileNameStyle,
+                                        }, StyledString {
+                                            text: format!(":{}:{}: {}:{} ", lo.line, lo.col.0 + 1,
+                                                hi.line, hi.col.0+1),
+                                            style: Style::LineAndColumn,
+                                        }, StyledString {
+                                            text: format!("note: "),
+                                            style: Style::OldSkoolNote,
+                                        }, StyledString {
+                                            text: format!("{}", ann),
+                                            style: Style::OldSkoolNoteText,
+                                        }],
+                                        kind: RenderedLineKind::Annotations,
+                                    });
+                                }
+                            }
                             rendered_lines[0].text.insert(0, StyledString {
                                 text: format!(":{} ", lo.line),
                                 style: Style::LineAndColumn,
@@ -598,7 +628,7 @@ impl FileInfo {
                             if annotation.is_primary {
                                 Style::UnderlinePrimary
                             } else {
-                                Style::UnderlineSecondary
+                                Style::OldSkoolNote
                             });
                     }
                     else {
@@ -606,7 +636,7 @@ impl FileInfo {
                             if annotation.is_primary {
                                 Style::UnderlinePrimary
                             } else {
-                                Style::UnderlineSecondary
+                                Style::OldSkoolNote
                             });
                     }
                 }
@@ -615,10 +645,14 @@ impl FileInfo {
                 for p in annotation.start_col .. annotation.end_col {
                     if annotation.is_primary {
                         styled_buffer.putc(1, p, '^', Style::UnderlinePrimary);
-                        styled_buffer.set_style(0, p, Style::UnderlinePrimary);
+                        if !annotation.is_minimized {
+                            styled_buffer.set_style(0, p, Style::UnderlinePrimary);
+                        }
                     } else {
                         styled_buffer.putc(1, p, '-', Style::UnderlineSecondary);
-                        styled_buffer.set_style(0, p, Style::UnderlineSecondary);
+                        if !annotation.is_minimized {
+                            styled_buffer.set_style(0, p, Style::UnderlineSecondary);
+                        }
                     }
                 }
             }
@@ -819,11 +853,13 @@ impl Line {
                        start: CharPos,
                        end: CharPos,
                        is_primary: bool,
+                       is_minimized: bool,
                        label: Option<String>) {
         self.annotations.push(Annotation {
             start_col: start.0,
             end_col: end.0,
             is_primary: is_primary,
+            is_minimized: is_minimized,
             label: label,
         });
     }
diff --git a/src/test/compile-fail/borrowck/borrowck-move-error-with-note.rs b/src/test/compile-fail/borrowck/borrowck-move-error-with-note.rs
index ffa7d192556..438a548819b 100644
--- a/src/test/compile-fail/borrowck/borrowck-move-error-with-note.rs
+++ b/src/test/compile-fail/borrowck/borrowck-move-error-with-note.rs
@@ -19,8 +19,8 @@ enum Foo {
 fn blah() {
     let f = &Foo::Foo1(box 1, box 2);
     match *f {             //~ ERROR cannot move out of
-                           //~| move occurs here
-        Foo::Foo1(num1,         //~ NOTE attempting to move value to here
+                           //~| cannot move out
+        Foo::Foo1(num1,         //~ NOTE to prevent move
                   num2) => (),  //~ NOTE and here
         Foo::Foo2(num) => (),   //~ NOTE and here
         Foo::Foo3 => ()
@@ -38,8 +38,8 @@ impl Drop for S {
 fn move_in_match() {
     match (S {f: "foo".to_string(), g: "bar".to_string()}) {
         S {         //~ ERROR cannot move out of type `S`, which defines the `Drop` trait
-        //~| can not move out of here
-            f: _s,  //~ NOTE attempting to move value to here
+        //~| cannot move out of here
+            f: _s,  //~ NOTE to prevent move
             g: _t   //~ NOTE and here
         } => {}
     }
@@ -55,8 +55,8 @@ fn free<T>(_: T) {}
 fn blah2() {
     let a = &A { a: box 1 };
     match a.a {           //~ ERROR cannot move out of
-                          //~| move occurs here
-        n => {            //~ NOTE attempting to move value to here
+                          //~| cannot move out
+        n => {            //~ NOTE to prevent move
             free(n)
         }
     }
diff --git a/src/test/compile-fail/borrowck/borrowck-move-out-of-vec-tail.rs b/src/test/compile-fail/borrowck/borrowck-move-out-of-vec-tail.rs
index 2f1c69d0d7d..15771295743 100644
--- a/src/test/compile-fail/borrowck/borrowck-move-out-of-vec-tail.rs
+++ b/src/test/compile-fail/borrowck/borrowck-move-out-of-vec-tail.rs
@@ -29,8 +29,8 @@ pub fn main() {
             match tail {
                 [Foo { string: a },
                 //~^ ERROR cannot move out of borrowed content
-                //~| move occurs here
-                //~| attempting to move value to here
+                //~| cannot move out
+                //~| to prevent move
                  Foo { string: b }] => {
                     //~^ NOTE and here
                 }
diff --git a/src/test/compile-fail/borrowck/borrowck-vec-pattern-nesting.rs b/src/test/compile-fail/borrowck/borrowck-vec-pattern-nesting.rs
index 1c63b458e62..eec6c8473eb 100644
--- a/src/test/compile-fail/borrowck/borrowck-vec-pattern-nesting.rs
+++ b/src/test/compile-fail/borrowck/borrowck-vec-pattern-nesting.rs
@@ -19,7 +19,7 @@ fn a() {
         [box ref _a, _, _] => {
         //~^ borrow of `vec[..]` occurs here
             vec[0] = box 4; //~ ERROR cannot assign
-            //~^ assignment to `vec[..]` occurs here
+            //~^ assignment to borrowed `vec[..]` occurs here
         }
     }
 }
@@ -31,7 +31,7 @@ fn b() {
         [_b..] => {
         //~^ borrow of `vec[..]` occurs here
             vec[0] = box 4; //~ ERROR cannot assign
-            //~^ assignment to `vec[..]` occurs here
+            //~^ assignment to borrowed `vec[..]` occurs here
         }
     }
 }
@@ -41,8 +41,8 @@ fn c() {
     let vec: &mut [Box<isize>] = &mut vec;
     match vec {
         [_a,         //~ ERROR cannot move out
-        //~| move occurs here
-        //~| attempting to move value to here
+        //~| cannot move out
+        //~| to prevent move
          _b..] => {
             // Note: `_a` is *moved* here, but `b` is borrowing,
             // hence illegal.
@@ -53,8 +53,8 @@ fn c() {
         _ => {}
     }
     let a = vec[0]; //~ ERROR cannot move out
-    //~^ NOTE attempting to move value to here
-    //~| can not move out of here
+    //~^ NOTE to prevent move
+    //~| cannot move out of here
 }
 
 fn d() {
@@ -62,13 +62,13 @@ fn d() {
     let vec: &mut [Box<isize>] = &mut vec;
     match vec {
         [_a..,     //~ ERROR cannot move out
-        //~^ move occurs here
-         _b] => {} //~ NOTE attempting to move value to here
+        //~^ cannot move out
+         _b] => {} //~ NOTE to prevent move
         _ => {}
     }
     let a = vec[0]; //~ ERROR cannot move out
-    //~^ NOTE attempting to move value to here
-    //~| can not move out of here
+    //~^ NOTE to prevent move
+    //~| cannot move out of here
 }
 
 fn e() {
@@ -76,15 +76,15 @@ fn e() {
     let vec: &mut [Box<isize>] = &mut vec;
     match vec {
         [_a, _b, _c] => {}  //~ ERROR cannot move out
-        //~| move occurs here
-        //~| NOTE attempting to move value to here
+        //~| cannot move out
+        //~| NOTE to prevent move
         //~| NOTE and here
         //~| NOTE and here
         _ => {}
     }
     let a = vec[0]; //~ ERROR cannot move out
-    //~^ NOTE attempting to move value to here
-    //~| can not move out of here
+    //~^ NOTE to prevent move
+    //~| cannot move out of here
 }
 
 fn main() {}
diff --git a/src/test/compile-fail/issue-26480.rs b/src/test/compile-fail/issue-26480.rs
index adcf8484f78..634a4014e11 100644
--- a/src/test/compile-fail/issue-26480.rs
+++ b/src/test/compile-fail/issue-26480.rs
@@ -26,6 +26,8 @@ macro_rules! write {
                   $arr.len() * size_of($arr[0]));
             //~^ ERROR mismatched types
             //~| expected u64, found usize
+            //~| expected type
+            //~| found type
         }
     }}
 }
@@ -38,6 +40,8 @@ fn main() {
     let hello = ['H', 'e', 'y'];
     write!(hello);
     //~^ NOTE in this expansion of write!
+    //~| NOTE in this expansion of write!
+    //~| NOTE in this expansion of write!
 
     cast!(2);
     //~^ NOTE in this expansion of cast!
diff --git a/src/test/compile-fail/moves-based-on-type-block-bad.rs b/src/test/compile-fail/moves-based-on-type-block-bad.rs
index 1fd69e2dbfe..deaff3c3521 100644
--- a/src/test/compile-fail/moves-based-on-type-block-bad.rs
+++ b/src/test/compile-fail/moves-based-on-type-block-bad.rs
@@ -32,9 +32,10 @@ fn main() {
     loop {
         f(&s, |hellothere| {
             match hellothere.x { //~ ERROR cannot move out
-                                 //~| move occurs here
+                                 //~| cannot move out of borrowed content
                 box E::Foo(_) => {}
-                box E::Bar(x) => println!("{}", x.to_string()), //~ NOTE attempting to move value to here
+                box E::Bar(x) => println!("{}", x.to_string()),
+                //~^ NOTE to prevent move
                 box E::Baz => {}
             }
         })
diff --git a/src/tools/compiletest/src/json.rs b/src/tools/compiletest/src/json.rs
index 3501b335205..84b78547ab9 100644
--- a/src/tools/compiletest/src/json.rs
+++ b/src/tools/compiletest/src/json.rs
@@ -12,6 +12,7 @@ use errors::{Error, ErrorKind};
 use rustc_serialize::json;
 use std::str::FromStr;
 use std::path::Path;
+use runtest::{ProcRes};
 
 // These structs are a subset of the ones found in
 // `syntax::errors::json`.
@@ -55,13 +56,13 @@ struct DiagnosticCode {
     explanation: Option<String>,
 }
 
-pub fn parse_output(file_name: &str, output: &str) -> Vec<Error> {
+pub fn parse_output(file_name: &str, output: &str, proc_res: &ProcRes) -> Vec<Error> {
     output.lines()
-          .flat_map(|line| parse_line(file_name, line))
+          .flat_map(|line| parse_line(file_name, line, output, proc_res))
           .collect()
 }
 
-fn parse_line(file_name: &str, line: &str) -> Vec<Error> {
+fn parse_line(file_name: &str, line: &str, output: &str, proc_res: &ProcRes) -> Vec<Error> {
     // The compiler sometimes intermingles non-JSON stuff into the
     // output.  This hack just skips over such lines. Yuck.
     if line.chars().next() == Some('{') {
@@ -72,7 +73,9 @@ fn parse_line(file_name: &str, line: &str) -> Vec<Error> {
                 expected_errors
             }
             Err(error) => {
-                panic!("failed to decode compiler output as json: `{}`", error);
+                proc_res.fatal(Some(&format!(
+                    "failed to decode compiler output as json: `{}`\noutput: {}\nline: {}",
+                    error, line, output)));
             }
         }
     } else {
diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs
index f89ff6b3849..e6dc3a9d360 100644
--- a/src/tools/compiletest/src/runtest.rs
+++ b/src/tools/compiletest/src/runtest.rs
@@ -1003,7 +1003,7 @@ actual:\n\
         let expect_note = expected_errors.iter().any(|ee| ee.kind == Some(ErrorKind::Note));
 
         // Parse the JSON output from the compiler and extract out the messages.
-        let actual_errors = json::parse_output(&file_name, &proc_res.stderr);
+        let actual_errors = json::parse_output(&file_name, &proc_res.stderr, &proc_res);
         let mut unexpected = 0;
         let mut not_found = 0;
         let mut found = vec![false; expected_errors.len()];
@@ -1547,21 +1547,7 @@ actual:\n\
 
     fn fatal_proc_rec(&self, err: &str, proc_res: &ProcRes) -> ! {
         self.error(err);
-        print!("\
-            status: {}\n\
-            command: {}\n\
-            stdout:\n\
-            ------------------------------------------\n\
-            {}\n\
-            ------------------------------------------\n\
-            stderr:\n\
-            ------------------------------------------\n\
-            {}\n\
-            ------------------------------------------\n\
-            \n",
-               proc_res.status, proc_res.cmdline, proc_res.stdout,
-               proc_res.stderr);
-        panic!();
+        proc_res.fatal(None);
     }
 
     fn _arm_exec_compiled_test(&self, env: Vec<(String, String)>) -> ProcRes {
@@ -2211,7 +2197,7 @@ struct ProcArgs {
     args: Vec<String>,
 }
 
-struct ProcRes {
+pub struct ProcRes {
     status: Status,
     stdout: String,
     stderr: String,
@@ -2223,6 +2209,29 @@ enum Status {
     Normal(ExitStatus),
 }
 
+impl ProcRes {
+    pub fn fatal(&self, err: Option<&str>) -> ! {
+        if let Some(e) = err {
+            println!("\nerror: {}", e);
+        }
+        print!("\
+            status: {}\n\
+            command: {}\n\
+            stdout:\n\
+            ------------------------------------------\n\
+            {}\n\
+            ------------------------------------------\n\
+            stderr:\n\
+            ------------------------------------------\n\
+            {}\n\
+            ------------------------------------------\n\
+            \n",
+               self.status, self.cmdline, self.stdout,
+               self.stderr);
+        panic!();
+    }
+}
+
 impl Status {
     fn code(&self) -> Option<i32> {
         match *self {