Structured resolver errors (#2482)

* [wit-parser] Migrate to structured errors in the resolver

* Rename structs

* Remove unneeded methods

* Remove dangling doc

* Doc improvements

* Update snapshot

* Don't reference a non-existent method in docs

* Fix bad merge

* sm_idx -> *source_maps_index

* Revert "sm_idx -> *source_maps_index"

This reverts commit 0692157.

* Remove push_groups, push_group returns a structured error

* Remove stale reference

* Use fewer abbreviations

* Introduce ResolveError::new_semantic

* Bring back push_groups because it makes it easier to test things

* Rename for consistency

* Fix typo

* Preserve downcasting

* Add new variants to ResolveError

* Pass a more specific span

* Bless the snapshots

* Mention function name

* fmt

* bin fmt

* Trim down comment

* Tighter downcasting tests

Author: PhoebeSzmucer

crates/wit-parser/src/ast.rs

@@ -1845,25 +1845,6 @@ impl SourceMap {
         Ok((resolver.resolve()?, nested))
     }
 
-    /// Runs `f` and, on error, attempts to add source highlighting to resolver
-    /// error types that still use `anyhow`. Only needed until the resolver is
-    /// migrated to structured errors.
-    pub(crate) fn rewrite_error<F, T>(&self, f: F) -> anyhow::Result<T>
-    where
-        F: FnOnce() -> anyhow::Result<T>,
-    {
-        let mut err = match f() {
-            Ok(t) => return Ok(t),
-            Err(e) => e,
-        };
-        if let Some(e) = err.downcast_mut::<crate::Error>() {
-            e.highlight(self);
-        } else if let Some(e) = err.downcast_mut::<crate::PackageNotFoundError>() {
-            e.highlight(self);
-        }
-        Err(err)
-    }
-
     pub(crate) fn highlight_span(&self, span: Span, err: impl fmt::Display) -> Option<String> {
         if !span.is_known() {
             return None;

crates/wit-parser/src/ast/error.rs

@@ -1,11 +1,15 @@
+//! Error types for WIT parsing.
+
 use alloc::boxed::Box;
 use alloc::string::{String, ToString};
 use core::fmt;
 
 use crate::{SourceMap, Span, ast::lex};
 
+/// Convenience alias for a `Result` whose error type is [`ParseError`].
 pub type ParseResult<T, E = ParseError> = Result<T, E>;
 
+/// The category of error that occurred while parsing a WIT package.
 #[non_exhaustive]
 #[derive(Debug, PartialEq, Eq)]
 pub enum ParseErrorKind {
@@ -31,6 +35,7 @@ pub enum ParseErrorKind {
 }
 
 impl ParseErrorKind {
+    /// Returns the source span associated with this error.
     pub fn span(&self) -> Span {
         match self {
             ParseErrorKind::Lex(e) => Span::new(e.position(), e.position() + 1),
@@ -62,6 +67,7 @@ impl fmt::Display for ParseErrorKind {
     }
 }
 
+/// A single structured error from parsing a WIT package.
 #[derive(Debug, PartialEq, Eq)]
 pub struct ParseError(Box<ParseErrorKind>);
 
@@ -74,10 +80,12 @@ impl ParseError {
         .into()
     }
 
+    /// Returns the underlying error kind
     pub fn kind(&self) -> &ParseErrorKind {
         &self.0
     }
 
+    /// Returns the underlying error kind (mutable).
     pub fn kind_mut(&mut self) -> &mut ParseErrorKind {
         &mut self.0
     }

crates/wit-parser/src/decoding.rs

@@ -2,6 +2,7 @@ use crate::*;
 use alloc::string::{String, ToString};
 use alloc::vec;
 use alloc::vec::Vec;
+use anyhow::Result;
 use anyhow::{Context, anyhow, bail};
 use core::mem;
 use std::io::Read;

crates/wit-parser/src/lib.rs

@@ -10,8 +10,7 @@ use alloc::format;
 use alloc::string::{String, ToString};
 use alloc::vec::Vec;
 #[cfg(feature = "std")]
-use anyhow::Context;
-use anyhow::{Result, bail};
+use anyhow::Context as _;
 use id_arena::{Arena, Id};
 use semver::Version;
 
@@ -52,6 +51,7 @@ pub use ast::{ParsedUsePath, parse_use_path};
 mod sizealign;
 pub use sizealign::*;
 mod resolve;
+pub use resolve::error::*;
 pub use resolve::*;
 mod live;
 pub use live::{LiveTypes, TypeIdVisitor};
@@ -64,11 +64,38 @@ mod serde_;
 use serde_::*;
 
 /// Checks if the given string is a legal identifier in wit.
-pub fn validate_id(s: &str) -> Result<()> {
+pub fn validate_id(s: &str) -> anyhow::Result<()> {
     ast::validate_id(0, s)?;
     Ok(())
 }
 
+/// Renders an [`anyhow::Error`] chain produced by this crate, substituting
+/// snippet-bearing output for any [`ResolveError`] or [`ParseError`] layers.
+///
+/// For each layer in the chain, this calls [`ResolveError::highlight`] or
+/// [`ParseError::highlight`] to format typed errors with file/line/column and
+/// a source snippet. Other layers are formatted via their [`fmt::Display`]
+/// impl. Layers are joined with `": "`, matching `format!("{err:#}")`.
+///
+/// `source_map` must be the [`SourceMap`] in which every typed error's spans
+/// are valid; combining typed errors from different source maps in one chain
+/// is unsupported.
+#[cfg(feature = "std")]
+pub fn render_anyhow_error(err: &anyhow::Error, source_map: &SourceMap) -> String {
+    err.chain()
+        .map(|layer| {
+            if let Some(re) = layer.downcast_ref::<ResolveError>() {
+                re.highlight(source_map)
+            } else if let Some(pe) = layer.downcast_ref::<ParseError>() {
+                pe.highlight(source_map)
+            } else {
+                layer.to_string()
+            }
+        })
+        .collect::<Vec<_>>()
+        .join(": ")
+}
+
 pub type WorldId = Id<World>;
 pub type InterfaceId = Id<Interface>;
 pub type TypeId = Id<TypeDef>;
@@ -294,99 +321,14 @@ impl fmt::Display for PackageName {
     }
 }
 
-#[derive(Debug)]
-struct Error {
-    span: Span,
-    msg: String,
-    highlighted: Option<String>,
-}
-
-impl Error {
-    fn new(span: Span, msg: impl Into<String>) -> Error {
-        Error {
-            span,
-            msg: msg.into(),
-            highlighted: None,
-        }
-    }
-
-    /// Highlights this error using the given source map, if the span is known.
-    fn highlight(&mut self, source_map: &ast::SourceMap) {
-        if self.highlighted.is_none() {
-            self.highlighted = source_map.highlight_span(self.span, &self.msg);
-        }
-    }
-}
-
-impl fmt::Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.highlighted.as_ref().unwrap_or(&self.msg).fmt(f)
-    }
-}
-
-impl core::error::Error for Error {}
-
-#[derive(Debug)]
-struct PackageNotFoundError {
-    span: Span,
-    requested: PackageName,
-    known: Vec<PackageName>,
-    highlighted: Option<String>,
-}
-
-impl PackageNotFoundError {
-    pub fn new(span: Span, requested: PackageName, known: Vec<PackageName>) -> Self {
-        Self {
-            span,
-            requested,
-            known,
-            highlighted: None,
-        }
-    }
-
-    /// Highlights this error using the given source map, if the span is known.
-    fn highlight(&mut self, source_map: &ast::SourceMap) {
-        if self.highlighted.is_none() {
-            self.highlighted = source_map.highlight_span(self.span, &format!("{self}"));
-        }
-    }
-}
-
-impl fmt::Display for PackageNotFoundError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        if let Some(highlighted) = &self.highlighted {
-            return highlighted.fmt(f);
-        }
-        if self.known.is_empty() {
-            write!(
-                f,
-                "package '{}' not found. no known packages.",
-                self.requested
-            )?;
-        } else {
-            write!(
-                f,
-                "package '{}' not found. known packages:\n",
-                self.requested
-            )?;
-            for known in self.known.iter() {
-                write!(f, "    {known}\n")?;
-            }
-        }
-        Ok(())
-    }
-}
-
-impl core::error::Error for PackageNotFoundError {}
-
 impl UnresolvedPackageGroup {
     /// Parses the given string as a wit document.
     ///
     /// The `path` argument is used for error reporting. The `contents` provided
     /// are considered to be the contents of `path`. This function does not read
     /// the filesystem.
     #[cfg(feature = "std")]
-    pub fn parse(path: impl AsRef<Path>, contents: &str) -> Result<UnresolvedPackageGroup> {
+    pub fn parse(path: impl AsRef<Path>, contents: &str) -> anyhow::Result<UnresolvedPackageGroup> {
         let path = path
             .as_ref()
             .to_str()
@@ -404,7 +346,7 @@ impl UnresolvedPackageGroup {
     /// grouping. This is useful when a WIT package is split across multiple
     /// files.
     #[cfg(feature = "std")]
-    pub fn parse_dir(path: impl AsRef<Path>) -> Result<UnresolvedPackageGroup> {
+    pub fn parse_dir(path: impl AsRef<Path>) -> anyhow::Result<UnresolvedPackageGroup> {
         let path = path.as_ref();
         let mut map = SourceMap::default();
         let cx = || format!("failed to read directory {path:?}");
@@ -1162,12 +1104,12 @@ pub enum Mangling {
 impl core::str::FromStr for Mangling {
     type Err = anyhow::Error;
 
-    fn from_str(s: &str) -> Result<Mangling> {
+    fn from_str(s: &str) -> anyhow::Result<Mangling> {
         match s {
             "legacy" => Ok(Mangling::Legacy),
             "standard32" => Ok(Mangling::Standard32),
             _ => {
-                bail!(
+                anyhow::bail!(
                     "unknown name mangling `{s}`, \
                      supported values are `legacy` or `standard32`"
                 )