Allow updating syntax tests with ranges

Author: LeonMatthes

internal/compiler/diagnostics.rs

@@ -274,18 +274,36 @@ impl Diagnostic {
         }
     }
 
+    /// The exclusive end of this diagnostic.
+    /// Returns a tuple with the line (starting at 1) and column number (starting at 1)
+    ///
+    /// Can also return (0, 0) if the span is invalid
     pub fn end_line_column(&self) -> (usize, usize) {
         if !self.span.span.is_valid() {
             return (0, 0);
         }
-        let offset = self.span.span.offset + self.span.span.length;
+        // The end_line_column is exclusive.
+        // Even if the span indicates a length of 0, the diagnostic should always
+        // return an end_line_column that is at least one offset further, so that at least one
+        // character is covered by the diagnostic.
+        let offset = self.span.span.offset + self.span.span.length.max(1);
 
         match &self.span.source_file {
             None => (0, 0),
             Some(sl) => sl.line_column(offset),
         }
     }
 
+    /// Return the length of this diagnostic in characters.
+    pub fn length(&self) -> usize {
+        // Like in end_line_column, the length should always be 1, even if the span indicates a
+        // length of 0, as otherwise there is no character to display the diagnostic on.
+        self.span.span.length.max(1)
+    }
+
+    // NOTE: The return-type differs from the Spanned trait.
+    // Because this is public API (Diagnostic is re-exported by the Interpreter), we cannot change
+    // this.
     /// return the path of the source file where this error is attached
     pub fn source_file(&self) -> Option<&Path> {
         self.span.source_file().map(|sf| sf.path())
@@ -418,8 +436,10 @@ impl BuildDiagnostics {
                     });
                     let file_span = file.span;
                     let s = codemap_diagnostic::SpanLabel {
-                        span: file_span
-                            .subspan(d.span.span.offset as u64, (d.span.span.offset + d.span.span.length) as u64),
+                        span: file_span.subspan(
+                            d.span.span.offset as u64,
+                            (d.span.span.offset + d.span.span.length) as u64,
+                        ),
                         style: codemap_diagnostic::SpanStyle::Primary,
                         label: None,
                     };

internal/compiler/tests/syntax_tests.rs

@@ -212,8 +212,6 @@ fn process_diagnostics(
         .filter_map(|(i, c)| if c == b'\n' { Some(i) } else { None })
         .collect::<Vec<usize>>();
 
-    let diag_copy = diags.clone();
-
     let mut expected = extract_expected_diags(source);
     let captures: Vec<_> =
         expected.iter().map(|expected| &expected.comment_range).cloned().collect();
@@ -305,7 +303,7 @@ fn process_diagnostics(
 
     if !success && update {
         let mut source = source.to_string();
-        self::update(diag_copy, &mut source, lines, &captures);
+        self::update(&diags, &mut source, lines, &captures);
         std::fs::write(path, source).unwrap();
     }
 
@@ -314,7 +312,7 @@ fn process_diagnostics(
 
 /// Rewrite the source to remove the old comments and add accurate error comments
 fn update(
-    mut diags: Vec<&Diagnostic>,
+    diags: &[&Diagnostic],
     source: &mut String,
     mut lines: Vec<usize>,
     to_remove: &[std::ops::Range<usize>],
@@ -328,42 +326,57 @@ fn update(
         }
     }
 
-    diags.sort_by_key(|d| {
-        let (l, c) = d.line_column();
-        (usize::MAX - l, c)
-    });
-
-    let mut last_line = 0;
-    let mut last_line_adjust = 0;
+    let mut last_line_adjust = Vec::from_iter(std::iter::repeat_n(0, lines.len()));
 
     for d in diags {
-        let (l, c) = d.line_column();
-        if c < 3 {
-            panic!("Error message cannot be on the column < 3: {d:?}")
-        }
+        let mut insert_range_at = |range: &str, l, c: usize| {
+            let column_adjust = if c < 3 { "<".repeat(3 - c) } else { "".to_string() };
+            let byte_offset = lines[l - 1] + 1;
+            let to_insert = format!(
+                "//{indent}{range}{adjust}{column_adjust}{error_or_warning}{{{message}}}\n",
+                indent = " ".repeat(c.max(3) - 3),
+                adjust = "^".repeat(last_line_adjust[l - 1]),
+                error_or_warning =
+                    if d.level() == DiagnosticLevel::Error { "error" } else { "warning" },
+                message = d.message().replace('\n', "↵").replace(env!("CARGO_MANIFEST_DIR"), "📂")
+            );
+            if byte_offset > source.len() {
+                source.push('\n');
+            }
+            source.insert_str(byte_offset, &to_insert);
+            for line in (l - 1)..lines.len() {
+                lines[line] += to_insert.len();
+            }
+            last_line_adjust[l - 1] += 1;
+        };
 
-        if last_line == l {
-            last_line_adjust += 1;
+        let (line_start, column_start) = d.line_column();
+        let (line_end, column_end) = d.end_line_column();
+
+        // The end column is exclusive, therefore use - 1 here
+        let range = if d.length() <= 1 {
+            // Single-character diagnostic, use "^" for the marker
+            "^".to_owned()
         } else {
-            last_line = l;
-            last_line_adjust = 0;
-        }
+            let end = if line_start == line_end {
+                // Same line, we can insert the closing "<"
+                " ".repeat(column_end - column_start - 2) + "<"
+            } else {
+                // End is on a different line, we'll emit it later
+                "".to_owned()
+            };
+            format!(">{end}")
+        };
 
-        let byte_offset = lines[l - 1] + 1;
+        insert_range_at(&range, line_start, column_start);
 
-        let to_insert = format!(
-            "//{indent}^{adjust}{error_or_warning}{{{message}}}\n",
-            indent = " ".repeat(c - 3),
-            adjust = "^".repeat(last_line_adjust),
-            error_or_warning =
-                if d.level() == DiagnosticLevel::Error { "error" } else { "warning" },
-            message = d.message().replace('\n', "↵").replace(env!("CARGO_MANIFEST_DIR"), "📂")
-        );
-        if byte_offset > source.len() {
-            source.push('\n');
+        // Insert the closing `<` at another line if necessary
+        // Edge-case: If a single-character diagnostic is on a newline character (\n), its
+        // end_line_column is technically on a new line, but the single ^ marker is enough, so no
+        // closing character is needed.
+        if line_start != line_end && d.length() > 1 {
+            insert_range_at("<", line_end, column_end - 1);
         }
-        source.insert_str(byte_offset, &to_insert);
-        lines[l - 1] += to_insert.len();
     }
 }