From 14721592c87ca6a4b47016f62cb92bc481a0bbfe Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 23 Sep 2024 08:02:32 -0700 Subject: [PATCH] Fix testing for errors during fuzzing Create a first-class error instead of testing for error strings to ensure this is more resilient over time. --- crates/wit-parser/src/lib.rs | 2 +- crates/wit-parser/src/resolve.rs | 32 ++++++++++++++++++++++++++++++-- crates/wit-smith/src/lib.rs | 7 ++----- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/crates/wit-parser/src/lib.rs b/crates/wit-parser/src/lib.rs index f26b87f01f..ad0f9af3d5 100644 --- a/crates/wit-parser/src/lib.rs +++ b/crates/wit-parser/src/lib.rs @@ -21,7 +21,7 @@ pub use ast::{parse_use_path, ParsedUsePath}; mod sizealign; pub use sizealign::*; mod resolve; -pub use resolve::{Package, PackageId, Remap, Resolve}; +pub use resolve::{InvalidTransitiveDependency, Package, PackageId, Remap, Resolve}; mod live; pub use live::{LiveTypes, TypeIdVisitor}; diff --git a/crates/wit-parser/src/resolve.rs b/crates/wit-parser/src/resolve.rs index 40379ae9a8..9799f1a294 100644 --- a/crates/wit-parser/src/resolve.rs +++ b/crates/wit-parser/src/resolve.rs @@ -1,4 +1,5 @@ use std::collections::{BTreeMap, HashMap, HashSet}; +use std::fmt; use std::mem; use std::path::{Path, PathBuf}; @@ -1888,8 +1889,7 @@ package {name} is defined in two different locations:\n\ // 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. - "interface `{name}` transitively depends on an interface in \ - incompatible ways", + InvalidTransitiveDependency(name), ); } } @@ -3218,6 +3218,34 @@ fn update_stability(from: &Stability, into: &mut Stability) -> Result<()> { bail!("mismatch in stability attributes") } +/// 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 + ) + } +} + +impl std::error::Error for InvalidTransitiveDependency {} + #[cfg(test)] mod tests { use crate::Resolve; diff --git a/crates/wit-smith/src/lib.rs b/crates/wit-smith/src/lib.rs index b702c5903e..10e753547d 100644 --- a/crates/wit-smith/src/lib.rs +++ b/crates/wit-smith/src/lib.rs @@ -6,7 +6,7 @@ //! type structures. use arbitrary::{Result, Unstructured}; -use wit_parser::Resolve; +use wit_parser::{InvalidTransitiveDependency, Resolve}; mod config; pub use self::config::Config; @@ -25,10 +25,7 @@ pub fn smith(config: &Config, u: &mut Unstructured<'_>) -> Result> { let id = match resolve.push_group(group) { Ok(id) => id, Err(e) => { - if e.to_string().contains( - "interface transitively depends on an interface in \ - incompatible ways", - ) { + if e.is::() { return Err(arbitrary::Error::IncorrectFormat); } panic!("bad wit parse: {e:?}")