idanarye / rust-typed-builder

Compile-time type-checked builder derive
https://crates.io/crates/typed-builder
Apache License 2.0
904 stars 52 forks source link

Add support for paths with angle brackets #122

Closed aumetra closed 10 months ago

aumetra commented 10 months ago

This PR adds support for paths of the build_method(into) field that contain angle brackets (such as Arc<Foo>)

idanarye commented 10 months ago

I'm not a fan of Either (and certainly not of unwrap_left - panics make messy compilation errors). I think a better solution would be to make KeyValue::value a verbatim - a TokenStream with no structure behind tokenization and grouping. Settings that don't need to inspect the internal structure of their value can pass it as is, and the ones that do can just parse it on the spot. KeyValue can even have a convenience parse_value function to streamline the syntax.

This would even make existing implementations simpler. For example, instead of this:

"vis" => {
    let value = expr.key_value()?.value;
    let syn::Expr::Lit(expr_lit) = value else {
        return Err(Error::new_spanned(value, "invalid visibility found"));
    };
    let syn::Lit::Str(expr_str) = expr_lit.lit else {
        return Err(Error::new_spanned(expr_lit, "invalid visibility found"));
    };
    self.vis = Some(syn::parse_str(&expr_str.value())?);
    Ok(())
}

We would have this:

"vis" => {
    let value = expr.key_value()?.parse_value::<syn::LitStr>()?.value();
    self.vis = Some(syn::parse_str(value)?);
    Ok(())
}

If this seems too much, I'm willing to just merge this and then do these changes myself.

aumetra commented 10 months ago

The issue here is that a .parse() call on a TokenStream apparently consumes the source ParseStream beyond its punctuated boundaries (for some reason)

Not quite sure why though, as I'm not really an expert when it comes to proc macros

aumetra commented 10 months ago

Just the transform element is being problematic at the moment, the rest is working just fine

idanarye commented 10 months ago

Yea. I guess my assumption about all commas hidden inside token trees was incorrect. And now that I think about it - it would also be a problem for generic (< and > are also not creating token groups)

I'll fix it myself after merging.