Fix parsing of --with on the CLI (bytecodealliance#999)

This commit fixes the `--with` option in the Rust generate from changes
in bytecodealliance#972. Notably the fixes here are:

* The `--with` option is no longer required.
* Multiple `--with` options are now accepted again.
* A new `--generate-all` option was added.
* The `generate_all` macro option was documented.
* Error messages on the CLI and the macro now mention all the variants
  of how to fix the error.

Closes bytecodealliance#995

Author: alexcrichton

crates/guest-rust/macro/src/lib.rs

@@ -18,13 +18,28 @@ pub fn generate(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
 }
 
 fn anyhow_to_syn(span: Span, err: anyhow::Error) -> Error {
+    let err = attach_with_context(err);
     let mut msg = err.to_string();
     for cause in err.chain().skip(1) {
         msg.push_str(&format!("\n\nCaused by:\n  {cause}"));
     }
     Error::new(span, msg)
 }
 
+fn attach_with_context(err: anyhow::Error) -> anyhow::Error {
+    if let Some(e) = err.downcast_ref::<wit_bindgen_rust::MissingWith>() {
+        let option = e.0.clone();
+        return err.context(format!(
+            "missing one of:\n\
+            * `generate_all` option\n\
+            * `with: {{ \"{option}\": path::to::bindings, }}`\n\
+            * `with: {{ \"{option}\": generate, }}`\
+            "
+        ));
+    }
+    err
+}
+
 struct Config {
     opts: Opts,
     resolve: Resolve,
@@ -100,7 +115,7 @@ impl Parse for Config {
                     }
                     Opt::With(with) => opts.with.extend(with),
                     Opt::GenerateAll => {
-                        opts.with.generate_by_default = true;
+                        opts.generate_all = true;
                     }
                     Opt::TypeSectionSuffix(suffix) => {
                         opts.type_section_suffix = Some(suffix.value());
@@ -186,7 +201,7 @@ impl Config {
         let mut generator = self.opts.build();
         generator
             .generate(&self.resolve, self.world, &mut files)
-            .map_err(|e| Error::new(Span::call_site(), e))?;
+            .map_err(|e| anyhow_to_syn(Span::call_site(), e))?;
         let (_, src) = files.iter().next().unwrap();
         let mut src = std::str::from_utf8(src).unwrap().to_string();
 

crates/guest-rust/src/lib.rs

@@ -694,6 +694,10 @@
 ///         "some:package/my-interface": generate,
 ///     },
 ///
+///     // Indicates that all interfaces not present in `with` should be assumed
+///     // to be marked with `generate`.
+///     generate_all,
+///
 ///     // An optional list of function names to skip generating bindings for.
 ///     // This is only applicable to imports and the name specified is the name
 ///     // of the function.

crates/rust/src/lib.rs

@@ -105,6 +105,18 @@ pub enum ExportKey {
     Name(String),
 }
 
+#[cfg(feature = "clap")]
+fn parse_with(s: &str) -> Result<(String, WithOption), String> {
+    let (k, v) = s.split_once('=').ok_or_else(|| {
+        format!("expected string of form `<key>=<value>[,<key>=<value>...]`; got `{s}`")
+    })?;
+    let v = match v {
+        "generate" => WithOption::Generate,
+        other => WithOption::Path(other.to_string()),
+    };
+    Ok((k.to_string(), v))
+}
+
 #[derive(Default, Debug, Clone)]
 #[cfg_attr(feature = "clap", derive(clap::Args))]
 pub struct Opts {
@@ -179,8 +191,13 @@ pub struct Opts {
     /// Argument must be of the form `k=v` and this option can be passed
     /// multiple times or one option can be comma separated, for example
     /// `k1=v1,k2=v2`.
-    #[cfg_attr(feature = "clap", arg(long, value_parser = clap::value_parser!(WithGeneration), value_delimiter = ','))]
-    pub with: WithGeneration,
+    #[cfg_attr(feature = "clap", arg(long, value_parser = parse_with, value_delimiter = ','))]
+    pub with: Vec<(String, WithOption)>,
+
+    /// Indicates that all interfaces not specified in `with` should be
+    /// generated.
+    #[cfg_attr(feature = "clap", arg(long))]
+    pub generate_all: bool,
 
     /// Add the specified suffix to the name of the custome section containing
     /// the component type.
@@ -307,10 +324,7 @@ impl RustWasm {
     ) -> Result<bool> {
         let with_name = resolve.name_world_key(name);
         let Some(remapping) = self.with.get(&with_name) else {
-            bail!("no remapping found for {with_name:?} - use the `generate!` macro's `with` option to force the interface to be generated or specify where it is already defined:
-```
-with: {{\n\t{with_name:?}: generate\n}}
-```")
+            bail!(MissingWith(with_name));
         };
         self.generated_interfaces.insert(with_name);
         let entry = match remapping {
@@ -872,7 +886,7 @@ impl WorldGenerator for RustWasm {
         for (k, v) in self.opts.with.iter() {
             self.with.insert(k.clone(), v.clone().into());
         }
-        self.with.generate_by_default = self.opts.with.generate_by_default;
+        self.with.generate_by_default = self.opts.generate_all;
     }
 
     fn import_interface(
@@ -1192,51 +1206,6 @@ impl fmt::Display for Ownership {
     }
 }
 
-/// Configuration for how interfaces are generated.
-#[derive(Debug, Clone, Default)]
-pub struct WithGeneration {
-    /// How interface should be generated
-    with: HashMap<String, WithOption>,
-    /// Whether to generate interfaces not present in the `with` map
-    pub generate_by_default: bool,
-}
-
-impl WithGeneration {
-    fn iter(&self) -> impl Iterator<Item = (&String, &WithOption)> {
-        self.with.iter()
-    }
-
-    pub fn extend(&mut self, with: HashMap<String, WithOption>) {
-        self.with.extend(with);
-    }
-}
-
-impl FromStr for WithGeneration {
-    type Err = String;
-
-    fn from_str(s: &str) -> std::prelude::v1::Result<Self, Self::Err> {
-        let with = s
-            .trim()
-            .split(',')
-            .map(|s| {
-                let (k, v) = s.trim().split_once('=').ok_or_else(|| {
-                    format!("expected string of form `<key>=<value>[,<key>=<value>...]`; got `{s}`")
-                })?;
-                let k = k.trim().to_string();
-                let v = match v.trim() {
-                    "generate" => WithOption::Generate,
-                    v => WithOption::Path(v.to_string()),
-                };
-                Ok((k, v))
-            })
-            .collect::<Result<HashMap<_, _>, Self::Err>>()?;
-        Ok(WithGeneration {
-            with,
-            generate_by_default: false,
-        })
-    }
-}
-
 /// Options for with "with" remappings.
 #[derive(Debug, Clone)]
 pub enum WithOption {
@@ -1462,3 +1431,19 @@ impl fmt::Display for RustFlagsRepr {
         }
     }
 }
+
+#[derive(Debug, Clone)]
+pub struct MissingWith(pub String);
+
+impl fmt::Display for MissingWith {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "missing `with` mapping for the key `{}`", self.0)
+    }
+}
+
+impl std::error::Error for MissingWith {}
+
+// bail!("no remapping found for {with_name:?} - use the `generate!` macro's `with` option to force the interface to be generated or specify where it is already defined:
+// ```
+// with: {{\n\t{with_name:?}: generate\n}}
+// ```")

crates/rust/tests/codegen.rs

@@ -513,3 +513,46 @@ mod gated_features {
         z();
     }
 }
+
+#[allow(unused)]
+mod simple_with_option {
+    mod a {
+        wit_bindgen::generate!({
+            inline: r#"
+                package foo:bar {
+                    interface a {
+                        x: func();
+                    }
+                }
+
+                package foo:baz {
+                    world w {
+                        import foo:bar/a;
+                    }
+                }
+            "#,
+            world: "foo:baz/w",
+            generate_all,
+        });
+    }
+
+    mod b {
+        wit_bindgen::generate!({
+            inline: r#"
+                package foo:bar {
+                    interface a {
+                        x: func();
+                    }
+                }
+
+                package foo:baz {
+                    world w {
+                        import foo:bar/a;
+                    }
+                }
+            "#,
+            world: "foo:baz/w",
+            with: { "foo:bar/a": generate },
+        });
+    }
+}

src/bin/wit-bindgen.rs

@@ -1,4 +1,4 @@
-use anyhow::{bail, Context, Result};
+use anyhow::{bail, Context, Error, Result};
 use clap::Parser;
 use std::path::PathBuf;
 use std::str;
@@ -123,7 +123,7 @@ fn main() -> Result<()> {
         Opt::CSharp { opts, args } => (opts.build(), args),
     };
 
-    gen_world(generator, &opt, &mut files)?;
+    gen_world(generator, &opt, &mut files).map_err(attach_with_context)?;
 
     for (name, contents) in files.iter() {
         let dst = match &opt.out_dir {
@@ -166,6 +166,17 @@ fn main() -> Result<()> {
     Ok(())
 }
 
+fn attach_with_context(err: Error) -> Error {
+    #[cfg(feature = "rust")]
+    if let Some(e) = err.downcast_ref::<wit_bindgen_rust::MissingWith>() {
+        let option = e.0.clone();
+        return err.context(format!(
+            "missing either `--generate-all` or `--with {option}=(...|generate)`"
+        ));
+    }
+    err
+}
+
 fn gen_world(
     mut generator: Box<dyn WorldGenerator>,
     opts: &Common,