From 7d82e7ef6a32b6189da0c1384d8799dbf6ba93c6 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 11 Jul 2024 14:20:24 -0500 Subject: [PATCH] Fix parsing of `--with` on the CLI (#999) This commit fixes the `--with` option in the Rust generate from changes in #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 #995 --- crates/guest-rust/macro/src/lib.rs | 19 ++++++- crates/guest-rust/src/lib.rs | 4 ++ crates/rust/src/lib.rs | 89 +++++++++++++----------------- crates/rust/tests/codegen.rs | 43 +++++++++++++++ src/bin/wit-bindgen.rs | 15 ++++- 5 files changed, 114 insertions(+), 56 deletions(-) diff --git a/crates/guest-rust/macro/src/lib.rs b/crates/guest-rust/macro/src/lib.rs index 24e7fdf44..56e36ac64 100644 --- a/crates/guest-rust/macro/src/lib.rs +++ b/crates/guest-rust/macro/src/lib.rs @@ -18,6 +18,7 @@ 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}")); @@ -25,6 +26,20 @@ fn anyhow_to_syn(span: Span, err: anyhow::Error) -> Error { Error::new(span, msg) } +fn attach_with_context(err: anyhow::Error) -> anyhow::Error { + if let Some(e) = err.downcast_ref::() { + 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(); diff --git a/crates/guest-rust/src/lib.rs b/crates/guest-rust/src/lib.rs index a9770f0b4..240f31bf3 100644 --- a/crates/guest-rust/src/lib.rs +++ b/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. diff --git a/crates/rust/src/lib.rs b/crates/rust/src/lib.rs index 108b2f634..de76c75e8 100644 --- a/crates/rust/src/lib.rs +++ b/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 `=[,=...]`; 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 { 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, - /// Whether to generate interfaces not present in the `with` map - pub generate_by_default: bool, -} - -impl WithGeneration { - fn iter(&self) -> impl Iterator { - self.with.iter() - } - - pub fn extend(&mut self, with: HashMap) { - self.with.extend(with); - } -} - -impl FromStr for WithGeneration { - type Err = String; - - fn from_str(s: &str) -> std::prelude::v1::Result { - let with = s - .trim() - .split(',') - .map(|s| { - let (k, v) = s.trim().split_once('=').ok_or_else(|| { - format!("expected string of form `=[,=...]`; 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::, 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}} +// ```") diff --git a/crates/rust/tests/codegen.rs b/crates/rust/tests/codegen.rs index 6e6919a96..f1ec1ffb8 100644 --- a/crates/rust/tests/codegen.rs +++ b/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 }, + }); + } +} diff --git a/src/bin/wit-bindgen.rs b/src/bin/wit-bindgen.rs index 7abcc067d..7e3c74425 100644 --- a/src/bin/wit-bindgen.rs +++ b/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::() { + let option = e.0.clone(); + return err.context(format!( + "missing either `--generate-all` or `--with {option}=(...|generate)`" + )); + } + err +} + fn gen_world( mut generator: Box, opts: &Common,