Report errors in external files (#6308)

Co-authored-by: Laurenz <laurmaedje@gmail.com>

11 files changed, 165 insertions, 47 deletions

diff --git a/tests/src/collect.rs b/tests/src/collect.rs
index 84af04d2..173488b0 100644
--- a/tests/src/collect.rs
+++ b/tests/src/collect.rs
@@ -7,7 +7,9 @@ use std::sync::LazyLock;
 
 use ecow::{eco_format, EcoString};
 use typst_syntax::package::PackageVersion;
-use typst_syntax::{is_id_continue, is_ident, is_newline, FileId, Source, VirtualPath};
+use typst_syntax::{
+    is_id_continue, is_ident, is_newline, FileId, Lines, Source, VirtualPath,
+};
 use unscanny::Scanner;
 
 /// Collects all tests from all files.
@@ -79,6 +81,8 @@ impl Display for FileSize {
 pub struct Note {
     pub pos: FilePos,
     pub kind: NoteKind,
+    /// The file [`Self::range`] belongs to.
+    pub file: FileId,
     pub range: Option<Range<usize>>,
     pub message: String,
 }
@@ -341,9 +345,28 @@ impl<'a> Parser<'a> {
         let kind: NoteKind = head.parse().ok()?;
         self.s.eat_if(' ');
 
+        let mut file = None;
+        if self.s.eat_if('"') {
+            let path = self.s.eat_until(|c| is_newline(c) || c == '"');
+            if !self.s.eat_if('"') {
+                self.error("expected closing quote after file path");
+                return None;
+            }
+
+            let vpath = VirtualPath::new(path);
+            file = Some(FileId::new(None, vpath));
+
+            self.s.eat_if(' ');
+        }
+
         let mut range = None;
         if self.s.at('-') || self.s.at(char::is_numeric) {
-            range = self.parse_range(source);
+            if let Some(file) = file {
+                range = self.parse_range_external(file);
+            } else {
+                range = self.parse_range(source);
+            }
+
             if range.is_none() {
                 self.error("range is malformed");
                 return None;
@@ -359,11 +382,78 @@ impl<'a> Parser<'a> {
         Some(Note {
             pos: FilePos::new(self.path, self.line),
             kind,
+            file: file.unwrap_or(source.id()),
             range,
             message,
         })
     }
 
+    #[cfg(not(feature = "default"))]
+    fn parse_range_external(&mut self, _file: FileId) -> Option<Range<usize>> {
+        panic!("external file ranges are not expected when testing `typst_syntax`");
+    }
+
+    /// Parse a range in an external file, optionally abbreviated as just a position
+    /// if the range is empty.
+    #[cfg(feature = "default")]
+    fn parse_range_external(&mut self, file: FileId) -> Option<Range<usize>> {
+        use typst::foundations::Bytes;
+
+        use crate::world::{read, system_path};
+
+        let path = match system_path(file) {
+            Ok(path) => path,
+            Err(err) => {
+                self.error(err.to_string());
+                return None;
+            }
+        };
+
+        let bytes = match read(&path) {
+            Ok(data) => Bytes::new(data),
+            Err(err) => {
+                self.error(err.to_string());
+                return None;
+            }
+        };
+
+        let start = self.parse_line_col()?;
+        let lines = Lines::try_from(&bytes).expect(
+            "errors shouldn't be annotated for files \
+            that aren't human readable (not valid utf-8)",
+        );
+        let range = if self.s.eat_if('-') {
+            let (line, col) = start;
+            let start = lines.line_column_to_byte(line, col);
+            let (line, col) = self.parse_line_col()?;
+            let end = lines.line_column_to_byte(line, col);
+            Option::zip(start, end).map(|(a, b)| a..b)
+        } else {
+            let (line, col) = start;
+            lines.line_column_to_byte(line, col).map(|i| i..i)
+        };
+        if range.is_none() {
+            self.error("range is out of bounds");
+        }
+        range
+    }
+
+    /// Parses absolute `line:column` indices in an external file.
+    fn parse_line_col(&mut self) -> Option<(usize, usize)> {
+        let line = self.parse_number()?;
+        if !self.s.eat_if(':') {
+            self.error("positions in external files always require both `<line>:<col>`");
+            return None;
+        }
+        let col = self.parse_number()?;
+        if line < 0 || col < 0 {
+            self.error("line and column numbers must be positive");
+            return None;
+        }
+
+        Some(((line as usize).saturating_sub(1), (col as usize).saturating_sub(1)))
+    }
+
     /// Parse a range, optionally abbreviated as just a position if the range
     /// is empty.
     fn parse_range(&mut self, source: &Source) -> Option<Range<usize>> {
@@ -389,13 +479,13 @@ impl<'a> Parser<'a> {
         let line_idx = (line_idx_in_test + comments).checked_add_signed(line_delta)?;
         let column_idx = if column < 0 {
             // Negative column index is from the back.
-            let range = source.line_to_range(line_idx)?;
+            let range = source.lines().line_to_range(line_idx)?;
             text[range].chars().count().saturating_add_signed(column)
         } else {
             usize::try_from(column).ok()?.checked_sub(1)?
         };
 
-        source.line_column_to_byte(line_idx, column_idx)
+        source.lines().line_column_to_byte(line_idx, column_idx)
     }
 
     /// Parse a number.
diff --git a/tests/src/run.rs b/tests/src/run.rs
index 4d08362c..a34e38db 100644
--- a/tests/src/run.rs
+++ b/tests/src/run.rs
@@ -10,10 +10,11 @@ use typst::layout::{Abs, Frame, FrameItem, PagedDocument, Transform};
 use typst::visualize::Color;
 use typst::{Document, WorldExt};
 use typst_pdf::PdfOptions;
+use typst_syntax::FileId;
 
 use crate::collect::{Attr, FileSize, NoteKind, Test};
 use crate::logger::TestResult;
-use crate::world::TestWorld;
+use crate::world::{system_path, TestWorld};
 
 /// Runs a single test.
 ///
@@ -117,7 +118,7 @@ impl<'a> Runner<'a> {
             if seen {
                 continue;
             }
-            let note_range = self.format_range(&note.range);
+            let note_range = self.format_range(note.file, &note.range);
             if first {
                 log!(self, "not emitted");
                 first = false;
@@ -208,10 +209,6 @@ impl<'a> Runner<'a> {
     /// Compare a subset of notes with a given kind against diagnostics of
     /// that same kind.
     fn check_diagnostic(&mut self, kind: NoteKind, diag: &SourceDiagnostic) {
-        // Ignore diagnostics from other sources than the test file itself.
-        if diag.span.id().is_some_and(|id| id != self.test.source.id()) {
-            return;
-        }
         // TODO: remove this once HTML export is stable
         if diag.message == "html export is under active development and incomplete" {
             return;
@@ -219,11 +216,11 @@ impl<'a> Runner<'a> {
 
         let message = diag.message.replace("\\", "/");
         let range = self.world.range(diag.span);
-        self.validate_note(kind, range.clone(), &message);
+        self.validate_note(kind, diag.span.id(), range.clone(), &message);
 
         // Check hints.
         for hint in &diag.hints {
-            self.validate_note(NoteKind::Hint, range.clone(), hint);
+            self.validate_note(NoteKind::Hint, diag.span.id(), range.clone(), hint);
         }
     }
 
@@ -235,15 +232,18 @@ impl<'a> Runner<'a> {
     fn validate_note(
         &mut self,
         kind: NoteKind,
+        file: Option<FileId>,
         range: Option<Range<usize>>,
         message: &str,
     ) {
         // Try to find perfect match.
+        let file = file.unwrap_or(self.test.source.id());
         if let Some((i, _)) = self.test.notes.iter().enumerate().find(|&(i, note)| {
             !self.seen[i]
                 && note.kind == kind
                 && note.range == range
                 && note.message == message
+                && note.file == file
         }) {
             self.seen[i] = true;
             return;
@@ -257,7 +257,7 @@ impl<'a> Runner<'a> {
                 && (note.range == range || note.message == message)
         }) else {
             // Not even a close match, diagnostic is not annotated.
-            let diag_range = self.format_range(&range);
+            let diag_range = self.format_range(file, &range);
             log!(into: self.not_annotated, "  {kind}: {diag_range} {}", message);
             return;
         };
@@ -267,10 +267,10 @@ impl<'a> Runner<'a> {
 
         // Range is wrong.
         if range != note.range {
-            let note_range = self.format_range(&note.range);
-            let note_text = self.text_for_range(&note.range);
-            let diag_range = self.format_range(&range);
-            let diag_text = self.text_for_range(&range);
+            let note_range = self.format_range(note.file, &note.range);
+            let note_text = self.text_for_range(note.file, &note.range);
+            let diag_range = self.format_range(file, &range);
+            let diag_text = self.text_for_range(file, &range);
             log!(self, "mismatched range ({}):", note.pos);
             log!(self, "  message   | {}", note.message);
             log!(self, "  annotated | {note_range:<9} | {note_text}");
@@ -286,39 +286,49 @@ impl<'a> Runner<'a> {
     }
 
     /// Display the text for a range.
-    fn text_for_range(&self, range: &Option<Range<usize>>) -> String {
+    fn text_for_range(&self, file: FileId, range: &Option<Range<usize>>) -> String {
         let Some(range) = range else { return "No text".into() };
         if range.is_empty() {
-            "(empty)".into()
-        } else {
-            format!("`{}`", self.test.source.text()[range.clone()].replace('\n', "\\n"))
+            return "(empty)".into();
         }
+
+        let lines = self.world.lookup(file);
+        lines.text()[range.clone()].replace('\n', "\\n").replace('\r', "\\r")
     }
 
     /// Display a byte range as a line:column range.
-    fn format_range(&self, range: &Option<Range<usize>>) -> String {
+    fn format_range(&self, file: FileId, range: &Option<Range<usize>>) -> String {
         let Some(range) = range else { return "No range".into() };
+
+        let mut preamble = String::new();
+        if file != self.test.source.id() {
+            preamble = format!("\"{}\" ", system_path(file).unwrap().display());
+        }
+
         if range.start == range.end {
-            self.format_pos(range.start)
+            format!("{preamble}{}", self.format_pos(file, range.start))
         } else {
-            format!("{}-{}", self.format_pos(range.start,), self.format_pos(range.end,))
+            format!(
+                "{preamble}{}-{}",
+                self.format_pos(file, range.start),
+                self.format_pos(file, range.end)
+            )
         }
     }
 
     /// Display a position as a line:column pair.
-    fn format_pos(&self, pos: usize) -> String {
-        if let (Some(line_idx), Some(column_idx)) =
-            (self.test.source.byte_to_line(pos), self.test.source.byte_to_column(pos))
-        {
-            let line = self.test.pos.line + line_idx;
-            let column = column_idx + 1;
-            if line == 1 {
-                format!("{column}")
-            } else {
-                format!("{line}:{column}")
-            }
+    fn format_pos(&self, file: FileId, pos: usize) -> String {
+        let lines = self.world.lookup(file);
+
+        let res = lines.byte_to_line_column(pos).map(|(line, col)| (line + 1, col + 1));
+        let Some((line, col)) = res else {
+            return "oob".into();
+        };
+
+        if line == 1 {
+            format!("{col}")
         } else {
-            "oob".into()
+            format!("{line}:{col}")
         }
     }
 }
diff --git a/tests/src/world.rs b/tests/src/world.rs
index fe2bd45e..bc3e690b 100644
--- a/tests/src/world.rs
+++ b/tests/src/world.rs
@@ -20,6 +20,7 @@ use typst::text::{Font, FontBook, TextElem, TextSize};
 use typst::utils::{singleton, LazyHash};
 use typst::visualize::Color;
 use typst::{Feature, Library, World};
+use typst_syntax::Lines;
 
 /// A world that provides access to the tests environment.
 #[derive(Clone)]
@@ -84,6 +85,22 @@ impl TestWorld {
         let mut map = self.base.slots.lock();
         f(map.entry(id).or_insert_with(|| FileSlot::new(id)))
     }
+
+    /// Lookup line metadata for a file by id.
+    #[track_caller]
+    pub fn lookup(&self, id: FileId) -> Lines<String> {
+        self.slot(id, |slot| {
+            if let Some(source) = slot.source.get() {
+                let source = source.as_ref().expect("file is not valid");
+                source.lines()
+            } else if let Some(bytes) = slot.file.get() {
+                let bytes = bytes.as_ref().expect("file is not valid");
+                Lines::try_from(bytes).expect("file is not valid utf-8")
+            } else {
+                panic!("file id does not point to any source file");
+            }
+        })
+    }
 }
 
 /// Shared foundation of all test worlds.
@@ -149,7 +166,7 @@ impl FileSlot {
 }
 
 /// The file system path for a file ID.
-fn system_path(id: FileId) -> FileResult<PathBuf> {
+pub(crate) fn system_path(id: FileId) -> FileResult<PathBuf> {
     let root: PathBuf = match id.package() {
         Some(spec) => format!("tests/packages/{}-{}", spec.name, spec.version).into(),
         None => PathBuf::new(),
@@ -159,7 +176,7 @@ fn system_path(id: FileId) -> FileResult<PathBuf> {
 }
 
 /// Read a file.
-fn read(path: &Path) -> FileResult<Cow<'static, [u8]>> {
+pub(crate) fn read(path: &Path) -> FileResult<Cow<'static, [u8]>> {
     // Resolve asset.
     if let Ok(suffix) = path.strip_prefix("assets/") {
         return typst_dev_assets::get(&suffix.to_string_lossy())
diff --git a/tests/suite/loading/csv.typ b/tests/suite/loading/csv.typ
index 6f57ec45..046345be 100644
--- a/tests/suite/loading/csv.typ
+++ b/tests/suite/loading/csv.typ
@@ -18,12 +18,12 @@
 #csv("nope.csv")
 
 --- csv-invalid ---
-// Error: 6-28 failed to parse CSV (found 3 instead of 2 fields in line 3)
+// Error: "/assets/data/bad.csv" 3:1 failed to parse CSV (found 3 instead of 2 fields in line 3)
 #csv("/assets/data/bad.csv")
 
 --- csv-invalid-row-type-dict ---
 // Test error numbering with dictionary rows.
-// Error: 6-28 failed to parse CSV (found 3 instead of 2 fields in line 3)
+// Error: "/assets/data/bad.csv" 3:1 failed to parse CSV (found 3 instead of 2 fields in line 3)
 #csv("/assets/data/bad.csv", row-type: dictionary)
 
 --- csv-invalid-delimiter ---
diff --git a/tests/suite/loading/json.typ b/tests/suite/loading/json.typ
index c8df1ff6..9e433992 100644
--- a/tests/suite/loading/json.typ
+++ b/tests/suite/loading/json.typ
@@ -6,7 +6,7 @@
 #test(data.at(2).weight, 150)
 
 --- json-invalid ---
-// Error: 7-30 failed to parse JSON (expected value at line 3 column 14)
+// Error: "/assets/data/bad.json" 3:14 failed to parse JSON (expected value at line 3 column 14)
 #json("/assets/data/bad.json")
 
 --- json-decode-deprecated ---
diff --git a/tests/suite/loading/read.typ b/tests/suite/loading/read.typ
index b5c9c089..57bfc1d5 100644
--- a/tests/suite/loading/read.typ
+++ b/tests/suite/loading/read.typ
@@ -8,5 +8,5 @@
 #let data = read("/assets/text/missing.txt")
 
 --- read-invalid-utf-8 ---
-// Error: 18-40 file is not valid utf-8
+// Error: 18-40 failed to convert to string (file is not valid utf-8 in assets/text/bad.txt:1:1)
 #let data = read("/assets/text/bad.txt")
diff --git a/tests/suite/loading/toml.typ b/tests/suite/loading/toml.typ
index a4318a01..9d65da45 100644
--- a/tests/suite/loading/toml.typ
+++ b/tests/suite/loading/toml.typ
@@ -37,7 +37,7 @@
 ))
 
 --- toml-invalid ---
-// Error: 7-30 failed to parse TOML (expected `.`, `=` at line 1 column 16)
+// Error: "/assets/data/bad.toml" 1:16-2:1 failed to parse TOML (expected `.`, `=`)
 #toml("/assets/data/bad.toml")
 
 --- toml-decode-deprecated ---
diff --git a/tests/suite/loading/xml.typ b/tests/suite/loading/xml.typ
index 933f3c48..eed7db0a 100644
--- a/tests/suite/loading/xml.typ
+++ b/tests/suite/loading/xml.typ
@@ -24,7 +24,7 @@
 ),))
 
 --- xml-invalid ---
-// Error: 6-28 failed to parse XML (found closing tag 'data' instead of 'hello' in line 3)
+// Error: "/assets/data/bad.xml" 3:0 failed to parse XML (found closing tag 'data' instead of 'hello')
 #xml("/assets/data/bad.xml")
 
 --- xml-decode-deprecated ---
diff --git a/tests/suite/loading/yaml.typ b/tests/suite/loading/yaml.typ
index a8089052..ad171c6e 100644
--- a/tests/suite/loading/yaml.typ
+++ b/tests/suite/loading/yaml.typ
@@ -13,7 +13,7 @@
 #test(data.at("1"), "ok")
 
 --- yaml-invalid ---
-// Error: 7-30 failed to parse YAML (did not find expected ',' or ']' at line 2 column 1, while parsing a flow sequence at line 1 column 18)
+// Error: "/assets/data/bad.yaml" 2:1 failed to parse YAML (did not find expected ',' or ']' at line 2 column 1, while parsing a flow sequence at line 1 column 18)
 #yaml("/assets/data/bad.yaml")
 
 --- yaml-decode-deprecated ---
diff --git a/tests/suite/scripting/import.typ b/tests/suite/scripting/import.typ
index 49b66ee5..382e444c 100644
--- a/tests/suite/scripting/import.typ
+++ b/tests/suite/scripting/import.typ
@@ -334,6 +334,7 @@
 
 --- import-cyclic-in-other-file ---
 // Cyclic import in other file.
+// Error: "tests/suite/scripting/modules/cycle2.typ" 2:9-2:21 cyclic import
 #import "./modules/cycle1.typ": *
 
 This is never reached.
diff --git a/tests/suite/visualize/image.typ b/tests/suite/visualize/image.typ
index 73c4feff..45c70c4b 100644
--- a/tests/suite/visualize/image.typ
+++ b/tests/suite/visualize/image.typ
@@ -167,7 +167,7 @@ A #box(image("/assets/images/tiger.jpg", height: 1cm, width: 80%)) B
 #image("/assets/plugins/hello.wasm")
 
 --- image-bad-svg ---
-// Error: 2-33 failed to parse SVG (found closing tag 'g' instead of 'style' in line 4)
+// Error: "/assets/images/bad.svg" 4:3 failed to parse SVG (found closing tag 'g' instead of 'style')
 #image("/assets/images/bad.svg")
 
 --- image-decode-svg ---
@@ -176,7 +176,7 @@ A #box(image("/assets/images/tiger.jpg", height: 1cm, width: 80%)) B
 #image.decode(`<svg xmlns="http://www.w3.org/2000/svg" height="140" width="500"><ellipse cx="200" cy="80" rx="100" ry="50" style="fill:yellow;stroke:purple;stroke-width:2" /></svg>`.text, format: "svg")
 
 --- image-decode-bad-svg ---
-// Error: 2-168 failed to parse SVG (missing root node)
+// Error: 15-152 failed to parse SVG (missing root node at 1:1)
 // Warning: 8-14 `image.decode` is deprecated, directly pass bytes to `image` instead
 #image.decode(`<svg height="140" width="500"><ellipse cx="200" cy="80" rx="100" ry="50" style="fill:yellow;stroke:purple;stroke-width:2" /></svg>`.text, format: "svg")