Structured resolver errors

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,7 +64,7 @@ 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(())
 }
@@ -294,99 +294,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 +319,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 +1077,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`"
                 )

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

@@ -0,0 +1,136 @@
+//! Error types for WIT package resolution.
+
+use alloc::boxed::Box;
+use alloc::format;
+use alloc::string::{String, ToString};
+use alloc::vec::Vec;
+use core::fmt;
+
+use crate::{PackageName, SourceMap, Span};
+
+/// Convenience alias for a `Result` whose error type is [`ResolveError`].
+pub type ResolveResult<T, E = ResolveError> = Result<T, E>;
+
+/// The category of error that occurred while resolving a WIT package.
+#[non_exhaustive]
+#[derive(Debug, PartialEq, Eq)]
+pub enum ResolveErrorKind {
+    /// A referenced package could not be found among the known packages.
+    PackageNotFound {
+        span: Span,
+        requested: PackageName,
+        known: Vec<PackageName>,
+    },
+    /// An interface has a transitive dependency that creates an incompatible
+    /// import relationship.
+    InvalidTransitiveDependency { span: Span, name: String },
+    /// The same package is defined in two different locations.
+    DuplicatePackage {
+        name: PackageName,
+        span1: Span,
+        span2: Span,
+    },
+    /// Packages form a dependency cycle.
+    PackageCycle { package: PackageName, span: Span },
+    /// A semantic error during resolution (type mismatch, invalid use, etc.)
+    Semantic { span: Span, message: String },
+}
+
+impl ResolveErrorKind {
+    /// Returns the source span associated with this error.
+    pub fn span(&self) -> Span {
+        match self {
+            ResolveErrorKind::PackageNotFound { span, .. }
+            | ResolveErrorKind::InvalidTransitiveDependency { span, .. }
+            | ResolveErrorKind::PackageCycle { span, .. }
+            | ResolveErrorKind::Semantic { span, .. } => *span,
+            ResolveErrorKind::DuplicatePackage { span1, .. } => *span1,
+        }
+    }
+}
+
+impl fmt::Display for ResolveErrorKind {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            ResolveErrorKind::PackageNotFound {
+                requested, known, ..
+            } => {
+                if known.is_empty() {
+                    write!(f, "package '{requested}' not found")
+                } else {
+                    write!(f, "package '{requested}' not found. known packages:")?;
+                    for k in known {
+                        write!(f, "\n    {k}")?;
+                    }
+                    Ok(())
+                }
+            }
+            ResolveErrorKind::InvalidTransitiveDependency { name, .. } => write!(
+                f,
+                "interface `{name}` transitively depends on an interface in incompatible ways",
+            ),
+            ResolveErrorKind::DuplicatePackage { name, .. } => {
+                write!(f, "package `{name}` is defined in two different locations",)
+            }
+            ResolveErrorKind::PackageCycle { package, .. } => {
+                write!(f, "package `{package}` creates a dependency cycle")
+            }
+            ResolveErrorKind::Semantic { message, .. } => message.fmt(f),
+        }
+    }
+}
+
+/// A single structured error from resolving a WIT package.
+#[derive(Debug, PartialEq, Eq)]
+pub struct ResolveError(Box<ResolveErrorKind>);
+
+impl ResolveError {
+    /// Creates a [`ResolveError`] with the [`ResolveErrorKind::Semantic`] variant.
+    pub fn new_semantic(span: Span, message: impl Into<String>) -> Self {
+        ResolveErrorKind::Semantic {
+            span,
+            message: message.into(),
+        }
+        .into()
+    }
+
+    /// Returns the underlying error kind.
+    pub fn kind(&self) -> &ResolveErrorKind {
+        &self.0
+    }
+
+    /// Returns the underlying error kind (mutable).
+    pub fn kind_mut(&mut self) -> &mut ResolveErrorKind {
+        &mut self.0
+    }
+
+    /// Format this error with source context (file:line:col + snippet).
+    pub fn highlight(&self, source_map: &SourceMap) -> String {
+        let e = self.kind();
+        let msg = e.to_string();
+        match e {
+            ResolveErrorKind::DuplicatePackage { name, span1, span2 } => {
+                let loc1 = source_map.render_location(*span1);
+                let loc2 = source_map.render_location(*span2);
+                format!(
+                    "package `{name}` is defined in two different locations:\n  * {loc1}\n  * {loc2}"
+                )
+            }
+            _ => source_map.highlight_span(e.span(), &msg).unwrap_or(msg),
+        }
+    }
+}
+
+impl fmt::Display for ResolveError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt::Display::fmt(self.kind(), f)
+    }
+}
+
+impl core::error::Error for ResolveError {}
+
+impl From<ResolveErrorKind> for ResolveError {
+    fn from(kind: ResolveErrorKind) -> Self {
+        ResolveError(Box::new(kind))
+    }
+}

crates/wit-parser/src/resolve/fs.rs

@@ -131,7 +131,9 @@ impl Resolve {
             .parse_deps_dir(&deps)
             .with_context(|| format!("failed to parse dependency directory: {}", deps.display()))?;
 
-        let (pkg_id, inner) = self.sort_unresolved_packages(top_pkg, deps)?;
+        let sort_result = self.sort_unresolved_packages(top_pkg, deps);
+        let (pkg_id, inner) =
+            sort_result.map_err(|e| anyhow::anyhow!("{}", e.highlight(&self.source_map)))?;
         Ok((pkg_id, PackageSourceMap::from_inner(inner)))
     }
 
@@ -195,7 +197,9 @@ impl Resolve {
         match self._push_file(path.as_ref())? {
             #[cfg(feature = "decoding")]
             ParsedFile::Package(id) => Ok(id),
-            ParsedFile::Unresolved(pkg) => self.push_group(pkg),
+            ParsedFile::Unresolved(pkg) => self
+                .push_group(pkg)
+                .map_err(|e| anyhow::anyhow!("{}", e.highlight(&self.source_map))),
         }
     }