Skip to content

Commit

Permalink
Fix parsing of --with on the CLI (#999)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alexcrichton authored Jul 11, 2024
1 parent 8ff0e17 commit 7d82e7e
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 56 deletions.
19 changes: 17 additions & 2 deletions crates/guest-rust/macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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();

Expand Down
4 changes: 4 additions & 0 deletions crates/guest-rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
89 changes: 37 additions & 52 deletions crates/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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}}
// ```")
43 changes: 43 additions & 0 deletions crates/rust/tests/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
});
}
}
15 changes: 13 additions & 2 deletions src/bin/wit-bindgen.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 7d82e7e

Please sign in to comment.