Report errors in external files (#6308)

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

1 files changed, 66 insertions, 53 deletions

diff --git a/crates/typst-library/src/model/bibliography.rs b/crates/typst-library/src/model/bibliography.rs
index 51e3b03b..11435657 100644
--- a/crates/typst-library/src/model/bibliography.rs
+++ b/crates/typst-library/src/model/bibliography.rs
@@ -19,7 +19,10 @@ use smallvec::{smallvec, SmallVec};
 use typst_syntax::{Span, Spanned};
 use typst_utils::{Get, ManuallyHash, NonZeroExt, PicoStr};
 
-use crate::diag::{bail, error, At, FileError, HintedStrResult, SourceResult, StrResult};
+use crate::diag::{
+    bail, error, At, HintedStrResult, LoadError, LoadResult, LoadedWithin, ReportPos,
+    SourceResult, StrResult,
+};
 use crate::engine::{Engine, Sink};
 use crate::foundations::{
     elem, Bytes, CastInfo, Content, Derived, FromValue, IntoValue, Label, NativeElement,
@@ -31,7 +34,7 @@ use crate::layout::{
     BlockBody, BlockElem, Em, GridCell, GridChild, GridElem, GridItem, HElem, PadElem,
     Sides, Sizing, TrackSizings,
 };
-use crate::loading::{DataSource, Load};
+use crate::loading::{format_yaml_error, DataSource, Load, LoadSource, Loaded};
 use crate::model::{
     CitationForm, CiteGroup, Destination, FootnoteElem, HeadingElem, LinkElem, ParElem,
     Url,
@@ -294,24 +297,21 @@ impl Bibliography {
         world: Tracked<dyn World + '_>,
         sources: Spanned<OneOrMultiple<DataSource>>,
     ) -> SourceResult<Derived<OneOrMultiple<DataSource>, Self>> {
-        let data = sources.load(world)?;
-        let bibliography = Self::decode(&sources.v, &data).at(sources.span)?;
+        let loaded = sources.load(world)?;
+        let bibliography = Self::decode(&loaded)?;
         Ok(Derived::new(sources.v, bibliography))
     }
 
     /// Decode a bibliography from loaded data sources.
     #[comemo::memoize]
     #[typst_macros::time(name = "load bibliography")]
-    fn decode(
-        sources: &OneOrMultiple<DataSource>,
-        data: &[Bytes],
-    ) -> StrResult<Bibliography> {
+    fn decode(data: &[Loaded]) -> SourceResult<Bibliography> {
         let mut map = IndexMap::new();
         let mut duplicates = Vec::<EcoString>::new();
 
         // We might have multiple bib/yaml files
-        for (source, data) in sources.0.iter().zip(data) {
-            let library = decode_library(source, data)?;
+        for d in data.iter() {
+            let library = decode_library(d)?;
             for entry in library {
                 match map.entry(Label::new(PicoStr::intern(entry.key()))) {
                     indexmap::map::Entry::Vacant(vacant) => {
@@ -325,7 +325,11 @@ impl Bibliography {
         }
 
         if !duplicates.is_empty() {
-            bail!("duplicate bibliography keys: {}", duplicates.join(", "));
+            // TODO: Store spans of entries for duplicate key error messages.
+            // Requires hayagriva entries to store their location, which should
+            // be fine, since they are 1kb anyway.
+            let span = data.first().unwrap().source.span;
+            bail!(span, "duplicate bibliography keys: {}", duplicates.join(", "));
         }
 
         Ok(Bibliography(Arc::new(ManuallyHash::new(map, typst_utils::hash128(data)))))
@@ -351,36 +355,47 @@ impl Debug for Bibliography {
 }
 
 /// Decode on library from one data source.
-fn decode_library(source: &DataSource, data: &Bytes) -> StrResult<Library> {
-    let src = data.as_str().map_err(FileError::from)?;
+fn decode_library(loaded: &Loaded) -> SourceResult<Library> {
+    let data = loaded.data.as_str().within(loaded)?;
 
-    if let DataSource::Path(path) = source {
+    if let LoadSource::Path(file_id) = loaded.source.v {
         // If we got a path, use the extension to determine whether it is
         // YAML or BibLaTeX.
-        let ext = Path::new(path.as_str())
+        let ext = file_id
+            .vpath()
+            .as_rooted_path()
             .extension()
             .and_then(OsStr::to_str)
             .unwrap_or_default();
 
         match ext.to_lowercase().as_str() {
-            "yml" | "yaml" => hayagriva::io::from_yaml_str(src)
-                .map_err(|err| eco_format!("failed to parse YAML ({err})")),
-            "bib" => hayagriva::io::from_biblatex_str(src)
-                .map_err(|errors| format_biblatex_error(src, Some(path), errors)),
-            _ => bail!("unknown bibliography format (must be .yml/.yaml or .bib)"),
+            "yml" | "yaml" => hayagriva::io::from_yaml_str(data)
+                .map_err(format_yaml_error)
+                .within(loaded),
+            "bib" => hayagriva::io::from_biblatex_str(data)
+                .map_err(format_biblatex_error)
+                .within(loaded),
+            _ => bail!(
+                loaded.source.span,
+                "unknown bibliography format (must be .yml/.yaml or .bib)"
+            ),
         }
     } else {
         // If we just got bytes, we need to guess. If it can be decoded as
         // hayagriva YAML, we'll use that.
-        let haya_err = match hayagriva::io::from_yaml_str(src) {
+        let haya_err = match hayagriva::io::from_yaml_str(data) {
             Ok(library) => return Ok(library),
             Err(err) => err,
         };
 
         // If it can be decoded as BibLaTeX, we use that isntead.
-        let bib_errs = match hayagriva::io::from_biblatex_str(src) {
-            Ok(library) => return Ok(library),
-            Err(err) => err,
+        let bib_errs = match hayagriva::io::from_biblatex_str(data) {
+            // If the file is almost valid yaml, but contains no `@` character
+            // it will be successfully parsed as an empty BibLaTeX library,
+            // since BibLaTeX does support arbitrary text outside of entries.
+            Ok(library) if !library.is_empty() => return Ok(library),
+            Ok(_) => None,
+            Err(err) => Some(err),
         };
 
         // If neither decoded correctly, check whether `:` or `{` appears
@@ -388,7 +403,7 @@ fn decode_library(source: &DataSource, data: &Bytes) -> StrResult<Library> {
         // and emit the more appropriate error.
         let mut yaml = 0;
         let mut biblatex = 0;
-        for c in src.chars() {
+        for c in data.chars() {
             match c {
                 ':' => yaml += 1,
                 '{' => biblatex += 1,
@@ -396,37 +411,33 @@ fn decode_library(source: &DataSource, data: &Bytes) -> StrResult<Library> {
             }
         }
 
-        if yaml > biblatex {
-            bail!("failed to parse YAML ({haya_err})")
-        } else {
-            Err(format_biblatex_error(src, None, bib_errs))
+        match bib_errs {
+            Some(bib_errs) if biblatex >= yaml => {
+                Err(format_biblatex_error(bib_errs)).within(loaded)
+            }
+            _ => Err(format_yaml_error(haya_err)).within(loaded),
         }
     }
 }
 
 /// Format a BibLaTeX loading error.
-fn format_biblatex_error(
-    src: &str,
-    path: Option<&str>,
-    errors: Vec<BibLaTeXError>,
-) -> EcoString {
-    let Some(error) = errors.first() else {
-        return match path {
-            Some(path) => eco_format!("failed to parse BibLaTeX file ({path})"),
-            None => eco_format!("failed to parse BibLaTeX"),
-        };
+fn format_biblatex_error(errors: Vec<BibLaTeXError>) -> LoadError {
+    // TODO: return multiple errors?
+    let Some(error) = errors.into_iter().next() else {
+        // TODO: can this even happen, should we just unwrap?
+        return LoadError::new(
+            ReportPos::None,
+            "failed to parse BibLaTeX",
+            "something went wrong",
+        );
     };
 
-    let (span, msg) = match error {
-        BibLaTeXError::Parse(error) => (&error.span, error.kind.to_string()),
-        BibLaTeXError::Type(error) => (&error.span, error.kind.to_string()),
+    let (range, msg) = match error {
+        BibLaTeXError::Parse(error) => (error.span, error.kind.to_string()),
+        BibLaTeXError::Type(error) => (error.span, error.kind.to_string()),
     };
 
-    let line = src.get(..span.start).unwrap_or_default().lines().count();
-    match path {
-        Some(path) => eco_format!("failed to parse BibLaTeX file ({path}:{line}: {msg})"),
-        None => eco_format!("failed to parse BibLaTeX ({line}: {msg})"),
-    }
+    LoadError::new(range, "failed to parse BibLaTeX", msg)
 }
 
 /// A loaded CSL style.
@@ -442,8 +453,8 @@ impl CslStyle {
         let style = match &source {
             CslSource::Named(style) => Self::from_archived(*style),
             CslSource::Normal(source) => {
-                let data = Spanned::new(source, span).load(world)?;
-                Self::from_data(data).at(span)?
+                let loaded = Spanned::new(source, span).load(world)?;
+                Self::from_data(&loaded.data).within(&loaded)?
             }
         };
         Ok(Derived::new(source, style))
@@ -464,16 +475,18 @@ impl CslStyle {
 
     /// Load a CSL style from file contents.
     #[comemo::memoize]
-    pub fn from_data(data: Bytes) -> StrResult<CslStyle> {
-        let text = data.as_str().map_err(FileError::from)?;
+    pub fn from_data(bytes: &Bytes) -> LoadResult<CslStyle> {
+        let text = bytes.as_str()?;
         citationberg::IndependentStyle::from_xml(text)
             .map(|style| {
                 Self(Arc::new(ManuallyHash::new(
                     style,
-                    typst_utils::hash128(&(TypeId::of::<Bytes>(), data)),
+                    typst_utils::hash128(&(TypeId::of::<Bytes>(), bytes)),
                 )))
             })
-            .map_err(|err| eco_format!("failed to load CSL style ({err})"))
+            .map_err(|err| {
+                LoadError::new(ReportPos::None, "failed to load CSL style", err)
+            })
     }
 
     /// Get the underlying independent style.