From 45237cc9c11527998e2e302a37f6e13d0889ca05 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Tue, 10 Mar 2026 12:58:29 +0000 Subject: [PATCH 1/2] [WIP] Structured errors in wit-parser --- crates/wit-parser/src/ast.rs | 435 +++++++++++------ crates/wit-parser/src/ast/lex.rs | 68 ++- crates/wit-parser/src/ast/resolve.rs | 382 ++++++++------- crates/wit-parser/src/ast/toposort.rs | 46 +- crates/wit-parser/src/decoding.rs | 69 +-- crates/wit-parser/src/lib.rs | 121 +++-- crates/wit-parser/src/resolve/fs.rs | 4 +- crates/wit-parser/src/resolve/mod.rs | 656 +++++++++++++------------- crates/wit-smith/src/lib.rs | 7 +- 9 files changed, 1007 insertions(+), 781 deletions(-) diff --git a/crates/wit-parser/src/ast.rs b/crates/wit-parser/src/ast.rs index a90ba12889..015e5de7ae 100644 --- a/crates/wit-parser/src/ast.rs +++ b/crates/wit-parser/src/ast.rs @@ -1,12 +1,13 @@ -use crate::{Error, PackageNotFoundError, UnresolvedPackageGroup}; +use crate::{PackageNotFoundError, UnresolvedPackageGroup}; use alloc::borrow::Cow; use alloc::boxed::Box; use alloc::format; use alloc::string::{String, ToString}; use alloc::vec::Vec; -use anyhow::{Context, Result, bail}; +use anyhow::Context as _; use core::fmt; use core::mem; +use core::result::Result; use lex::{Span, Token, Tokenizer}; use semver::Version; #[cfg(feature = "std")] @@ -33,7 +34,7 @@ impl<'a> PackageFile<'a> { /// /// This will optionally start with `package foo:bar;` and then will have a /// list of ast items after it. - fn parse(tokens: &mut Tokenizer<'a>) -> Result { + fn parse(tokens: &mut Tokenizer<'a>) -> Result { let mut package_name_tokens_peek = tokens.clone(); let docs = parse_docs(&mut package_name_tokens_peek)?; @@ -62,13 +63,13 @@ impl<'a> PackageFile<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result { + ) -> Result { let span = tokens.expect(Token::Package)?; if !attributes.is_empty() { - bail!(Error::new( - span, - format!("cannot place attributes on nested packages"), - )); + return Err(WitError::Parse(ParseError { + span: span, + message: format!("cannot place attributes on nested packages"), + })); } let package_id = PackageName::parse(tokens, docs)?; tokens.expect(Token::LeftBrace)?; @@ -121,7 +122,10 @@ pub struct DeclList<'a> { } impl<'a> DeclList<'a> { - fn parse_until(tokens: &mut Tokenizer<'a>, end: Option) -> Result> { + fn parse_until( + tokens: &mut Tokenizer<'a>, + end: Option, + ) -> Result, WitError> { let mut items = Vec::new(); let mut docs = parse_docs(tokens)?; loop { @@ -151,8 +155,8 @@ impl<'a> DeclList<'a> { &'b UsePath<'a>, Option<&'b [UseName<'a>]>, WorldOrInterface, - ) -> Result<()>, - ) -> Result<()> { + ) -> Result<(), WitError>, + ) -> Result<(), WitError> { for item in self.items.iter() { match item { AstItem::World(world) => { @@ -259,7 +263,7 @@ enum AstItem<'a> { } impl<'a> AstItem<'a> { - fn parse(tokens: &mut Tokenizer<'a>, docs: Docs<'a>) -> Result { + fn parse(tokens: &mut Tokenizer<'a>, docs: Docs<'a>) -> Result { let attributes = Attribute::parse_list(tokens)?; match tokens.clone().next()? { Some((_span, Token::Interface)) => { @@ -285,7 +289,7 @@ struct PackageName<'a> { } impl<'a> PackageName<'a> { - fn parse(tokens: &mut Tokenizer<'a>, docs: Docs<'a>) -> Result { + fn parse(tokens: &mut Tokenizer<'a>, docs: Docs<'a>) -> Result { let namespace = parse_id(tokens)?; tokens.expect(Token::Colon)?; let name = parse_id(tokens)?; @@ -322,7 +326,7 @@ struct ToplevelUse<'a> { } impl<'a> ToplevelUse<'a> { - fn parse(tokens: &mut Tokenizer<'a>, attributes: Vec>) -> Result { + fn parse(tokens: &mut Tokenizer<'a>, attributes: Vec>) -> Result { let span = tokens.expect(Token::Use)?; let item = UsePath::parse(tokens)?; let as_ = if tokens.eat(Token::As)? { @@ -352,7 +356,7 @@ impl<'a> World<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result { + ) -> Result { tokens.expect(Token::World)?; let name = parse_id(tokens)?; let items = Self::parse_items(tokens)?; @@ -364,7 +368,7 @@ impl<'a> World<'a> { }) } - fn parse_items(tokens: &mut Tokenizer<'a>) -> Result>> { + fn parse_items(tokens: &mut Tokenizer<'a>) -> Result>, WitError> { tokens.expect(Token::LeftBrace)?; let mut items = Vec::new(); loop { @@ -392,7 +396,7 @@ impl<'a> WorldItem<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result> { + ) -> Result, WitError> { match tokens.clone().next()? { Some((_span, Token::Import)) => { Import::parse(tokens, docs, attributes).map(WorldItem::Import) @@ -443,7 +447,7 @@ impl<'a> Import<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result> { + ) -> Result, WitError> { tokens.expect(Token::Import)?; let kind = ExternKind::parse(tokens)?; Ok(Import { @@ -465,7 +469,7 @@ impl<'a> Export<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result> { + ) -> Result, WitError> { tokens.expect(Token::Export)?; let kind = ExternKind::parse(tokens)?; Ok(Export { @@ -483,7 +487,7 @@ enum ExternKind<'a> { } impl<'a> ExternKind<'a> { - fn parse(tokens: &mut Tokenizer<'a>) -> Result> { + fn parse(tokens: &mut Tokenizer<'a>) -> Result, WitError> { // Create a copy of the token stream to test out if this is a function // or an interface import. In those situations the token stream gets // reset to the state of the clone and we continue down those paths. @@ -540,7 +544,7 @@ impl<'a> Interface<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result { + ) -> Result { tokens.expect(Token::Interface)?; let name = parse_id(tokens)?; let items = Self::parse_items(tokens)?; @@ -552,7 +556,9 @@ impl<'a> Interface<'a> { }) } - pub(super) fn parse_items(tokens: &mut Tokenizer<'a>) -> Result>> { + pub(super) fn parse_items( + tokens: &mut Tokenizer<'a>, + ) -> Result>, WitError> { tokens.expect(Token::LeftBrace)?; let mut items = Vec::new(); loop { @@ -593,7 +599,7 @@ enum UsePath<'a> { } impl<'a> UsePath<'a> { - fn parse(tokens: &mut Tokenizer<'a>) -> Result { + fn parse(tokens: &mut Tokenizer<'a>) -> Result { let id = parse_id(tokens)?; if tokens.eat(Token::Colon)? { // `foo:bar/baz@1.0` @@ -632,7 +638,7 @@ struct UseName<'a> { } impl<'a> Use<'a> { - fn parse(tokens: &mut Tokenizer<'a>, attributes: Vec>) -> Result { + fn parse(tokens: &mut Tokenizer<'a>, attributes: Vec>) -> Result { tokens.expect(Token::Use)?; let from = UsePath::parse(tokens)?; tokens.expect(Token::Period)?; @@ -674,7 +680,7 @@ struct IncludeName<'a> { } impl<'a> Include<'a> { - fn parse(tokens: &mut Tokenizer<'a>, attributes: Vec>) -> Result { + fn parse(tokens: &mut Tokenizer<'a>, attributes: Vec>) -> Result { tokens.expect(Token::Include)?; let from = UsePath::parse(tokens)?; @@ -801,7 +807,7 @@ impl<'a> ResourceFunc<'a> { docs: Docs<'a>, attributes: Vec>, tokens: &mut Tokenizer<'a>, - ) -> Result { + ) -> Result { match tokens.clone().next()? { Some((span, Token::Constructor)) => { tokens.expect(Token::Constructor)?; @@ -965,8 +971,11 @@ struct Func<'a> { } impl<'a> Func<'a> { - fn parse(tokens: &mut Tokenizer<'a>) -> Result> { - fn parse_params<'a>(tokens: &mut Tokenizer<'a>, left_paren: bool) -> Result> { + fn parse(tokens: &mut Tokenizer<'a>) -> Result, WitError> { + fn parse_params<'a>( + tokens: &mut Tokenizer<'a>, + left_paren: bool, + ) -> Result, WitError> { if left_paren { tokens.expect(Token::LeftParen)?; }; @@ -1001,7 +1010,7 @@ impl<'a> InterfaceItem<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result> { + ) -> Result, WitError> { match tokens.clone().next()? { Some((_span, Token::Type)) => { TypeDef::parse(tokens, docs, attributes).map(InterfaceItem::TypeDef) @@ -1035,7 +1044,7 @@ impl<'a> TypeDef<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result { + ) -> Result { tokens.expect(Token::Type)?; let name = parse_id(tokens)?; tokens.expect(Token::Equals)?; @@ -1053,7 +1062,7 @@ impl<'a> TypeDef<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result { + ) -> Result { tokens.expect(Token::Flags)?; let name = parse_id(tokens)?; let ty = Type::Flags(Flags { @@ -1080,7 +1089,7 @@ impl<'a> TypeDef<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result { + ) -> Result { tokens.expect(Token::Resource)?; let name = parse_id(tokens)?; let mut funcs = Vec::new(); @@ -1109,7 +1118,7 @@ impl<'a> TypeDef<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result { + ) -> Result { tokens.expect(Token::Record)?; let name = parse_id(tokens)?; let ty = Type::Record(Record { @@ -1138,7 +1147,7 @@ impl<'a> TypeDef<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result { + ) -> Result { tokens.expect(Token::Variant)?; let name = parse_id(tokens)?; let ty = Type::Variant(Variant { @@ -1172,7 +1181,7 @@ impl<'a> TypeDef<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result { + ) -> Result { tokens.expect(Token::Enum)?; let name = parse_id(tokens)?; let ty = Type::Enum(Enum { @@ -1201,7 +1210,7 @@ impl<'a> NamedFunc<'a> { tokens: &mut Tokenizer<'a>, docs: Docs<'a>, attributes: Vec>, - ) -> Result { + ) -> Result { let name = parse_id(tokens)?; tokens.expect(Token::Colon)?; let func = Func::parse(tokens)?; @@ -1215,7 +1224,7 @@ impl<'a> NamedFunc<'a> { } } -fn parse_id<'a>(tokens: &mut Tokenizer<'a>) -> Result> { +fn parse_id<'a>(tokens: &mut Tokenizer<'a>) -> Result, WitError> { match tokens.next()? { Some((span, Token::Id)) => Ok(Id { name: tokens.parse_id(span)?, @@ -1225,11 +1234,11 @@ fn parse_id<'a>(tokens: &mut Tokenizer<'a>) -> Result> { name: tokens.parse_explicit_id(span)?, span, }), - other => Err(err_expected(tokens, "an identifier or string", other).into()), + other => Err(err_expected(tokens, "an identifier or string", other)), } } -fn parse_opt_version(tokens: &mut Tokenizer<'_>) -> Result> { +fn parse_opt_version(tokens: &mut Tokenizer<'_>) -> Result, WitError> { if tokens.eat(Token::At)? { parse_version(tokens).map(Some) } else { @@ -1237,7 +1246,7 @@ fn parse_opt_version(tokens: &mut Tokenizer<'_>) -> Result) -> Result<(Span, Version)> { +fn parse_version(tokens: &mut Tokenizer<'_>) -> Result<(Span, Version), WitError> { let start = tokens.expect(Token::Integer)?.start(); tokens.expect(Token::Period)?; tokens.expect(Token::Integer)?; @@ -1247,7 +1256,12 @@ fn parse_version(tokens: &mut Tokenizer<'_>) -> Result<(Span, Version)> { eat_ids(tokens, Token::Minus, &mut span)?; eat_ids(tokens, Token::Plus, &mut span)?; let string = tokens.get_span(span); - let version = Version::parse(string).map_err(|e| Error::new(span, e.to_string()))?; + let version = Version::parse(string).map_err(|e| { + WitError::Parse(ParseError { + span, + message: e.to_string(), + }) + })?; return Ok((span, version)); // According to `semver.org` this is what we're parsing: @@ -1303,7 +1317,11 @@ fn parse_version(tokens: &mut Tokenizer<'_>) -> Result<(Span, Version)> { // Note that this additionally doesn't try to return any first-class errors. // Instead this bails out on something unrecognized for something else in // the system to return an error. - fn eat_ids(tokens: &mut Tokenizer<'_>, prefix: Token, end: &mut Span) -> Result<()> { + fn eat_ids( + tokens: &mut Tokenizer<'_>, + prefix: Token, + end: &mut Span, + ) -> Result<(), lex::Error> { if !tokens.eat(prefix)? { return Ok(()); } @@ -1327,7 +1345,7 @@ fn parse_version(tokens: &mut Tokenizer<'_>) -> Result<(Span, Version)> { } } -fn parse_docs<'a>(tokens: &mut Tokenizer<'a>) -> Result> { +fn parse_docs<'a>(tokens: &mut Tokenizer<'a>) -> Result, lex::Error> { let mut docs = Docs::default(); let mut clone = tokens.clone(); let mut started = false; @@ -1356,7 +1374,7 @@ fn parse_docs<'a>(tokens: &mut Tokenizer<'a>) -> Result> { } impl<'a> Type<'a> { - fn parse(tokens: &mut Tokenizer<'a>) -> Result { + fn parse(tokens: &mut Tokenizer<'a>) -> Result { match tokens.next()? { Some((span, Token::U8)) => Ok(Type::U8(span)), Some((span, Token::U16)) => Ok(Type::U16(span)), @@ -1392,7 +1410,12 @@ impl<'a> Type<'a> { let size = if tokens.eat(Token::Comma)? { let number = tokens.next()?; if let Some((span, Token::Integer)) = number { - let size: u32 = tokens.get_span(span).parse()?; + let size: u32 = tokens.get_span(span).parse().map_err(|e| { + WitError::Parse(ParseError { + span, + message: format!("invalid list size: {e}"), + }) + })?; Some(size) } else { return Err(err_expected(tokens, "fixed-length", number).into()); @@ -1560,8 +1583,8 @@ fn parse_list<'a, T>( tokens: &mut Tokenizer<'a>, start: Token, end: Token, - parse: impl FnMut(Docs<'a>, &mut Tokenizer<'a>) -> Result, -) -> Result> { + parse: impl FnMut(Docs<'a>, &mut Tokenizer<'a>) -> Result, +) -> Result, WitError> { tokens.expect(start)?; parse_list_trailer(tokens, end, parse) } @@ -1569,8 +1592,8 @@ fn parse_list<'a, T>( fn parse_list_trailer<'a, T>( tokens: &mut Tokenizer<'a>, end: Token, - mut parse: impl FnMut(Docs<'a>, &mut Tokenizer<'a>) -> Result, -) -> Result> { + mut parse: impl FnMut(Docs<'a>, &mut Tokenizer<'a>) -> Result, +) -> Result, WitError> { let mut items = Vec::new(); loop { // get docs before we skip them to try to eat the end token @@ -1598,13 +1621,16 @@ fn err_expected( tokens: &Tokenizer<'_>, expected: &'static str, found: Option<(Span, Token)>, -) -> Error { +) -> WitError { match found { - Some((span, token)) => Error::new( + Some((span, token)) => WitError::Parse(ParseError { span, - format!("expected {}, found {}", expected, token.describe()), - ), - None => Error::new(tokens.eof_span(), format!("expected {expected}, found eof")), + message: format!("expected {}, found {}", expected, token.describe()), + }), + None => WitError::Parse(ParseError { + span: tokens.eof_span(), + message: format!("expected {expected}, found eof"), + }), } } @@ -1615,7 +1641,7 @@ enum Attribute<'a> { } impl<'a> Attribute<'a> { - fn parse_list(tokens: &mut Tokenizer<'a>) -> Result>> { + fn parse_list(tokens: &mut Tokenizer<'a>) -> Result>, WitError> { let mut ret = Vec::new(); while tokens.eat(Token::At)? { let id = parse_id(tokens)?; @@ -1654,7 +1680,10 @@ impl<'a> Attribute<'a> { } } other => { - bail!(Error::new(id.span, format!("unknown attribute `{other}`"),)) + return Err(WitError::Parse(ParseError { + span: id.span, + message: format!("unknown attribute `{other}`"), + })); } }; ret.push(attr); @@ -1671,13 +1700,13 @@ impl<'a> Attribute<'a> { } } -fn eat_id(tokens: &mut Tokenizer<'_>, expected: &str) -> Result { +fn eat_id(tokens: &mut Tokenizer<'_>, expected: &str) -> Result { let id = parse_id(tokens)?; if id.name != expected { - bail!(Error::new( - id.span, - format!("expected `{expected}`, found `{}`", id.name), - )); + return Err(WitError::Parse(ParseError { + span: id.span, + message: format!("expected `{expected}`, found `{}`", id.name), + })); } Ok(id.span) } @@ -1699,6 +1728,146 @@ struct Source { contents: String, } +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SpanLocation { + /// File to which this span belongs + pub file: String, + /// UTF-8 byte offet within the file (start) + pub start: usize, + /// UTF-8 byte offet within the file (end) + pub end: usize, +} + +#[derive(Debug)] +pub struct ParseError { + pub span: Span, + pub message: String, +} + +#[non_exhaustive] +#[derive(Debug)] +pub enum WitError { + Lex(lex::Error), + Parse(ParseError), + PackageNotFound(PackageNotFoundError), + TopoSortError(toposort::Error), + /// An interface transitively depends on another interface in an + /// incompatible way with respect to imports and exports. + /// + /// The `name` is the interface with the invalid transitive dependency. + /// The `span` points to the world where the conflict was detected. + /// + /// Note: a future improvement would be to also provide a path showing + /// how the interfaces are connected, to help the user understand why + /// the dependency is invalid. + InvalidTransitiveDependency { + name: String, + span: Span, + }, + /// Two separate source locations define the same package name. + /// + /// `loc1` and `loc2` are pre-rendered location strings (e.g. + /// `"file.wit:3:1"`) because the two spans live in different source maps + /// that may not be available to the caller. + DuplicatePackage { + name: crate::PackageName, + loc1: String, + loc2: String, + }, + /// A dependency cycle was detected among packages. + /// + /// `package` is the package whose `use` statement closes the cycle. + /// `span_location` points to that `use` statement for programmatic + /// consumers (e.g. an LSP). `highlighted` is a pre-rendered source + /// excerpt for CLI display, produced at detection time when the + /// individual package's source map is still available. + PackageCycle { + package: crate::PackageName, + span_location: Option, + highlighted: Option, + }, +} + +impl From for WitError { + fn from(e: lex::Error) -> Self { + WitError::Lex(e) + } +} + +impl From for WitError { + fn from(e: toposort::Error) -> Self { + WitError::TopoSortError(e) + } +} + +impl core::fmt::Display for WitError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + WitError::Lex(e) => fmt::Display::fmt(e, f), + WitError::Parse(e) => e.message.fmt(f), + WitError::PackageNotFound(e) => fmt::Display::fmt(e, f), + WitError::TopoSortError(e) => fmt::Display::fmt(e, f), + WitError::InvalidTransitiveDependency { name, .. } => write!( + f, + "interface `{name}` transitively depends on an interface in incompatible ways", + ), + WitError::DuplicatePackage { name, loc1, loc2 } => write!( + f, + "package `{name}` is defined in two different locations:\n * {loc1}\n * {loc2}", + ), + WitError::PackageCycle { package, .. } => { + write!(f, "package `{package}` creates a dependency cycle") + } + } + } +} + +impl core::error::Error for WitError {} + +impl WitError { + /// Format this error with source context from `source_map`. + /// + /// Returns a string with the error message and a highlighted excerpt of the + /// relevant source, suitable for display to end users. Falls back to the + /// plain message if no span information is available. + pub fn highlight(&self, source_map: &SourceMap) -> String { + match self { + WitError::Parse(e) => source_map + .highlight_span(e.span, &e.message) + .unwrap_or_else(|| e.message.clone()), + WitError::Lex(e) => { + let span = Span::new(e.position(), e.position() + 1); + source_map + .highlight_span(span, e) + .unwrap_or_else(|| e.to_string()) + } + WitError::PackageNotFound(e) => { + let msg = e.to_string(); + source_map.highlight_span(e.span, &msg).unwrap_or(msg) + } + WitError::TopoSortError(e) => { + let msg = e.to_string(); + source_map.highlight_span(e.span(), &msg).unwrap_or(msg) + } + WitError::InvalidTransitiveDependency { span, .. } => { + let msg = self.to_string(); + source_map.highlight_span(*span, &msg).unwrap_or(msg) + } + // DuplicatePackage has no span — locations are pre-rendered strings. + WitError::DuplicatePackage { .. } => self.to_string(), + // Cycle carries a pre-rendered highlighted string computed at + // detection time when the package's source map was still available. + WitError::PackageCycle { + highlighted: Some(s), + .. + } => s.clone(), + WitError::PackageCycle { + highlighted: None, .. + } => self.to_string(), + } + } +} + impl SourceMap { /// Creates a new empty source map. pub fn new() -> SourceMap { @@ -1708,7 +1877,7 @@ impl SourceMap { /// Reads the file `path` on the filesystem and appends its contents to this /// [`SourceMap`]. #[cfg(feature = "std")] - pub fn push_file(&mut self, path: &Path) -> Result<()> { + pub fn push_file(&mut self, path: &Path) -> anyhow::Result<()> { let contents = std::fs::read_to_string(path) .with_context(|| format!("failed to read file {path:?}"))?; self.push(path, contents); @@ -1768,101 +1937,55 @@ impl SourceMap { /// Parses the files added to this source map into a /// [`UnresolvedPackageGroup`]. - pub fn parse(self) -> Result { + pub fn parse(self) -> Result { let mut nested = Vec::new(); - let main = self.rewrite_error(|| { - let mut resolver = Resolver::default(); - let mut srcs = self.sources.iter().collect::>(); - srcs.sort_by_key(|src| &src.path); - - // Parse each source file individually. A tokenizer is created here - // form settings and then `PackageFile` is used to parse the whole - // stream of tokens. - for src in srcs { - let mut tokens = Tokenizer::new( - // chop off the forcibly appended `\n` character when - // passing through the source to get tokenized. - &src.contents[..src.contents.len() - 1], - src.offset, - ) - .with_context(|| format!("failed to tokenize path: {}", src.path))?; - let mut file = PackageFile::parse(&mut tokens)?; - - // Filter out any nested packages and resolve them separately. - // Nested packages have only a single "file" so only one item - // is pushed into a `Resolver`. Note that a nested `Resolver` - // is used here, not the outer one. - // - // Note that filtering out `Package` items is required due to - // how the implementation of disallowing nested packages in - // nested packages currently works. - for item in mem::take(&mut file.decl_list.items) { - match item { - AstItem::Package(nested_pkg) => { - let mut resolve = Resolver::default(); - resolve.push(nested_pkg).with_context(|| { - format!("failed to handle nested package in: {}", src.path) - })?; - - nested.push(resolve.resolve()?); - } - other => file.decl_list.items.push(other), + let mut resolver = Resolver::default(); + let mut srcs = self.sources.iter().collect::>(); + srcs.sort_by_key(|src| &src.path); + + // Parse each source file individually. A tokenizer is created here + // from settings and then `PackageFile` is used to parse the whole + // stream of tokens. + for src in srcs { + let mut tokens = Tokenizer::new( + // chop off the forcibly appended `\n` character when + // passing through the source to get tokenized. + &src.contents[..src.contents.len() - 1], + src.offset, + )?; + let mut file = PackageFile::parse(&mut tokens)?; + + // Filter out any nested packages and resolve them separately. + // Nested packages have only a single "file" so only one item + // is pushed into a `Resolver`. Note that a nested `Resolver` + // is used here, not the outer one. + // + // Note that filtering out `Package` items is required due to + // how the implementation of disallowing nested packages in + // nested packages currently works. + for item in mem::take(&mut file.decl_list.items) { + match item { + AstItem::Package(nested_pkg) => { + let mut resolve = Resolver::default(); + resolve.push(nested_pkg)?; + nested.push(resolve.resolve()?); } + other => file.decl_list.items.push(other), } - - // With nested packages handled push this file into the - // resolver. - resolver - .push(file) - .with_context(|| format!("failed to start resolving path: {}", src.path))?; } - Ok(resolver.resolve()?) - })?; + + // With nested packages handled push this file into the resolver. + resolver.push(file)?; + } + Ok(UnresolvedPackageGroup { - main, + main: resolver.resolve()?, nested, source_map: self, }) } - pub(crate) fn rewrite_error(&self, f: F) -> Result - where - F: FnOnce() -> Result, - { - let mut err = match f() { - Ok(t) => return Ok(t), - Err(e) => e, - }; - if let Some(parse) = err.downcast_mut::() { - parse.highlight(self); - return Err(err); - } - if let Some(notfound) = err.downcast_mut::() { - notfound.highlight(self); - return Err(err); - } - - if let Some(lex) = err.downcast_ref::() { - let pos = match lex { - lex::Error::Unexpected(at, _) - | lex::Error::UnterminatedComment(at) - | lex::Error::Wanted { at, .. } - | lex::Error::InvalidCharInId(at, _) - | lex::Error::IdPartEmpty(at) - | lex::Error::InvalidEscape(at, _) => *at, - }; - let msg = self.highlight_err(pos, None, lex); - bail!("{msg}") - } - - if let Some(sort) = err.downcast_mut::() { - sort.highlight(self); - } - - Err(err) - } - - pub(crate) fn highlight_span(&self, span: Span, err: impl fmt::Display) -> Option { + pub fn highlight_span(&self, span: Span, err: impl fmt::Display) -> Option { if !span.is_known() { return None; } @@ -1922,6 +2045,20 @@ impl SourceMap { ) } + pub fn get_location(&self, span: Span) -> Option { + if !span.is_known() { + return None; + } + let start = span.start(); + let end = span.end(); + let src = self.source_for_offset(start); + Some(SpanLocation { + file: src.path.clone(), + start: src.to_relative_offset(start), + end: src.to_relative_offset(end), + }) + } + fn source_for_offset(&self, start: u32) -> &Source { let i = match self.sources.binary_search_by_key(&start, |src| src.offset) { Ok(i) => i, @@ -1967,11 +2104,11 @@ pub enum ParsedUsePath { Package(crate::PackageName, String), } -pub fn parse_use_path(s: &str) -> Result { +pub fn parse_use_path(s: &str) -> anyhow::Result { let mut tokens = Tokenizer::new(s, 0)?; let path = UsePath::parse(&mut tokens)?; if tokens.next()?.is_some() { - bail!("trailing tokens in path specifier"); + anyhow::bail!("trailing tokens in path specifier"); } Ok(match path { UsePath::Id(id) => ParsedUsePath::Name(id.name.to_string()), diff --git a/crates/wit-parser/src/ast/lex.rs b/crates/wit-parser/src/ast/lex.rs index a7b306da5e..e8915ccc7d 100644 --- a/crates/wit-parser/src/ast/lex.rs +++ b/crates/wit-parser/src/ast/lex.rs @@ -1,8 +1,8 @@ #[cfg(test)] use alloc::{vec, vec::Vec}; -use anyhow::{Result, bail}; use core::char; use core::fmt; +use core::result::Result; use core::str; use unicode_xid::UnicodeXID; @@ -171,6 +171,9 @@ pub enum Error { InvalidEscape(u32, char), Unexpected(u32, char), UnterminatedComment(u32), + ForbiddenCodepoint(u32, char), + DeprecatedCodepoint(u32, char), + ControlCodepoint(u32, char), Wanted { at: u32, expected: &'static str, @@ -179,7 +182,7 @@ pub enum Error { } impl<'a> Tokenizer<'a> { - pub fn new(input: &'a str, span_offset: u32) -> Result> { + pub fn new(input: &'a str, span_offset: u32) -> Result, Error> { detect_invalid_input(input)?; let mut t = Tokenizer { @@ -194,7 +197,7 @@ impl<'a> Tokenizer<'a> { Ok(t) } - pub fn expect_semicolon(&mut self) -> Result<()> { + pub fn expect_semicolon(&mut self) -> Result<(), Error> { self.expect(Token::Semicolon)?; Ok(()) } @@ -205,13 +208,13 @@ impl<'a> Tokenizer<'a> { &self.input[start..end] } - pub fn parse_id(&self, span: Span) -> Result<&'a str> { + pub fn parse_id(&self, span: Span) -> Result<&'a str, Error> { let ret = self.get_span(span); validate_id(span.start(), &ret)?; Ok(ret) } - pub fn parse_explicit_id(&self, span: Span) -> Result<&'a str> { + pub fn parse_explicit_id(&self, span: Span) -> Result<&'a str, Error> { let token = self.get_span(span); let id_part = token.strip_prefix('%').unwrap(); validate_id(span.start(), id_part)?; @@ -456,13 +459,11 @@ impl<'a> Iterator for CrlfFold<'a> { } } -fn detect_invalid_input(input: &str) -> Result<()> { +fn detect_invalid_input(input: &str) -> Result<(), Error> { // Disallow specific codepoints. - let mut line = 1; - for ch in input.chars() { + for (pos, ch) in input.char_indices() { match ch { - '\n' => line += 1, - '\r' | '\t' => {} + '\n' | '\r' | '\t' => {} // Bidirectional override codepoints can be used to craft source code that // appears to have a different meaning than its actual meaning. See @@ -471,11 +472,7 @@ fn detect_invalid_input(input: &str) -> Result<()> { // [CVE-2021-42574]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-42574 '\u{202a}' | '\u{202b}' | '\u{202c}' | '\u{202d}' | '\u{202e}' | '\u{2066}' | '\u{2067}' | '\u{2068}' | '\u{2069}' => { - bail!( - "Input contains bidirectional override codepoint {:?} at line {}", - ch.escape_unicode(), - line - ); + return Err(Error::ForbiddenCodepoint(u32::try_from(pos).unwrap(), ch)); } // Disallow several characters which are deprecated or discouraged in Unicode. @@ -487,18 +484,14 @@ fn detect_invalid_input(input: &str) -> Result<()> { // Unicode 13.0.0, sec. 16.4 Khmer, Characters Whose Use Is Discouraged. '\u{149}' | '\u{673}' | '\u{f77}' | '\u{f79}' | '\u{17a3}' | '\u{17a4}' | '\u{17b4}' | '\u{17b5}' => { - bail!( - "Codepoint {:?} at line {} is discouraged by Unicode", - ch.escape_unicode(), - line - ); + return Err(Error::DeprecatedCodepoint(u32::try_from(pos).unwrap(), ch)); } // Disallow control codes other than the ones explicitly recognized above, // so that viewing a wit file on a terminal doesn't have surprising side // effects or appear to have a different meaning than its actual meaning. ch if ch.is_control() => { - bail!("Control code '{}' at line {}", ch.escape_unicode(), line); + return Err(Error::ControlCodepoint(u32::try_from(pos).unwrap(), ch)); } _ => {} @@ -635,6 +628,22 @@ impl Token { impl core::error::Error for Error {} +impl Error { + pub fn position(&self) -> u32 { + match self { + Error::InvalidCharInId(at, _) + | Error::IdPartEmpty(at) + | Error::InvalidEscape(at, _) + | Error::Unexpected(at, _) + | Error::UnterminatedComment(at) + | Error::ForbiddenCodepoint(at, _) + | Error::DeprecatedCodepoint(at, _) + | Error::ControlCodepoint(at, _) => *at, + Error::Wanted { at, .. } => *at, + } + } +} + impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -646,6 +655,21 @@ impl fmt::Display for Error { Error::InvalidCharInId(_, ch) => write!(f, "invalid character in identifier {ch:?}"), Error::IdPartEmpty(_) => write!(f, "identifiers must have characters between '-'s"), Error::InvalidEscape(_, ch) => write!(f, "invalid escape in string {ch:?}"), + Error::ForbiddenCodepoint(_, ch) => { + write!( + f, + "Input contains bidirectional override codepoint {:?}", + ch.escape_unicode() + ) + } + Error::DeprecatedCodepoint(_, ch) => { + write!( + f, + "Codepoint {:?} is discouraged by Unicode", + ch.escape_unicode() + ) + } + Error::ControlCodepoint(_, ch) => write!(f, "Control code '{}'", ch.escape_unicode()), } } } @@ -712,6 +736,8 @@ fn test_validate_id() { #[test] fn test_tokenizer() { + use anyhow::Result; + fn collect(s: &str) -> Result> { let mut t = Tokenizer::new(s, 0)?; let mut tokens = Vec::new(); diff --git a/crates/wit-parser/src/ast/resolve.rs b/crates/wit-parser/src/ast/resolve.rs index e77ac0f260..916f2e0caf 100644 --- a/crates/wit-parser/src/ast/resolve.rs +++ b/crates/wit-parser/src/ast/resolve.rs @@ -1,11 +1,13 @@ use super::{ParamList, WorldOrInterface}; use crate::ast::toposort::toposort; +use crate::ast::{ParseError, WitError}; use crate::*; use alloc::string::{String, ToString}; use alloc::vec::Vec; use alloc::{format, vec}; -use anyhow::bail; use core::mem; +use core::result::Result; +use std::borrow::ToOwned; #[derive(Default)] pub struct Resolver<'a> { @@ -105,7 +107,7 @@ enum TypeOrItem { } impl<'a> Resolver<'a> { - pub(super) fn push(&mut self, file: ast::PackageFile<'a>) -> Result<()> { + pub(super) fn push(&mut self, file: ast::PackageFile<'a>) -> Result<(), WitError> { // As each WIT file is pushed into this resolver keep track of the // current package name assigned. Only one file needs to mention it, but // if multiple mention it then they must all match. @@ -113,13 +115,13 @@ impl<'a> Resolver<'a> { let cur_name = cur.package_name(); if let Some((prev, _)) = &self.package_name { if cur_name != *prev { - bail!(Error::new( - cur.span, - format!( + return Err(WitError::Parse(ParseError { + span: cur.span, + message: format!( "package identifier `{cur_name}` does not match \ previous package name of `{prev}`" ), - )) + })); } } self.package_name = Some((cur_name, cur.span)); @@ -128,10 +130,10 @@ impl<'a> Resolver<'a> { let docs = self.docs(&cur.docs); if docs.contents.is_some() { if self.package_docs.contents.is_some() { - bail!(Error::new( - cur.docs.span, - "found doc comments on multiple 'package' items" - )) + return Err(WitError::Parse(ParseError { + span: cur.docs.span, + message: "found doc comments on multiple 'package' items".to_owned(), + })); } self.package_docs = docs; } @@ -145,22 +147,26 @@ impl<'a> Resolver<'a> { ast::AstItem::Package(pkg) => pkg.package_id.as_ref().unwrap().span, _ => continue, }; - bail!(Error::new( - span, - "nested packages must be placed at the top-level" - )) + return Err(WitError::Parse(ParseError { + span: span, + message: "nested packages must be placed at the top-level".to_owned(), + })); } self.decl_lists.push(file.decl_list); Ok(()) } - pub(crate) fn resolve(&mut self) -> Result { + pub(crate) fn resolve(&mut self) -> Result { // At least one of the WIT files must have a `package` annotation. let (name, package_name_span) = match &self.package_name { Some(name) => name.clone(), None => { - bail!("no `package` header was found in any WIT file for this package") + return Err(WitError::Parse(ParseError { + span: Span::default(), + message: "no `package` header was found in any WIT file for this package" + .to_owned(), + })); } }; @@ -336,7 +342,7 @@ impl<'a> Resolver<'a> { fn populate_ast_items( &mut self, decl_lists: &[ast::DeclList<'a>], - ) -> Result<(Vec, Vec)> { + ) -> Result<(Vec, Vec), WitError> { let mut package_items = IndexMap::default(); // Validate that all worlds and interfaces have unique names within this @@ -350,10 +356,10 @@ impl<'a> Resolver<'a> { match item { ast::AstItem::Interface(i) => { if package_items.insert(i.name.name, i.name.span).is_some() { - bail!(Error::new( - i.name.span, - format!("duplicate item named `{}`", i.name.name), - )) + return Err(WitError::Parse(ParseError { + span: i.name.span, + message: format!("duplicate item named `{}`", i.name.name), + })); } let prev = decl_list_ns.insert(i.name.name, ()); assert!(prev.is_none()); @@ -364,10 +370,10 @@ impl<'a> Resolver<'a> { } ast::AstItem::World(w) => { if package_items.insert(w.name.name, w.name.span).is_some() { - bail!(Error::new( - w.name.span, - format!("duplicate item named `{}`", w.name.name), - )) + return Err(WitError::Parse(ParseError { + span: w.name.span, + message: format!("duplicate item named `{}`", w.name.name), + })); } let prev = decl_list_ns.insert(w.name.name, ()); assert!(prev.is_none()); @@ -415,10 +421,10 @@ impl<'a> Resolver<'a> { ast::AstItem::Package(_) => unreachable!(), }; if decl_list_ns.insert(name.name, (name.span, src)).is_some() { - bail!(Error::new( - name.span, - format!("duplicate name `{}` in this file", name.name), - )); + return Err(WitError::Parse(ParseError { + span: name.span, + message: format!("duplicate name `{}` in this file", name.name), + })); } } @@ -446,13 +452,13 @@ impl<'a> Resolver<'a> { order[iface.name].push(used_name.clone()); } None => { - bail!(Error::new( - used_name.span, - format!( + return Err(WitError::Parse(ParseError { + span: used_name.span, + message: format!( "interface or world `{name}` not found in package", name = used_name.name ), - )) + })); } }, } @@ -494,21 +500,21 @@ impl<'a> Resolver<'a> { let (name, ast_item) = match item { ast::AstItem::Use(u) => { if !u.attributes.is_empty() { - bail!(Error::new( - u.span, - format!("attributes not allowed on top-level use"), - )) + return Err(WitError::Parse(ParseError { + span: u.span, + message: format!("attributes not allowed on top-level use"), + })); } let name = u.as_.as_ref().unwrap_or(u.item.name()); let item = match &u.item { ast::UsePath::Id(name) => *ids.get(name.name).ok_or_else(|| { - Error::new( - name.span, - format!( + WitError::Parse(ParseError { + span: name.span, + message: format!( "interface or world `{name}` does not exist", name = name.name ), - ) + }) })?, ast::UsePath::Package { id, name } => { self.foreign_deps[&id.package_name()][name.name].0 @@ -549,7 +555,7 @@ impl<'a> Resolver<'a> { /// This is done after all interfaces are generated so `self.resolve_path` /// can be used to determine if what's being imported from is a foreign /// interface or not. - fn populate_foreign_types(&mut self, decl_lists: &[ast::DeclList<'a>]) -> Result<()> { + fn populate_foreign_types(&mut self, decl_lists: &[ast::DeclList<'a>]) -> Result<(), WitError> { for (i, decl_list) in decl_lists.iter().enumerate() { self.cur_ast_index = i; decl_list.for_each_path(&mut |_, attrs, path, names, _| { @@ -593,7 +599,11 @@ impl<'a> Resolver<'a> { Ok(()) } - fn resolve_world(&mut self, world_id: WorldId, world: &ast::World<'a>) -> Result { + fn resolve_world( + &mut self, + world_id: WorldId, + world: &ast::World<'a>, + ) -> Result { let docs = self.docs(&world.docs); self.worlds[world_id].docs = docs; let stability = self.stability(&world.attributes)?; @@ -627,10 +637,12 @@ impl<'a> Resolver<'a> { WorldItem::Type { id, span: *span }, ); if prev.is_some() { - bail!(Error::new( - *span, - format!("import `{name}` conflicts with prior import of same name"), - )) + return Err(WitError::Parse(ParseError { + span: *span, + message: format!( + "import `{name}` conflicts with prior import of same name" + ), + })); } } TypeOrItem::Item(_) => unreachable!(), @@ -704,10 +716,10 @@ impl<'a> Resolver<'a> { }; if let WorldItem::Interface { id, .. } = world_item { if !interfaces.insert(id) { - bail!(Error::new( - kind.span(), - format!("interface cannot be {desc}ed more than once"), - )) + return Err(WitError::Parse(ParseError { + span: kind.span(), + message: format!("interface cannot be {desc}ed more than once"), + })); } } let dst = if desc == "import" { @@ -726,10 +738,10 @@ impl<'a> Resolver<'a> { WorldKey::Name(name) => name, WorldKey::Interface(..) => unreachable!(), }; - bail!(Error::new( - kind.span(), - format!("{desc} `{name}` conflicts with prior {prev} of same name",), - )) + return Err(WitError::Parse(ParseError { + span: kind.span(), + message: format!("{desc} `{name}` conflicts with prior {prev} of same name",), + })); } } self.type_lookup.clear(); @@ -742,7 +754,7 @@ impl<'a> Resolver<'a> { docs: &ast::Docs<'a>, attrs: &[ast::Attribute<'a>], kind: &ast::ExternKind<'a>, - ) -> Result { + ) -> Result { match kind { ast::ExternKind::Interface(name, items) => { let prev = mem::take(&mut self.type_lookup); @@ -790,7 +802,7 @@ impl<'a> Resolver<'a> { fields: &[ast::InterfaceItem<'a>], docs: &ast::Docs<'a>, attrs: &[ast::Attribute<'a>], - ) -> Result<()> { + ) -> Result<(), WitError> { let docs = self.docs(docs); self.interfaces[interface_id].docs = docs; let stability = self.stability(attrs)?; @@ -866,7 +878,7 @@ impl<'a> Resolver<'a> { &mut self, owner: TypeOwner, fields: impl Iterator> + Clone, - ) -> Result<()> + ) -> Result<(), WitError> where 'a: 'b, { @@ -893,10 +905,10 @@ impl<'a> Resolver<'a> { TypeItem::Def(t) => { let prev = type_defs.insert(t.name.name, Some(t)); if prev.is_some() { - bail!(Error::new( - t.name.span, - format!("name `{}` is defined more than once", t.name.name), - )) + return Err(WitError::Parse(ParseError { + span: t.name.span, + message: format!("name `{}` is defined more than once", t.name.name), + })); } let mut deps = Vec::new(); collect_deps(&t.ty, &mut deps); @@ -911,7 +923,9 @@ impl<'a> Resolver<'a> { } } } - let order = toposort("type", &type_deps).map_err(attach_old_float_type_context)?; + let order = toposort("type", &type_deps) + .map_err(attach_old_float_type_context) + .map_err(WitError::TopoSortError)?; for ty in order { let def = match type_defs.swap_remove(&ty).unwrap() { Some(def) => def, @@ -932,28 +946,25 @@ impl<'a> Resolver<'a> { } return Ok(()); - fn attach_old_float_type_context(err: ast::toposort::Error) -> anyhow::Error { - let name = match &err { - ast::toposort::Error::NonexistentDep { name, .. } => name, - _ => return err.into(), - }; - let new = match name.as_str() { - "float32" => "f32", - "float64" => "f64", - _ => return err.into(), - }; - - let context = format!( - "the `{name}` type has been renamed to `{new}` and is \ - no longer accepted, but the `WIT_REQUIRE_F32_F64=0` \ - environment variable can be used to temporarily \ - disable this error" - ); - anyhow::Error::from(err).context(context) + fn attach_old_float_type_context(mut err: ast::toposort::Error) -> ast::toposort::Error { + if let ast::toposort::Error::NonexistentDep { name, hint, .. } = &mut err { + let new = match name.as_str() { + "float32" => "f32", + "float64" => "f64", + _ => return err, + }; + *hint = Some(format!( + "the `{name}` type has been renamed to `{new}` and is \ + no longer accepted, but the `WIT_REQUIRE_F32_F64=0` \ + environment variable can be used to temporarily \ + disable this error" + )); + } + err } } - fn resolve_use(&mut self, owner: TypeOwner, u: &ast::Use<'a>) -> Result<()> { + fn resolve_use(&mut self, owner: TypeOwner, u: &ast::Use<'a>) -> Result<(), WitError> { let (item, name, span) = self.resolve_ast_item_path(&u.from)?; let use_from = self.extract_iface_from_item(&item, &name, span)?; let stability = self.stability(&u.attributes)?; @@ -963,15 +974,17 @@ impl<'a> Resolver<'a> { let id = match lookup.get(name.name.name) { Some((TypeOrItem::Type(id), _)) => *id, Some((TypeOrItem::Item(s), _)) => { - bail!(Error::new( - name.name.span, - format!("cannot import {s} `{}`", name.name.name), - )) + return Err(WitError::Parse(ParseError { + span: name.name.span, + message: format!("cannot import {s} `{}`", name.name.name), + })); + } + None => { + return Err(WitError::Parse(ParseError { + span: name.name.span, + message: format!("name `{}` is not defined", name.name.name), + })); } - None => bail!(Error::new( - name.name.span, - format!("name `{}` is not defined", name.name.name), - )), }; let span = name.name.span; let name = name.as_.as_ref().unwrap_or(&name.name); @@ -989,7 +1002,7 @@ impl<'a> Resolver<'a> { } /// For each name in the `include`, resolve the path of the include, add it to the self.includes - fn resolve_include(&mut self, world_id: WorldId, i: &ast::Include<'a>) -> Result<()> { + fn resolve_include(&mut self, world_id: WorldId, i: &ast::Include<'a>) -> Result<(), WitError> { let stability = self.stability(&i.attributes)?; let (item, name, span) = self.resolve_ast_item_path(&i.from)?; let include_from = self.extract_world_from_item(&item, &name, span)?; @@ -1013,7 +1026,7 @@ impl<'a> Resolver<'a> { &mut self, func: &ast::ResourceFunc<'_>, resource: &ast::Id<'_>, - ) -> Result { + ) -> Result { let resource_id = match self.type_lookup.get(resource.name) { Some((TypeOrItem::Type(id), _)) => *id, _ => panic!("type lookup for resource failed"), @@ -1062,7 +1075,7 @@ impl<'a> Resolver<'a> { name_span: Span, func: &ast::Func, kind: FunctionKind, - ) -> Result { + ) -> Result { let docs = self.docs(docs); let stability = self.stability(attrs)?; let params = self.resolve_params(&func.params, &kind, func.span)?; @@ -1078,7 +1091,10 @@ impl<'a> Resolver<'a> { }) } - fn resolve_ast_item_path(&self, path: &ast::UsePath<'a>) -> Result<(AstItem, String, Span)> { + fn resolve_ast_item_path( + &self, + path: &ast::UsePath<'a>, + ) -> Result<(AstItem, String, Span), WitError> { match path { ast::UsePath::Id(id) => { let item = self.ast_items[self.cur_ast_index] @@ -1087,10 +1103,10 @@ impl<'a> Resolver<'a> { match item { Some(item) => Ok((*item, id.name.into(), id.span)), None => { - bail!(Error::new( - id.span, - format!("interface or world `{}` does not exist", id.name), - )) + return Err(WitError::Parse(ParseError { + span: id.span, + message: format!("interface or world `{}` does not exist", id.name), + })); } } } @@ -1107,37 +1123,46 @@ impl<'a> Resolver<'a> { item: &AstItem, name: &str, span: Span, - ) -> Result { + ) -> Result { match item { AstItem::Interface(id) => Ok(*id), AstItem::World(_) => { - bail!(Error::new( + return Err(WitError::Parse(ParseError { span, - format!("name `{name}` is defined as a world, not an interface"), - )) + message: format!("name `{name}` is defined as a world, not an interface"), + })); } } } - fn extract_world_from_item(&self, item: &AstItem, name: &str, span: Span) -> Result { + fn extract_world_from_item( + &self, + item: &AstItem, + name: &str, + span: Span, + ) -> Result { match item { AstItem::World(id) => Ok(*id), AstItem::Interface(_) => { - bail!(Error::new( + return Err(WitError::Parse(ParseError { span, - format!("name `{name}` is defined as an interface, not a world"), - )) + message: format!("name `{name}` is defined as an interface, not a world"), + })); } } } - fn define_interface_name(&mut self, name: &ast::Id<'a>, item: TypeOrItem) -> Result<()> { + fn define_interface_name( + &mut self, + name: &ast::Id<'a>, + item: TypeOrItem, + ) -> Result<(), WitError> { let prev = self.type_lookup.insert(name.name, (item, name.span)); if prev.is_some() { - bail!(Error::new( - name.span, - format!("name `{}` is defined more than once", name.name), - )) + return Err(WitError::Parse(ParseError { + span: name.span, + message: format!("name `{}` is defined more than once", name.name), + })); } else { Ok(()) } @@ -1147,7 +1172,7 @@ impl<'a> Resolver<'a> { &mut self, ty: &ast::Type<'_>, stability: &Stability, - ) -> Result { + ) -> Result { Ok(match ty { ast::Type::Bool(_) => TypeDefKind::Type(Type::Bool), ast::Type::U8(_) => TypeDefKind::Type(Type::U8), @@ -1188,10 +1213,10 @@ impl<'a> Resolver<'a> { | Type::Char | Type::String => {} _ => { - bail!(Error::new( - map.span, - "invalid map key type: map keys must be bool, u8, u16, u32, u64, s8, s16, s32, s64, char, or string", - )) + return Err(WitError::Parse(ParseError { + span: map.span, + message: "invalid map key type: map keys must be bool, u8, u16, u32, u64, s8, s16, s32, s64, char, or string".to_owned(), + })); } } @@ -1216,16 +1241,19 @@ impl<'a> Resolver<'a> { match func { ast::ResourceFunc::Method(f) | ast::ResourceFunc::Static(f) => { if !names.insert(&f.name.name) { - bail!(Error::new( - f.name.span, - format!("duplicate function name `{}`", f.name.name), - )) + return Err(WitError::Parse(ParseError { + span: f.name.span, + message: format!("duplicate function name `{}`", f.name.name), + })); } } ast::ResourceFunc::Constructor(f) => { ctors += 1; if ctors > 1 { - bail!(Error::new(f.name.span, "duplicate constructors")) + return Err(WitError::Parse(ParseError { + span: f.name.span, + message: "duplicate constructors".to_owned(), + })); } } } @@ -1245,7 +1273,7 @@ impl<'a> Resolver<'a> { span: field.name.span, }) }) - .collect::>>()?; + .collect::, WitError>>()?; TypeDefKind::Record(Record { fields }) } ast::Type::Flags(flags) => { @@ -1265,12 +1293,15 @@ impl<'a> Resolver<'a> { .types .iter() .map(|ty| self.resolve_type(ty, stability)) - .collect::>>()?; + .collect::, WitError>>()?; TypeDefKind::Tuple(Tuple { types }) } ast::Type::Variant(variant) => { if variant.cases.is_empty() { - bail!(Error::new(variant.span, "empty variant")) + return Err(WitError::Parse(ParseError { + span: variant.span, + message: "empty variant".to_owned(), + })); } let cases = variant .cases @@ -1283,12 +1314,15 @@ impl<'a> Resolver<'a> { span: case.name.span, }) }) - .collect::>>()?; + .collect::, WitError>>()?; TypeDefKind::Variant(Variant { cases }) } ast::Type::Enum(e) => { if e.cases.is_empty() { - bail!(Error::new(e.span, "empty enum")) + return Err(WitError::Parse(ParseError { + span: e.span, + message: "empty enum".to_owned(), + })); } let cases = e .cases @@ -1300,7 +1334,7 @@ impl<'a> Resolver<'a> { span: case.name.span, }) }) - .collect::>>()?; + .collect::, WitError>>()?; TypeDefKind::Enum(Enum { cases }) } ast::Type::Option(ty) => TypeDefKind::Option(self.resolve_type(&ty.ty, stability)?), @@ -1317,21 +1351,25 @@ impl<'a> Resolver<'a> { }) } - fn resolve_type_name(&mut self, name: &ast::Id<'_>) -> Result { + fn resolve_type_name(&mut self, name: &ast::Id<'_>) -> Result { match self.type_lookup.get(name.name) { Some((TypeOrItem::Type(id), _)) => Ok(*id), - Some((TypeOrItem::Item(s), _)) => bail!(Error::new( - name.span, - format!("cannot use {s} `{name}` as a type", name = name.name), - )), - None => bail!(Error::new( - name.span, - format!("name `{name}` is not defined", name = name.name), - )), + Some((TypeOrItem::Item(s), _)) => { + return Err(WitError::Parse(ParseError { + span: name.span, + message: format!("cannot use {s} `{name}` as a type", name = name.name), + })); + } + None => { + return Err(WitError::Parse(ParseError { + span: name.span, + message: format!("name `{name}` is not defined", name = name.name), + })); + } } } - fn validate_resource(&mut self, name: &ast::Id<'_>) -> Result { + fn validate_resource(&mut self, name: &ast::Id<'_>) -> Result { let id = self.resolve_type_name(name)?; let mut cur = id; loop { @@ -1342,10 +1380,15 @@ impl<'a> Resolver<'a> { self.required_resource_types.push((cur, name.span)); break Ok(id); } - _ => bail!(Error::new( - name.span, - format!("type `{}` used in a handle must be a resource", name.name), - )), + _ => { + return Err(WitError::Parse(ParseError { + span: name.span, + message: format!( + "type `{}` used in a handle must be a resource", + name.name + ), + })); + } } } } @@ -1419,7 +1462,11 @@ impl<'a> Resolver<'a> { } } - fn resolve_type(&mut self, ty: &super::Type<'_>, stability: &Stability) -> Result { + fn resolve_type( + &mut self, + ty: &super::Type<'_>, + stability: &Stability, + ) -> Result { // Resources must be declared at the top level to have their methods // processed appropriately, but resources also shouldn't show up // recursively so assert that's not happening here. @@ -1443,7 +1490,7 @@ impl<'a> Resolver<'a> { &mut self, ty: Option<&super::Type<'_>>, stability: &Stability, - ) -> Result> { + ) -> Result, WitError> { match ty { Some(ty) => Ok(Some(self.resolve_type(ty, stability)?)), None => Ok(None), @@ -1549,7 +1596,7 @@ impl<'a> Resolver<'a> { Docs { contents } } - fn stability(&mut self, attrs: &[ast::Attribute<'_>]) -> Result { + fn stability(&mut self, attrs: &[ast::Attribute<'_>]) -> Result { match attrs { [] => Ok(Stability::Unknown), @@ -1593,16 +1640,16 @@ impl<'a> Resolver<'a> { deprecated: Some(version.clone()), }), [ast::Attribute::Deprecated { span, .. }] => { - bail!(Error::new( - *span, - "must pair @deprecated with either @since or @unstable", - )) + return Err(WitError::Parse(ParseError { + span: *span, + message: "must pair @deprecated with either @since or @unstable".to_owned(), + })); } [_, b, ..] => { - bail!(Error::new( - b.span(), - "unsupported combination of attributes", - )) + return Err(WitError::Parse(ParseError { + span: b.span(), + message: "unsupported combination of attributes".to_owned(), + })); } } } @@ -1612,7 +1659,7 @@ impl<'a> Resolver<'a> { params: &ParamList<'_>, kind: &FunctionKind, span: Span, - ) -> Result> { + ) -> Result, WitError> { let mut ret = Vec::new(); match *kind { // These kinds of methods don't have any adjustments to the @@ -1645,10 +1692,10 @@ impl<'a> Resolver<'a> { } for (name, ty) in params { if ret.iter().any(|p| p.name == name.name) { - bail!(Error::new( - name.span, - format!("param `{}` is defined more than once", name.name), - )) + return Err(WitError::Parse(ParseError { + span: name.span, + message: format!("param `{}` is defined more than once", name.name), + })); } ret.push(Param { name: name.name.to_string(), @@ -1664,7 +1711,7 @@ impl<'a> Resolver<'a> { result: &Option>, kind: &FunctionKind, _span: Span, - ) -> Result> { + ) -> Result, WitError> { match *kind { // These kinds of methods don't have any adjustments to the return // values, so plumb them through as-is. @@ -1696,7 +1743,7 @@ impl<'a> Resolver<'a> { &mut self, resource_id: TypeId, result_ast: &ast::Type<'_>, - ) -> Result { + ) -> Result { let result = self.resolve_type(result_ast, &Stability::Unknown)?; let ok_type = match result { Type::Id(id) => match &self.types[id].kind { @@ -1706,10 +1753,11 @@ impl<'a> Resolver<'a> { _ => None, }; let Some(ok_type) = ok_type else { - bail!(Error::new( - result_ast.span(), - "if a constructor return type is declared it must be a `result`", - )); + return Err(WitError::Parse(ParseError { + span: result_ast.span(), + message: "if a constructor return type is declared it must be a `result`" + .to_owned(), + })); }; match ok_type { Some(Type::Id(ok_id)) if resource_id == ok_id => Ok(result), @@ -1720,10 +1768,10 @@ impl<'a> Resolver<'a> { } else { result_ast.span() }; - bail!(Error::new( - ok_span, - "the `ok` type must be the resource being constructed", - )); + return Err(WitError::Parse(ParseError { + span: ok_span, + message: "the `ok` type must be the resource being constructed".to_owned(), + })); } } } diff --git a/crates/wit-parser/src/ast/toposort.rs b/crates/wit-parser/src/ast/toposort.rs index b54b3f647b..5dd36fb4a7 100644 --- a/crates/wit-parser/src/ast/toposort.rs +++ b/crates/wit-parser/src/ast/toposort.rs @@ -1,13 +1,12 @@ use crate::IndexMap; use crate::ast::{Id, Span}; use alloc::collections::BinaryHeap; -use alloc::format; use alloc::string::{String, ToString}; use alloc::vec; use alloc::vec::Vec; -use anyhow::Result; use core::fmt; use core::mem; +use core::result::Result; #[derive(Default, Clone)] struct State { @@ -59,7 +58,7 @@ pub fn toposort<'a>( span: edge.span, name: edge.name.to_string(), kind: kind.to_string(), - highlighted: None, + hint: None, })?; states[j].reverse_deps.push(i); } @@ -120,7 +119,6 @@ pub fn toposort<'a>( span: dep.span, name: dep.name.to_string(), kind: kind.to_string(), - highlighted: None, }); } } @@ -128,56 +126,42 @@ pub fn toposort<'a>( unreachable!() } -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum Error { NonexistentDep { span: Span, name: String, kind: String, - highlighted: Option, + /// Optional hint to display after the main error message, e.g. to + /// suggest a renamed type. + hint: Option, }, Cycle { span: Span, name: String, kind: String, - highlighted: Option, }, } impl Error { - pub(crate) fn highlighted(&self) -> Option<&str> { + pub fn span(&self) -> Span { match self { - Error::NonexistentDep { highlighted, .. } | Error::Cycle { highlighted, .. } => { - highlighted.as_deref() - } - } - } - - /// Highlights this error using the given source map, if the span is known. - pub(crate) fn highlight(&mut self, source_map: &crate::ast::SourceMap) { - if self.highlighted().is_some() { - return; - } - let span = match self { Error::NonexistentDep { span, .. } | Error::Cycle { span, .. } => *span, - }; - let msg = source_map.highlight_span(span, &format!("{self}")); - match self { - Error::NonexistentDep { highlighted, .. } | Error::Cycle { highlighted, .. } => { - *highlighted = msg; - } } } } impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if let Some(s) = self.highlighted() { - return f.write_str(s); - } match self { - Error::NonexistentDep { kind, name, .. } => { - write!(f, "{kind} `{name}` does not exist") + Error::NonexistentDep { + kind, name, hint, .. + } => { + write!(f, "{kind} `{name}` does not exist")?; + if let Some(hint) = hint { + write!(f, "\n{hint}")?; + } + Ok(()) } Error::Cycle { kind, name, .. } => { write!(f, "{kind} `{name}` depends on itself") diff --git a/crates/wit-parser/src/decoding.rs b/crates/wit-parser/src/decoding.rs index 88253a3b6e..47f54c34bb 100644 --- a/crates/wit-parser/src/decoding.rs +++ b/crates/wit-parser/src/decoding.rs @@ -48,7 +48,7 @@ enum WitEncodingVersion { impl ComponentInfo { /// Creates a new component info by parsing the given WebAssembly component bytes. - fn from_reader(mut reader: impl Read) -> Result { + fn from_reader(mut reader: impl Read) -> anyhow::Result { let mut validator = Validator::new_with_features(WasmFeatures::all()); let mut externs = Vec::new(); let mut depth = 1; @@ -185,7 +185,7 @@ impl ComponentInfo { } } - fn decode_wit_v1_package(&self) -> Result<(Resolve, PackageId)> { + fn decode_wit_v1_package(&self) -> anyhow::Result<(Resolve, PackageId)> { let mut decoder = WitPackageDecoder::new(&self.types); let mut pkg = None; @@ -215,7 +215,7 @@ impl ComponentInfo { Ok((resolve, package)) } - fn decode_wit_v2_package(&self) -> Result<(Resolve, PackageId)> { + fn decode_wit_v2_package(&self) -> anyhow::Result<(Resolve, PackageId)> { let mut decoder = WitPackageDecoder::new(&self.types); let mut pkg_name = None; @@ -295,7 +295,7 @@ impl ComponentInfo { Ok((resolve, package)) } - fn decode_component(&self) -> Result<(Resolve, WorldId)> { + fn decode_component(&self) -> anyhow::Result<(Resolve, WorldId)> { assert!(self.is_wit_package().is_none()); let mut decoder = WitPackageDecoder::new(&self.types); // Note that this name is arbitrarily chosen. We may one day perhaps @@ -383,7 +383,7 @@ impl DecodedWasm { } /// Decode for incremental reading -pub fn decode_reader(reader: impl Read) -> Result { +pub fn decode_reader(reader: impl Read) -> anyhow::Result { let info = ComponentInfo::from_reader(reader)?; if let Some(version) = info.is_wit_package() { @@ -412,7 +412,7 @@ pub fn decode_reader(reader: impl Read) -> Result { /// The WebAssembly binary provided here can either be a /// WIT-package-encoded-as-binary or an actual component itself. A [`Resolve`] /// is always created and the return value indicates which was detected. -pub fn decode(bytes: &[u8]) -> Result { +pub fn decode(bytes: &[u8]) -> anyhow::Result { decode_reader(bytes) } @@ -423,7 +423,7 @@ pub fn decode(bytes: &[u8]) -> Result { /// itself imports nothing and exports a single component, and the single /// component export represents the world. The name of the export is also the /// name of the package/world/etc. -pub fn decode_world(wasm: &[u8]) -> Result<(Resolve, WorldId)> { +pub fn decode_world(wasm: &[u8]) -> anyhow::Result<(Resolve, WorldId)> { let mut validator = Validator::new_with_features(WasmFeatures::all()); let mut exports = Vec::new(); let mut depth = 1; @@ -538,7 +538,11 @@ impl WitPackageDecoder<'_> { } } - fn decode_v1_package(&mut self, name: &ComponentName, ty: &ComponentType) -> Result { + fn decode_v1_package( + &mut self, + name: &ComponentName, + ty: &ComponentType, + ) -> anyhow::Result { // Process all imports for this package first, where imports are // importing from remote packages. for (name, ty) in ty.imports.iter() { @@ -594,7 +598,7 @@ impl WitPackageDecoder<'_> { imports: &wasmparser::collections::IndexMap, ty: &ComponentInstanceType, fields: &mut PackageFields<'a>, - ) -> Result { + ) -> anyhow::Result { let component_name = self .parse_component_name(name) .context("expected world name to have an ID form")?; @@ -623,7 +627,7 @@ impl WitPackageDecoder<'_> { name: &str, ty: &ComponentType, fields: &mut PackageFields<'a>, - ) -> Result { + ) -> anyhow::Result { let kebab_name = self .parse_component_name(name) .context("expected world name to have an ID form")?; @@ -643,7 +647,7 @@ impl WitPackageDecoder<'_> { name: &str, world: WorldId, package: &mut PackageFields<'a>, - ) -> Result<()> { + ) -> anyhow::Result<()> { log::debug!("decoding component import `{name}`"); let ty = self .types @@ -704,7 +708,7 @@ impl WitPackageDecoder<'_> { export: &DecodingExport, world: WorldId, package: &mut PackageFields<'a>, - ) -> Result<()> { + ) -> anyhow::Result<()> { let name = &export.name; log::debug!("decoding component export `{name}`"); let types = self.types.as_ref(); @@ -753,7 +757,11 @@ impl WitPackageDecoder<'_> { /// dependency the foreign package structure, types, etc, all need to be /// created. For a local dependency it's instead ensured that all the types /// line up with the previous definitions. - fn register_import(&mut self, name: &str, ty: &ComponentInstanceType) -> Result { + fn register_import( + &mut self, + name: &str, + ty: &ComponentInstanceType, + ) -> anyhow::Result { let (is_local, interface) = match self.named_interfaces.get(name) { Some(id) => (true, *id), None => (false, self.extract_dep_interface(name)?), @@ -883,7 +891,7 @@ impl WitPackageDecoder<'_> { /// This will parse the `name_string` as a component model ID string and /// ensure that there's an `InterfaceId` corresponding to its components. - fn extract_dep_interface(&mut self, name_string: &str) -> Result { + fn extract_dep_interface(&mut self, name_string: &str) -> anyhow::Result { let name = ComponentName::new(name_string, 0).unwrap(); let name = match name.kind() { ComponentNameKind::Interface(name) => name, @@ -947,7 +955,7 @@ impl WitPackageDecoder<'_> { name: &str, ty: &ComponentInstanceType, package: &mut PackageFields<'a>, - ) -> Result<(WorldKey, InterfaceId)> { + ) -> anyhow::Result<(WorldKey, InterfaceId)> { // If this interface's name is already known then that means this is an // interface that's both imported and exported. Use `register_import` // to draw connections between types and this interface's types. @@ -1017,12 +1025,15 @@ impl WitPackageDecoder<'_> { Ok((key, id)) } - fn parse_component_name(&self, name: &str) -> Result { + fn parse_component_name(&self, name: &str) -> anyhow::Result { ComponentName::new(name, 0) .with_context(|| format!("cannot extract item name from: {name}")) } - fn extract_interface_name_from_component_name(&self, name: &str) -> Result> { + fn extract_interface_name_from_component_name( + &self, + name: &str, + ) -> anyhow::Result> { let component_name = self.parse_component_name(name)?; match component_name.kind() { ComponentNameKind::Interface(name) => Ok(Some(name.interface().to_string())), @@ -1037,7 +1048,7 @@ impl WitPackageDecoder<'_> { owner: TypeOwner, referenced: ComponentAnyTypeId, created: ComponentAnyTypeId, - ) -> Result { + ) -> anyhow::Result { let kind = match self.find_alias(referenced) { // If this `TypeId` points to a type which has // previously been defined, meaning we're aliasing a @@ -1091,7 +1102,7 @@ impl WitPackageDecoder<'_> { name: &str, ty: &ComponentType, package: &mut PackageFields<'a>, - ) -> Result { + ) -> anyhow::Result { let name = self .extract_interface_name_from_component_name(name)? .context("expected world name to have an ID form")?; @@ -1204,7 +1215,7 @@ impl WitPackageDecoder<'_> { name: &str, ty: &ComponentFuncType, owner: TypeOwner, - ) -> Result { + ) -> anyhow::Result { let name = ComponentName::new(name, 0).unwrap(); let params = ty .params @@ -1216,7 +1227,7 @@ impl WitPackageDecoder<'_> { span: Default::default(), }) }) - .collect::>>() + .collect::>>() .context("failed to convert params")?; let result = match &ty.result { Some(ty) => Some( @@ -1272,7 +1283,7 @@ impl WitPackageDecoder<'_> { }) } - fn convert_valtype(&mut self, ty: &ComponentValType) -> Result { + fn convert_valtype(&mut self, ty: &ComponentValType) -> anyhow::Result { let id = match ty { ComponentValType::Primitive(ty) => return Ok(self.convert_primitive(*ty)), ComponentValType::Type(id) => *id, @@ -1328,7 +1339,7 @@ impl WitPackageDecoder<'_> { /// Converts a wasmparser `ComponentDefinedType`, the definition of a type /// in the component model, to a WIT `TypeDefKind` to get inserted into the /// types arena by the caller. - fn convert_defined(&mut self, ty: &ComponentDefinedType) -> Result { + fn convert_defined(&mut self, ty: &ComponentDefinedType) -> anyhow::Result { match ty { ComponentDefinedType::Primitive(t) => Ok(TypeDefKind::Type(self.convert_primitive(*t))), @@ -1353,7 +1364,7 @@ impl WitPackageDecoder<'_> { .types .iter() .map(|t| self.convert_valtype(t)) - .collect::>()?; + .collect::>()?; Ok(TypeDefKind::Tuple(Tuple { types })) } @@ -1388,7 +1399,7 @@ impl WitPackageDecoder<'_> { span: Default::default(), }) }) - .collect::>()?; + .collect::>()?; Ok(TypeDefKind::Record(Record { fields })) } @@ -1407,7 +1418,7 @@ impl WitPackageDecoder<'_> { span: Default::default(), }) }) - .collect::>()?; + .collect::>()?; Ok(TypeDefKind::Variant(Variant { cases })) } @@ -1477,7 +1488,7 @@ impl WitPackageDecoder<'_> { } } - fn register_defined(&mut self, id: TypeId, def: &ComponentDefinedType) -> Result<()> { + fn register_defined(&mut self, id: TypeId, def: &ComponentDefinedType) -> anyhow::Result<()> { Registrar { types: &self.types, type_map: &mut self.type_map, @@ -1624,7 +1635,7 @@ struct Registrar<'a> { impl Registrar<'_> { /// Verifies that the wasm structure of `def` matches the wit structure of /// `id` and recursively registers types. - fn defined(&mut self, id: TypeId, def: &ComponentDefinedType) -> Result<()> { + fn defined(&mut self, id: TypeId, def: &ComponentDefinedType) -> anyhow::Result<()> { match def { ComponentDefinedType::Primitive(_) => Ok(()), @@ -1780,7 +1791,7 @@ impl Registrar<'_> { } } - fn valtype(&mut self, wasm: &ComponentValType, wit: &Type) -> Result<()> { + fn valtype(&mut self, wasm: &ComponentValType, wit: &Type) -> anyhow::Result<()> { let wasm = match wasm { ComponentValType::Type(wasm) => *wasm, ComponentValType::Primitive(_wasm) => { diff --git a/crates/wit-parser/src/lib.rs b/crates/wit-parser/src/lib.rs index 742d75159d..a980c8b9c5 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; @@ -33,6 +32,7 @@ pub(crate) use hashbrown::{HashMap, HashSet}; use alloc::borrow::Cow; use core::fmt; use core::hash::{Hash, Hasher}; +use core::result::Result; #[cfg(feature = "std")] use std::path::Path; @@ -46,8 +46,9 @@ pub use metadata::PackageMetadata; pub mod abi; mod ast; pub use ast::SourceMap; +pub use ast::WitError; pub use ast::lex::Span; -pub use ast::{ParsedUsePath, parse_use_path}; +pub use ast::{ParseError, ParsedUsePath, SpanLocation, parse_use_path}; mod sizealign; pub use sizealign::*; mod resolve; @@ -63,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(()) } @@ -293,44 +294,43 @@ 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, +// #[derive(Clone, 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(Clone, Debug)] +pub struct PackageNotFoundError { + pub span: Span, + pub requested: PackageName, + pub known: Vec, } impl PackageNotFoundError { @@ -339,23 +339,12 @@ impl PackageNotFoundError { 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, @@ -385,12 +374,18 @@ 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() .ok_or_else(|| anyhow::anyhow!("path is not valid utf-8: {:?}", path.as_ref()))?; - Self::parse_str(path, contents) + let mut map = ast::SourceMap::default(); + map.push_str(path, contents); + // TODO: avoid clone by changing `SourceMap::parse` to return the map + // back on error, e.g. `Err((SourceMap, WitError))`. + let map_for_err = map.clone(); + map.parse() + .map_err(|e| anyhow::anyhow!("{}", e.highlight(&map_for_err))) } /// Parses the given string as a wit document. @@ -398,7 +393,7 @@ 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 { + pub fn parse_str(path: &str, contents: &str) -> Result { let mut map = SourceMap::default(); map.push_str(path, contents); map.parse() @@ -410,7 +405,7 @@ impl UnresolvedPackageGroup { /// 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) -> Result { + pub fn parse_path(path: impl AsRef) -> anyhow::Result { let path = path.as_ref(); if path.is_dir() { UnresolvedPackageGroup::parse_dir(path) @@ -424,7 +419,7 @@ impl UnresolvedPackageGroup { /// 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) -> Result { + 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:?}"))?; @@ -438,7 +433,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:?}"); @@ -463,7 +458,11 @@ impl UnresolvedPackageGroup { } map.push_file(&path)?; } + // TODO: avoid clone by changing `SourceMap::parse` to return the map + // back on error, e.g. `Err((SourceMap, WitError))`. + let map_for_err = map.clone(); map.parse() + .map_err(|e| anyhow::anyhow!("{}", e.highlight(&map_for_err))) } } @@ -1195,12 +1194,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/fs.rs b/crates/wit-parser/src/resolve/fs.rs index 39ff86721c..a6dae0eead 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 f7d7b5ac92..7d5b627224 100644 --- a/crates/wit-parser/src/resolve/mod.rs +++ b/crates/wit-parser/src/resolve/mod.rs @@ -4,11 +4,11 @@ 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::*; -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}; @@ -19,11 +19,11 @@ use serde_derive::Serialize; use std::collections::hash_map::Entry; use crate::ast::lex::Span; -use crate::ast::{ParsedUsePath, parse_use_path}; +use crate::ast::{ParseError, 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, + AstItem, Docs, Function, FunctionKind, Handle, IncludeName, Interface, InterfaceId, LiftLowerAbi, ManglingAndAbi, PackageName, PackageNotFoundError, SourceMap, Stability, Type, TypeDef, TypeDefKind, TypeId, TypeIdVisitor, TypeOwner, UnresolvedPackage, UnresolvedPackageGroup, World, WorldId, WorldItem, WorldKey, @@ -196,32 +196,32 @@ fn visit<'a>( order: &mut IndexSet, visiting: &mut HashSet<&'a PackageName>, source_maps: &[SourceMap], -) -> Result<()> { +) -> Result<(), WitError> { 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(()) - }) + for (i, (dep, _)) in pkg.foreign_deps.iter().enumerate() { + let span = pkg.foreign_dep_spans[i]; + if !visiting.insert(dep) { + let (_, source_map_index) = pkg_details_map.get(&pkg.name).unwrap(); + let source_map = &source_maps[*source_map_index]; + let span_location = source_map.get_location(span); + let highlighted = source_map.highlight_span(span, "package creates a dependency cycle"); + return Err(WitError::PackageCycle { + package: dep.clone(), + span_location, + highlighted, + }); + } + if let Some(dep) = pkg_details_map.get(dep) { + let (dep_pkg, _) = dep; + visit(dep_pkg, pkg_details_map, order, visiting, source_maps)?; } - 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 { @@ -254,11 +254,11 @@ impl Resolve { &mut self, main: UnresolvedPackageGroup, deps: Vec, - ) -> Result<(PackageId, PackageSources)> { + ) -> Result<(PackageId, PackageSources), WitError> { let mut pkg_details_map = BTreeMap::new(); let mut source_maps = Vec::new(); - let mut insert = |group: UnresolvedPackageGroup| { + let mut insert = |group: UnresolvedPackageGroup| -> Result<(), WitError> { let UnresolvedPackageGroup { main, nested, @@ -276,13 +276,7 @@ impl Resolve { }; 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\ - " - ) + return Err(WitError::DuplicatePackage { name, loc1, loc2 }); } 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 [`WitError`] 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 [`WitError`] 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) } @@ -397,7 +430,7 @@ 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 { + pub fn push_source(&mut self, path: &str, contents: &str) -> anyhow::Result { self.push_group(UnresolvedPackageGroup::parse_str(path, contents)?) } @@ -464,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(), @@ -519,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 => { @@ -538,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 => { @@ -557,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, @@ -577,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); @@ -685,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 @@ -699,7 +741,7 @@ package {name} is defined in two different locations:\n\ WorldItem::Interface { id: aid, stability: astability, - .. + span: aspan, }, WorldItem::Interface { id: bid, @@ -709,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!(), @@ -742,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(); @@ -859,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, .. }) => { @@ -924,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); @@ -956,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) { @@ -1050,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 @@ -1091,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(); @@ -1252,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( @@ -1806,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 @@ -1827,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 \ + WitError::Parse(ParseError { + 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(WitError::Parse(ParseError { + span, + message: format!( + "feature gate cannot reference unreleased version \ {since} of package [{}] (current version {package_version})", - p.name - ))); + p.name + ), + })); } true @@ -1853,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. /// @@ -1877,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<(), WitError> { // 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. @@ -1982,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 @@ -2075,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<(), WitError> { let mut required_imports = HashSet::new(); for (id, (key, stability)) in export_interfaces.iter() { let name = self.name_world_key(&key); @@ -2091,26 +2131,24 @@ 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(WitError::InvalidTransitiveDependency { name, span }); } } return Ok(()); @@ -2185,7 +2223,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 @@ -2311,13 +2349,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)] @@ -2993,7 +3026,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, WitError> { match map.get(id.index()) { Some(Some(id)) => Ok(*id), Some(None) => { @@ -3001,7 +3039,7 @@ 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(WitError::Parse(ParseError { span, message: msg })) } None => panic!("request to remap a {desc} that has not yet been registered"), } @@ -3022,38 +3060,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| { + WitError::Parse(ParseError { + 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| { + WitError::Parse(ParseError { + 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(); @@ -3067,7 +3124,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; } @@ -3101,20 +3158,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; } @@ -3136,10 +3180,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 => {} } @@ -3154,15 +3195,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; } @@ -3182,10 +3215,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 => {} } @@ -3214,15 +3244,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 @@ -3258,7 +3280,7 @@ impl Remap { resolve: &mut Resolve, pkgid: PackageId, unresolved: &UnresolvedPackage, - ) -> Result<()> { + ) -> Result<(), WitError> { // 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(); @@ -3310,10 +3332,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(WitError::Parse(ParseError { + span: *span, + message: format!("type used in a handle must be a resource"), + })); + } } } } @@ -3330,7 +3354,7 @@ impl Remap { interface_to_package: &HashMap)>, resolve: &mut Resolve, parent_pkg_id: &PackageId, - ) -> Result<(), anyhow::Error> { + ) -> Result<(), WitError> { for (unresolved_iface_id, unresolved_iface) in unresolved.interfaces.iter() { let (pkg_name, interface, span, stabilities) = match interface_to_package.get(&unresolved_iface_id) { @@ -3346,11 +3370,11 @@ impl Remap { .get(pkg_name) .copied() .ok_or_else(|| { - PackageNotFoundError::new( + WitError::PackageNotFound(PackageNotFoundError::new( span, pkg_name.clone(), resolve.package_names.keys().cloned().collect(), - ) + )) })?; // Functions can't be imported so this should be empty. @@ -3372,11 +3396,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(|| { + WitError::Parse(ParseError { + 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)); } @@ -3395,7 +3420,7 @@ impl Remap { world_to_package: &HashMap)>, resolve: &mut Resolve, parent_pkg_id: &PackageId, - ) -> Result<(), anyhow::Error> { + ) -> Result<(), WitError> { for (unresolved_world_id, unresolved_world) in unresolved.worlds.iter() { let (pkg_name, world, span, stabilities) = match world_to_package.get(&unresolved_world_id) { @@ -3409,7 +3434,12 @@ impl Remap { .package_names .get(pkg_name) .copied() - .ok_or_else(|| Error::new(span, "package not found"))?; + .ok_or_else(|| { + WitError::Parse(ParseError { + span, + message: "package not found".to_owned(), + }) + })?; let pkg = &resolve.packages[pkgid]; let world_span = unresolved_world.span; @@ -3426,11 +3456,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(|| { + WitError::Parse(ParseError { + 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)); } @@ -3448,7 +3479,7 @@ impl Remap { unresolved: &UnresolvedPackage, pkgid: PackageId, resolve: &mut Resolve, - ) -> Result<(), anyhow::Error> { + ) -> Result<(), WitError> { 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 @@ -3459,7 +3490,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; } @@ -3475,7 +3506,10 @@ impl Remap { .types .get(name) .ok_or_else(|| { - Error::new(span, format!("type `{name}` not defined in interface")) + WitError::Parse(ParseError { + span, + message: format!("type `{name}` not defined in interface"), + }) })?; assert_eq!(self.types.len(), unresolved_type_id.index()); self.types.push(Some(type_id)); @@ -3493,7 +3527,7 @@ impl Remap { resolve: &mut Resolve, ty: &mut TypeDef, span: Span, - ) -> Result<()> { + ) -> Result<(), WitError> { // 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::*; @@ -3506,8 +3540,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) => { @@ -3556,7 +3589,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<(), WitError> { let id = match ty { Type::Id(id) => id, _ => return Ok(()), @@ -3591,12 +3629,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<(), WitError> { *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<(), WitError> { iface.types.retain(|_, ty| self.types[ty.index()].is_some()); let iface_pkg_id = iface.package.as_ref().unwrap_or_else(|| { panic!( @@ -3614,21 +3656,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 @@ -3647,7 +3680,7 @@ impl Remap { resolve: &mut Resolve, func: &mut Function, span: Span, - ) -> Result<()> { + ) -> Result<(), WitError> { if let Some(id) = func.kind.resource_mut() { self.update_type_id(id, span)?; } @@ -3660,13 +3693,13 @@ impl Remap { if let Some(ty) = &func.result { if self.type_has_borrow(resolve, ty) { - bail!(Error::new( + return Err(WitError::Parse(ParseError { span, - format!( + message: format!( "function returns a type which contains \ a `borrow` which is not supported" - ) - )) + ), + })); } } @@ -3678,7 +3711,7 @@ impl Remap { world: &mut World, resolve: &mut Resolve, pkg_id: &PackageId, - ) -> Result<()> { + ) -> Result<(), WitError> { // 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. @@ -3694,10 +3727,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)?; @@ -3730,22 +3760,13 @@ impl Remap { id: WorldId, resolve: &mut Resolve, pkg_id: &PackageId, - ) -> Result<()> { + ) -> Result<(), WitError> { 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( @@ -3767,47 +3788,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<(), WitError> { 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<(), WitError> { + 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(WitError::Parse(ParseError { + 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<(), WitError> { match key { WorldKey::Name(_) => {} WorldKey::Interface(id) => { @@ -3825,7 +3854,7 @@ impl Remap { span: Span, pkg_id: &PackageId, resolve: &mut Resolve, - ) -> Result<()> { + ) -> Result<(), WitError> { 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(); @@ -3840,13 +3869,13 @@ impl Remap { self.remove_matching_name(export, &mut names_); } if !names_.is_empty() { - bail!(Error::new( + return Err(WitError::Parse(ParseError { 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(); @@ -3899,7 +3928,7 @@ impl Remap { span: Span, item_type: &str, is_external_include: bool, - ) -> Result<()> { + ) -> Result<(), WitError> { match item.0 { WorldKey::Name(n) => { let n = names @@ -3923,10 +3952,12 @@ impl Remap { let prev = get_items(cloner.resolve).insert(key, new_item); if prev.is_some() { - bail!(Error::new( + return Err(WitError::Parse(ParseError { 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(_) => { @@ -3938,7 +3969,7 @@ impl Remap { WorldItem::Interface { id: aid, stability: astability, - .. + span: aspan, }, WorldItem::Interface { id: bid, @@ -3947,7 +3978,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!(), @@ -4070,7 +4106,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) { @@ -4093,7 +4129,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()); @@ -4138,7 +4174,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()); @@ -4182,7 +4222,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. @@ -4191,7 +4231,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)?; @@ -4202,7 +4242,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 `{}`", @@ -4264,7 +4304,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()); @@ -4323,7 +4363,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 ( @@ -4373,7 +4413,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) -> Result<()> { +fn update_stability(from: &Stability, into: &mut Stability, span: Span) -> Result<(), WitError> { // If `from` is unknown or the two stability annotations are equal then // there's nothing to do here. if from == into || from.is_unknown() { @@ -4388,51 +4428,27 @@ 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(WitError::Parse(ParseError { + 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<(), WitError> { 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; diff --git a/crates/wit-smith/src/lib.rs b/crates/wit-smith/src/lib.rs index ebc0e09fa2..47a0ca86a1 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, WitError}; mod config; pub use self::config::Config; @@ -27,7 +27,10 @@ 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(WitError::InvalidTransitiveDependency { .. }) + ) { return Err(arbitrary::Error::IncorrectFormat); } let err = e.to_string(); From 9482030be969597a09eae9d70f3d9382ab53f6f6 Mon Sep 17 00:00:00 2001 From: Phoebe Szmucer <129869709+PhoebeSzmucer@users.noreply.github.com> Date: Tue, 10 Mar 2026 12:59:48 +0000 Subject: [PATCH 2/2] Remove commented out code --- crates/wit-parser/src/lib.rs | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/crates/wit-parser/src/lib.rs b/crates/wit-parser/src/lib.rs index a980c8b9c5..fc890f2e07 100644 --- a/crates/wit-parser/src/lib.rs +++ b/crates/wit-parser/src/lib.rs @@ -294,38 +294,6 @@ impl fmt::Display for PackageName { } } -// #[derive(Clone, 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(Clone, Debug)] pub struct PackageNotFoundError { pub span: Span,