From b1bdd14e343844f6d6b245f237e18aed6c573a06 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Sun, 15 Mar 2026 14:32:44 +0000 Subject: [PATCH 01/12] [wit-parser] Migrate to structured errors in the resolver --- crates/wit-parser/src/ast.rs | 19 - crates/wit-parser/src/decoding.rs | 1 + crates/wit-parser/src/lib.rs | 141 ++- crates/wit-parser/src/resolve/error.rs | 115 +++ crates/wit-parser/src/resolve/fs.rs | 4 +- crates/wit-parser/src/resolve/mod.rs | 829 ++++++++++-------- .../tests/ui/parse-fail/bad-gate3.wit.result | 2 +- .../tests/ui/parse-fail/bad-gate4.wit.result | 2 +- .../tests/ui/parse-fail/bad-gate5.wit.result | 2 +- .../tests/ui/parse-fail/bad-pkg6.wit.result | 1 - .../case-insensitive-duplicates.wit.result | 2 +- .../parse-fail/import-and-export1.wit.result | 4 +- .../parse-fail/import-and-export2.wit.result | 4 +- .../parse-fail/import-and-export3.wit.result | 4 +- .../parse-fail/import-and-export4.wit.result | 4 +- .../parse-fail/import-and-export5.wit.result | 4 +- .../multi-package-deps-share-nest.wit.result | 6 +- .../multiple-package-inline-cycle.wit.result | 2 +- ...nested-packages-colliding-names.wit.result | 6 +- .../tests/ui/parse-fail/pkg-cycle.wit.result | 2 +- .../tests/ui/parse-fail/pkg-cycle2.wit.result | 2 +- .../ui/parse-fail/return-borrow1.wit.result | 2 +- .../ui/parse-fail/return-borrow2.wit.result | 2 +- .../ui/parse-fail/return-borrow6.wit.result | 2 +- .../ui/parse-fail/return-borrow7.wit.result | 2 +- .../ui/parse-fail/return-borrow8.wit.result | 2 +- .../unresolved-interface4.wit.result | 1 - crates/wit-smith/src/lib.rs | 8 +- 28 files changed, 650 insertions(+), 525 deletions(-) create mode 100644 crates/wit-parser/src/resolve/error.rs diff --git a/crates/wit-parser/src/ast.rs b/crates/wit-parser/src/ast.rs index 06543df063..6321c3ee4c 100644 --- a/crates/wit-parser/src/ast.rs +++ b/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(&self, f: F) -> anyhow::Result - where - F: FnOnce() -> anyhow::Result, - { - let mut err = match f() { - Ok(t) => return Ok(t), - Err(e) => e, - }; - if let Some(e) = err.downcast_mut::() { - e.highlight(self); - } else if let Some(e) = err.downcast_mut::() { - e.highlight(self); - } - Err(err) - } - pub(crate) fn highlight_span(&self, span: Span, err: impl fmt::Display) -> Option { if !span.is_known() { return None; diff --git a/crates/wit-parser/src/decoding.rs b/crates/wit-parser/src/decoding.rs index 88253a3b6e..d1d6a8100e 100644 --- a/crates/wit-parser/src/decoding.rs +++ b/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; diff --git a/crates/wit-parser/src/lib.rs b/crates/wit-parser/src/lib.rs index 76461df98c..2346687e65 100644 --- a/crates/wit-parser/src/lib.rs +++ b/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,91 +294,6 @@ impl fmt::Display for PackageName { } } -#[derive(Debug)] -struct Error { - span: Span, - msg: String, - highlighted: Option, -} - -impl Error { - fn new(span: Span, msg: impl Into) -> 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, - highlighted: Option, -} - -impl PackageNotFoundError { - pub fn new(span: Span, requested: PackageName, known: Vec) -> 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. /// @@ -386,7 +301,7 @@ impl UnresolvedPackageGroup { /// are considered to be the contents of `path`. This function does not read /// the filesystem. #[cfg(feature = "std")] - pub fn parse(path: impl AsRef, contents: &str) -> Result { + pub fn parse(path: impl AsRef, contents: &str) -> anyhow::Result { let path = path .as_ref() .to_str() @@ -397,6 +312,48 @@ impl UnresolvedPackageGroup { .map_err(|(map, e)| anyhow::anyhow!("{}", e.highlight(&map))) } + /// 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. + pub fn parse_str( + path: &str, + contents: &str, + ) -> Result { + let mut map = SourceMap::default(); + map.push_str(path, contents); + map.parse() + } + + /// Parse a WIT package at the provided path. + /// + /// The path provided is inferred whether it's a file or a directory. A file + /// is parsed with [`UnresolvedPackageGroup::parse_file`] and a directory is + /// parsed with [`UnresolvedPackageGroup::parse_dir`]. + #[cfg(feature = "std")] + pub fn parse_path(path: impl AsRef) -> anyhow::Result { + let path = path.as_ref(); + if path.is_dir() { + UnresolvedPackageGroup::parse_dir(path) + } else { + UnresolvedPackageGroup::parse_file(path) + } + } + + /// Parses a WIT package from the file provided. + /// + /// The return value represents all packages found in the WIT file which + /// might be either one or multiple depending on the syntax used. + #[cfg(feature = "std")] + pub fn parse_file(path: impl AsRef) -> anyhow::Result { + let path = path.as_ref(); + let contents = std::fs::read_to_string(path) + .with_context(|| format!("failed to read file {path:?}"))?; + Self::parse(path, &contents) + } + + /// Parses a WIT package from the directory provided. /// /// This method will look at all files under the `path` specified. All @@ -404,7 +361,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) -> Result { + pub fn parse_dir(path: impl AsRef) -> anyhow::Result { let path = path.as_ref(); let mut map = SourceMap::default(); let cx = || format!("failed to read directory {path:?}"); @@ -1162,12 +1119,12 @@ pub enum Mangling { impl core::str::FromStr for Mangling { type Err = anyhow::Error; - fn from_str(s: &str) -> Result { + fn from_str(s: &str) -> anyhow::Result { match s { "legacy" => Ok(Mangling::Legacy), "standard32" => Ok(Mangling::Standard32), _ => { - bail!( + anyhow::bail!( "unknown name mangling `{s}`, \ supported values are `legacy` or `standard32`" ) diff --git a/crates/wit-parser/src/resolve/error.rs b/crates/wit-parser/src/resolve/error.rs new file mode 100644 index 0000000000..df3c249f5c --- /dev/null +++ b/crates/wit-parser/src/resolve/error.rs @@ -0,0 +1,115 @@ +use alloc::boxed::Box; +use alloc::format; +use alloc::string::{String, ToString}; +use alloc::vec::Vec; +use core::fmt; + +use crate::{PackageName, SourceMap, Span}; + +#[non_exhaustive] +#[derive(Debug, PartialEq, Eq)] +pub enum ResolveErrorKind { + PackageNotFound { + span: Span, + requested: PackageName, + known: Vec, + }, + InvalidTransitiveDependency { + span: Span, + name: String, + }, + DuplicatePackage { + name: PackageName, + span1: Span, + span2: Span, + }, + PackageCycle { + package: PackageName, + span: Span, + }, + Semantic { + span: Span, + message: String, + }, +} + +impl ResolveErrorKind { + 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), + } + } +} + +#[derive(Debug, PartialEq, Eq)] +pub struct ResolveErrors(Box); + +impl ResolveErrors { + pub fn kind(&self) -> &ResolveErrorKind { + &self.0 + } + + 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 ResolveErrors { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(self.kind(), f) + } +} + +impl core::error::Error for ResolveErrors {} + +impl From for ResolveErrors { + fn from(kind: ResolveErrorKind) -> Self { + ResolveErrors(Box::new(kind)) + } +} diff --git a/crates/wit-parser/src/resolve/fs.rs b/crates/wit-parser/src/resolve/fs.rs index 39ff86721c..a979156ed0 100644 --- a/crates/wit-parser/src/resolve/fs.rs +++ b/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))) } diff --git a/crates/wit-parser/src/resolve/mod.rs b/crates/wit-parser/src/resolve/mod.rs index c7d302c4c7..49f5fed5b9 100644 --- a/crates/wit-parser/src/resolve/mod.rs +++ b/crates/wit-parser/src/resolve/mod.rs @@ -4,11 +4,12 @@ use alloc::string::{String, ToString}; use alloc::vec::Vec; use alloc::{format, vec}; use core::cmp::Ordering; -use core::fmt; use core::mem; +use core::result::Result; +use crate::resolve::error::{ResolveErrorKind, ResolveErrors}; use crate::*; -use anyhow::{Context, Result, anyhow, bail}; +use anyhow::{Context, anyhow, bail}; #[cfg(not(feature = "std"))] use hashbrown::hash_map::Entry; use id_arena::{Arena, Id}; @@ -23,15 +24,16 @@ use crate::ast::{ParsedUsePath, parse_use_path}; #[cfg(feature = "serde")] use crate::serde_::{serialize_arena, serialize_id_map}; use crate::{ - AstItem, Docs, Error, Function, FunctionKind, Handle, IncludeName, Interface, InterfaceId, - LiftLowerAbi, ManglingAndAbi, PackageName, PackageNotFoundError, SourceMap, Stability, Type, - TypeDef, TypeDefKind, TypeId, TypeIdVisitor, TypeOwner, UnresolvedPackage, - UnresolvedPackageGroup, World, WorldId, WorldItem, WorldKey, + AstItem, Docs, Function, FunctionKind, Handle, IncludeName, Interface, InterfaceId, + LiftLowerAbi, ManglingAndAbi, PackageName, SourceMap, Stability, Type, TypeDef, TypeDefKind, + TypeId, TypeIdVisitor, TypeOwner, UnresolvedPackage, UnresolvedPackageGroup, World, WorldId, + WorldItem, WorldKey, }; pub use clone::CloneMaps; mod clone; +pub mod error; #[cfg(feature = "std")] mod fs; @@ -195,33 +197,38 @@ fn visit<'a>( pkg_details_map: &'a BTreeMap, order: &mut IndexSet, visiting: &mut HashSet<&'a PackageName>, - source_maps: &[SourceMap], -) -> Result<()> { + source_map_offsets: &[u32], +) -> Result<(), ResolveErrors> { if order.contains(&pkg.name) { return Ok(()); } - match pkg_details_map.get(&pkg.name) { - Some(pkg_details) => { - let (_, source_maps_index) = pkg_details; - source_maps[*source_maps_index].rewrite_error(|| { - for (i, (dep, _)) in pkg.foreign_deps.iter().enumerate() { - let span = pkg.foreign_dep_spans[i]; - if !visiting.insert(dep) { - bail!(Error::new(span, "package depends on itself")); - } - if let Some(dep) = pkg_details_map.get(dep) { - let (dep_pkg, _) = dep; - visit(dep_pkg, pkg_details_map, order, visiting, source_maps)?; - } - assert!(visiting.remove(dep)); - } - assert!(order.insert(pkg.name.clone())); - Ok(()) - }) + let (_, sm_idx) = pkg_details_map + .get(&pkg.name) + .expect("No pkg_details found for package when doing topological sort"); + let offset = source_map_offsets[*sm_idx]; + for (i, (dep, _)) in pkg.foreign_deps.iter().enumerate() { + let mut span = pkg.foreign_dep_spans[i]; + span.adjust(offset); + if !visiting.insert(dep) { + return Err(ResolveErrors::from(ResolveErrorKind::PackageCycle { + package: dep.clone(), + span, + })); + } + if let Some((dep_pkg, _)) = pkg_details_map.get(dep) { + visit( + dep_pkg, + pkg_details_map, + order, + visiting, + source_map_offsets, + )?; } - None => panic!("No pkg_details found for package when doing topological sort"), + assert!(visiting.remove(dep)); } + assert!(order.insert(pkg.name.clone())); + Ok(()) } impl Resolve { @@ -230,35 +237,18 @@ impl Resolve { Resolve::default() } - /// Parse WIT packages from the input `path`. - /// - /// The input `path` can be one of: - /// - /// * A directory containing a WIT package with an optional `deps` directory - /// for any dependent WIT packages it references. - /// * A single standalone WIT file. - /// * A wasm-encoded WIT package as a single file in the wasm binary format. - /// * A wasm-encoded WIT package as a single file in the wasm text format. - /// - /// In all of these cases packages are allowed to depend on previously - /// inserted packages into this `Resolve`. Resolution for packages is based - /// on the name of each package and reference. - /// - /// This method returns a `PackageId` and additionally a `PackageSourceMap`. - /// The `PackageId` represent the main package that was parsed. For example if a single WIT - /// file was specified this will be the main package found in the file. For a directory this - /// will be all the main package in the directory itself. The `PackageId` value is useful - /// to pass to [`Resolve::select_world`] to take a user-specified world in a - /// conventional fashion and select which to use for bindings generation. + /// Merge `main` and `deps` into this [`Resolve`], topologically sorting + /// them internally. Returns the [`PackageId`] of `main` and a + /// [`PackageSources`] covering all groups. fn sort_unresolved_packages( &mut self, main: UnresolvedPackageGroup, deps: Vec, - ) -> Result<(PackageId, PackageSources)> { - let mut pkg_details_map = BTreeMap::new(); - let mut source_maps = Vec::new(); + ) -> Result<(PackageId, PackageSources), ResolveErrors> { + let mut source_maps: Vec = Vec::new(); + let mut all_packages: Vec<(UnresolvedPackage, usize)> = Vec::new(); - let mut insert = |group: UnresolvedPackageGroup| { + let mut collect = |group: UnresolvedPackageGroup| { let UnresolvedPackageGroup { main, nested, @@ -266,31 +256,45 @@ impl Resolve { } = group; let i = source_maps.len(); source_maps.push(source_map); - for pkg in nested.into_iter().chain([main]) { - let name = pkg.name.clone(); - let my_span = pkg.package_name_span; - let (prev_pkg, prev_i) = match pkg_details_map.insert(name.clone(), (pkg, i)) { - Some(pair) => pair, - None => continue, - }; - let loc1 = source_maps[i].render_location(my_span); - let loc2 = source_maps[prev_i].render_location(prev_pkg.package_name_span); - bail!( - "\ -package {name} is defined in two different locations:\n\ - * {loc1}\n\ - * {loc2}\n\ - " - ) + all_packages.push((pkg, i)); } - Ok(()) }; let main_name = main.main.name.clone(); - insert(main)?; + collect(main); for dep in deps { - insert(dep)?; + collect(dep); + } + + // Merge all source maps into resolve.source_map upfront so that every + // span produced during toposort and duplicate detection is valid in + // resolve.source_map and can be located by rewrite_error below. + // Each group has exactly one source map, so a Vec of offsets suffices. + let source_map_offsets: Vec = source_maps + .iter() + .map(|sm| self.push_source_map(sm.clone())) + .collect(); + + let mut pkg_details_map: BTreeMap = + BTreeMap::new(); + for (pkg, sm_idx) in all_packages { + let name = pkg.name.clone(); + let my_span = pkg.package_name_span; + let offset = source_map_offsets[sm_idx]; + if let Some((prev_pkg, prev_idx)) = pkg_details_map.insert(name.clone(), (pkg, sm_idx)) + { + let prev_offset = source_map_offsets[prev_idx]; + let mut span1 = my_span; + span1.adjust(offset); + let mut span2 = prev_pkg.package_name_span; + span2.adjust(prev_offset); + return Err(ResolveErrors::from(ResolveErrorKind::DuplicatePackage { + name, + span1, + span2, + })); + } } // Perform a simple topological sort which will bail out on cycles @@ -299,39 +303,29 @@ package {name} is defined in two different locations:\n\ let mut order = IndexSet::default(); { let mut visiting = HashSet::new(); - for pkg_details in pkg_details_map.values() { - let (pkg, _) = pkg_details; + for (pkg, _) in pkg_details_map.values() { visit( pkg, &pkg_details_map, &mut order, &mut visiting, - &source_maps, + &source_map_offsets, )?; } } - // Ensure that the final output is topologically sorted. Track which source maps - // have been appended and their byte offsets to avoid duplicating them. let mut package_id_to_source_map_idx = BTreeMap::new(); let mut main_pkg_id = None; - let mut source_map_offsets: HashMap = HashMap::new(); for name in order { - let (pkg, source_map_index) = pkg_details_map.remove(&name).unwrap(); - let source_map = &source_maps[source_map_index]; + let (pkg, sm_idx) = pkg_details_map.remove(&name).unwrap(); + let span_offset = source_map_offsets[sm_idx]; let is_main = pkg.name == main_name; - - // Get or compute the span offset for this source map - let span_offset = *source_map_offsets - .entry(source_map_index) - .or_insert_with(|| self.push_source_map(source_map.clone())); - let id = self.push(pkg, span_offset)?; if is_main { assert!(main_pkg_id.is_none()); main_pkg_id = Some(id); } - package_id_to_source_map_idx.insert(id, source_map_index); + package_id_to_source_map_idx.insert(id, sm_idx); } Ok(( @@ -368,14 +362,14 @@ package {name} is defined in two different locations:\n\ &mut self, mut unresolved: UnresolvedPackage, span_offset: u32, - ) -> Result { + ) -> Result { unresolved.adjust_spans(span_offset); let ret = Remap::default().append(self, unresolved); if ret.is_ok() { #[cfg(debug_assertions)] self.assert_valid(); } - self.source_map.rewrite_error(|| ret) + ret } /// Appends new [`UnresolvedPackageGroup`] to this [`Resolve`], creating a @@ -385,9 +379,48 @@ package {name} is defined in two different locations:\n\ /// will be returned here, if successful a package identifier is returned /// which corresponds to the package that was just inserted. /// - /// The returned [`PackageId`]s are listed in topologically sorted order. - pub fn push_group(&mut self, unresolved_group: UnresolvedPackageGroup) -> Result { - let (pkg_id, _) = self.sort_unresolved_packages(unresolved_group, Vec::new())?; + /// If the package has dependencies that have not yet been pushed into this + /// [`Resolve`], use [`Resolve::push_groups`] instead to pass them all at + /// once and have dependency ordering and cycle detection handled internally. + /// Appends new [`UnresolvedPackageGroup`] to this [`Resolve`], creating a + /// fully resolved package with no dangling references. + /// + /// Any dependency resolution error or otherwise world-elaboration error + /// will be returned here, if successful a package identifier is returned + /// which corresponds to the package that was just inserted. + /// + /// If the package has dependencies that have not yet been pushed into this + /// [`Resolve`], use [`Resolve::push_groups`] instead to pass them all at + /// once and have dependency ordering and cycle detection handled internally. + pub fn push_group( + &mut self, + unresolved_group: UnresolvedPackageGroup, + ) -> anyhow::Result { + self.push_groups(unresolved_group, Vec::new()) + .map_err(|e| anyhow::anyhow!("{}", e.highlight(&self.source_map))) + } + + /// Appends a main [`UnresolvedPackageGroup`] and its dependencies to this + /// [`Resolve`] in a single call, returning a structured [`ResolveErrors`] on + /// failure. + /// + /// This is the preferred alternative to calling [`Resolve::push_group`] + /// repeatedly when you have a package and its local dependencies available + /// as in-memory [`UnresolvedPackageGroup`]s (e.g. from [`SourceMap::parse`] + /// or [`UnresolvedPackageGroup::parse_str`]). Wit-parser sorts them into + /// the correct topological order internally and detects dependency cycles. + /// + /// On error, spans in the returned [`ResolveErrors`] are absolute within + /// `self.source_map` and can be resolved with + /// [`SourceMap::get_location`]. + /// + /// The returned [`PackageId`] corresponds to `main`. + pub fn push_groups( + &mut self, + main: UnresolvedPackageGroup, + deps: Vec, + ) -> Result { + let (pkg_id, _) = self.sort_unresolved_packages(main, deps)?; Ok(pkg_id) } @@ -396,13 +429,9 @@ package {name} is defined in two different locations:\n\ /// The `path` provided is used for error messages but otherwise is not /// read. This method does not touch the filesystem. The `contents` provided /// are the contents of a WIT package. - pub fn push_source(&mut self, path: &str, contents: &str) -> Result { - let mut map = SourceMap::default(); - map.push_str(path, contents); - self.push_group( - map.parse() - .map_err(|(map, e)| anyhow::anyhow!("{}", e.highlight(&map)))?, - ) + pub fn push_source(&mut self, path: &str, contents: &str) -> anyhow::Result { + let group = UnresolvedPackageGroup::parse_str(path, contents).map_err(|(_, e)| e)?; + self.push_group(group) } /// Renders a span as a human-readable location string (e.g., "file.wit:10:5"). @@ -468,7 +497,7 @@ package {name} is defined in two different locations:\n\ /// URLs present. If found then it's assumed that both `Resolve` instances /// were originally created from the same contents and are two views /// of the same package. - pub fn merge(&mut self, resolve: Resolve) -> Result { + pub fn merge(&mut self, resolve: Resolve) -> anyhow::Result { log::trace!( "merging {} packages into {} packages", resolve.packages.len(), @@ -523,7 +552,7 @@ package {name} is defined in two different locations:\n\ for (id, mut ty) in types { let new_id = match type_map.get(&id).copied() { Some(id) => { - update_stability(&ty.stability, &mut self.types[id].stability)?; + update_stability(&ty.stability, &mut self.types[id].stability, ty.span)?; id } None => { @@ -542,7 +571,11 @@ package {name} is defined in two different locations:\n\ for (id, mut iface) in interfaces { let new_id = match interface_map.get(&id).copied() { Some(id) => { - update_stability(&iface.stability, &mut self.interfaces[id].stability)?; + update_stability( + &iface.stability, + &mut self.interfaces[id].stability, + iface.span, + )?; id } None => { @@ -561,7 +594,11 @@ package {name} is defined in two different locations:\n\ for (id, mut world) in worlds { let new_id = match world_map.get(&id).copied() { Some(world_id) => { - update_stability(&world.stability, &mut self.worlds[world_id].stability)?; + update_stability( + &world.stability, + &mut self.worlds[world_id].stability, + world.span, + )?; for from_import in world.imports.iter() { Resolve::update_world_imports_stability( from_import, @@ -581,24 +618,25 @@ package {name} is defined in two different locations:\n\ None => { log::debug!("moving world {}", world.name); moved_worlds.push(id); - let mut update = |map: &mut IndexMap| -> Result<_> { - for (mut name, mut item) in mem::take(map) { - remap.update_world_key(&mut name, Default::default())?; - match &mut item { - WorldItem::Function(f) => { - remap.update_function(self, f, Default::default())? - } - WorldItem::Interface { id, .. } => { - *id = remap.map_interface(*id, Default::default())? - } - WorldItem::Type { id, .. } => { - *id = remap.map_type(*id, Default::default())? + let mut update = + |map: &mut IndexMap| -> anyhow::Result<_> { + for (mut name, mut item) in mem::take(map) { + remap.update_world_key(&mut name, Default::default())?; + match &mut item { + WorldItem::Function(f) => { + remap.update_function(self, f, Default::default())? + } + WorldItem::Interface { id, .. } => { + *id = remap.map_interface(*id, Default::default())? + } + WorldItem::Type { id, .. } => { + *id = remap.map_type(*id, Default::default())? + } } + map.insert(name, item); } - map.insert(name, item); - } - Ok(()) - }; + Ok(()) + }; update(&mut world.imports)?; update(&mut world.exports)?; world.adjust_spans(span_offset); @@ -689,7 +727,7 @@ package {name} is defined in two different locations:\n\ from_item: (&WorldKey, &WorldItem), into_items: &mut IndexMap, interface_map: &HashMap, Id>, - ) -> Result<()> { + ) -> anyhow::Result<()> { match from_item.0 { WorldKey::Name(_) => { // No stability info to update here, only updating import/include stability @@ -703,7 +741,7 @@ package {name} is defined in two different locations:\n\ WorldItem::Interface { id: aid, stability: astability, - .. + span: aspan, }, WorldItem::Interface { id: bid, @@ -713,7 +751,7 @@ package {name} is defined in two different locations:\n\ ) => { let aid = interface_map.get(aid).copied().unwrap_or(*aid); assert_eq!(aid, *bid); - update_stability(astability, bstability)?; + update_stability(astability, bstability, *aspan)?; Ok(()) } _ => unreachable!(), @@ -746,7 +784,7 @@ package {name} is defined in two different locations:\n\ from: WorldId, into: WorldId, clone_maps: &mut CloneMaps, - ) -> Result<()> { + ) -> anyhow::Result<()> { let mut new_imports = Vec::new(); let mut new_exports = Vec::new(); @@ -863,7 +901,7 @@ package {name} is defined in two different locations:\n\ Ok(()) } - fn merge_world_item(&self, from: &WorldItem, into: &WorldItem) -> Result<()> { + fn merge_world_item(&self, from: &WorldItem, into: &WorldItem) -> anyhow::Result<()> { let mut map = MergeMap::new(self, self); match (from, into) { (WorldItem::Interface { id: from, .. }, WorldItem::Interface { id: into, .. }) => { @@ -928,7 +966,7 @@ package {name} is defined in two different locations:\n\ name: &WorldKey, item: &WorldItem, must_be_imported: &HashMap, - ) -> Result<()> { + ) -> anyhow::Result<()> { assert!(!into.exports.contains_key(name)); let name = self.name_world_key(name); @@ -960,7 +998,7 @@ package {name} is defined in two different locations:\n\ Ok(()) } - fn ensure_not_exported(&self, world: &World, id: InterfaceId) -> Result<()> { + fn ensure_not_exported(&self, world: &World, id: InterfaceId) -> anyhow::Result<()> { let key = WorldKey::Interface(id); let name = self.name_world_key(&key); if world.exports.contains_key(&key) { @@ -1054,7 +1092,11 @@ package {name} is defined in two different locations:\n\ /// bindings in a context that is importing the original world. This /// is intended to be used as part of language tooling when depending on /// other components. - pub fn importize(&mut self, world_id: WorldId, out_world_name: Option) -> Result<()> { + pub fn importize( + &mut self, + world_id: WorldId, + out_world_name: Option, + ) -> anyhow::Result<()> { // Rename the world to avoid having it get confused with the original // name of the world. Add `-importized` to it for now. Precisely how // this new world is created may want to be updated over time if this @@ -1095,7 +1137,8 @@ package {name} is defined in two different locations:\n\ // Fill out any missing transitive interface imports by elaborating this // world which does that for us. - self.elaborate_world(world_id)?; + let world_span = self.worlds[world_id].span; + self.elaborate_world(world_id, world_span)?; #[cfg(debug_assertions)] self.assert_valid(); @@ -1256,7 +1299,7 @@ package {name} is defined in two different locations:\n\ &self, main_packages: &[PackageId], world: Option<&str>, - ) -> Result { + ) -> anyhow::Result { // Determine if `world` is a kebab-name or an ID. let world_path = match world { Some(world) => Some( @@ -1810,8 +1853,7 @@ package {name} is defined in two different locations:\n\ stability: &Stability, pkg_id: &PackageId, span: Span, - ) -> Result { - let err = |msg: String| -> anyhow::Error { Error::new(span, msg).into() }; + ) -> Result { Ok(match stability { Stability::Unknown => true, // NOTE: deprecations are intentionally omitted -- an existing @@ -1831,22 +1873,28 @@ package {name} is defined in two different locations:\n\ // Use of feature gating with version specifiers inside a // package that is not versioned is not allowed let package_version = p.name.version.as_ref().ok_or_else(|| { - err(format!( - "package [{}] contains a feature gate with a version \ + ResolveErrors::from(ResolveErrorKind::Semantic { + span: span, + message: format!( + "package [{}] contains a feature gate with a version \ specifier, so it must have a version", - p.name - )) + p.name + ), + }) })?; // If the version on the feature gate is: // - released, then we can include it // - unreleased, then we must check the feature (if present) if since > package_version { - return Err(err(format!( - "feature gate cannot reference unreleased version \ + return Err(ResolveErrors::from(ResolveErrorKind::Semantic { + span, + message: format!( + "feature gate cannot reference unreleased version \ {since} of package [{}] (current version {package_version})", - p.name - ))); + p.name + ), + })); } true @@ -1857,19 +1905,6 @@ package {name} is defined in two different locations:\n\ }) } - /// Convenience wrapper around `include_stability` specialized for types - /// with a more targeted error message. - fn include_type(&self, ty: &TypeDef, pkgid: PackageId, span: Span) -> Result { - self.include_stability(&ty.stability, &pkgid, span) - .with_context(|| { - format!( - "failed to process feature gate for type [{}] in package [{}]", - ty.name.as_ref().map(String::as_str).unwrap_or(""), - self.packages[pkgid].name, - ) - }) - } - /// Performs the "elaboration process" necessary for the `world_id` /// specified to ensure that all of its transitive imports are listed. /// @@ -1881,7 +1916,7 @@ package {name} is defined in two different locations:\n\ /// noted on `elaborate_world_exports`. /// /// The world is mutated in-place in this `Resolve`. - fn elaborate_world(&mut self, world_id: WorldId) -> Result<()> { + fn elaborate_world(&mut self, world_id: WorldId, span: Span) -> Result<(), ResolveErrors> { // First process all imports. This is easier than exports since the only // requirement here is that all interfaces need to be added with a // topological order between them. @@ -1986,7 +2021,7 @@ package {name} is defined in two different locations:\n\ } } - self.elaborate_world_exports(&export_interfaces, &mut new_imports, &mut new_exports)?; + self.elaborate_world_exports(&export_interfaces, &mut new_imports, &mut new_exports, span)?; // In addition to sorting at the start of elaboration also sort here at // the end of elaboration to handle types being interspersed with @@ -2079,7 +2114,8 @@ package {name} is defined in two different locations:\n\ export_interfaces: &IndexMap, imports: &mut IndexMap, exports: &mut IndexMap, - ) -> Result<()> { + span: Span, + ) -> Result<(), ResolveErrors> { let mut required_imports = HashSet::new(); for (id, (key, stability)) in export_interfaces.iter() { let name = self.name_world_key(&key); @@ -2095,26 +2131,26 @@ package {name} is defined in two different locations:\n\ stability, ); if !ok { - bail!( - // FIXME: this is not a great error message and basically no - // one will know what to do when it gets printed. Improving - // this error message, however, is a chunk of work that may - // not be best spent doing this at this time, so I'm writing - // this comment instead. - // - // More-or-less what should happen here is that a "path" - // from this interface to the conflicting interface should - // be printed. It should be explained why an import is being - // injected, why that's conflicting with an export, and - // ideally with a suggestion of "add this interface to the - // export list to fix this error". - // - // That's a lot of info that's not easy to get at without - // more refactoring, so it's left to a future date in the - // hopes that most folks won't actually run into this for - // the time being. - InvalidTransitiveDependency(name), - ); + // FIXME: this is not a great error message and basically no + // one will know what to do when it gets printed. Improving + // this error message, however, is a chunk of work that may + // not be best spent doing this at this time, so I'm writing + // this comment instead. + // + // More-or-less what should happen here is that a "path" + // from this interface to the conflicting interface should + // be printed. It should be explained why an import is being + // injected, why that's conflicting with an export, and + // ideally with a suggestion of "add this interface to the + // export list to fix this error". + // + // That's a lot of info that's not easy to get at without + // more refactoring, so it's left to a future date in the + // hopes that most folks won't actually run into this for + // the time being. + return Err(ResolveErrors::from( + ResolveErrorKind::InvalidTransitiveDependency { name, span }, + )); } } return Ok(()); @@ -2189,7 +2225,7 @@ package {name} is defined in two different locations:\n\ /// and 0.2.1 then the result afterwards will be that it imports /// 0.2.1. If, however, 0.3.0 where imported then the final result would /// import both 0.2.0 and 0.3.0. - pub fn merge_world_imports_based_on_semver(&mut self, world_id: WorldId) -> Result<()> { + pub fn merge_world_imports_based_on_semver(&mut self, world_id: WorldId) -> anyhow::Result<()> { let world = &self.worlds[world_id]; // The first pass here is to build a map of "semver tracks" where they @@ -2315,13 +2351,8 @@ package {name} is defined in two different locations:\n\ // modified directly. let ids = self.worlds.iter().map(|(id, _)| id).collect::>(); for world_id in ids { - self.elaborate_world(world_id).with_context(|| { - let name = &self.worlds[world_id].name; - format!( - "failed to elaborate world `{name}` after deduplicating imports \ - based on semver" - ) - })?; + let world_span = self.worlds[world_id].span; + self.elaborate_world(world_id, world_span)?; } #[cfg(debug_assertions)] @@ -2997,7 +3028,12 @@ pub struct Remap { type_has_borrow: Vec>, } -fn apply_map(map: &[Option>], id: Id, desc: &str, span: Span) -> Result> { +fn apply_map( + map: &[Option>], + id: Id, + desc: &str, + span: Span, +) -> Result, ResolveErrors> { match map.get(id.index()) { Some(Some(id)) => Ok(*id), Some(None) => { @@ -3005,7 +3041,10 @@ fn apply_map(map: &[Option>], id: Id, desc: &str, span: Span) -> Res "found a reference to a {desc} which is excluded \ due to its feature not being activated" ); - Err(Error::new(span, msg).into()) + Err(ResolveErrors::from(ResolveErrorKind::Semantic { + span, + message: msg, + })) } None => panic!("request to remap a {desc} that has not yet been registered"), } @@ -3026,38 +3065,57 @@ fn rename(original_name: &str, include_name: &IncludeName) -> Option { } impl Remap { - pub fn map_type(&self, id: TypeId, span: Span) -> Result { + pub fn map_type(&self, id: TypeId, span: Span) -> Result { apply_map(&self.types, id, "type", span) } - pub fn map_interface(&self, id: InterfaceId, span: Span) -> Result { + pub fn map_interface(&self, id: InterfaceId, span: Span) -> Result { apply_map(&self.interfaces, id, "interface", span) } - pub fn map_world(&self, id: WorldId, span: Span) -> Result { + pub fn map_world(&self, id: WorldId, span: Span) -> Result { apply_map(&self.worlds, id, "world", span) } + pub fn map_world_for_type(&self, id: WorldId, span: Span) -> Result { + self.map_world(id, span).map_err(|e| { + ResolveErrors::from(ResolveErrorKind::Semantic { + span, + message: format!("{e}; this type is not gated by a feature but its world is"), + }) + }) + } + + pub fn map_interface_for_type( + &self, + id: InterfaceId, + span: Span, + ) -> Result { + self.map_interface(id, span).map_err(|e| { + ResolveErrors::from(ResolveErrorKind::Semantic { + span, + message: format!("{e}; this type is not gated by a feature but its interface is"), + }) + }) + } + fn append( &mut self, resolve: &mut Resolve, unresolved: UnresolvedPackage, - ) -> Result { + ) -> Result { let pkgid = resolve.packages.alloc(Package { name: unresolved.name.clone(), docs: unresolved.docs.clone(), interfaces: Default::default(), worlds: Default::default(), }); - let prev = resolve.package_names.insert(unresolved.name.clone(), pkgid); - if let Some(prev) = prev { - resolve.package_names.insert(unresolved.name.clone(), prev); - bail!( - "attempting to re-add package `{}` when it's already present in this `Resolve`", - unresolved.name, - ); - } - + assert!( + !resolve.package_names.contains_key(&unresolved.name), + "attempting to re-add package `{}` when it's already present in this `Resolve`", + unresolved.name, + ); + resolve.package_names.insert(unresolved.name.clone(), pkgid); self.process_foreign_deps(resolve, pkgid, &unresolved)?; let foreign_types = self.types.len(); @@ -3071,7 +3129,7 @@ impl Remap { // yet. for (id, mut ty) in unresolved.types.into_iter().skip(foreign_types) { let span = ty.span; - if !resolve.include_type(&ty, pkgid, span)? { + if !resolve.include_stability(&ty.stability, &pkgid, span)? { self.types.push(None); continue; } @@ -3105,20 +3163,7 @@ impl Remap { // referenced along the way. for (id, mut iface) in unresolved.interfaces.into_iter().skip(foreign_interfaces) { let span = iface.span; - if !resolve - .include_stability(&iface.stability, &pkgid, span) - .with_context(|| { - format!( - "failed to process feature gate for interface [{}] in package [{}]", - iface - .name - .as_ref() - .map(String::as_str) - .unwrap_or(""), - resolve.packages[pkgid].name, - ) - })? - { + if !resolve.include_stability(&iface.stability, &pkgid, span)? { self.interfaces.push(None); continue; } @@ -3140,10 +3185,7 @@ impl Remap { let span = resolve.types[id].span; match &mut resolve.types[id].owner { TypeOwner::Interface(iface_id) => { - *iface_id = self.map_interface(*iface_id, span) - .with_context(|| { - "this type is not gated by a feature but its interface is gated by a feature" - })?; + *iface_id = self.map_interface_for_type(*iface_id, span)?; } TypeOwner::World(_) | TypeOwner::None => {} } @@ -3158,15 +3200,7 @@ impl Remap { // here. for (id, mut world) in unresolved.worlds.into_iter().skip(foreign_worlds) { let world_span = world.span; - if !resolve - .include_stability(&world.stability, &pkgid, world_span) - .with_context(|| { - format!( - "failed to process feature gate for world [{}] in package [{}]", - world.name, resolve.packages[pkgid].name, - ) - })? - { + if !resolve.include_stability(&world.stability, &pkgid, world_span)? { self.worlds.push(None); continue; } @@ -3186,10 +3220,7 @@ impl Remap { let span = resolve.types[id].span; match &mut resolve.types[id].owner { TypeOwner::World(world_id) => { - *world_id = self.map_world(*world_id, span) - .with_context(|| { - "this type is not gated by a feature but its interface is gated by a feature" - })?; + *world_id = self.map_world_for_type(*world_id, span)?; } TypeOwner::Interface(_) | TypeOwner::None => {} } @@ -3218,15 +3249,7 @@ impl Remap { self.process_world_includes(id, resolve, &pkgid)?; let world_span = resolve.worlds[id].span; - resolve.elaborate_world(id).with_context(|| { - Error::new( - world_span, - format!( - "failed to elaborate world imports/exports of `{}`", - resolve.worlds[id].name - ), - ) - })?; + resolve.elaborate_world(id, world_span)?; } // Fixup "parent" ids now that everything has been identified @@ -3262,7 +3285,7 @@ impl Remap { resolve: &mut Resolve, pkgid: PackageId, unresolved: &UnresolvedPackage, - ) -> Result<()> { + ) -> Result<(), ResolveErrors> { // Invert the `foreign_deps` map to be keyed by world id to get // used in the loops below. let mut world_to_package = HashMap::new(); @@ -3314,10 +3337,12 @@ impl Remap { match resolve.types[id].kind { TypeDefKind::Type(Type::Id(i)) => id = i, TypeDefKind::Resource => break, - _ => bail!(Error::new( - *span, - format!("type used in a handle must be a resource"), - )), + _ => { + return Err(ResolveErrors::from(ResolveErrorKind::Semantic { + span: *span, + message: format!("type used in a handle must be a resource"), + })); + } } } } @@ -3334,7 +3359,7 @@ impl Remap { interface_to_package: &HashMap)>, resolve: &mut Resolve, parent_pkg_id: &PackageId, - ) -> Result<(), anyhow::Error> { + ) -> Result<(), ResolveErrors> { for (unresolved_iface_id, unresolved_iface) in unresolved.interfaces.iter() { let (pkg_name, interface, span, stabilities) = match interface_to_package.get(&unresolved_iface_id) { @@ -3350,11 +3375,11 @@ impl Remap { .get(pkg_name) .copied() .ok_or_else(|| { - PackageNotFoundError::new( + ResolveErrors::from(ResolveErrorKind::PackageNotFound { span, - pkg_name.clone(), - resolve.package_names.keys().cloned().collect(), - ) + requested: pkg_name.clone(), + known: resolve.package_names.keys().cloned().collect(), + }) })?; // Functions can't be imported so this should be empty. @@ -3376,11 +3401,12 @@ impl Remap { continue; } - let iface_id = pkg - .interfaces - .get(interface) - .copied() - .ok_or_else(|| Error::new(iface_span, "interface not found in package"))?; + let iface_id = pkg.interfaces.get(interface).copied().ok_or_else(|| { + ResolveErrors::from(ResolveErrorKind::Semantic { + span: iface_span, + message: "interface not found in package".to_owned(), + }) + })?; assert_eq!(self.interfaces.len(), unresolved_iface_id.index()); self.interfaces.push(Some(iface_id)); } @@ -3399,7 +3425,7 @@ impl Remap { world_to_package: &HashMap)>, resolve: &mut Resolve, parent_pkg_id: &PackageId, - ) -> Result<(), anyhow::Error> { + ) -> Result<(), ResolveErrors> { for (unresolved_world_id, unresolved_world) in unresolved.worlds.iter() { let (pkg_name, world, span, stabilities) = match world_to_package.get(&unresolved_world_id) { @@ -3413,7 +3439,13 @@ impl Remap { .package_names .get(pkg_name) .copied() - .ok_or_else(|| Error::new(span, "package not found"))?; + .ok_or_else(|| { + ResolveErrors::from(ResolveErrorKind::PackageNotFound { + span, + requested: pkg_name.clone(), + known: resolve.package_names.keys().cloned().collect(), + }) + })?; let pkg = &resolve.packages[pkgid]; let world_span = unresolved_world.span; @@ -3430,11 +3462,12 @@ impl Remap { continue; } - let world_id = pkg - .worlds - .get(world) - .copied() - .ok_or_else(|| Error::new(world_span, "world not found in package"))?; + let world_id = pkg.worlds.get(world).copied().ok_or_else(|| { + ResolveErrors::from(ResolveErrorKind::Semantic { + span: world_span, + message: "world not found in package".to_owned(), + }) + })?; assert_eq!(self.worlds.len(), unresolved_world_id.index()); self.worlds.push(Some(world_id)); } @@ -3452,7 +3485,7 @@ impl Remap { unresolved: &UnresolvedPackage, pkgid: PackageId, resolve: &mut Resolve, - ) -> Result<(), anyhow::Error> { + ) -> Result<(), ResolveErrors> { for (unresolved_type_id, unresolved_ty) in unresolved.types.iter() { // All "Unknown" types should appear first so once we're no longer // in unknown territory it's package-defined types so break out of @@ -3463,7 +3496,7 @@ impl Remap { } let span = unresolved_ty.span; - if !resolve.include_type(unresolved_ty, pkgid, span)? { + if !resolve.include_stability(&unresolved_ty.stability, &pkgid, span)? { self.types.push(None); continue; } @@ -3479,7 +3512,10 @@ impl Remap { .types .get(name) .ok_or_else(|| { - Error::new(span, format!("type `{name}` not defined in interface")) + ResolveErrors::from(ResolveErrorKind::Semantic { + span: span, + message: format!("type `{name}` not defined in interface"), + }) })?; assert_eq!(self.types.len(), unresolved_type_id.index()); self.types.push(Some(type_id)); @@ -3497,7 +3533,7 @@ impl Remap { resolve: &mut Resolve, ty: &mut TypeDef, span: Span, - ) -> Result<()> { + ) -> Result<(), ResolveErrors> { // NB: note that `ty.owner` is not updated here since interfaces // haven't been mapped yet and that's done in a separate step. use crate::TypeDefKind::*; @@ -3510,8 +3546,7 @@ impl Remap { Resource => {} Record(r) => { for field in r.fields.iter_mut() { - self.update_ty(resolve, &mut field.ty, span) - .with_context(|| format!("failed to update field `{}`", field.name))?; + self.update_ty(resolve, &mut field.ty, span)? } } Tuple(t) => { @@ -3560,7 +3595,12 @@ impl Remap { Ok(()) } - fn update_ty(&mut self, resolve: &mut Resolve, ty: &mut Type, span: Span) -> Result<()> { + fn update_ty( + &mut self, + resolve: &mut Resolve, + ty: &mut Type, + span: Span, + ) -> Result<(), ResolveErrors> { let id = match ty { Type::Id(id) => id, _ => return Ok(()), @@ -3595,12 +3635,16 @@ impl Remap { Ok(()) } - fn update_type_id(&self, id: &mut TypeId, span: Span) -> Result<()> { + fn update_type_id(&self, id: &mut TypeId, span: Span) -> Result<(), ResolveErrors> { *id = self.map_type(*id, span)?; Ok(()) } - fn update_interface(&mut self, resolve: &mut Resolve, iface: &mut Interface) -> Result<()> { + fn update_interface( + &mut self, + resolve: &mut Resolve, + iface: &mut Interface, + ) -> Result<(), ResolveErrors> { iface.types.retain(|_, ty| self.types[ty.index()].is_some()); let iface_pkg_id = iface.package.as_ref().unwrap_or_else(|| { panic!( @@ -3618,21 +3662,12 @@ impl Remap { for (_name, ty) in iface.types.iter_mut() { self.update_type_id(ty, iface.span)?; } - for (func_name, func) in iface.functions.iter_mut() { + for (_, func) in iface.functions.iter_mut() { let span = func.span; - if !resolve - .include_stability(&func.stability, iface_pkg_id, span) - .with_context(|| { - format!( - "failed to process feature gate for function [{func_name}] in package [{}]", - resolve.packages[*iface_pkg_id].name, - ) - })? - { + if !resolve.include_stability(&func.stability, iface_pkg_id, span)? { continue; } - self.update_function(resolve, func, span) - .with_context(|| format!("failed to update function `{}`", func.name))?; + self.update_function(resolve, func, span)? } // Filter out all of the existing functions in interface which fail the @@ -3651,7 +3686,7 @@ impl Remap { resolve: &mut Resolve, func: &mut Function, span: Span, - ) -> Result<()> { + ) -> Result<(), ResolveErrors> { if let Some(id) = func.kind.resource_mut() { self.update_type_id(id, span)?; } @@ -3664,13 +3699,13 @@ impl Remap { if let Some(ty) = &func.result { if self.type_has_borrow(resolve, ty) { - bail!(Error::new( + return Err(ResolveErrors::from(ResolveErrorKind::Semantic { span, - format!( + message: format!( "function returns a type which contains \ a `borrow` which is not supported" - ) - )) + ), + })); } } @@ -3682,7 +3717,7 @@ impl Remap { world: &mut World, resolve: &mut Resolve, pkg_id: &PackageId, - ) -> Result<()> { + ) -> Result<(), ResolveErrors> { // Rewrite imports/exports with their updated versions. Note that this // may involve updating the key of the imports/exports maps so this // starts by emptying them out and then everything is re-inserted. @@ -3698,10 +3733,7 @@ impl Remap { *id = self.map_type(*id, span)?; } let stability = item.stability(resolve); - if !resolve - .include_stability(stability, pkg_id, span) - .with_context(|| format!("failed to process world item in `{}`", world.name))? - { + if !resolve.include_stability(stability, pkg_id, span)? { continue; } self.update_world_key(&mut name, span)?; @@ -3734,22 +3766,13 @@ impl Remap { id: WorldId, resolve: &mut Resolve, pkg_id: &PackageId, - ) -> Result<()> { + ) -> Result<(), ResolveErrors> { let world = &mut resolve.worlds[id]; // Resolve all `include` statements of the world which will add more // entries to the imports/exports list for this world. let includes = mem::take(&mut world.includes); for include in includes { - if !resolve - .include_stability(&include.stability, pkg_id, include.span) - .with_context(|| { - format!( - "failed to process feature gate for included world [{}] in package [{}]", - resolve.worlds[include.id].name.as_str(), - resolve.packages[*pkg_id].name - ) - })? - { + if !resolve.include_stability(&include.stability, pkg_id, include.span)? { continue; } self.resolve_include( @@ -3771,47 +3794,55 @@ impl Remap { /// Validates that a world's imports and exports don't have case-insensitive /// duplicate names. Per the WIT specification, kebab-case identifiers are /// case-insensitive within the same scope. - fn validate_world_case_insensitive_names(resolve: &Resolve, world_id: WorldId) -> Result<()> { + fn validate_world_case_insensitive_names( + resolve: &Resolve, + world_id: WorldId, + ) -> Result<(), ResolveErrors> { let world = &resolve.worlds[world_id]; // Helper closure to check for case-insensitive duplicates in a map - let validate_names = |items: &IndexMap, - item_type: &str| - -> Result<()> { - let mut seen_lowercase: HashMap = HashMap::new(); - - for key in items.keys() { - // Only WorldKey::Name variants can have case-insensitive conflicts - if let WorldKey::Name(name) = key { - let lowercase_name = name.to_lowercase(); - - if let Some(existing_name) = seen_lowercase.get(&lowercase_name) { - // Only error on case-insensitive duplicates (e.g., "foo" vs "FOO"). - // Exact duplicates would have been caught earlier. - if existing_name != name { - bail!( - "{item_type} `{name}` conflicts with {item_type} `{existing_name}` \ - (kebab-case identifiers are case-insensitive)" - ); + let validate_names = + |items: &IndexMap, item_type: &str| -> Result<(), ResolveErrors> { + let mut seen_lowercase: HashMap = HashMap::new(); + + for key in items.keys() { + // Only WorldKey::Name variants can have case-insensitive conflicts + if let WorldKey::Name(name) = key { + let lowercase_name = name.to_lowercase(); + + if let Some(existing_name) = seen_lowercase.get(&lowercase_name) { + // Only error on case-insensitive duplicates (e.g., "foo" vs "FOO"). + // Exact duplicates would have been caught earlier. + if existing_name != name { + // TODO: `WorldKey::Name` does not carry a `Span`, so we + // cannot point at the conflicting item. Add a span to + // `WorldKey::Name` to improve this error. + return Err(ResolveErrors::from(ResolveErrorKind::Semantic { + span: Span::default(), + message: format!( + "{item_type} `{name}` in world `{}` conflicts with \ + {item_type} `{existing_name}` \ + (kebab-case identifiers are case-insensitive)", + world.name, + ), + })); + } } - } - seen_lowercase.insert(lowercase_name, name.clone()); + seen_lowercase.insert(lowercase_name, name.clone()); + } } - } - Ok(()) - }; + Ok(()) + }; - validate_names(&world.imports, "import") - .with_context(|| format!("failed to validate imports in world `{}`", world.name))?; - validate_names(&world.exports, "export") - .with_context(|| format!("failed to validate exports in world `{}`", world.name))?; + validate_names(&world.imports, "import")?; + validate_names(&world.exports, "export")?; Ok(()) } - fn update_world_key(&self, key: &mut WorldKey, span: Span) -> Result<()> { + fn update_world_key(&self, key: &mut WorldKey, span: Span) -> Result<(), ResolveErrors> { match key { WorldKey::Name(_) => {} WorldKey::Interface(id) => { @@ -3829,7 +3860,7 @@ impl Remap { span: Span, pkg_id: &PackageId, resolve: &mut Resolve, - ) -> Result<()> { + ) -> Result<(), ResolveErrors> { let world = &resolve.worlds[id]; let include_world_id = self.map_world(include_world_id_orig, span)?; let include_world = resolve.worlds[include_world_id].clone(); @@ -3844,13 +3875,13 @@ impl Remap { self.remove_matching_name(export, &mut names_); } if !names_.is_empty() { - bail!(Error::new( + return Err(ResolveErrors::from(ResolveErrorKind::Semantic { span, - format!( + message: format!( "no import or export kebab-name `{}`. Note that an ID does not support renaming", names_[0].name ), - )); + })); } let mut maps = Default::default(); @@ -3903,7 +3934,7 @@ impl Remap { span: Span, item_type: &str, is_external_include: bool, - ) -> Result<()> { + ) -> Result<(), ResolveErrors> { match item.0 { WorldKey::Name(n) => { let n = names @@ -3927,10 +3958,12 @@ impl Remap { let prev = get_items(cloner.resolve).insert(key, new_item); if prev.is_some() { - bail!(Error::new( + return Err(ResolveErrors::from(ResolveErrorKind::Semantic { span, - format!("{item_type} of `{n}` shadows previously {item_type}ed items"), - )) + message: format!( + "{item_type} of `{n}` shadows previously {item_type}ed items" + ), + })); } } key @ WorldKey::Interface(_) => { @@ -3942,7 +3975,7 @@ impl Remap { WorldItem::Interface { id: aid, stability: astability, - .. + span: aspan, }, WorldItem::Interface { id: bid, @@ -3951,7 +3984,12 @@ impl Remap { }, ) => { assert_eq!(*aid, *bid); - merge_include_stability(astability, bstability, is_external_include)?; + merge_include_stability( + astability, + bstability, + is_external_include, + *aspan, + )?; } (WorldItem::Interface { .. }, _) => unreachable!(), (WorldItem::Function(_), _) => unreachable!(), @@ -4074,7 +4112,7 @@ impl<'a> MergeMap<'a> { } } - fn build(&mut self) -> Result<()> { + fn build(&mut self) -> anyhow::Result<()> { for from_id in self.from.topological_packages() { let from = &self.from.packages[from_id]; let into_id = match self.into.package_names.get(&from.name) { @@ -4097,7 +4135,7 @@ impl<'a> MergeMap<'a> { Ok(()) } - fn build_package(&mut self, from_id: PackageId, into_id: PackageId) -> Result<()> { + fn build_package(&mut self, from_id: PackageId, into_id: PackageId) -> anyhow::Result<()> { let prev = self.package_map.insert(from_id, into_id); assert!(prev.is_none()); @@ -4142,7 +4180,11 @@ impl<'a> MergeMap<'a> { Ok(()) } - fn build_interface(&mut self, from_id: InterfaceId, into_id: InterfaceId) -> Result<()> { + fn build_interface( + &mut self, + from_id: InterfaceId, + into_id: InterfaceId, + ) -> anyhow::Result<()> { let prev = self.interface_map.insert(from_id, into_id); assert!(prev.is_none()); @@ -4186,7 +4228,7 @@ impl<'a> MergeMap<'a> { Ok(()) } - fn build_type_id(&mut self, from_id: TypeId, into_id: TypeId) -> Result<()> { + fn build_type_id(&mut self, from_id: TypeId, into_id: TypeId) -> anyhow::Result<()> { // FIXME: ideally the types should be "structurally // equal" but that's not trivial to do in the face of // resources. @@ -4195,7 +4237,7 @@ impl<'a> MergeMap<'a> { Ok(()) } - fn build_type(&mut self, from_ty: &Type, into_ty: &Type) -> Result<()> { + fn build_type(&mut self, from_ty: &Type, into_ty: &Type) -> anyhow::Result<()> { match (from_ty, into_ty) { (Type::Id(from), Type::Id(into)) => { self.build_type_id(*from, *into)?; @@ -4206,7 +4248,7 @@ impl<'a> MergeMap<'a> { Ok(()) } - fn build_function(&mut self, from_func: &Function, into_func: &Function) -> Result<()> { + fn build_function(&mut self, from_func: &Function, into_func: &Function) -> anyhow::Result<()> { if from_func.name != into_func.name { bail!( "different function names `{}` and `{}`", @@ -4268,7 +4310,7 @@ impl<'a> MergeMap<'a> { Ok(()) } - fn build_world(&mut self, from_id: WorldId, into_id: WorldId) -> Result<()> { + fn build_world(&mut self, from_id: WorldId, into_id: WorldId) -> anyhow::Result<()> { let prev = self.world_map.insert(from_id, into_id); assert!(prev.is_none()); @@ -4327,7 +4369,7 @@ impl<'a> MergeMap<'a> { } } - fn match_world_item(&mut self, from: &WorldItem, into: &WorldItem) -> Result<()> { + fn match_world_item(&mut self, from: &WorldItem, into: &WorldItem) -> anyhow::Result<()> { match (from, into) { (WorldItem::Interface { id: from, .. }, WorldItem::Interface { id: into, .. }) => { match ( @@ -4377,7 +4419,11 @@ impl<'a> MergeMap<'a> { /// This is done to keep up-to-date stability information if possible. /// Components for example don't carry stability information but WIT does so /// this tries to move from "unknown" to stable/unstable if possible. -fn update_stability(from: &Stability, into: &mut Stability) -> Result<()> { +fn update_stability( + from: &Stability, + into: &mut Stability, + span: Span, +) -> Result<(), ResolveErrors> { // If `from` is unknown or the two stability annotations are equal then // there's nothing to do here. if from == into || from.is_unknown() { @@ -4392,56 +4438,33 @@ fn update_stability(from: &Stability, into: &mut Stability) -> Result<()> { // Failing all that this means that the two attributes are different so // generate an error. - bail!("mismatch in stability from '{:?}' to '{:?}'", from, into) + Err(ResolveErrors::from(ResolveErrorKind::Semantic { + span, + message: format!("mismatch in stability from '{from:?}' to '{into:?}'"), + })) } fn merge_include_stability( from: &Stability, into: &mut Stability, is_external_include: bool, -) -> Result<()> { + span: Span, +) -> Result<(), ResolveErrors> { if is_external_include && from.is_stable() { log::trace!("dropped stability from external package"); *into = Stability::Unknown; return Ok(()); } - return update_stability(from, into); -} - -/// An error that can be returned during "world elaboration" during various -/// [`Resolve`] operations. -/// -/// Methods on [`Resolve`] which mutate its internals, such as -/// [`Resolve::push_dir`] or [`Resolve::importize`] can fail if `world` imports -/// in WIT packages are invalid. This error indicates one of these situations -/// where an invalid dependency graph between imports and exports are detected. -/// -/// Note that at this time this error is subtle and not easy to understand, and -/// work needs to be done to explain this better and additionally provide a -/// better error message. For now though this type enables callers to test for -/// the exact kind of error emitted. -#[derive(Debug, Clone)] -pub struct InvalidTransitiveDependency(String); - -impl fmt::Display for InvalidTransitiveDependency { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "interface `{}` transitively depends on an interface in \ - incompatible ways", - self.0 - ) - } + update_stability(from, into, span) } -impl core::error::Error for InvalidTransitiveDependency {} - #[cfg(test)] mod tests { use crate::alloc::format; use crate::alloc::string::ToString; - use crate::{Resolve, WorldItem, WorldKey}; + use crate::alloc::vec; + use crate::{Resolve, UnresolvedPackageGroup, WorldItem, WorldKey}; use anyhow::Result; #[test] @@ -5501,6 +5524,50 @@ interface iface { Ok(()) } + #[test] + fn push_groups_resolves_dep_before_main() -> Result<()> { + // push_groups must topologically sort main + deps internally and succeed + // even when the dep is listed after main in the caller's mental model. + let dep = UnresolvedPackageGroup::parse_str( + "file:///dep.wit", + "package foo:dep;\ninterface i { type t = u32; }", + ) + .map_err(|(_, e)| e)?; + let main = UnresolvedPackageGroup::parse_str( + "file:///main.wit", + "package foo:main;\ninterface j { use foo:dep/i.{t}; type u = t; }", + ) + .map_err(|(_, e)| e)?; + let mut resolve = Resolve::default(); + resolve.push_groups(main, vec![dep])?; + assert_eq!(resolve.packages.len(), 2); + Ok(()) + } + + #[test] + fn push_groups_cycle_error_contains_location() { + // A cross-group cycle must produce an error message with a file URI and + // line/col. This validates that source maps are merged into resolve.source_map + // *before* toposort runs, so the span in the cycle error is resolvable. + let a = UnresolvedPackageGroup::parse_str( + "file:///a.wit", + "package foo:a;\ninterface i { use foo:b/j.{}; }", + ) + .unwrap(); + let b = UnresolvedPackageGroup::parse_str( + "file:///b.wit", + "package foo:b;\ninterface j { use foo:a/i.{}; }", + ) + .unwrap(); + let mut resolve = Resolve::default(); + let err = resolve.push_groups(a, vec![b]).unwrap_err(); + let msg = err.highlight(&resolve.source_map); + assert!( + msg.contains("file:///"), + "cycle error should contain a file URI, got: {msg}" + ); + } + #[test] fn param_spans_preserved_through_merge() -> Result<()> { let mut resolve1 = Resolve::default(); diff --git a/crates/wit-parser/tests/ui/parse-fail/bad-gate3.wit.result b/crates/wit-parser/tests/ui/parse-fail/bad-gate3.wit.result index 181a55f67e..6838ae3369 100644 --- a/crates/wit-parser/tests/ui/parse-fail/bad-gate3.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/bad-gate3.wit.result @@ -1,4 +1,4 @@ -this type is not gated by a feature but its interface is gated by a feature: found a reference to a interface which is excluded due to its feature not being activated +found a reference to a interface which is excluded due to its feature not being activated; this type is not gated by a feature but its interface is --> tests/ui/parse-fail/bad-gate3.wit:5:8 | 5 | type a = u32; diff --git a/crates/wit-parser/tests/ui/parse-fail/bad-gate4.wit.result b/crates/wit-parser/tests/ui/parse-fail/bad-gate4.wit.result index 956ba063aa..3558d373be 100644 --- a/crates/wit-parser/tests/ui/parse-fail/bad-gate4.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/bad-gate4.wit.result @@ -1,4 +1,4 @@ -failed to update function `[constructor]a`: found a reference to a type which is excluded due to its feature not being activated +found a reference to a type which is excluded due to its feature not being activated --> tests/ui/parse-fail/bad-gate4.wit:6:5 | 6 | constructor(); diff --git a/crates/wit-parser/tests/ui/parse-fail/bad-gate5.wit.result b/crates/wit-parser/tests/ui/parse-fail/bad-gate5.wit.result index 6c36fb3f80..ad28d61c66 100644 --- a/crates/wit-parser/tests/ui/parse-fail/bad-gate5.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/bad-gate5.wit.result @@ -1,4 +1,4 @@ -failed to update function `[static]a.x`: found a reference to a type which is excluded due to its feature not being activated +found a reference to a type which is excluded due to its feature not being activated --> tests/ui/parse-fail/bad-gate5.wit:9:5 | 9 | x: static func(); diff --git a/crates/wit-parser/tests/ui/parse-fail/bad-pkg6.wit.result b/crates/wit-parser/tests/ui/parse-fail/bad-pkg6.wit.result index 0adea11b98..96d0344722 100644 --- a/crates/wit-parser/tests/ui/parse-fail/bad-pkg6.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/bad-pkg6.wit.result @@ -1,7 +1,6 @@ failed to resolve directory while parsing WIT for path [tests/ui/parse-fail/bad-pkg6]: package 'foo:bar' not found. known packages: foo:baz foo:foo - --> tests/ui/parse-fail/bad-pkg6/root.wit:3:7 | 3 | use foo:bar/baz.{}; diff --git a/crates/wit-parser/tests/ui/parse-fail/case-insensitive-duplicates.wit.result b/crates/wit-parser/tests/ui/parse-fail/case-insensitive-duplicates.wit.result index d46da42b70..e48535d03f 100644 --- a/crates/wit-parser/tests/ui/parse-fail/case-insensitive-duplicates.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/case-insensitive-duplicates.wit.result @@ -1 +1 @@ -failed to validate exports in world `example`: export `GET-USER` conflicts with export `get-user` (kebab-case identifiers are case-insensitive) \ No newline at end of file +export `GET-USER` in world `example` conflicts with export `get-user` (kebab-case identifiers are case-insensitive) \ No newline at end of file diff --git a/crates/wit-parser/tests/ui/parse-fail/import-and-export1.wit.result b/crates/wit-parser/tests/ui/parse-fail/import-and-export1.wit.result index 2642cf4c47..bfbc7a17dc 100644 --- a/crates/wit-parser/tests/ui/parse-fail/import-and-export1.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/import-and-export1.wit.result @@ -1,5 +1,5 @@ -failed to elaborate world imports/exports of `test` +interface `foo:foo/i3` transitively depends on an interface in incompatible ways --> tests/ui/parse-fail/import-and-export1.wit:12:7 | 12 | world test { - | ^---: interface `foo:foo/i3` transitively depends on an interface in incompatible ways \ No newline at end of file + | ^--- \ No newline at end of file diff --git a/crates/wit-parser/tests/ui/parse-fail/import-and-export2.wit.result b/crates/wit-parser/tests/ui/parse-fail/import-and-export2.wit.result index 76406d791a..19f11b9db9 100644 --- a/crates/wit-parser/tests/ui/parse-fail/import-and-export2.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/import-and-export2.wit.result @@ -1,5 +1,5 @@ -failed to elaborate world imports/exports of `baz` +interface `anon` transitively depends on an interface in incompatible ways --> tests/ui/parse-fail/import-and-export2.wit:11:7 | 11 | world baz { - | ^--: interface `anon` transitively depends on an interface in incompatible ways \ No newline at end of file + | ^-- \ No newline at end of file diff --git a/crates/wit-parser/tests/ui/parse-fail/import-and-export3.wit.result b/crates/wit-parser/tests/ui/parse-fail/import-and-export3.wit.result index 857a927999..89a8ecdd70 100644 --- a/crates/wit-parser/tests/ui/parse-fail/import-and-export3.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/import-and-export3.wit.result @@ -1,5 +1,5 @@ -failed to elaborate world imports/exports of `test` +interface `foo:foo/i3` transitively depends on an interface in incompatible ways --> tests/ui/parse-fail/import-and-export3.wit:33:7 | 33 | world test { - | ^---: interface `foo:foo/i3` transitively depends on an interface in incompatible ways \ No newline at end of file + | ^--- \ No newline at end of file diff --git a/crates/wit-parser/tests/ui/parse-fail/import-and-export4.wit.result b/crates/wit-parser/tests/ui/parse-fail/import-and-export4.wit.result index 9028ce7324..a7ab331087 100644 --- a/crates/wit-parser/tests/ui/parse-fail/import-and-export4.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/import-and-export4.wit.result @@ -1,5 +1,5 @@ -failed to elaborate world imports/exports of `test` +interface `foo:foo/i3` transitively depends on an interface in incompatible ways --> tests/ui/parse-fail/import-and-export4.wit:40:7 | 40 | world test { - | ^---: interface `foo:foo/i3` transitively depends on an interface in incompatible ways \ No newline at end of file + | ^--- \ No newline at end of file diff --git a/crates/wit-parser/tests/ui/parse-fail/import-and-export5.wit.result b/crates/wit-parser/tests/ui/parse-fail/import-and-export5.wit.result index 4bb5face1c..460634730e 100644 --- a/crates/wit-parser/tests/ui/parse-fail/import-and-export5.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/import-and-export5.wit.result @@ -1,5 +1,5 @@ -failed to elaborate world imports/exports of `w` +interface `anon` transitively depends on an interface in incompatible ways --> tests/ui/parse-fail/import-and-export5.wit:12:7 | 12 | world w { - | ^: interface `anon` transitively depends on an interface in incompatible ways \ No newline at end of file + | ^ \ No newline at end of file diff --git a/crates/wit-parser/tests/ui/parse-fail/multi-package-deps-share-nest.wit.result b/crates/wit-parser/tests/ui/parse-fail/multi-package-deps-share-nest.wit.result index 053a98b163..8c31ec8689 100644 --- a/crates/wit-parser/tests/ui/parse-fail/multi-package-deps-share-nest.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/multi-package-deps-share-nest.wit.result @@ -1,3 +1,3 @@ -failed to resolve directory while parsing WIT for path [tests/ui/parse-fail/multi-package-deps-share-nest]: package foo:shared is defined in two different locations: -* tests/ui/parse-fail/multi-package-deps-share-nest/deps/dep2/types.wit:3:9 -* tests/ui/parse-fail/multi-package-deps-share-nest/deps/dep1/types.wit:3:9 \ No newline at end of file +failed to resolve directory while parsing WIT for path [tests/ui/parse-fail/multi-package-deps-share-nest]: package `foo:shared` is defined in two different locations: + * tests/ui/parse-fail/multi-package-deps-share-nest/deps/dep2/types.wit:3:9 + * tests/ui/parse-fail/multi-package-deps-share-nest/deps/dep1/types.wit:3:9 \ No newline at end of file diff --git a/crates/wit-parser/tests/ui/parse-fail/multiple-package-inline-cycle.wit.result b/crates/wit-parser/tests/ui/parse-fail/multiple-package-inline-cycle.wit.result index 420ca84d72..ebbbaeac33 100644 --- a/crates/wit-parser/tests/ui/parse-fail/multiple-package-inline-cycle.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/multiple-package-inline-cycle.wit.result @@ -1,4 +1,4 @@ -package depends on itself +package `foo:qux` creates a dependency cycle --> tests/ui/parse-fail/multiple-package-inline-cycle.wit:4:9 | 4 | use foo:qux/i.{}; diff --git a/crates/wit-parser/tests/ui/parse-fail/nested-packages-colliding-names.wit.result b/crates/wit-parser/tests/ui/parse-fail/nested-packages-colliding-names.wit.result index 1376351db3..156887fc0a 100644 --- a/crates/wit-parser/tests/ui/parse-fail/nested-packages-colliding-names.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/nested-packages-colliding-names.wit.result @@ -1,3 +1,3 @@ -package foo:name is defined in two different locations: -* tests/ui/parse-fail/nested-packages-colliding-names.wit:5:9 -* tests/ui/parse-fail/nested-packages-colliding-names.wit:3:9 \ No newline at end of file +package `foo:name` is defined in two different locations: + * tests/ui/parse-fail/nested-packages-colliding-names.wit:5:9 + * tests/ui/parse-fail/nested-packages-colliding-names.wit:3:9 \ No newline at end of file diff --git a/crates/wit-parser/tests/ui/parse-fail/pkg-cycle.wit.result b/crates/wit-parser/tests/ui/parse-fail/pkg-cycle.wit.result index 250e158141..233e0cc79e 100644 --- a/crates/wit-parser/tests/ui/parse-fail/pkg-cycle.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/pkg-cycle.wit.result @@ -1,4 +1,4 @@ -failed to resolve directory while parsing WIT for path [tests/ui/parse-fail/pkg-cycle]: package depends on itself +failed to resolve directory while parsing WIT for path [tests/ui/parse-fail/pkg-cycle]: package `foo:a1` creates a dependency cycle --> tests/ui/parse-fail/pkg-cycle/deps/a1/root.wit:3:7 | 3 | use foo:a1/foo.{}; diff --git a/crates/wit-parser/tests/ui/parse-fail/pkg-cycle2.wit.result b/crates/wit-parser/tests/ui/parse-fail/pkg-cycle2.wit.result index ec10e18c27..b890772931 100644 --- a/crates/wit-parser/tests/ui/parse-fail/pkg-cycle2.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/pkg-cycle2.wit.result @@ -1,4 +1,4 @@ -failed to resolve directory while parsing WIT for path [tests/ui/parse-fail/pkg-cycle2]: package depends on itself +failed to resolve directory while parsing WIT for path [tests/ui/parse-fail/pkg-cycle2]: package `foo:a2` creates a dependency cycle --> tests/ui/parse-fail/pkg-cycle2/deps/a1/root.wit:3:7 | 3 | use foo:a2/foo.{}; diff --git a/crates/wit-parser/tests/ui/parse-fail/return-borrow1.wit.result b/crates/wit-parser/tests/ui/parse-fail/return-borrow1.wit.result index a4ddb59f1a..aa7dd50790 100644 --- a/crates/wit-parser/tests/ui/parse-fail/return-borrow1.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/return-borrow1.wit.result @@ -1,4 +1,4 @@ -failed to update function `x`: function returns a type which contains a `borrow` which is not supported +function returns a type which contains a `borrow` which is not supported --> tests/ui/parse-fail/return-borrow1.wit:6:3 | 6 | x: func() -> borrow; diff --git a/crates/wit-parser/tests/ui/parse-fail/return-borrow2.wit.result b/crates/wit-parser/tests/ui/parse-fail/return-borrow2.wit.result index 01c7f6dad1..d29f0ee7aa 100644 --- a/crates/wit-parser/tests/ui/parse-fail/return-borrow2.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/return-borrow2.wit.result @@ -1,4 +1,4 @@ -failed to update function `[method]y.x`: function returns a type which contains a `borrow` which is not supported +function returns a type which contains a `borrow` which is not supported --> tests/ui/parse-fail/return-borrow2.wit:5:5 | 5 | x: func() -> borrow; diff --git a/crates/wit-parser/tests/ui/parse-fail/return-borrow6.wit.result b/crates/wit-parser/tests/ui/parse-fail/return-borrow6.wit.result index e1264b574d..4c0f86ab16 100644 --- a/crates/wit-parser/tests/ui/parse-fail/return-borrow6.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/return-borrow6.wit.result @@ -1,4 +1,4 @@ -failed to update function `x`: function returns a type which contains a `borrow` which is not supported +function returns a type which contains a `borrow` which is not supported --> tests/ui/parse-fail/return-borrow6.wit:6:3 | 6 | x: func() -> tuple>; diff --git a/crates/wit-parser/tests/ui/parse-fail/return-borrow7.wit.result b/crates/wit-parser/tests/ui/parse-fail/return-borrow7.wit.result index f2175ae6d6..5d8532f944 100644 --- a/crates/wit-parser/tests/ui/parse-fail/return-borrow7.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/return-borrow7.wit.result @@ -1,4 +1,4 @@ -failed to update function `x`: function returns a type which contains a `borrow` which is not supported +function returns a type which contains a `borrow` which is not supported --> tests/ui/parse-fail/return-borrow7.wit:10:3 | 10 | x: func() -> y2; diff --git a/crates/wit-parser/tests/ui/parse-fail/return-borrow8.wit.result b/crates/wit-parser/tests/ui/parse-fail/return-borrow8.wit.result index 3ace7b8be7..386c05e7d2 100644 --- a/crates/wit-parser/tests/ui/parse-fail/return-borrow8.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/return-borrow8.wit.result @@ -1,4 +1,4 @@ -failed to resolve directory while parsing WIT for path [tests/ui/parse-fail/return-borrow8]: failed to update function `x`: function returns a type which contains a `borrow` which is not supported +failed to resolve directory while parsing WIT for path [tests/ui/parse-fail/return-borrow8]: function returns a type which contains a `borrow` which is not supported --> tests/ui/parse-fail/return-borrow8/foo.wit:6:3 | 6 | x: func() -> r; diff --git a/crates/wit-parser/tests/ui/parse-fail/unresolved-interface4.wit.result b/crates/wit-parser/tests/ui/parse-fail/unresolved-interface4.wit.result index 2c9268f995..ca541065c7 100644 --- a/crates/wit-parser/tests/ui/parse-fail/unresolved-interface4.wit.result +++ b/crates/wit-parser/tests/ui/parse-fail/unresolved-interface4.wit.result @@ -1,6 +1,5 @@ package 'some:dependency' not found. known packages: foo:foo - --> tests/ui/parse-fail/unresolved-interface4.wit:6:10 | 6 | import some:dependency/iface; diff --git a/crates/wit-smith/src/lib.rs b/crates/wit-smith/src/lib.rs index ebc0e09fa2..ed5714dffe 100644 --- a/crates/wit-smith/src/lib.rs +++ b/crates/wit-smith/src/lib.rs @@ -6,7 +6,7 @@ //! type structures. use arbitrary::{Result, Unstructured}; -use wit_parser::{InvalidTransitiveDependency, Resolve}; +use wit_parser::{Resolve, ResolveErrorKind, ResolveErrors}; mod config; pub use self::config::Config; @@ -27,7 +27,11 @@ pub fn smith(config: &Config, u: &mut Unstructured<'_>) -> Result> { let id = match resolve.push_group(group) { Ok(id) => id, Err(e) => { - if e.is::() { + if matches!( + e.downcast_ref::(), + Some(e) if matches!(e.kind(), + ResolveErrorKind::InvalidTransitiveDependency { .. }) + ) { return Err(arbitrary::Error::IncorrectFormat); } let err = e.to_string(); From 2aeff69509a39d6e824bbd96b18e63df64f66ed8 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:41:23 +0000 Subject: [PATCH 02/12] Rename structs --- crates/wit-parser/src/resolve/error.rs | 50 ++++++---- crates/wit-parser/src/resolve/mod.rs | 123 +++++++++++-------------- crates/wit-smith/src/lib.rs | 4 +- 3 files changed, 88 insertions(+), 89 deletions(-) diff --git a/crates/wit-parser/src/resolve/error.rs b/crates/wit-parser/src/resolve/error.rs index df3c249f5c..b44aca8ab2 100644 --- a/crates/wit-parser/src/resolve/error.rs +++ b/crates/wit-parser/src/resolve/error.rs @@ -1,3 +1,9 @@ +//! Error types for WIT package resolution. +//! +//! The main types are [`ResolveError`] (a single structured error) and +//! [`ResolveErrorKind`] (the underlying variant). [`ResolveResult`] is a +//! convenience alias for `Result`. + use alloc::boxed::Box; use alloc::format; use alloc::string::{String, ToString}; @@ -6,34 +12,36 @@ use core::fmt; use crate::{PackageName, SourceMap, Span}; +/// Convenience alias for a `Result` whose error type is [`ResolveError`]. +pub type ResolveResult = Result; + +/// 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, }, - InvalidTransitiveDependency { - span: Span, - name: String, - }, + /// 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, }, - PackageCycle { - package: PackageName, - span: Span, - }, - Semantic { - span: Span, - message: String, - }, + /// 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, .. } @@ -76,14 +84,20 @@ impl fmt::Display for ResolveErrorKind { } } +/// A single structured error from resolving a WIT package. +/// +/// Carries a [`ResolveErrorKind`] and can be formatted with source context via +/// [`ResolveError::highlight`]. #[derive(Debug, PartialEq, Eq)] -pub struct ResolveErrors(Box); +pub struct ResolveError(Box); -impl ResolveErrors { +impl ResolveError { + /// Returns the underlying error kind. pub fn kind(&self) -> &ResolveErrorKind { &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(); @@ -100,16 +114,16 @@ impl ResolveErrors { } } -impl fmt::Display for ResolveErrors { +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 ResolveErrors {} +impl core::error::Error for ResolveError {} -impl From for ResolveErrors { +impl From for ResolveError { fn from(kind: ResolveErrorKind) -> Self { - ResolveErrors(Box::new(kind)) + ResolveError(Box::new(kind)) } } diff --git a/crates/wit-parser/src/resolve/mod.rs b/crates/wit-parser/src/resolve/mod.rs index 49f5fed5b9..c647ef4b53 100644 --- a/crates/wit-parser/src/resolve/mod.rs +++ b/crates/wit-parser/src/resolve/mod.rs @@ -5,9 +5,8 @@ use alloc::vec::Vec; use alloc::{format, vec}; use core::cmp::Ordering; use core::mem; -use core::result::Result; -use crate::resolve::error::{ResolveErrorKind, ResolveErrors}; +use crate::resolve::error::{ResolveError, ResolveErrorKind}; use crate::*; use anyhow::{Context, anyhow, bail}; #[cfg(not(feature = "std"))] @@ -198,7 +197,7 @@ fn visit<'a>( order: &mut IndexSet, visiting: &mut HashSet<&'a PackageName>, source_map_offsets: &[u32], -) -> Result<(), ResolveErrors> { +) -> ResolveResult<()> { if order.contains(&pkg.name) { return Ok(()); } @@ -211,7 +210,7 @@ fn visit<'a>( let mut span = pkg.foreign_dep_spans[i]; span.adjust(offset); if !visiting.insert(dep) { - return Err(ResolveErrors::from(ResolveErrorKind::PackageCycle { + return Err(ResolveError::from(ResolveErrorKind::PackageCycle { package: dep.clone(), span, })); @@ -244,7 +243,7 @@ impl Resolve { &mut self, main: UnresolvedPackageGroup, deps: Vec, - ) -> Result<(PackageId, PackageSources), ResolveErrors> { + ) -> ResolveResult<(PackageId, PackageSources)> { let mut source_maps: Vec = Vec::new(); let mut all_packages: Vec<(UnresolvedPackage, usize)> = Vec::new(); @@ -289,7 +288,7 @@ impl Resolve { span1.adjust(offset); let mut span2 = prev_pkg.package_name_span; span2.adjust(prev_offset); - return Err(ResolveErrors::from(ResolveErrorKind::DuplicatePackage { + return Err(ResolveError::from(ResolveErrorKind::DuplicatePackage { name, span1, span2, @@ -362,7 +361,7 @@ impl Resolve { &mut self, mut unresolved: UnresolvedPackage, span_offset: u32, - ) -> Result { + ) -> ResolveResult { unresolved.adjust_spans(span_offset); let ret = Remap::default().append(self, unresolved); if ret.is_ok() { @@ -401,7 +400,7 @@ impl Resolve { } /// Appends a main [`UnresolvedPackageGroup`] and its dependencies to this - /// [`Resolve`] in a single call, returning a structured [`ResolveErrors`] on + /// [`Resolve`] in a single call, returning a structured [`ResolveError`] on /// failure. /// /// This is the preferred alternative to calling [`Resolve::push_group`] @@ -410,7 +409,7 @@ impl Resolve { /// or [`UnresolvedPackageGroup::parse_str`]). Wit-parser sorts them into /// the correct topological order internally and detects dependency cycles. /// - /// On error, spans in the returned [`ResolveErrors`] are absolute within + /// On error, spans in the returned [`ResolveError`] are absolute within /// `self.source_map` and can be resolved with /// [`SourceMap::get_location`]. /// @@ -419,7 +418,7 @@ impl Resolve { &mut self, main: UnresolvedPackageGroup, deps: Vec, - ) -> Result { + ) -> ResolveResult { let (pkg_id, _) = self.sort_unresolved_packages(main, deps)?; Ok(pkg_id) } @@ -1853,7 +1852,7 @@ impl Resolve { stability: &Stability, pkg_id: &PackageId, span: Span, - ) -> Result { + ) -> ResolveResult { Ok(match stability { Stability::Unknown => true, // NOTE: deprecations are intentionally omitted -- an existing @@ -1873,7 +1872,7 @@ impl Resolve { // Use of feature gating with version specifiers inside a // package that is not versioned is not allowed let package_version = p.name.version.as_ref().ok_or_else(|| { - ResolveErrors::from(ResolveErrorKind::Semantic { + ResolveError::from(ResolveErrorKind::Semantic { span: span, message: format!( "package [{}] contains a feature gate with a version \ @@ -1887,7 +1886,7 @@ impl Resolve { // - released, then we can include it // - unreleased, then we must check the feature (if present) if since > package_version { - return Err(ResolveErrors::from(ResolveErrorKind::Semantic { + return Err(ResolveError::from(ResolveErrorKind::Semantic { span, message: format!( "feature gate cannot reference unreleased version \ @@ -1916,7 +1915,7 @@ impl Resolve { /// noted on `elaborate_world_exports`. /// /// The world is mutated in-place in this `Resolve`. - fn elaborate_world(&mut self, world_id: WorldId, span: Span) -> Result<(), ResolveErrors> { + fn elaborate_world(&mut self, world_id: WorldId, span: Span) -> ResolveResult<()> { // First process all imports. This is easier than exports since the only // requirement here is that all interfaces need to be added with a // topological order between them. @@ -2115,7 +2114,7 @@ impl Resolve { imports: &mut IndexMap, exports: &mut IndexMap, span: Span, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { let mut required_imports = HashSet::new(); for (id, (key, stability)) in export_interfaces.iter() { let name = self.name_world_key(&key); @@ -2148,7 +2147,7 @@ impl Resolve { // more refactoring, so it's left to a future date in the // hopes that most folks won't actually run into this for // the time being. - return Err(ResolveErrors::from( + return Err(ResolveError::from( ResolveErrorKind::InvalidTransitiveDependency { name, span }, )); } @@ -3028,12 +3027,7 @@ pub struct Remap { type_has_borrow: Vec>, } -fn apply_map( - map: &[Option>], - id: Id, - desc: &str, - span: Span, -) -> Result, ResolveErrors> { +fn apply_map(map: &[Option>], id: Id, desc: &str, span: Span) -> ResolveResult> { match map.get(id.index()) { Some(Some(id)) => Ok(*id), Some(None) => { @@ -3041,7 +3035,7 @@ fn apply_map( "found a reference to a {desc} which is excluded \ due to its feature not being activated" ); - Err(ResolveErrors::from(ResolveErrorKind::Semantic { + Err(ResolveError::from(ResolveErrorKind::Semantic { span, message: msg, })) @@ -3065,21 +3059,21 @@ fn rename(original_name: &str, include_name: &IncludeName) -> Option { } impl Remap { - pub fn map_type(&self, id: TypeId, span: Span) -> Result { + pub fn map_type(&self, id: TypeId, span: Span) -> ResolveResult { apply_map(&self.types, id, "type", span) } - pub fn map_interface(&self, id: InterfaceId, span: Span) -> Result { + pub fn map_interface(&self, id: InterfaceId, span: Span) -> ResolveResult { apply_map(&self.interfaces, id, "interface", span) } - pub fn map_world(&self, id: WorldId, span: Span) -> Result { + pub fn map_world(&self, id: WorldId, span: Span) -> ResolveResult { apply_map(&self.worlds, id, "world", span) } - pub fn map_world_for_type(&self, id: WorldId, span: Span) -> Result { + pub fn map_world_for_type(&self, id: WorldId, span: Span) -> ResolveResult { self.map_world(id, span).map_err(|e| { - ResolveErrors::from(ResolveErrorKind::Semantic { + ResolveError::from(ResolveErrorKind::Semantic { span, message: format!("{e}; this type is not gated by a feature but its world is"), }) @@ -3090,9 +3084,9 @@ impl Remap { &self, id: InterfaceId, span: Span, - ) -> Result { + ) -> ResolveResult { self.map_interface(id, span).map_err(|e| { - ResolveErrors::from(ResolveErrorKind::Semantic { + ResolveError::from(ResolveErrorKind::Semantic { span, message: format!("{e}; this type is not gated by a feature but its interface is"), }) @@ -3103,7 +3097,7 @@ impl Remap { &mut self, resolve: &mut Resolve, unresolved: UnresolvedPackage, - ) -> Result { + ) -> ResolveResult { let pkgid = resolve.packages.alloc(Package { name: unresolved.name.clone(), docs: unresolved.docs.clone(), @@ -3285,7 +3279,7 @@ impl Remap { resolve: &mut Resolve, pkgid: PackageId, unresolved: &UnresolvedPackage, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { // Invert the `foreign_deps` map to be keyed by world id to get // used in the loops below. let mut world_to_package = HashMap::new(); @@ -3338,7 +3332,7 @@ impl Remap { TypeDefKind::Type(Type::Id(i)) => id = i, TypeDefKind::Resource => break, _ => { - return Err(ResolveErrors::from(ResolveErrorKind::Semantic { + return Err(ResolveError::from(ResolveErrorKind::Semantic { span: *span, message: format!("type used in a handle must be a resource"), })); @@ -3359,7 +3353,7 @@ impl Remap { interface_to_package: &HashMap)>, resolve: &mut Resolve, parent_pkg_id: &PackageId, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { for (unresolved_iface_id, unresolved_iface) in unresolved.interfaces.iter() { let (pkg_name, interface, span, stabilities) = match interface_to_package.get(&unresolved_iface_id) { @@ -3375,7 +3369,7 @@ impl Remap { .get(pkg_name) .copied() .ok_or_else(|| { - ResolveErrors::from(ResolveErrorKind::PackageNotFound { + ResolveError::from(ResolveErrorKind::PackageNotFound { span, requested: pkg_name.clone(), known: resolve.package_names.keys().cloned().collect(), @@ -3402,7 +3396,7 @@ impl Remap { } let iface_id = pkg.interfaces.get(interface).copied().ok_or_else(|| { - ResolveErrors::from(ResolveErrorKind::Semantic { + ResolveError::from(ResolveErrorKind::Semantic { span: iface_span, message: "interface not found in package".to_owned(), }) @@ -3425,7 +3419,7 @@ impl Remap { world_to_package: &HashMap)>, resolve: &mut Resolve, parent_pkg_id: &PackageId, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { for (unresolved_world_id, unresolved_world) in unresolved.worlds.iter() { let (pkg_name, world, span, stabilities) = match world_to_package.get(&unresolved_world_id) { @@ -3440,7 +3434,7 @@ impl Remap { .get(pkg_name) .copied() .ok_or_else(|| { - ResolveErrors::from(ResolveErrorKind::PackageNotFound { + ResolveError::from(ResolveErrorKind::PackageNotFound { span, requested: pkg_name.clone(), known: resolve.package_names.keys().cloned().collect(), @@ -3463,7 +3457,7 @@ impl Remap { } let world_id = pkg.worlds.get(world).copied().ok_or_else(|| { - ResolveErrors::from(ResolveErrorKind::Semantic { + ResolveError::from(ResolveErrorKind::Semantic { span: world_span, message: "world not found in package".to_owned(), }) @@ -3485,7 +3479,7 @@ impl Remap { unresolved: &UnresolvedPackage, pkgid: PackageId, resolve: &mut Resolve, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { for (unresolved_type_id, unresolved_ty) in unresolved.types.iter() { // All "Unknown" types should appear first so once we're no longer // in unknown territory it's package-defined types so break out of @@ -3512,7 +3506,7 @@ impl Remap { .types .get(name) .ok_or_else(|| { - ResolveErrors::from(ResolveErrorKind::Semantic { + ResolveError::from(ResolveErrorKind::Semantic { span: span, message: format!("type `{name}` not defined in interface"), }) @@ -3533,7 +3527,7 @@ impl Remap { resolve: &mut Resolve, ty: &mut TypeDef, span: Span, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { // NB: note that `ty.owner` is not updated here since interfaces // haven't been mapped yet and that's done in a separate step. use crate::TypeDefKind::*; @@ -3595,12 +3589,7 @@ impl Remap { Ok(()) } - fn update_ty( - &mut self, - resolve: &mut Resolve, - ty: &mut Type, - span: Span, - ) -> Result<(), ResolveErrors> { + fn update_ty(&mut self, resolve: &mut Resolve, ty: &mut Type, span: Span) -> ResolveResult<()> { let id = match ty { Type::Id(id) => id, _ => return Ok(()), @@ -3635,7 +3624,7 @@ impl Remap { Ok(()) } - fn update_type_id(&self, id: &mut TypeId, span: Span) -> Result<(), ResolveErrors> { + fn update_type_id(&self, id: &mut TypeId, span: Span) -> ResolveResult<()> { *id = self.map_type(*id, span)?; Ok(()) } @@ -3644,7 +3633,7 @@ impl Remap { &mut self, resolve: &mut Resolve, iface: &mut Interface, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { iface.types.retain(|_, ty| self.types[ty.index()].is_some()); let iface_pkg_id = iface.package.as_ref().unwrap_or_else(|| { panic!( @@ -3686,7 +3675,7 @@ impl Remap { resolve: &mut Resolve, func: &mut Function, span: Span, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { if let Some(id) = func.kind.resource_mut() { self.update_type_id(id, span)?; } @@ -3699,7 +3688,7 @@ impl Remap { if let Some(ty) = &func.result { if self.type_has_borrow(resolve, ty) { - return Err(ResolveErrors::from(ResolveErrorKind::Semantic { + return Err(ResolveError::from(ResolveErrorKind::Semantic { span, message: format!( "function returns a type which contains \ @@ -3717,7 +3706,7 @@ impl Remap { world: &mut World, resolve: &mut Resolve, pkg_id: &PackageId, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { // Rewrite imports/exports with their updated versions. Note that this // may involve updating the key of the imports/exports maps so this // starts by emptying them out and then everything is re-inserted. @@ -3766,7 +3755,7 @@ impl Remap { id: WorldId, resolve: &mut Resolve, pkg_id: &PackageId, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { let world = &mut resolve.worlds[id]; // Resolve all `include` statements of the world which will add more // entries to the imports/exports list for this world. @@ -3797,12 +3786,12 @@ impl Remap { fn validate_world_case_insensitive_names( resolve: &Resolve, world_id: WorldId, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { let world = &resolve.worlds[world_id]; // Helper closure to check for case-insensitive duplicates in a map let validate_names = - |items: &IndexMap, item_type: &str| -> Result<(), ResolveErrors> { + |items: &IndexMap, item_type: &str| -> ResolveResult<()> { let mut seen_lowercase: HashMap = HashMap::new(); for key in items.keys() { @@ -3817,7 +3806,7 @@ impl Remap { // TODO: `WorldKey::Name` does not carry a `Span`, so we // cannot point at the conflicting item. Add a span to // `WorldKey::Name` to improve this error. - return Err(ResolveErrors::from(ResolveErrorKind::Semantic { + return Err(ResolveError::from(ResolveErrorKind::Semantic { span: Span::default(), message: format!( "{item_type} `{name}` in world `{}` conflicts with \ @@ -3842,7 +3831,7 @@ impl Remap { Ok(()) } - fn update_world_key(&self, key: &mut WorldKey, span: Span) -> Result<(), ResolveErrors> { + fn update_world_key(&self, key: &mut WorldKey, span: Span) -> ResolveResult<()> { match key { WorldKey::Name(_) => {} WorldKey::Interface(id) => { @@ -3860,7 +3849,7 @@ impl Remap { span: Span, pkg_id: &PackageId, resolve: &mut Resolve, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { let world = &resolve.worlds[id]; let include_world_id = self.map_world(include_world_id_orig, span)?; let include_world = resolve.worlds[include_world_id].clone(); @@ -3875,7 +3864,7 @@ impl Remap { self.remove_matching_name(export, &mut names_); } if !names_.is_empty() { - return Err(ResolveErrors::from(ResolveErrorKind::Semantic { + return Err(ResolveError::from(ResolveErrorKind::Semantic { span, message: format!( "no import or export kebab-name `{}`. Note that an ID does not support renaming", @@ -3934,7 +3923,7 @@ impl Remap { span: Span, item_type: &str, is_external_include: bool, - ) -> Result<(), ResolveErrors> { + ) -> ResolveResult<()> { match item.0 { WorldKey::Name(n) => { let n = names @@ -3958,7 +3947,7 @@ impl Remap { let prev = get_items(cloner.resolve).insert(key, new_item); if prev.is_some() { - return Err(ResolveErrors::from(ResolveErrorKind::Semantic { + return Err(ResolveError::from(ResolveErrorKind::Semantic { span, message: format!( "{item_type} of `{n}` shadows previously {item_type}ed items" @@ -4419,11 +4408,7 @@ impl<'a> MergeMap<'a> { /// This is done to keep up-to-date stability information if possible. /// Components for example don't carry stability information but WIT does so /// this tries to move from "unknown" to stable/unstable if possible. -fn update_stability( - from: &Stability, - into: &mut Stability, - span: Span, -) -> Result<(), ResolveErrors> { +fn update_stability(from: &Stability, into: &mut Stability, span: Span) -> ResolveResult<()> { // If `from` is unknown or the two stability annotations are equal then // there's nothing to do here. if from == into || from.is_unknown() { @@ -4438,7 +4423,7 @@ fn update_stability( // Failing all that this means that the two attributes are different so // generate an error. - Err(ResolveErrors::from(ResolveErrorKind::Semantic { + Err(ResolveError::from(ResolveErrorKind::Semantic { span, message: format!("mismatch in stability from '{from:?}' to '{into:?}'"), })) @@ -4449,7 +4434,7 @@ fn merge_include_stability( into: &mut Stability, is_external_include: bool, span: Span, -) -> Result<(), ResolveErrors> { +) -> ResolveResult<()> { if is_external_include && from.is_stable() { log::trace!("dropped stability from external package"); *into = Stability::Unknown; diff --git a/crates/wit-smith/src/lib.rs b/crates/wit-smith/src/lib.rs index ed5714dffe..2b611f94a1 100644 --- a/crates/wit-smith/src/lib.rs +++ b/crates/wit-smith/src/lib.rs @@ -6,7 +6,7 @@ //! type structures. use arbitrary::{Result, Unstructured}; -use wit_parser::{Resolve, ResolveErrorKind, ResolveErrors}; +use wit_parser::{Resolve, ResolveError, ResolveErrorKind}; mod config; pub use self::config::Config; @@ -28,7 +28,7 @@ pub fn smith(config: &Config, u: &mut Unstructured<'_>) -> Result> { Ok(id) => id, Err(e) => { if matches!( - e.downcast_ref::(), + e.downcast_ref::(), Some(e) if matches!(e.kind(), ResolveErrorKind::InvalidTransitiveDependency { .. }) ) { From 2bb93f0df0e4cde46268076830b7b106926de2e9 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Wed, 1 Apr 2026 14:59:44 +0100 Subject: [PATCH 03/12] Remove unneeded methods --- crates/wit-parser/src/lib.rs | 36 ---------------- crates/wit-parser/src/resolve/mod.rs | 64 +++++++++++++++++----------- 2 files changed, 40 insertions(+), 60 deletions(-) diff --git a/crates/wit-parser/src/lib.rs b/crates/wit-parser/src/lib.rs index 2346687e65..9eecbd6781 100644 --- a/crates/wit-parser/src/lib.rs +++ b/crates/wit-parser/src/lib.rs @@ -317,42 +317,6 @@ impl UnresolvedPackageGroup { /// 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. - pub fn parse_str( - path: &str, - contents: &str, - ) -> Result { - let mut map = SourceMap::default(); - map.push_str(path, contents); - map.parse() - } - - /// Parse a WIT package at the provided path. - /// - /// The path provided is inferred whether it's a file or a directory. A file - /// is parsed with [`UnresolvedPackageGroup::parse_file`] and a directory is - /// parsed with [`UnresolvedPackageGroup::parse_dir`]. - #[cfg(feature = "std")] - pub fn parse_path(path: impl AsRef) -> anyhow::Result { - let path = path.as_ref(); - if path.is_dir() { - UnresolvedPackageGroup::parse_dir(path) - } else { - UnresolvedPackageGroup::parse_file(path) - } - } - - /// Parses a WIT package from the file provided. - /// - /// The return value represents all packages found in the WIT file which - /// might be either one or multiple depending on the syntax used. - #[cfg(feature = "std")] - pub fn parse_file(path: impl AsRef) -> anyhow::Result { - let path = path.as_ref(); - let contents = std::fs::read_to_string(path) - .with_context(|| format!("failed to read file {path:?}"))?; - Self::parse(path, &contents) - } - /// Parses a WIT package from the directory provided. /// diff --git a/crates/wit-parser/src/resolve/mod.rs b/crates/wit-parser/src/resolve/mod.rs index c647ef4b53..c092e037cc 100644 --- a/crates/wit-parser/src/resolve/mod.rs +++ b/crates/wit-parser/src/resolve/mod.rs @@ -406,7 +406,7 @@ impl Resolve { /// This is the preferred alternative to calling [`Resolve::push_group`] /// repeatedly when you have a package and its local dependencies available /// as in-memory [`UnresolvedPackageGroup`]s (e.g. from [`SourceMap::parse`] - /// or [`UnresolvedPackageGroup::parse_str`]). Wit-parser sorts them into + /// or [`SourceMap::parse`]). Wit-parser sorts them into /// the correct topological order internally and detects dependency cycles. /// /// On error, spans in the returned [`ResolveError`] are absolute within @@ -429,8 +429,12 @@ impl Resolve { /// read. This method does not touch the filesystem. The `contents` provided /// are the contents of a WIT package. pub fn push_source(&mut self, path: &str, contents: &str) -> anyhow::Result { - let group = UnresolvedPackageGroup::parse_str(path, contents).map_err(|(_, e)| e)?; - self.push_group(group) + let mut map = SourceMap::default(); + map.push_str(path, contents); + self.push_group( + map.parse() + .map_err(|(map, e)| anyhow::anyhow!("{}", e.highlight(&map)))?, + ) } /// Renders a span as a human-readable location string (e.g., "file.wit:10:5"). @@ -4449,7 +4453,7 @@ mod tests { use crate::alloc::format; use crate::alloc::string::ToString; use crate::alloc::vec; - use crate::{Resolve, UnresolvedPackageGroup, WorldItem, WorldKey}; + use crate::{Resolve, SourceMap, WorldItem, WorldKey}; use anyhow::Result; #[test] @@ -5513,16 +5517,22 @@ interface iface { fn push_groups_resolves_dep_before_main() -> Result<()> { // push_groups must topologically sort main + deps internally and succeed // even when the dep is listed after main in the caller's mental model. - let dep = UnresolvedPackageGroup::parse_str( - "file:///dep.wit", - "package foo:dep;\ninterface i { type t = u32; }", - ) - .map_err(|(_, e)| e)?; - let main = UnresolvedPackageGroup::parse_str( - "file:///main.wit", - "package foo:main;\ninterface j { use foo:dep/i.{t}; type u = t; }", - ) - .map_err(|(_, e)| e)?; + let dep = { + let mut map = SourceMap::default(); + map.push_str( + "file:///dep.wit", + "package foo:dep;\ninterface i { type t = u32; }", + ); + map.parse().map_err(|(_, e)| e)? + }; + let main = { + let mut map = SourceMap::default(); + map.push_str( + "file:///main.wit", + "package foo:main;\ninterface j { use foo:dep/i.{t}; type u = t; }", + ); + map.parse().map_err(|(_, e)| e)? + }; let mut resolve = Resolve::default(); resolve.push_groups(main, vec![dep])?; assert_eq!(resolve.packages.len(), 2); @@ -5534,16 +5544,22 @@ interface iface { // A cross-group cycle must produce an error message with a file URI and // line/col. This validates that source maps are merged into resolve.source_map // *before* toposort runs, so the span in the cycle error is resolvable. - let a = UnresolvedPackageGroup::parse_str( - "file:///a.wit", - "package foo:a;\ninterface i { use foo:b/j.{}; }", - ) - .unwrap(); - let b = UnresolvedPackageGroup::parse_str( - "file:///b.wit", - "package foo:b;\ninterface j { use foo:a/i.{}; }", - ) - .unwrap(); + let a = { + let mut map = SourceMap::default(); + map.push_str( + "file:///a.wit", + "package foo:a;\ninterface i { use foo:b/j.{}; }", + ); + map.parse().unwrap() + }; + let b = { + let mut map = SourceMap::default(); + map.push_str( + "file:///b.wit", + "package foo:b;\ninterface j { use foo:a/i.{}; }", + ); + map.parse().unwrap() + }; let mut resolve = Resolve::default(); let err = resolve.push_groups(a, vec![b]).unwrap_err(); let msg = err.highlight(&resolve.source_map); From 9177badf6efd6f13576778ba0427f059b668db03 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Wed, 1 Apr 2026 18:58:14 +0100 Subject: [PATCH 04/12] Remove dangling doc --- crates/wit-parser/src/lib.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/wit-parser/src/lib.rs b/crates/wit-parser/src/lib.rs index 9eecbd6781..2ff2abe510 100644 --- a/crates/wit-parser/src/lib.rs +++ b/crates/wit-parser/src/lib.rs @@ -312,12 +312,6 @@ impl UnresolvedPackageGroup { .map_err(|(map, e)| anyhow::anyhow!("{}", e.highlight(&map))) } - /// 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. - /// Parses a WIT package from the directory provided. /// /// This method will look at all files under the `path` specified. All From f9244437b115c662d19646a9ca990dd0295ab850 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Tue, 7 Apr 2026 12:38:32 +0100 Subject: [PATCH 05/12] Doc improvements --- crates/wit-parser/src/ast/error.rs | 8 ++++++++ crates/wit-parser/src/resolve/error.rs | 12 +++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/crates/wit-parser/src/ast/error.rs b/crates/wit-parser/src/ast/error.rs index 55a3ffa959..8541e8e0d7 100644 --- a/crates/wit-parser/src/ast/error.rs +++ b/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 = Result; +/// 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 resolving a WIT package. #[derive(Debug, PartialEq, Eq)] pub struct ParseError(Box); @@ -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 } diff --git a/crates/wit-parser/src/resolve/error.rs b/crates/wit-parser/src/resolve/error.rs index b44aca8ab2..851263f74c 100644 --- a/crates/wit-parser/src/resolve/error.rs +++ b/crates/wit-parser/src/resolve/error.rs @@ -1,8 +1,4 @@ //! Error types for WIT package resolution. -//! -//! The main types are [`ResolveError`] (a single structured error) and -//! [`ResolveErrorKind`] (the underlying variant). [`ResolveResult`] is a -//! convenience alias for `Result`. use alloc::boxed::Box; use alloc::format; @@ -85,9 +81,6 @@ impl fmt::Display for ResolveErrorKind { } /// A single structured error from resolving a WIT package. -/// -/// Carries a [`ResolveErrorKind`] and can be formatted with source context via -/// [`ResolveError::highlight`]. #[derive(Debug, PartialEq, Eq)] pub struct ResolveError(Box); @@ -97,6 +90,11 @@ impl ResolveError { &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(); From 06415056ab53bfba8c7d1935f2f4e9bd3ff615d0 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Tue, 7 Apr 2026 12:40:52 +0100 Subject: [PATCH 06/12] Update snapshot --- tests/cli/since-on-future-package.wit.stderr | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/tests/cli/since-on-future-package.wit.stderr b/tests/cli/since-on-future-package.wit.stderr index c575764151..4994ed6115 100644 --- a/tests/cli/since-on-future-package.wit.stderr +++ b/tests/cli/since-on-future-package.wit.stderr @@ -1,8 +1,5 @@ -error: failed to process feature gate for function [b] in package [test:invalid@0.1.0] - -Caused by: - 0: feature gate cannot reference unreleased version 0.1.1 of package [test:invalid@0.1.0] (current version 0.1.0) - --> tests/cli/since-on-future-package.wit:9:3 - | - 9 | b: func(s: string) -> string; - | ^ +error: feature gate cannot reference unreleased version 0.1.1 of package [test:invalid@0.1.0] (current version 0.1.0) + --> tests/cli/since-on-future-package.wit:9:3 + | + 9 | b: func(s: string) -> string; + | ^ From 2043c6c46fb0d615c2f75efd6368741ba21b7ce0 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Tue, 7 Apr 2026 12:44:46 +0100 Subject: [PATCH 07/12] Don't reference a non-existent method in docs --- crates/wit-parser/src/resolve/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/wit-parser/src/resolve/mod.rs b/crates/wit-parser/src/resolve/mod.rs index 177b539351..a9bcba5c62 100644 --- a/crates/wit-parser/src/resolve/mod.rs +++ b/crates/wit-parser/src/resolve/mod.rs @@ -410,8 +410,7 @@ impl Resolve { /// the correct topological order internally and detects dependency cycles. /// /// On error, spans in the returned [`ResolveError`] are absolute within - /// `self.source_map` and can be resolved with - /// [`SourceMap::get_location`]. + /// `self.source_map`. /// /// The returned [`PackageId`] corresponds to `main`. pub fn push_groups( From 89df4c45e085d946c2330d10b123953d1a0597a5 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Wed, 8 Apr 2026 11:21:19 +0100 Subject: [PATCH 08/12] Fix bad merge --- crates/wit-parser/src/resolve/mod.rs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/crates/wit-parser/src/resolve/mod.rs b/crates/wit-parser/src/resolve/mod.rs index a9bcba5c62..13fb15b3b8 100644 --- a/crates/wit-parser/src/resolve/mod.rs +++ b/crates/wit-parser/src/resolve/mod.rs @@ -1121,20 +1121,7 @@ impl Resolve { world_id: WorldId, out_world_name: Option, ) -> anyhow::Result<()> { - // Rename the world to avoid having it get confused with the original - // name of the world. Add `-importized` to it for now. Precisely how - // this new world is created may want to be updated over time if this - // becomes problematic. - let world = &mut self.worlds[world_id]; - let pkg = &mut self.packages[world.package.unwrap()]; - pkg.worlds.shift_remove(&world.name); - if let Some(name) = out_world_name { - world.name = name.clone(); - pkg.worlds.insert(name, world_id); - } else { - world.name.push_str("-importized"); - pkg.worlds.insert(world.name.clone(), world_id); - } + self.rename_world(world_id, out_world_name, "-importized"); // Trim all non-type definitions from imports. Types can be used by // exported functions, for example, so they're preserved. @@ -1162,7 +1149,7 @@ impl Resolve { // Fill out any missing transitive interface imports by elaborating this // world which does that for us. - let world_span = self.worlds[world_id].span; + let world_span = world.span; self.elaborate_world(world_id, world_span)?; #[cfg(debug_assertions)] From 0692157cd0082dfee387f154d996eac776275eb1 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Wed, 8 Apr 2026 11:22:39 +0100 Subject: [PATCH 09/12] sm_idx -> *source_maps_index --- crates/wit-parser/src/resolve/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/wit-parser/src/resolve/mod.rs b/crates/wit-parser/src/resolve/mod.rs index 13fb15b3b8..fbd5f9305b 100644 --- a/crates/wit-parser/src/resolve/mod.rs +++ b/crates/wit-parser/src/resolve/mod.rs @@ -202,10 +202,10 @@ fn visit<'a>( return Ok(()); } - let (_, sm_idx) = pkg_details_map + let (_, source_maps_index) = pkg_details_map .get(&pkg.name) .expect("No pkg_details found for package when doing topological sort"); - let offset = source_map_offsets[*sm_idx]; + let offset = source_map_offsets[*source_maps_index]; for (i, (dep, _)) in pkg.foreign_deps.iter().enumerate() { let mut span = pkg.foreign_dep_spans[i]; span.adjust(offset); From 6496897fe551cddec4ae096b87e5557cd345df04 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Wed, 8 Apr 2026 11:23:27 +0100 Subject: [PATCH 10/12] Revert "sm_idx -> *source_maps_index" This reverts commit 0692157cd0082dfee387f154d996eac776275eb1. --- crates/wit-parser/src/resolve/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/wit-parser/src/resolve/mod.rs b/crates/wit-parser/src/resolve/mod.rs index fbd5f9305b..13fb15b3b8 100644 --- a/crates/wit-parser/src/resolve/mod.rs +++ b/crates/wit-parser/src/resolve/mod.rs @@ -202,10 +202,10 @@ fn visit<'a>( return Ok(()); } - let (_, source_maps_index) = pkg_details_map + let (_, sm_idx) = pkg_details_map .get(&pkg.name) .expect("No pkg_details found for package when doing topological sort"); - let offset = source_map_offsets[*source_maps_index]; + let offset = source_map_offsets[*sm_idx]; for (i, (dep, _)) in pkg.foreign_deps.iter().enumerate() { let mut span = pkg.foreign_dep_spans[i]; span.adjust(offset); From d90a9f2a0cf24a8fd621779958040bb956dd5ee5 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Wed, 8 Apr 2026 11:47:00 +0100 Subject: [PATCH 11/12] Remove push_groups, push_group returns a structured error --- crates/wit-parser/src/resolve/fs.rs | 4 +- crates/wit-parser/src/resolve/mod.rs | 85 +--------------------------- crates/wit-smith/src/lib.rs | 7 +-- 3 files changed, 9 insertions(+), 87 deletions(-) diff --git a/crates/wit-parser/src/resolve/fs.rs b/crates/wit-parser/src/resolve/fs.rs index a979156ed0..17988ddbef 100644 --- a/crates/wit-parser/src/resolve/fs.rs +++ b/crates/wit-parser/src/resolve/fs.rs @@ -197,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))), } } diff --git a/crates/wit-parser/src/resolve/mod.rs b/crates/wit-parser/src/resolve/mod.rs index 13fb15b3b8..5a84f8a319 100644 --- a/crates/wit-parser/src/resolve/mod.rs +++ b/crates/wit-parser/src/resolve/mod.rs @@ -394,31 +394,8 @@ impl Resolve { pub fn push_group( &mut self, unresolved_group: UnresolvedPackageGroup, - ) -> anyhow::Result { - self.push_groups(unresolved_group, Vec::new()) - .map_err(|e| anyhow::anyhow!("{}", e.highlight(&self.source_map))) - } - - /// Appends a main [`UnresolvedPackageGroup`] and its dependencies to this - /// [`Resolve`] in a single call, returning a structured [`ResolveError`] on - /// failure. - /// - /// This is the preferred alternative to calling [`Resolve::push_group`] - /// repeatedly when you have a package and its local dependencies available - /// as in-memory [`UnresolvedPackageGroup`]s (e.g. from [`SourceMap::parse`] - /// or [`SourceMap::parse`]). Wit-parser sorts them into - /// the correct topological order internally and detects dependency cycles. - /// - /// On error, spans in the returned [`ResolveError`] are absolute within - /// `self.source_map`. - /// - /// The returned [`PackageId`] corresponds to `main`. - pub fn push_groups( - &mut self, - main: UnresolvedPackageGroup, - deps: Vec, ) -> ResolveResult { - let (pkg_id, _) = self.sort_unresolved_packages(main, deps)?; + let (pkg_id, _) = self.sort_unresolved_packages(unresolved_group, Vec::new())?; Ok(pkg_id) } @@ -434,6 +411,7 @@ impl Resolve { map.parse() .map_err(|(map, e)| anyhow::anyhow!("{}", e.highlight(&map)))?, ) + .map_err(|e| anyhow::anyhow!("{}", e.highlight(&self.source_map))) } /// Renders a span as a human-readable location string (e.g., "file.wit:10:5"). @@ -4513,9 +4491,8 @@ fn merge_include_stability( mod tests { use crate::alloc::format; use crate::alloc::string::{String, ToString}; - use crate::alloc::vec; use crate::alloc::vec::Vec; - use crate::{Resolve, SourceMap, WorldItem, WorldKey}; + use crate::{Resolve, WorldItem, WorldKey}; use anyhow::Result; #[test] @@ -5575,62 +5552,6 @@ interface iface { Ok(()) } - #[test] - fn push_groups_resolves_dep_before_main() -> Result<()> { - // push_groups must topologically sort main + deps internally and succeed - // even when the dep is listed after main in the caller's mental model. - let dep = { - let mut map = SourceMap::default(); - map.push_str( - "file:///dep.wit", - "package foo:dep;\ninterface i { type t = u32; }", - ); - map.parse().map_err(|(_, e)| e)? - }; - let main = { - let mut map = SourceMap::default(); - map.push_str( - "file:///main.wit", - "package foo:main;\ninterface j { use foo:dep/i.{t}; type u = t; }", - ); - map.parse().map_err(|(_, e)| e)? - }; - let mut resolve = Resolve::default(); - resolve.push_groups(main, vec![dep])?; - assert_eq!(resolve.packages.len(), 2); - Ok(()) - } - - #[test] - fn push_groups_cycle_error_contains_location() { - // A cross-group cycle must produce an error message with a file URI and - // line/col. This validates that source maps are merged into resolve.source_map - // *before* toposort runs, so the span in the cycle error is resolvable. - let a = { - let mut map = SourceMap::default(); - map.push_str( - "file:///a.wit", - "package foo:a;\ninterface i { use foo:b/j.{}; }", - ); - map.parse().unwrap() - }; - let b = { - let mut map = SourceMap::default(); - map.push_str( - "file:///b.wit", - "package foo:b;\ninterface j { use foo:a/i.{}; }", - ); - map.parse().unwrap() - }; - let mut resolve = Resolve::default(); - let err = resolve.push_groups(a, vec![b]).unwrap_err(); - let msg = err.highlight(&resolve.source_map); - assert!( - msg.contains("file:///"), - "cycle error should contain a file URI, got: {msg}" - ); - } - #[test] fn param_spans_preserved_through_merge() -> Result<()> { let mut resolve1 = Resolve::default(); diff --git a/crates/wit-smith/src/lib.rs b/crates/wit-smith/src/lib.rs index 2b611f94a1..4a9f90e002 100644 --- a/crates/wit-smith/src/lib.rs +++ b/crates/wit-smith/src/lib.rs @@ -6,7 +6,7 @@ //! type structures. use arbitrary::{Result, Unstructured}; -use wit_parser::{Resolve, ResolveError, ResolveErrorKind}; +use wit_parser::{Resolve, ResolveErrorKind}; mod config; pub use self::config::Config; @@ -28,9 +28,8 @@ pub fn smith(config: &Config, u: &mut Unstructured<'_>) -> Result> { Ok(id) => id, Err(e) => { if matches!( - e.downcast_ref::(), - Some(e) if matches!(e.kind(), - ResolveErrorKind::InvalidTransitiveDependency { .. }) + e.kind(), + ResolveErrorKind::InvalidTransitiveDependency { .. } ) { return Err(arbitrary::Error::IncorrectFormat); } From 151cda019606de3e93b66b6904dfc4ebe9d0c37f Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Wed, 8 Apr 2026 13:48:07 +0100 Subject: [PATCH 12/12] Remove stale reference --- crates/wit-parser/src/resolve/mod.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/crates/wit-parser/src/resolve/mod.rs b/crates/wit-parser/src/resolve/mod.rs index 5a84f8a319..ad57932b1f 100644 --- a/crates/wit-parser/src/resolve/mod.rs +++ b/crates/wit-parser/src/resolve/mod.rs @@ -377,20 +377,6 @@ impl Resolve { /// Any dependency resolution error or otherwise world-elaboration error /// will be returned here, if successful a package identifier is returned /// which corresponds to the package that was just inserted. - /// - /// If the package has dependencies that have not yet been pushed into this - /// [`Resolve`], use [`Resolve::push_groups`] instead to pass them all at - /// once and have dependency ordering and cycle detection handled internally. - /// Appends new [`UnresolvedPackageGroup`] to this [`Resolve`], creating a - /// fully resolved package with no dangling references. - /// - /// Any dependency resolution error or otherwise world-elaboration error - /// will be returned here, if successful a package identifier is returned - /// which corresponds to the package that was just inserted. - /// - /// If the package has dependencies that have not yet been pushed into this - /// [`Resolve`], use [`Resolve::push_groups`] instead to pass them all at - /// once and have dependency ordering and cycle detection handled internally. pub fn push_group( &mut self, unresolved_group: UnresolvedPackageGroup,