Skip to content

Commit

Permalink
Accept a closure for with in lieu of a path for fields (#310)
Browse files Browse the repository at this point in the history
To avoid exposing the internals of generated impls, these closures are forbidden from capturing locals.
This change should be backwards-compatible, as previously only paths were accepted.

As part of this change, `trybuild` was updated to `1.0.89`; this was done in this commit for simplicity.

Fixes #309
  • Loading branch information
TedDriggs authored Oct 7, 2024
1 parent 1817119 commit b746a0c
Show file tree
Hide file tree
Showing 11 changed files with 136 additions and 16 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- Support `#[darling(with = ...)]` on the `data` field when deriving `FromDeriveInput`. This allows the use of simpler receiver types, such as a `Vec` of enum variants.
- Bump version of `proc-macro2` to 1.0.86.
- Accept closures for `#[darling(with = ...)]` on fields in `FromDeriveInput`, `FromMeta`, `FromField`, etc. [#309](https://github.com/TedDriggs/darling/issues/309)

## v0.20.10 (July 9, 2024)

Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ syn = "2.0.15"

[target.'cfg(compiletests)'.dev-dependencies]
rustversion = "1.0.9"
trybuild = "1.0.38"
trybuild = "1.0.89"

[features]
default = ["suggestions"]
Expand Down
2 changes: 1 addition & 1 deletion compiletests.sh
Original file line number Diff line number Diff line change
@@ -1 +1 @@
RUSTFLAGS="--cfg=compiletests" cargo +1.65.0 test --test compiletests
RUSTFLAGS="--cfg=compiletests" cargo +1.77.0 test --test compiletests
17 changes: 12 additions & 5 deletions core/src/codegen/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::borrow::Cow;

use proc_macro2::TokenStream;
use quote::{quote, quote_spanned, ToTokens, TokenStreamExt};
use syn::{spanned::Spanned, Ident, Path, Type};
use syn::{spanned::Spanned, Ident, Type};

use crate::codegen::{DefaultExpression, PostfixTransform};
use crate::usage::{self, IdentRefSet, IdentSet, UsesTypeParams};
Expand All @@ -22,7 +22,10 @@ pub struct Field<'a> {
/// The type of the field in the input.
pub ty: &'a Type,
pub default_expression: Option<DefaultExpression<'a>>,
pub with_path: Cow<'a, Path>,
/// An expression that will be wrapped in a call to [`core::convert::identity`] and
/// then used for converting a provided value into the field value _before_ postfix
/// transforms are called.
pub with_callable: Cow<'a, syn::Expr>,
pub post_transform: Option<&'a PostfixTransform>,
pub skip: bool,
pub multiple: bool,
Expand Down Expand Up @@ -156,7 +159,7 @@ impl<'a> ToTokens for MatchArm<'a> {

let name_str = &field.name_in_attr;
let ident = field.ident;
let with_path = &field.with_path;
let with_callable = &field.with_callable;
let post_transform = field.post_transform.as_ref();

// Errors include the location of the bad input, so we compute that here.
Expand All @@ -171,15 +174,19 @@ impl<'a> ToTokens for MatchArm<'a> {
quote!(#name_str)
};

// Give darling's generated code the span of the `with_path` so that if the target
// Give darling's generated code the span of the `with_callable` so that if the target
// type doesn't impl FromMeta, darling's immediate user gets a properly-spanned error.
//
// Within the generated code, add the span immediately on extraction failure, so that it's
// as specific as possible.
// The behavior of `with_span` makes this safe to do; if the child applied an
// even-more-specific span, our attempt here will not overwrite that and will only cost
// us one `if` check.
let extractor = quote_spanned!(with_path.span()=>#with_path(__inner)#post_transform.map_err(|e| e.with_span(&__inner).at(#location)));
let extractor = quote_spanned!(with_callable.span()=>
::darling::export::identity::<fn(&::syn::Meta) -> ::darling::Result<_>>(#with_callable)(__inner)
#post_transform
.map_err(|e| e.with_span(&__inner).at(#location))
);

tokens.append_all(if field.multiple {
quote!(
Expand Down
27 changes: 24 additions & 3 deletions core/src/options/input_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub struct InputField {
pub attr_name: Option<String>,
pub ty: syn::Type,
pub default: Option<DefaultExpression>,
pub with: Option<syn::Path>,
pub with: Option<With>,

/// If `true`, generated code will not look for this field in the input meta item,
/// instead always falling back to either `InputField::default` or `Default::default`.
Expand All @@ -34,7 +34,7 @@ impl InputField {
.map_or_else(|| Cow::Owned(self.ident.to_string()), Cow::Borrowed),
ty: &self.ty,
default_expression: self.as_codegen_default(),
with_path: self.with.as_ref().map_or_else(
with_callable: self.with.as_ref().map(|w| &w.call).map_or_else(
|| {
Cow::Owned(
parse_quote_spanned!(self.ty.span()=> ::darling::FromMeta::from_meta),
Expand Down Expand Up @@ -149,7 +149,7 @@ impl ParseAttribute for InputField {
return Err(Error::duplicate_field_path(path).with_span(mi));
}

self.with = Some(FromMeta::from_meta(mi)?);
self.with = Some(With::from_meta(mi)?);

if self.flatten.is_present() {
return Err(
Expand Down Expand Up @@ -239,3 +239,24 @@ impl ParseAttribute for InputField {
Ok(())
}
}

#[derive(Debug, Clone)]
pub struct With {
/// The callable
call: syn::Expr,
}

impl With {
pub fn from_meta(meta: &syn::Meta) -> Result<Self> {
if let syn::Meta::NameValue(nv) = meta {
match &nv.value {
syn::Expr::Path(_) | syn::Expr::Closure(_) => Ok(Self {
call: nv.value.clone(),
}),
_ => Err(Error::unexpected_expr_type(&nv.value)),
}
} else {
Err(Error::unsupported_format("non-value"))
}
}
}
11 changes: 9 additions & 2 deletions examples/expr_with.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
use darling::{util::parse_expr, FromDeriveInput};
use darling::{util::parse_expr, FromDeriveInput, FromMeta};
use syn::{parse_quote, Expr};

#[derive(FromDeriveInput)]
#[darling(attributes(demo))]
pub struct Receiver {
#[darling(with = parse_expr::preserve_str_literal, map = Some)]
example1: Option<Expr>,
#[darling(
// A closure can be used in lieu of a path.
with = |m| Ok(String::from_meta(m)?.to_uppercase()),
default
)]
example2: String,
}

fn main() {
let input = Receiver::from_derive_input(&parse_quote! {
#[demo(example1 = test::path)]
#[demo(example1 = test::path, example2 = "hello")]
struct Example;
})
.unwrap();

assert_eq!(input.example1, Some(parse_quote!(test::path)));
assert_eq!(input.example2, "HELLO".to_string());
}
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub use darling_core::ToTokens;
/// of the referenced types.
#[doc(hidden)]
pub mod export {
pub use core::convert::From;
pub use core::convert::{identity, From};
pub use core::default::Default;
pub use core::option::Option::{self, None, Some};
pub use core::result::Result::{self, Err, Ok};
Expand Down
6 changes: 3 additions & 3 deletions tests/compile-fail/attrs_with_bad_fn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ note: method defined here
| pub fn handle<T>(&mut self, result: Result<T>) -> Option<T> {
| ^^^^^^
help: try wrapping the expression in `Ok`
|
11 | #[darling(with = Ok(bad_converter))]
| +++ +
|
11 | #[darling(with = Ok(bad_converter))]
| +++ +
21 changes: 21 additions & 0 deletions tests/compile-fail/with_closure_capture.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
use darling::{FromDeriveInput, FromMeta};

#[derive(FromDeriveInput)]
#[darling(attributes(demo))]
pub struct Receiver {
example1: String,
#[darling(
// This should fail because `example1` is a local that's been captured
// from the `FromDeriveInput` impl. That's disallowed because exposing
// those internals would make any change to the derived method body a
// potentially-breaking change.
with = |m| Ok(
String::from_meta(m)?.to_uppercase()
+ example1.1.as_ref().map(|s| s.as_str()).unwrap_or("")
),
default
)]
example2: String,
}

fn main() {}
30 changes: 30 additions & 0 deletions tests/compile-fail/with_closure_capture.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
error[E0308]: mismatched types
--> tests/compile-fail/with_closure_capture.rs:12:16
|
12 | with = |m| Ok(
| ^ arguments to this function are incorrect
| ________________|
| |
13 | | String::from_meta(m)?.to_uppercase()
14 | | + example1.1.as_ref().map(|s| s.as_str()).unwrap_or("")
15 | | ),
| |_________^ expected fn pointer, found closure
|
= note: expected fn pointer `for<'a> fn(&'a syn::Meta) -> Result<String, darling::Error>`
found closure `{closure@$DIR/tests/compile-fail/with_closure_capture.rs:12:16: 12:19}`
note: closures can only be coerced to `fn` types if they do not capture any variables
--> tests/compile-fail/with_closure_capture.rs:14:15
|
14 | + example1.1.as_ref().map(|s| s.as_str()).unwrap_or("")
| ^^^^^^^^ `example1` captured here
help: the return type of this call is `{closure@$DIR/tests/compile-fail/with_closure_capture.rs:12:16: 12:19}` due to the type of the argument passed
--> tests/compile-fail/with_closure_capture.rs:12:16
|
12 | with = |m| Ok(
| ________________^
13 | | String::from_meta(m)?.to_uppercase()
14 | | + example1.1.as_ref().map(|s| s.as_str()).unwrap_or("")
15 | | ),
| |_________- this argument influences the return type of `{{root}}`
note: function defined here
--> $RUST/core/src/convert/mod.rs
33 changes: 33 additions & 0 deletions tests/meta_with.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use darling::{util::parse_expr, FromDeriveInput, FromMeta};
use syn::{parse_quote, Expr};

#[derive(FromDeriveInput)]
#[darling(attributes(demo))]
pub struct Receiver {
#[darling(with = parse_expr::preserve_str_literal, map = Some)]
example1: Option<Expr>,
#[darling(
with = |m| Ok(String::from_meta(m)?.to_uppercase()),
map = Some
)]
example2: Option<String>,
// This is deliberately strange - it keeps the field name, and ignores
// the rest of the attribute. In normal operation, this is strongly discouraged.
// It's used here to verify that the parameter type is known even if it can't be
// inferred from usage within the closure.
#[darling(with = |m| Ok(m.path().clone()))]
example3: syn::Path,
}

#[test]
fn handles_all_cases() {
let input = Receiver::from_derive_input(&parse_quote! {
#[demo(example1 = test::path, example2 = "hello", example3)]
struct Example;
})
.unwrap();

assert_eq!(input.example1, Some(parse_quote!(test::path)));
assert_eq!(input.example2, Some("HELLO".to_string()));
assert_eq!(input.example3, parse_quote!(example3));
}

0 comments on commit b746a0c

Please sign in to comment.