diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 77c6c368364..dc5195fa285 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -335,43 +335,56 @@ impl EmitterWriter { // which is...less weird, at least. In fact, in general, if // the rightmost span overlaps with any other span, we should // use the "hang below" version, so we can at least make it - // clear where the span *starts*. + // clear where the span *starts*. There's an exception for this + // logic, when the labels do not have a message: + // + // fn foo(x: u32) { + // -------------- + // | + // x_span + // + // instead of: + // + // fn foo(x: u32) { + // -------------- + // | | + // | x_span + // + // let mut annotations_position = vec![]; let mut line_len = 0; let mut p = 0; let mut ann_iter = annotations.iter().peekable(); while let Some(annotation) = ann_iter.next() { - let is_line = if let AnnotationType::MultilineLine(_) = annotation.annotation_type { - true - } else { - false - }; let peek = ann_iter.peek(); if let Some(next) = peek { - let next_is_line = if let AnnotationType::MultilineLine(_) = next.annotation_type { - true - } else { - false - }; - - if overlaps(next, annotation) && !is_line && !next_is_line { + if overlaps(next, annotation) && !annotation.is_line() && !next.is_line() + && annotation.has_label() + { + // This annotation needs a new line in the output. p += 1; } } annotations_position.push((p, annotation)); if let Some(next) = peek { - let next_is_line = if let AnnotationType::MultilineLine(_) = next.annotation_type { - true - } else { - false - }; let l = if let Some(ref label) = next.label { label.len() + 2 } else { 0 }; - if (overlaps(next, annotation) || next.end_col + l > annotation.start_col) - && !is_line && !next_is_line + if (overlaps(next, annotation) // Do not allow two labels to be in the same line + || next.end_col + l > annotation.start_col) // if they overlap including + // padding, to avoid situations like: + // + // fn foo(x: u32) { + // -------^------ + // | | + // fn_spanx_span + // + && !annotation.is_line() // Do not add a new line if this annotation or the + && !next.is_line() // next are vertical line placeholders. + && annotation.has_label() // Both labels must have some text, otherwise + && next.has_label() // they are not overlapping. { p += 1; } @@ -408,6 +421,17 @@ impl EmitterWriter { return; } + // Write the colunmn separator. + // + // After this we will have: + // + // 2 | fn foo() { + // | + // | + // | + // 3 | + // 4 | } + // | for pos in 0..line_len + 1 { draw_col_separator(buffer, line_offset + pos + 1, width_offset - 2); buffer.putc(line_offset + pos + 1, @@ -468,7 +492,8 @@ impl EmitterWriter { Style::UnderlineSecondary }; let pos = pos + 1; - if pos > 1 { + + if pos > 1 && annotation.has_label() { for p in line_offset + 1..line_offset + pos + 1 { buffer.putc(p, code_offset + annotation.start_col, @@ -546,16 +571,8 @@ impl EmitterWriter { // | | something about `foo` // | something about `fn foo()` annotations_position.sort_by(|a, b| { - fn len(a: &Annotation) -> usize { - // Account for usize underflows - if a.end_col > a.start_col { - a.end_col - a.start_col - } else { - a.start_col - a.end_col - } - } // Decreasing order - len(a.1).cmp(&len(b.1)).reverse() + a.1.len().cmp(&b.1.len()).reverse() }); // Write the underlines. diff --git a/src/librustc_errors/snippet.rs b/src/librustc_errors/snippet.rs index b8c1726443d..e70a5dd5ff8 100644 --- a/src/librustc_errors/snippet.rs +++ b/src/librustc_errors/snippet.rs @@ -151,6 +151,15 @@ impl Annotation { } } + /// Wether this annotation is a vertical line placeholder. + pub fn is_line(&self) -> bool { + if let AnnotationType::MultilineLine(_) = self.annotation_type { + true + } else { + false + } + } + pub fn is_multiline(&self) -> bool { match self.annotation_type { AnnotationType::Multiline(_) | @@ -161,6 +170,32 @@ impl Annotation { } } + pub fn len(&self) -> usize { + // Account for usize underflows + if self.end_col > self.start_col { + self.end_col - self.start_col + } else { + self.start_col - self.end_col + } + } + + pub fn has_label(&self) -> bool { + if let Some(ref label) = self.label { + // Consider labels with no text as effectively not being there + // to avoid weird output with unnecessary vertical lines, like: + // + // X | fn foo(x: u32) { + // | -------^------ + // | | | + // | | + // | + // + // Note that this would be the complete output users would see. + label.len() > 0 + } else { + false + } + } } #[derive(Debug)] diff --git a/src/libsyntax/test_snippet.rs b/src/libsyntax/test_snippet.rs index 98e574867b4..c6d6e6237f2 100644 --- a/src/libsyntax/test_snippet.rs +++ b/src/libsyntax/test_snippet.rs @@ -494,6 +494,7 @@ error: foo "#); } + #[test] fn overlaping_start_and_end() { test_harness(r#" @@ -544,3 +545,390 @@ error: foo "#); } + +#[test] +fn multiple_labels_primary_without_message() { + test_harness(r#" +fn foo() { + a { b { c } d } +} +"#, + vec![ + SpanLabel { + start: Position { + string: "b", + count: 1, + }, + end: Position { + string: "}", + count: 1, + }, + label: "", + }, + SpanLabel { + start: Position { + string: "a", + count: 1, + }, + end: Position { + string: "d", + count: 1, + }, + label: "`a` is a good letter", + }, + SpanLabel { + start: Position { + string: "c", + count: 1, + }, + end: Position { + string: "c", + count: 1, + }, + label: "", + }, + ], + r#" +error: foo + --> test.rs:3:7 + | +3 | a { b { c } d } + | ----^^^^-^^-- `a` is a good letter + +"#); +} + +#[test] +fn multiple_labels_secondary_without_message() { + test_harness(r#" +fn foo() { + a { b { c } d } +} +"#, + vec![ + SpanLabel { + start: Position { + string: "a", + count: 1, + }, + end: Position { + string: "d", + count: 1, + }, + label: "`a` is a good letter", + }, + SpanLabel { + start: Position { + string: "b", + count: 1, + }, + end: Position { + string: "}", + count: 1, + }, + label: "", + }, + ], + r#" +error: foo + --> test.rs:3:3 + | +3 | a { b { c } d } + | ^^^^-------^^ `a` is a good letter + +"#); +} + +#[test] +fn multiple_labels_primary_without_message_2() { + test_harness(r#" +fn foo() { + a { b { c } d } +} +"#, + vec![ + SpanLabel { + start: Position { + string: "b", + count: 1, + }, + end: Position { + string: "}", + count: 1, + }, + label: "`b` is a good letter", + }, + SpanLabel { + start: Position { + string: "a", + count: 1, + }, + end: Position { + string: "d", + count: 1, + }, + label: "", + }, + SpanLabel { + start: Position { + string: "c", + count: 1, + }, + end: Position { + string: "c", + count: 1, + }, + label: "", + }, + ], + r#" +error: foo + --> test.rs:3:7 + | +3 | a { b { c } d } + | ----^^^^-^^-- + | | + | `b` is a good letter + +"#); +} + +#[test] +fn multiple_labels_secondary_without_message_2() { + test_harness(r#" +fn foo() { + a { b { c } d } +} +"#, + vec![ + SpanLabel { + start: Position { + string: "a", + count: 1, + }, + end: Position { + string: "d", + count: 1, + }, + label: "", + }, + SpanLabel { + start: Position { + string: "b", + count: 1, + }, + end: Position { + string: "}", + count: 1, + }, + label: "`b` is a good letter", + }, + ], + r#" +error: foo + --> test.rs:3:3 + | +3 | a { b { c } d } + | ^^^^-------^^ + | | + | `b` is a good letter + +"#); +} + +#[test] +fn multiple_labels_without_message() { + test_harness(r#" +fn foo() { + a { b { c } d } +} +"#, + vec![ + SpanLabel { + start: Position { + string: "a", + count: 1, + }, + end: Position { + string: "d", + count: 1, + }, + label: "", + }, + SpanLabel { + start: Position { + string: "b", + count: 1, + }, + end: Position { + string: "}", + count: 1, + }, + label: "", + }, + ], + r#" +error: foo + --> test.rs:3:3 + | +3 | a { b { c } d } + | ^^^^-------^^ + +"#); +} + +#[test] +fn multiple_labels_without_message_2() { + test_harness(r#" +fn foo() { + a { b { c } d } +} +"#, + vec![ + SpanLabel { + start: Position { + string: "b", + count: 1, + }, + end: Position { + string: "}", + count: 1, + }, + label: "", + }, + SpanLabel { + start: Position { + string: "a", + count: 1, + }, + end: Position { + string: "d", + count: 1, + }, + label: "", + }, + SpanLabel { + start: Position { + string: "c", + count: 1, + }, + end: Position { + string: "c", + count: 1, + }, + label: "", + }, + ], + r#" +error: foo + --> test.rs:3:7 + | +3 | a { b { c } d } + | ----^^^^-^^-- + +"#); +} + +#[test] +fn multiple_labels_with_message() { + test_harness(r#" +fn foo() { + a { b { c } d } +} +"#, + vec![ + SpanLabel { + start: Position { + string: "a", + count: 1, + }, + end: Position { + string: "d", + count: 1, + }, + label: "`a` is a good letter", + }, + SpanLabel { + start: Position { + string: "b", + count: 1, + }, + end: Position { + string: "}", + count: 1, + }, + label: "`b` is a good letter", + }, + ], + r#" +error: foo + --> test.rs:3:3 + | +3 | a { b { c } d } + | ^^^^-------^^ + | | | + | | `b` is a good letter + | `a` is a good letter + +"#); +} + +#[test] +fn single_label_with_message() { + test_harness(r#" +fn foo() { + a { b { c } d } +} +"#, + vec![ + SpanLabel { + start: Position { + string: "a", + count: 1, + }, + end: Position { + string: "d", + count: 1, + }, + label: "`a` is a good letter", + }, + ], + r#" +error: foo + --> test.rs:3:3 + | +3 | a { b { c } d } + | ^^^^^^^^^^^^^ `a` is a good letter + +"#); +} + +#[test] +fn single_label_without_message() { + test_harness(r#" +fn foo() { + a { b { c } d } +} +"#, + vec![ + SpanLabel { + start: Position { + string: "a", + count: 1, + }, + end: Position { + string: "d", + count: 1, + }, + label: "", + }, + ], + r#" +error: foo + --> test.rs:3:3 + | +3 | a { b { c } d } + | ^^^^^^^^^^^^^ + +"#); +}