Repository: rust-lang/annotate-snippets-rs
Pull request #131 (2024-06-14 23:01:12)

Fix annotating line endings
There have been a number of problems recently with annotating line endings:
- #130
- #117

I went ahead and tried my best to fix all of the current problems with annotating line endings, and in doing so, #116 appears to be fixed as well.

The two root problems were that we never really supported highlighting the end of a line in multiline spans, and even in cases where we did support highlighting the end of the line, we only supported highlighting the first of the line endings, if the line ended with `\r\n`.


Patch:
diff --git a/src/renderer/display_list.rs b/src/renderer/display_list.rs
index d94a660..bc46f7f 100644
--- a/src/renderer/display_list.rs
+++ b/src/renderer/display_list.rs
@@ -524,6 +524,7 @@ pub(crate) enum DisplaySourceLine<'a> {
     Content {
         text: &'a str,
         range: (usize, usize), // meta information for annotation placement.
+        end_line: EndLine,
     },
     /// An empty source line.
     Empty,
@@ -658,7 +659,8 @@ impl<'a> CursorLines<'a> {
     }
 }
 
-enum EndLine {
+#[derive(Copy, Clone, Debug, PartialEq)]
+pub(crate) enum EndLine {
     Eof = 0,
     Crlf = 1,
     Lf = 2,
@@ -847,13 +849,20 @@ fn format_header<'a>(
 
         for item in body {
             if let DisplayLine::Source {
-                line: DisplaySourceLine::Content { text, range },
+                line:
+                    DisplaySourceLine::Content {
+                        text,
+                        range,
+                        end_line,
+                    },
                 lineno,
                 ..
             } = item
             {
-                if main_range >= range.0 && main_range <= range.1 {
-                    let char_column = text[0..(main_range - range.0)].chars().count();
+                if main_range >= range.0 && main_range <= range.1 + *end_line as usize {
+                    let char_column = text[0..(main_range - range.0).min(text.len())]
+                        .chars()
+                        .count();
                     col = char_column + 1;
                     line_offset = lineno.unwrap_or(1);
                     break;
@@ -927,8 +936,18 @@ fn fold_body(body: Vec<DisplayLine<'_>>) -> Vec<DisplayLine<'_>> {
     let mut unhighlighed_lines = vec![];
     for line in body {
         match &line {
-            DisplayLine::Source { annotations, .. } => {
-                if annotations.is_empty() {
+            DisplayLine::Source {
+                annotations,
+                inline_marks,
+                ..
+            } => {
+                if annotations.is_empty()
+                    // A multiline start mark (`/`) needs be treated as an
+                    // annotation or the line could get folded.
+                    && inline_marks
+                        .iter()
+                        .all(|m| m.mark_type != DisplayMarkType::AnnotationStart)
+                {
                     unhighlighed_lines.push(line);
                 } else {
                     if lines.is_empty() {
@@ -1016,12 +1035,14 @@ fn format_body(
     for (idx, (line, end_line)) in CursorLines::new(snippet.source).enumerate() {
         let line_length: usize = line.len();
         let line_range = (current_index, current_index + line_length);
+        let end_line_size = end_line as usize;
         body.push(DisplayLine::Source {
             lineno: Some(current_line),
             inline_marks: vec![],
             line: DisplaySourceLine::Content {
                 text: line,
                 range: line_range,
+                end_line,
             },
             annotations: vec![],
         });
@@ -1045,7 +1066,7 @@ fn format_body(
         let line_start_index = line_range.0;
         let line_end_index = line_range.1;
         current_line += 1;
-        current_index += line_length + end_line as usize;
+        current_index += line_length + end_line_size;
 
         // It would be nice to use filter_drain here once it's stable.
         annotations.retain(|annotation| {
@@ -1057,18 +1078,24 @@ fn format_body(
             };
             let label_right = annotation.label.map_or(0, |label| label.len() + 1);
             match annotation.range {
-                Range { start, .. } if start > line_end_index => true,
+                // This handles if the annotation is on the next line. We add
+                // the `end_line_size` to account for annotating the line end.
+                Range { start, .. } if start > line_end_index + end_line_size => true,
+                // This handles the case where an annotation is contained
+                // within the current line including any line-end characters.
                 Range { start, end }
-                    if start >= line_start_index && end <= line_end_index
-                        // Allow annotating eof or stripped eol
-                        || start == line_end_index && end - start <= 1 =>
+                    if start >= line_start_index
+                        // We add at least one to `line_end_index` to allow
+                        // highlighting the end of a file
+                        && end <= line_end_index + max(end_line_size, 1) =>
                 {
                     if let DisplayLine::Source {
                         ref mut annotations,
                         ..
                     } = body[body_idx]
                     {
-                        let annotation_start_col = line[0..(start - line_start_index)]
+                        let annotation_start_col = line
+                            [0..(start - line_start_index).min(line_length)]
                             .chars()
                             .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
                             .sum::<usize>();
@@ -1101,11 +1128,16 @@ fn format_body(
                     }
                     false
                 }
+                // This handles the case where a multiline annotation starts
+                // somewhere on the current line, including any line-end chars
                 Range { start, end }
                     if start >= line_start_index
-                        && start <= line_end_index
+                        // The annotation can start on a line ending
+                        && start <= line_end_index + end_line_size.saturating_sub(1)
                         && end > line_end_index =>
                 {
+                    // Special case for multiline annotations that start at the
+                    // beginning of a line, which requires a special mark (`/`)
                     if start - line_start_index == 0 {
                         if let DisplayLine::Source {
                             ref mut inline_marks,
@@ -1122,7 +1154,8 @@ fn format_body(
                         ..
                     } = body[body_idx]
                     {
-                        let annotation_start_col = line[0..(start - line_start_index)]
+                        let annotation_start_col = line
+                            [0..(start - line_start_index).min(line_length)]
                             .chars()
                             .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
                             .sum::<usize>();
@@ -1147,7 +1180,11 @@ fn format_body(
                     }
                     true
                 }
-                Range { start, end } if start < line_start_index && end > line_end_index => {
+                // This handles the case where a multiline annotation starts
+                // somewhere before this line and ends after it as well
+                Range { start, end }
+                    if start < line_start_index && end > line_end_index + max(end_line_size, 1) =>
+                {
                     if let DisplayLine::Source {
                         ref mut inline_marks,
                         ..
@@ -1160,10 +1197,14 @@ fn format_body(
                     }
                     true
                 }
+                // This handles the case where a multiline annotation ends
+                // somewhere on the current line, including any line-end chars
                 Range { start, end }
                     if start < line_start_index
                         && end >= line_start_index
-                        && end <= line_end_index =>
+                        // We add at least one to `line_end_index` to allow
+                        // highlighting the end of a file
+                        && end <= line_end_index + max(end_line_size, 1) =>
                 {
                     if let DisplayLine::Source {
                         ref mut inline_marks,
@@ -1175,13 +1216,21 @@ fn format_body(
                             mark_type: DisplayMarkType::AnnotationThrough,
                             annotation_type: DisplayAnnotationType::from(annotation.level),
                         });
-                        let end_mark = line[0..(end - line_start_index)]
+                        let end_mark = line[0..(end - line_start_index).min(line_length)]
                             .chars()
                             .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
                             .sum::<usize>()
                             .saturating_sub(1);
-
-                        let end_plus_one = end_mark + 1;
+                        // If the annotation ends on a line-end character, we
+                        // need to annotate one past the end of the line
+                        let (end_mark, end_plus_one) = if end > line_end_index
+                            // Special case for highlighting the end of a file
+                            || (end == line_end_index + 1 && end_line_size == 0)
+                        {
+                            (end_mark + 1, end_mark + 2)
+                        } else {
+                            (end_mark, end_mark + 1)
+                        };
 
                         span_left_margin = min(span_left_margin, end_mark);
                         span_right_margin = max(span_right_margin, end_plus_one);


Test patch:
diff --git a/tests/fixtures/no-color/ann_multiline2.svg b/tests/fixtures/no-color/ann_multiline2.svg
index 18a9bf6..49c2c4b 100644
--- a/tests/fixtures/no-color/ann_multiline2.svg
+++ b/tests/fixtures/no-color/ann_multiline2.svg
@@ -1,4 +1,4 @@
-<svg width="740px" height="182px" xmlns="http://www.w3.org/2000/svg">
+<svg width="740px" height="164px" xmlns="http://www.w3.org/2000/svg">
   <style>
     .fg { fill: #AAAAAA }
     .bg { background: #000000 }
@@ -22,17 +22,15 @@
 </tspan>
     <tspan x="10px" y="64px"><tspan>   |</tspan>
 </tspan>
-    <tspan x="10px" y="82px"><tspan>26 |   This is an exampl</tspan>
+    <tspan x="10px" y="82px"><tspan>26 | This is an example</tspan>
 </tspan>
-    <tspan x="10px" y="100px"><tspan>   |  ____________^</tspan>
+    <tspan x="10px" y="100px"><tspan>   |            ^^^^^^^ this should not be on separate lines</tspan>
 </tspan>
-    <tspan x="10px" y="118px"><tspan>27 | | e of an edge case of an annotation overflowing</tspan>
+    <tspan x="10px" y="118px"><tspan>27 | of an edge case of an annotation overflowing</tspan>
 </tspan>
-    <tspan x="10px" y="136px"><tspan>   | |_^ this should not be on separate lines</tspan>
+    <tspan x="10px" y="136px"><tspan>28 | to exactly one character on next line.</tspan>
 </tspan>
-    <tspan x="10px" y="154px"><tspan>28 |   to exactly one character on next line.</tspan>
-</tspan>
-    <tspan x="10px" y="172px"><tspan>   |</tspan>
+    <tspan x="10px" y="154px"><tspan>   |</tspan>
 </tspan>
   </text>
 
diff --git a/tests/fixtures/no-color/ann_multiline2.toml b/tests/fixtures/no-color/ann_multiline2.toml
index dd85332..afb3aa9 100644
--- a/tests/fixtures/no-color/ann_multiline2.toml
+++ b/tests/fixtures/no-color/ann_multiline2.toml
@@ -5,8 +5,8 @@ title = "spacing error found"
 
 [[message.snippets]]
 source = """
-This is an exampl
-e of an edge case of an annotation overflowing
+This is an example
+of an edge case of an annotation overflowing
 to exactly one character on next line.
 """
 line_start = 26
@@ -15,4 +15,4 @@ fold = false
 [[message.snippets.annotations]]
 label = "this should not be on separate lines"
 level = "Error"
-range = [11, 18]
+range = [11, 19]
diff --git a/tests/fixtures/no-color/fold_ann_multiline.svg b/tests/fixtures/no-color/fold_ann_multiline.svg
index 0d2d67c..f82fe25 100644
--- a/tests/fixtures/no-color/fold_ann_multiline.svg
+++ b/tests/fixtures/no-color/fold_ann_multiline.svg
@@ -1,4 +1,4 @@
-<svg width="869px" height="218px" xmlns="http://www.w3.org/2000/svg">
+<svg width="869px" height="236px" xmlns="http://www.w3.org/2000/svg">
   <style>
     .fg { fill: #AAAAAA }
     .bg { background: #000000 }
@@ -28,15 +28,17 @@
 </tspan>
     <tspan x="10px" y="118px"><tspan>52 | /     for ann in annotations {</tspan>
 </tspan>
-    <tspan x="10px" y="136px"><tspan>...  |</tspan>
+    <tspan x="10px" y="136px"><tspan>53 | |         match (ann.range.0, ann.range.1) {</tspan>
 </tspan>
-    <tspan x="10px" y="154px"><tspan>71 | |         }</tspan>
+    <tspan x="10px" y="154px"><tspan>...  |</tspan>
 </tspan>
-    <tspan x="10px" y="172px"><tspan>72 | |     }</tspan>
+    <tspan x="10px" y="172px"><tspan>71 | |         }</tspan>
 </tspan>
-    <tspan x="10px" y="190px"><tspan>   | |_____^ expected enum `std::option::Option`, found ()</tspan>
+    <tspan x="10px" y="190px"><tspan>72 | |     }</tspan>
 </tspan>
-    <tspan x="10px" y="208px"><tspan>   |</tspan>
+    <tspan x="10px" y="208px"><tspan>   | |_____^ expected enum `std::option::Option`, found ()</tspan>
+</tspan>
+    <tspan x="10px" y="226px"><tspan>   |</tspan>
 </tspan>
   </text>
 
diff --git a/tests/formatter.rs b/tests/formatter.rs
index 5b746e1..fa3927c 100644
--- a/tests/formatter.rs
+++ b/tests/formatter.rs
@@ -303,3 +303,432 @@ LL | abc
     let renderer = Renderer::plain().anonymized_line_numbers(true);
     assert_data_eq!(renderer.render(input).to_string(), expected);
 }
+
+#[test]
+fn issue_130() {
+    let input = Level::Error.title("dummy").snippet(
+        Snippet::source("foo\nbar\nbaz")
+            .origin("file/path")
+            .line_start(3)
+            .fold(true)
+            .annotation(Level::Error.span(4..11)), // bar\nbaz
+    );
+
+    let expected = str![[r#"
+error: dummy
+ --> file/path:4:1
+  |
+4 | / bar
+5 | | baz
+  | |___^
+  |
+"#]];
+    let renderer = Renderer::plain();
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn unterminated_string_multiline() {
+    let source = "\
+a\"
+// ...
+";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .fold(true)
+            .annotation(Level::Error.span(0..10)), // 1..10 works
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:1
+  |
+3 | / a"
+4 | | // ...
+  | |_______^
+  |
+"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn char_and_nl_annotate_char() {
+    let source = "a\r\nb";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(0..2)), // a\r
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:1
+  |
+3 | a
+  | ^
+4 | b
+  |"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn char_eol_annotate_char() {
+    let source = "a\r\nb";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(0..3)), // a\r\n
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:1
+  |
+3 | a
+  | ^
+4 | b
+  |"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn char_eol_annotate_char_double_width() {
+    let snippets = Level::Error.title("").snippet(
+        Snippet::source("こん\r\nにちは\r\n世界")
+            .origin("<current file>")
+            .annotation(Level::Error.span(3..8)), // ん\r\n
+    );
+
+    let expected = str![[r#"
+error
+ --> <current file>:1:2
+  |
+1 | こん
+  |   ^^
+2 | にちは
+3 | 世界
+  |
+"#]];
+
+    let renderer = Renderer::plain();
+    assert_data_eq!(renderer.render(snippets).to_string(), expected);
+}
+
+#[test]
+fn annotate_eol() {
+    let source = "a\r\nb";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(1..2)), // \r
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 | a
+  |  ^
+4 | b
+  |"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn annotate_eol2() {
+    let source = "a\r\nb";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(1..3)), // \r\n
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 | a
+  |  ^
+4 | b
+  |"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn annotate_eol3() {
+    let source = "a\r\nb";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(2..3)), // \n
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 | a
+  |  ^
+4 | b
+  |"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn annotate_eol4() {
+    let source = "a\r\nb";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(2..2)), // \n
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 | a
+  |  ^
+4 | b
+  |"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn annotate_eol_double_width() {
+    let snippets = Level::Error.title("").snippet(
+        Snippet::source("こん\r\nにちは\r\n世界")
+            .origin("<current file>")
+            .annotation(Level::Error.span(7..8)), // \n
+    );
+
+    let expected = str![[r#"
+error
+ --> <current file>:1:3
+  |
+1 | こん
+  |     ^
+2 | にちは
+3 | 世界
+  |
+"#]];
+
+    let renderer = Renderer::plain();
+    assert_data_eq!(renderer.render(snippets).to_string(), expected);
+}
+
+#[test]
+fn multiline_eol_start() {
+    let source = "a\r\nb";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(1..4)), // \r\nb
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 |   a
+  |  __^
+4 | | b
+  | |_^
+  |"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn multiline_eol_start2() {
+    let source = "a\r\nb";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(2..4)), // \nb
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 |   a
+  |  __^
+4 | | b
+  | |_^
+  |"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn multiline_eol_start3() {
+    let source = "a\nb";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(1..3)), // \nb
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 |   a
+  |  __^
+4 | | b
+  | |_^
+  |"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn multiline_eol_start_double_width() {
+    let snippets = Level::Error.title("").snippet(
+        Snippet::source("こん\r\nにちは\r\n世界")
+            .origin("<current file>")
+            .annotation(Level::Error.span(7..11)), // \r\nに
+    );
+
+    let expected = str![[r#"
+error
+ --> <current file>:1:3
+  |
+1 |   こん
+  |  _____^
+2 | | にちは
+  | |__^
+3 |   世界
+  |
+"#]];
+
+    let renderer = Renderer::plain();
+    assert_data_eq!(renderer.render(snippets).to_string(), expected);
+}
+
+#[test]
+fn multiline_eol_start_eol_end() {
+    let source = "a\nb\nc";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(1..4)), // \nb\n
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 |   a
+  |  __^
+4 | | b
+  | |__^
+5 |   c
+  |
+"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn multiline_eol_start_eol_end2() {
+    let source = "a\r\nb\r\nc";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(2..5)), // \nb\r
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 |   a
+  |  __^
+4 | | b
+  | |__^
+5 |   c
+  |
+"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn multiline_eol_start_eol_end3() {
+    let source = "a\r\nb\r\nc";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(2..6)), // \nb\r\n
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 |   a
+  |  __^
+4 | | b
+  | |__^
+5 |   c
+  |
+"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn multiline_eol_start_eof_end() {
+    let source = "a\r\nb";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(1..5)), // \r\nb(EOF)
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 |   a
+  |  __^
+4 | | b
+  | |__^
+  |
+"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn multiline_eol_start_eof_end_double_width() {
+    let source = "ん\r\nに";
+    let input = Level::Error.title("").snippet(
+        Snippet::source(source)
+            .origin("file/path")
+            .line_start(3)
+            .annotation(Level::Error.span(3..9)), // \r\nに(EOF)
+    );
+    let expected = str![[r#"
+error
+ --> file/path:3:2
+  |
+3 |   ん
+  |  ___^
+4 | | に
+  | |___^
+  |
+"#]];
+    let renderer = Renderer::plain().anonymized_line_numbers(false);
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}