Fold resolve_world_from_name helper into select_world (#1611)

* Fold `resolve_world_from_name` helper into `select_world`

This is a follow-up to #1577 to refactor a bit to have bindings
generators continue to use `Resolve::select_world` for figuring out what
to generate bindings for.

* Review comments and API changes

* Rename `Resolve::append` to `Resolve::push_group`
* Add `Resolve::push_str`
* Change `&Path` arguments to `impl AsRef<Path>`

* Fix fuzzer build

Author: alexcrichton

crates/wit-component/src/encoding.rs

@@ -2129,39 +2129,32 @@ impl ComponentEncoder {
 
 #[cfg(all(test, feature = "dummy-module"))]
 mod test {
-    use crate::{dummy_module, embed_component_metadata};
-
     use super::*;
-    use std::path::Path;
-    use wit_parser::UnresolvedPackageGroup;
+    use crate::{dummy_module, embed_component_metadata};
 
     #[test]
     fn it_renames_imports() {
         let mut resolve = Resolve::new();
-        let UnresolvedPackageGroup {
-            mut packages,
-            source_map,
-        } = UnresolvedPackageGroup::parse(
-            Path::new("test.wit"),
-            r#"
+        let pkgs = resolve
+            .push_str(
+                "test.wit",
+                r#"
 package test:wit;
 
 interface i {
-f: func();
+    f: func();
 }
 
 world test {
-import i;
-import foo: interface {
-f: func();
-}
+    import i;
+    import foo: interface {
+        f: func();
+    }
 }
 "#,
-        )
-        .unwrap();
-        let pkg = resolve.push(packages.remove(0), &source_map).unwrap();
-
-        let world = resolve.select_world(pkg, None).unwrap();
+            )
+            .unwrap();
+        let world = resolve.select_world(&pkgs, None).unwrap();
 
         let mut module = dummy_module(&resolve, world);
 

crates/wit-component/src/lib.rs

@@ -5,9 +5,9 @@
 use std::str::FromStr;
 use std::{borrow::Cow, fmt::Display};
 
-use anyhow::{bail, Context, Result};
+use anyhow::{bail, Result};
 use wasm_encoder::{CanonicalOption, Encode, Section};
-use wit_parser::{parse_use_path, PackageId, ParsedUsePath, Resolve, WorldId};
+use wit_parser::{Resolve, WorldId};
 
 mod encoding;
 mod gc;
@@ -79,43 +79,6 @@ impl From<StringEncoding> for wasm_encoder::CanonicalOption {
     }
 }
 
-/// Handles world name resolution for cases when multiple packages may have been resolved. If this
-/// is the case, and we're dealing with input that contains a user-supplied world name (like via a
-/// CLI command, for instance), we want to ensure that the world name follows the following rules:
-///
-///   * If there is a single resolved package with a single world, the world name name MAY be
-///     omitted.
-///   * If there is a single resolved package with multiple worlds, the world name MUST be supplied,
-///     but MAY or MAY NOT be fully-qualified.
-///   * If there are multiple resolved packages, the world name MUST be fully-qualified.
-pub fn resolve_world_from_name(
-    resolve: &Resolve,
-    resolved_packages: Vec<PackageId>,
-    world_name: Option<&str>,
-) -> Result<WorldId> {
-    match resolved_packages.len() {
-        0 => bail!("all of the supplied WIT source files were empty"),
-        1 => resolve.select_world(resolved_packages[0], world_name.as_deref()),
-        _ => match world_name.as_deref() {
-            Some(name) => {
-                let world_path = parse_use_path(name).with_context(|| {
-                    format!("failed to parse world specifier `{name}`")
-                })?;
-                match world_path {
-                        ParsedUsePath::Name(name) => bail!("the world specifier must be of the fully-qualified, id-based form (ex: \"wasi:http/proxy\" rather than \"proxy\"); you used {name}"),
-                        ParsedUsePath::Package(pkg_name, _) => {
-                            match resolve.package_names.get(&pkg_name) {
-                                Some(pkg_id) => resolve.select_world(pkg_id.clone(), world_name.as_deref()),
-                                None => bail!("the world specifier you provided named {pkg_name}, but no package with that name was found"),
-                            }
-                        }
-                    }
-            }
-            None => bail!("the supplied WIT source files describe multiple packages; please provide a fully-qualified world-specifier to the `embed` command"),
-        },
-    }
-}
-
 /// A producer section to be added to all modules and components synthesized by
 /// this crate
 pub(crate) fn base_producers() -> wasm_metadata::Producers {
@@ -145,11 +108,9 @@ pub fn embed_component_metadata(
 
 #[cfg(test)]
 mod tests {
-    use std::path::Path;
-
     use anyhow::Result;
     use wasmparser::Payload;
-    use wit_parser::{Resolve, UnresolvedPackageGroup};
+    use wit_parser::Resolve;
 
     use super::{embed_component_metadata, StringEncoding};
 
@@ -184,12 +145,8 @@ world test-world {}
 
         // Parse pre-canned WIT to build resolver
         let mut resolver = Resolve::default();
-        let UnresolvedPackageGroup {
-            mut packages,
-            source_map,
-        } = UnresolvedPackageGroup::parse(&Path::new("in-code.wit"), COMPONENT_WIT)?;
-        let pkg_id = resolver.push(packages.remove(0), &source_map)?;
-        let world = resolver.select_world(pkg_id, Some("test-world").into())?;
+        let pkgs = resolver.push_str("in-code.wit", COMPONENT_WIT)?;
+        let world = resolver.select_world(&pkgs, Some("test-world"))?;
 
         // Embed component metadata
         embed_component_metadata(&mut bytes, &resolver, world, StringEncoding::UTF8)?;

crates/wit-component/src/metadata.rs

@@ -42,7 +42,7 @@
 //! the three arguments originally passed to `encode`.
 
 use crate::validation::BARE_FUNC_MODULE_NAME;
-use crate::{resolve_world_from_name, DecodedWasm, StringEncoding};
+use crate::{DecodedWasm, StringEncoding};
 use anyhow::{bail, Context, Result};
 use indexmap::IndexMap;
 use std::borrow::Cow;
@@ -264,7 +264,7 @@ impl Bindgen {
                     DecodedWasm::Component(..) => bail!("expected encoded wit package(s)"),
                 };
                 resolve = r;
-                world = resolve_world_from_name(&resolve, pkgs, Some(world_name.into()))?;
+                world = resolve.select_world(&pkgs, Some(world_name.into()))?;
             }
 
             // Current format where `data` is a wasm component itself.

crates/wit-component/tests/components.rs

@@ -245,7 +245,7 @@ fn read_core_module(path: &Path, resolve: &Resolve, pkg: PackageId) -> Result<Ve
     let mut wasm = wat::parse_file(path)?;
     let name = path.file_stem().and_then(|s| s.to_str()).unwrap();
     let world = resolve
-        .select_world(pkg, Some(name))
+        .select_world(&[pkg], Some(name))
         .context("failed to select a world")?;
 
     // Add this producer data to the wit-component metadata so we can make sure it gets through the

crates/wit-component/tests/interfaces.rs

@@ -51,7 +51,7 @@ fn run_test(path: &Path, is_dir: bool) -> Result<()> {
     let packages = if is_dir {
         resolve.push_dir(path)?.0
     } else {
-        resolve.append(UnresolvedPackageGroup::parse_file(path)?)?
+        resolve.push_file(path)?
     };
 
     for package in packages {
@@ -107,8 +107,7 @@ fn assert_print(resolve: &Resolve, pkg_ids: &[PackageId], path: &Path, is_dir: b
         assert_output(&expected, &output)?;
     }
 
-    UnresolvedPackageGroup::parse("foo.wit".as_ref(), &output)
-        .context("failed to parse printed output")?;
+    UnresolvedPackageGroup::parse("foo.wit", &output).context("failed to parse printed output")?;
     Ok(())
 }
 

crates/wit-component/tests/linking.rs

@@ -1,8 +1,7 @@
 use {
     anyhow::{Context, Result},
-    std::path::Path,
     wit_component::StringEncoding,
-    wit_parser::{Resolve, UnresolvedPackageGroup},
+    wit_parser::Resolve,
 };
 
 const FOO: &str = r#"
@@ -141,12 +140,8 @@ fn encode(wat: &str, wit: Option<&str>) -> Result<Vec<u8>> {
 
     if let Some(wit) = wit {
         let mut resolve = Resolve::default();
-        let UnresolvedPackageGroup {
-            mut packages,
-            source_map,
-        } = UnresolvedPackageGroup::parse(Path::new("wit"), wit)?;
-        let pkg = resolve.push(packages.remove(0), &source_map)?;
-        let world = resolve.select_world(pkg, None)?;
+        let pkgs = resolve.push_str("test.wit", wit)?;
+        let world = resolve.select_world(&pkgs, None)?;
 
         wit_component::embed_component_metadata(
             &mut module,

crates/wit-component/tests/targets.rs

@@ -1,6 +1,6 @@
-use anyhow::{bail, Context, Result};
+use anyhow::{Context, Result};
 use std::{fs, path::Path};
-use wit_parser::{Resolve, UnresolvedPackageGroup, WorldId};
+use wit_parser::{Resolve, WorldId};
 
 /// Tests whether a component targets a world.
 ///
@@ -22,7 +22,7 @@ use wit_parser::{Resolve, UnresolvedPackageGroup, WorldId};
 /// Run the test with the environment variable `BLESS` set to update `error.txt`.
 ///
 /// Each test is effectively executing as:
-/// ```wasm-tools component targets -w foobar test.wit test.wat```
+/// `wasm-tools component targets -w foobar test.wit test.wat`
 #[test]
 fn targets() -> Result<()> {
     drop(env_logger::try_init());
@@ -79,23 +79,10 @@ fn load_test_wit(path: &Path) -> Result<(Resolve, WorldId)> {
     const TEST_TARGET_WORLD_ID: &str = "foobar";
 
     let test_wit_path = path.join("test.wit");
-    let UnresolvedPackageGroup {
-        mut packages,
-        source_map,
-    } = UnresolvedPackageGroup::parse_file(&test_wit_path)
-        .context("failed to parse WIT package")?;
-    if packages.is_empty() {
-        bail!("Files were completely empty - are you sure these are the files you're looking for?")
-    }
-    if packages.len() > 1 {
-        bail!("Multi-package targeting tests are not yet supported.")
-    }
-
     let mut resolve = Resolve::default();
-    let package_id = resolve.push(packages.remove(0), &source_map)?;
-
+    let pkgs = resolve.push_file(&test_wit_path)?;
     let world_id = resolve
-        .select_world(package_id, Some(TEST_TARGET_WORLD_ID))
+        .select_world(&pkgs, Some(TEST_TARGET_WORLD_ID))
         .with_context(|| "failed to select world from package".to_string())?;
 
     Ok((resolve, world_id))

crates/wit-component/tests/wit.rs

@@ -9,9 +9,9 @@ fn parse_wit_dir() -> Result<()> {
     drop(env_logger::try_init());
 
     let mut resolver = Resolve::default();
-    let package_id = resolver.push_path("tests/wit/parse-dir/wit")?.0[0];
+    let (package_ids, _) = resolver.push_path("tests/wit/parse-dir/wit")?;
     assert!(resolver
-        .select_world(package_id, "foo-world".into())
+        .select_world(&package_ids, "foo-world".into())
         .is_ok());
 
     Ok(())
@@ -23,10 +23,8 @@ fn parse_wit_file() -> Result<()> {
     drop(env_logger::try_init());
 
     let mut resolver = Resolve::default();
-    let package_id = resolver
-        .push_path("tests/wit/parse-dir/wit/deps/bar/bar.wit")?
-        .0[0];
-    resolver.select_world(package_id, "bar-world".into())?;
+    let (package_ids, _) = resolver.push_path("tests/wit/parse-dir/wit/deps/bar/bar.wit")?;
+    resolver.select_world(&package_ids, "bar-world".into())?;
     assert!(resolver
         .interfaces
         .iter()

crates/wit-parser/src/lib.rs

@@ -228,10 +228,11 @@ impl UnresolvedPackageGroup {
     /// Parses the given string as a wit document.
     ///
     /// The `path` argument is used for error reporting. The `contents` provided
-    /// will not be able to use `pkg` use paths to other documents.
-    pub fn parse(path: &Path, contents: &str) -> Result<UnresolvedPackageGroup> {
+    /// are considered to be the contents of `path`. This function does not read
+    /// the filesystem.
+    pub fn parse(path: impl AsRef<Path>, contents: &str) -> Result<UnresolvedPackageGroup> {
         let mut map = SourceMap::default();
-        map.push(path, contents);
+        map.push(path.as_ref(), contents);
         map.parse()
     }
 
@@ -240,7 +241,8 @@ impl UnresolvedPackageGroup {
     /// The path provided is inferred whether it's a file or a directory. A file
     /// is parsed with [`UnresolvedPackageGroup::parse_file`] and a directory is
     /// parsed with [`UnresolvedPackageGroup::parse_dir`].
-    pub fn parse_path(path: &Path) -> Result<UnresolvedPackageGroup> {
+    pub fn parse_path(path: impl AsRef<Path>) -> Result<UnresolvedPackageGroup> {
+        let path = path.as_ref();
         if path.is_dir() {
             UnresolvedPackageGroup::parse_dir(path)
         } else {
@@ -250,19 +252,23 @@ impl UnresolvedPackageGroup {
 
     /// Parses a WIT package from the file provided.
     ///
-    /// The WIT package returned will be a single-document package and will not
-    /// be able to use `pkg` paths to other documents.
-    pub fn parse_file(path: &Path) -> Result<UnresolvedPackageGroup> {
+    /// The return value represents all packages found in the WIT file which
+    /// might be either one or multiple depending on the syntax used.
+    pub fn parse_file(path: impl AsRef<Path>) -> Result<UnresolvedPackageGroup> {
+        let path = path.as_ref();
         let contents = std::fs::read_to_string(path)
             .with_context(|| format!("failed to read file {path:?}"))?;
         Self::parse(path, &contents)
     }
 
     /// Parses a WIT package from the directory provided.
     ///
-    /// All files with the extension `*.wit` or `*.wit.md` will be loaded from
-    /// `path` into the returned package.
-    pub fn parse_dir(path: &Path) -> Result<UnresolvedPackageGroup> {
+    /// This method will look at all files under the `path` specified. All
+    /// `*.wit` files are parsed and assumed to be part of the same package
+    /// grouping. This is useful when a WIT package is split across multiple
+    /// files.
+    pub fn parse_dir(path: impl AsRef<Path>) -> Result<UnresolvedPackageGroup> {
+        let path = path.as_ref();
         let mut map = SourceMap::default();
         let cx = || format!("failed to read directory {path:?}");
         for entry in path.read_dir().with_context(&cx)? {
@@ -281,7 +287,7 @@ impl UnresolvedPackageGroup {
                 Some(name) => name,
                 None => continue,
             };
-            if !filename.ends_with(".wit") && !filename.ends_with(".wit.md") {
+            if !filename.ends_with(".wit") {
                 continue;
             }
             map.push_file(&path)?;