Auto merge of #42904 - estebank:number-suggestions, r=nikomatsakis

bors · bors · commit d2ebb12a1d6f · 2017-07-06T22:22:21.000Z
Make suggestion include the line number When there're more than one suggestions in the same diagnostic, they are displayed in their own block, instead of inline. In order to reduce confusion, those blocks now display the line number. New output: ``` error[E0308]: mismatched types --> ../../src/test/ui/block-result/unexpected-return-on-unit.rs:19:5 | 19 | foo() | ^^^^^ expected (), found usize | = note: expected type `()` found type `usize` help: did you mean to add a semicolon here? | 19 | foo(); | ^ help: possibly return type missing here? | 18 | fn bar() -> usize { | ^^^^^^^^ error: aborting due to previous error(s) ``` Fix #39152.
diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs
@@ -1060,44 +1060,72 @@ impl EmitterWriter {
                                -> io::Result<()> {
         use std::borrow::Borrow;
 
-        let primary_span = suggestion.substitution_spans().next().unwrap();
+        let primary_sub = &suggestion.substitution_parts[0];
         if let Some(ref cm) = self.cm {
             let mut buffer = StyledBuffer::new();
 
-            let lines = cm.span_to_lines(primary_span).unwrap();
+            let lines = cm.span_to_lines(primary_sub.span).unwrap();
 
             assert!(!lines.lines.is_empty());
 
             buffer.append(0, &level.to_string(), Style::Level(level.clone()));
             buffer.append(0, ": ", Style::HeaderMsg);
             self.msg_to_buffer(&mut buffer,
-                            &[(suggestion.msg.to_owned(), Style::NoStyle)],
-                            max_line_num_len,
-                            "suggestion",
-                            Some(Style::HeaderMsg));
+                               &[(suggestion.msg.to_owned(), Style::NoStyle)],
+                               max_line_num_len,
+                               "suggestion",
+                               Some(Style::HeaderMsg));
 
             let suggestions = suggestion.splice_lines(cm.borrow());
-            let mut row_num = 1;
-            for complete in suggestions.iter().take(MAX_SUGGESTIONS) {
-
-                // print the suggestion without any line numbers, but leave
-                // space for them. This helps with lining up with previous
-                // snippets from the actual error being reported.
+            let span_start_pos = cm.lookup_char_pos(primary_sub.span.lo);
+            let line_start = span_start_pos.line;
+            draw_col_separator_no_space(&mut buffer, 1, max_line_num_len + 1);
+            let mut row_num = 2;
+            for (&(ref complete, show_underline), ref sub) in suggestions
+                    .iter().zip(primary_sub.substitutions.iter()).take(MAX_SUGGESTIONS)
+            {
+                let mut line_pos = 0;
+                // Only show underline if there's a single suggestion and it is a single line
                 let mut lines = complete.lines();
                 for line in lines.by_ref().take(MAX_HIGHLIGHT_LINES) {
+                    // Print the span column to avoid confusion
+                    buffer.puts(row_num,
+                                0,
+                                &((line_start + line_pos).to_string()),
+                                Style::LineNumber);
+                    // print the suggestion
                     draw_col_separator(&mut buffer, row_num, max_line_num_len + 1);
                     buffer.append(row_num, line, Style::NoStyle);
+                    line_pos += 1;
                     row_num += 1;
+                    // Only show an underline in the suggestions if the suggestion is not the
+                    // entirety of the code being shown and the displayed code is not multiline.
+                    if show_underline {
+                        draw_col_separator(&mut buffer, row_num, max_line_num_len + 1);
+                        let sub_len = sub.trim_right().len();
+                        let underline_start = span_start_pos.col.0;
+                        let underline_end = span_start_pos.col.0 + sub_len;
+                        for p in underline_start..underline_end {
+                            buffer.putc(row_num,
+                                        max_line_num_len + 3 + p,
+                                        '^',
+                                        Style::UnderlinePrimary);
+                        }
+                        row_num += 1;
+                    }
                 }
 
                 // if we elided some lines, add an ellipsis
                 if let Some(_) = lines.next() {
-                    buffer.append(row_num, "...", Style::NoStyle);
+                    buffer.puts(row_num, max_line_num_len - 1, "...", Style::LineNumber);
+                } else if !show_underline {
+                    draw_col_separator_no_space(&mut buffer, row_num, max_line_num_len + 1);
+                    row_num += 1;
                 }
             }
             if suggestions.len() > MAX_SUGGESTIONS {
                 let msg = format!("and {} other candidates", suggestions.len() - MAX_SUGGESTIONS);
-                buffer.append(row_num, &msg, Style::NoStyle);
+                buffer.puts(row_num, 0, &msg, Style::NoStyle);
             }
             emit_to_destination(&buffer.render(), level, &mut self.dst)?;
         }
diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs
@@ -114,8 +114,8 @@ impl CodeSuggestion {
         self.substitution_parts.iter().map(|sub| sub.span)
     }
 
-    /// Returns the assembled code suggestions.
-    pub fn splice_lines(&self, cm: &CodeMapper) -> Vec<String> {
+    /// Returns the assembled code suggestions and wether they should be shown with an underline.
+    pub fn splice_lines(&self, cm: &CodeMapper) -> Vec<(String, bool)> {
         use syntax_pos::{CharPos, Loc, Pos};
 
         fn push_trailing(buf: &mut String,
@@ -138,7 +138,7 @@ impl CodeSuggestion {
         }
 
         if self.substitution_parts.is_empty() {
-            return vec![String::new()];
+            return vec![(String::new(), false)];
         }
 
         let mut primary_spans: Vec<_> = self.substitution_parts
@@ -175,14 +175,25 @@ impl CodeSuggestion {
         prev_hi.col = CharPos::from_usize(0);
 
         let mut prev_line = fm.get_line(lines.lines[0].line_index);
-        let mut bufs = vec![String::new(); self.substitutions()];
+        let mut bufs = vec![(String::new(), false); self.substitutions()];
 
         for (sp, substitutes) in primary_spans {
             let cur_lo = cm.lookup_char_pos(sp.lo);
-            for (buf, substitute) in bufs.iter_mut().zip(substitutes) {
+            for (&mut (ref mut buf, ref mut underline), substitute) in bufs.iter_mut()
+                                                                           .zip(substitutes) {
                 if prev_hi.line == cur_lo.line {
                     push_trailing(buf, prev_line.as_ref(), &prev_hi, Some(&cur_lo));
+
+                    // Only show an underline in the suggestions if the suggestion is not the
+                    // entirety of the code being shown and the displayed code is not multiline.
+                    if prev_line.as_ref().unwrap().trim().len() > 0
+                        && !substitute.ends_with('\n')
+                        && substitute.lines().count() == 1
+                    {
+                        *underline = true;
+                    }
                 } else {
+                    *underline = false;
                     push_trailing(buf, prev_line.as_ref(), &prev_hi, None);
                     // push lines between the previous and current span (if any)
                     for idx in prev_hi.line..(cur_lo.line - 1) {
@@ -200,7 +211,7 @@ impl CodeSuggestion {
             prev_hi = cm.lookup_char_pos(sp.hi);
             prev_line = fm.get_line(prev_hi.line - 1);
         }
-        for buf in &mut bufs {
+        for &mut (ref mut buf, _) in &mut bufs {
             // if the replacement already ends with a newline, don't print the next line
             if !buf.ends_with('\n') {
                 push_trailing(buf, prev_line.as_ref(), &prev_hi, None);
diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs
@@ -359,7 +359,7 @@ impl DiagnosticCode {
 
 impl JsonEmitter {
     fn render(&self, suggestion: &CodeSuggestion) -> Vec<String> {
-        suggestion.splice_lines(&*self.cm)
+        suggestion.splice_lines(&*self.cm).iter().map(|line| line.0.to_owned()).collect()
     }
 }
 
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
@@ -2920,10 +2920,6 @@ impl<'a> Parser<'a> {
                         err.cancel();
                         let codemap = self.sess.codemap();
                         let suggestion_span = lhs_span.to(self.prev_span);
-                        let suggestion = match codemap.span_to_snippet(suggestion_span) {
-                            Ok(lstring) => format!("({})", lstring),
-                            _ => format!("(<expression> as <type>)")
-                        };
                         let warn_message = match codemap.span_to_snippet(self.prev_span) {
                             Ok(lstring) => format!("`{}`", lstring),
                             _ => "a type".to_string(),
@@ -2934,6 +2930,10 @@ impl<'a> Parser<'a> {
                         let mut err = self.sess.span_diagnostic.struct_span_err(sp, &msg);
                         err.span_label(sp, "interpreted as generic argument");
                         err.span_label(self.span, "not interpreted as comparison");
+                        let suggestion = match codemap.span_to_snippet(suggestion_span) {
+                            Ok(lstring) => format!("({})", lstring),
+                            _ => format!("(<expression> as <type>)")
+                        };
                         err.span_suggestion(suggestion_span,
                                             "if you want to compare the casted value then write:",
                                             suggestion);
diff --git a/src/test/ui/block-result/unexpected-return-on-unit.stderr b/src/test/ui/block-result/unexpected-return-on-unit.stderr
@@ -7,9 +7,13 @@ error[E0308]: mismatched types
    = note: expected type `()`
               found type `usize`
 help: did you mean to add a semicolon here?
-   |     foo();
+   |
+19 |     foo();
+   |          ^
 help: possibly return type missing here?
-   | fn bar() -> usize {
+   |
+18 | fn bar() -> usize {
+   |          ^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issue-22644.rs b/src/test/ui/issue-22644.rs
@@ -14,5 +14,18 @@ fn main() {
 
     println!("{}", a as usize > b);
     println!("{}", a as usize < b);
-    println!("{}", a as usize < 4);
+    println!("{}", a
+                   as
+                   usize
+                   <
+                   4);
+    println!("{}", a
+
+
+                   as
+
+
+                   usize
+                   <
+                   5);
 }
diff --git a/src/test/ui/issue-22644.stderr b/src/test/ui/issue-22644.stderr
@@ -7,18 +7,42 @@ error: `<` is interpreted as a start of generic arguments for `usize`, not a com
    |                               not interpreted as comparison
    |
 help: if you want to compare the casted value then write:
-   |     println!("{}", (a as usize) < b);
+   |
+16 |     println!("{}", (a as usize) < b);
+   |                    ^^^^^^^^^^^^
 
 error: `<` is interpreted as a start of generic arguments for `usize`, not a comparison
-  --> $DIR/issue-22644.rs:17:33
+  --> $DIR/issue-22644.rs:21:20
    |
-17 |     println!("{}", a as usize < 4);
-   |                               - ^ interpreted as generic argument
-   |                               |
-   |                               not interpreted as comparison
+20 |                    <
+   |                    - not interpreted as comparison
+21 |                    4);
+   |                    ^ interpreted as generic argument
+   |
+help: if you want to compare the casted value then write:
+   |
+17 |     println!("{}", (a
+18 |                    as
+19 |                    usize)
+   |
+
+error: `<` is interpreted as a start of generic arguments for `usize`, not a comparison
+  --> $DIR/issue-22644.rs:30:20
+   |
+29 |                    <
+   |                    - not interpreted as comparison
+30 |                    5);
+   |                    ^ interpreted as generic argument
    |
 help: if you want to compare the casted value then write:
-   |     println!("{}", (a as usize) < 4);
+   |
+22 |     println!("{}", (a
+23 | 
+24 | 
+25 |                    as
+26 | 
+27 | 
+ ...
 
-error: aborting due to 2 previous errors
+error: aborting due to 3 previous errors
 
diff --git a/src/test/ui/resolve/enums-are-namespaced-xc.stderr b/src/test/ui/resolve/enums-are-namespaced-xc.stderr
@@ -5,7 +5,9 @@ error[E0425]: cannot find value `A` in module `namespaced_enums`
    |                               ^ not found in `namespaced_enums`
    |
 help: possible candidate is found in another module, you can import it into scope
-   | use namespaced_enums::Foo::A;
+   |
+12 | use namespaced_enums::Foo::A;
+   |
 
 error[E0425]: cannot find function `B` in module `namespaced_enums`
   --> $DIR/enums-are-namespaced-xc.rs:18:31
@@ -14,7 +16,9 @@ error[E0425]: cannot find function `B` in module `namespaced_enums`
    |                               ^ not found in `namespaced_enums`
    |
 help: possible candidate is found in another module, you can import it into scope
-   | use namespaced_enums::Foo::B;
+   |
+12 | use namespaced_enums::Foo::B;
+   |
 
 error[E0422]: cannot find struct, variant or union type `C` in module `namespaced_enums`
   --> $DIR/enums-are-namespaced-xc.rs:21:31
@@ -23,7 +27,9 @@ error[E0422]: cannot find struct, variant or union type `C` in module `namespace
    |                               ^ not found in `namespaced_enums`
    |
 help: possible candidate is found in another module, you can import it into scope
-   | use namespaced_enums::Foo::C;
+   |
+12 | use namespaced_enums::Foo::C;
+   |
 
 error: aborting due to 3 previous errors
 
diff --git a/src/test/ui/resolve/issue-16058.stderr b/src/test/ui/resolve/issue-16058.stderr
@@ -5,9 +5,13 @@ error[E0574]: expected struct, variant or union type, found enum `Result`
    |         ^^^^^^ not a struct, variant or union type
    |
 help: possible better candidates are found in other modules, you can import them into scope
-   | use std::fmt::Result;
-   | use std::io::Result;
-   | use std::thread::Result;
+   |
+12 | use std::fmt::Result;
+   |
+12 | use std::io::Result;
+   |
+12 | use std::thread::Result;
+   |
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/resolve/issue-17518.stderr b/src/test/ui/resolve/issue-17518.stderr
@@ -5,7 +5,9 @@ error[E0422]: cannot find struct, variant or union type `E` in this scope
    |     ^ not found in this scope
    |
 help: possible candidate is found in another module, you can import it into scope
-   | use SomeEnum::E;
+   |
+11 | use SomeEnum::E;
+   |
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/resolve/issue-21221-1.stderr b/src/test/ui/resolve/issue-21221-1.stderr
@@ -5,9 +5,13 @@ error[E0405]: cannot find trait `Mul` in this scope
    |      ^^^ not found in this scope
    |
 help: possible candidates are found in other modules, you can import them into scope
-   | use mul1::Mul;
-   | use mul2::Mul;
-   | use std::ops::Mul;
+   |
+11 | use mul1::Mul;
+   |
+11 | use mul2::Mul;
+   |
+11 | use std::ops::Mul;
+   |
 
 error[E0412]: cannot find type `Mul` in this scope
   --> $DIR/issue-21221-1.rs:72:16
@@ -16,10 +20,15 @@ error[E0412]: cannot find type `Mul` in this scope
    |                ^^^ not found in this scope
    |
 help: possible candidates are found in other modules, you can import them into scope
-   | use mul1::Mul;
-   | use mul2::Mul;
-   | use mul3::Mul;
-   | use mul4::Mul;
+   |
+11 | use mul1::Mul;
+   |
+11 | use mul2::Mul;
+   |
+11 | use mul3::Mul;
+   |
+11 | use mul4::Mul;
+   |
 and 2 other candidates
 
 error[E0405]: cannot find trait `ThisTraitReallyDoesntExistInAnyModuleReally` in this scope
@@ -35,7 +44,9 @@ error[E0405]: cannot find trait `Div` in this scope
    |      ^^^ not found in this scope
    |
 help: possible candidate is found in another module, you can import it into scope
-   | use std::ops::Div;
+   |
+11 | use std::ops::Div;
+   |
 
 error: cannot continue compilation due to previous error
 
diff --git a/src/test/ui/resolve/issue-21221-2.stderr b/src/test/ui/resolve/issue-21221-2.stderr
@@ -5,7 +5,9 @@ error[E0405]: cannot find trait `T` in this scope
    |      ^ not found in this scope
    |
 help: possible candidate is found in another module, you can import it into scope
-   | use foo::bar::T;
+   |
+11 | use foo::bar::T;
+   |
 
 error[E0601]: main function not found
 
diff --git a/src/test/ui/resolve/issue-21221-3.stderr b/src/test/ui/resolve/issue-21221-3.stderr
@@ -5,7 +5,9 @@ error[E0405]: cannot find trait `OuterTrait` in this scope
    |      ^^^^^^^^^^ not found in this scope
    |
 help: possible candidate is found in another module, you can import it into scope
-   | use issue_21221_3::outer::OuterTrait;
+   |
+16 | use issue_21221_3::outer::OuterTrait;
+   |
 
 error: cannot continue compilation due to previous error
 
diff --git a/src/test/ui/resolve/issue-21221-4.stderr b/src/test/ui/resolve/issue-21221-4.stderr
diff --git a/src/test/ui/resolve/issue-3907.stderr b/src/test/ui/resolve/issue-3907.stderr
diff --git a/src/test/ui/resolve/privacy-struct-ctor.stderr b/src/test/ui/resolve/privacy-struct-ctor.stderr
diff --git a/src/test/ui/span/issue-35987.stderr b/src/test/ui/span/issue-35987.stderr
diff --git a/src/test/ui/span/issue-39018.stderr b/src/test/ui/span/issue-39018.stderr

Original file line number	Diff line number	Diff line change
`@@ -359,7 +359,7 @@ impl DiagnosticCode {`
`359`	`359`
`360`	`360`	`impl JsonEmitter {`
`361`	`361`	`fn render(&self, suggestion: &CodeSuggestion) -> Vec<String> {`
`362`		`- suggestion.splice_lines(&*self.cm)`
	`362`	`+ suggestion.splice_lines(&*self.cm).iter().map(\|line\| line.0.to_owned()).collect()`
`363`	`363`	`}`
`364`	`364`	`}`
`365`	`365`
Original file line number	Diff line number	Diff line change
@@ -5,7 +5,9 @@ error[E0422]: cannot find struct, variant or union type `E` in this scope
`5`	`5`	`\| ^ not found in this scope`
`6`	`6`	`\|`
`7`	`7`	`help: possible candidate is found in another module, you can import it into scope`
`8`		`- \| use SomeEnum::E;`
	`8`	`+ \|`
	`9`	`+11 \| use SomeEnum::E;`
	`10`	`+ \|`
`9`	`11`
`10`	`12`	`error: aborting due to previous error`
`11`	`13`
Original file line number	Diff line number	Diff line change
@@ -5,7 +5,9 @@ error[E0405]: cannot find trait `T` in this scope
`5`	`5`	`\| ^ not found in this scope`
`6`	`6`	`\|`
`7`	`7`	`help: possible candidate is found in another module, you can import it into scope`
`8`		`- \| use foo::bar::T;`
	`8`	`+ \|`
	`9`	`+11 \| use foo::bar::T;`
	`10`	`+ \|`
`9`	`11`
`10`	`12`	`error[E0601]: main function not found`
`11`	`13`
Original file line number	Diff line number	Diff line change
@@ -5,7 +5,9 @@ error[E0405]: cannot find trait `OuterTrait` in this scope
`5`	`5`	`\| ^^^^^^^^^^ not found in this scope`
`6`	`6`	`\|`
`7`	`7`	`help: possible candidate is found in another module, you can import it into scope`
`8`		`- \| use issue_21221_3::outer::OuterTrait;`
	`8`	`+ \|`
	`9`	`+16 \| use issue_21221_3::outer::OuterTrait;`
	`10`	`+ \|`
`9`	`11`
`10`	`12`	`error: cannot continue compilation due to previous error`
`11`	`13`