Skip to content

Commit 3c16c07

Browse files
TedDriggsTedDriggs
committed
Check fields earlier for FromVariant impls
Previously, a problem in a variant's attributes would prevent discovery of issues in the variant's fields. This commit also updates the tests of `supports` to ensure there is coverage of the interaction between `supports` and forwarding. Fixes #419
1 parent 4d4f455 commit 3c16c07

3 files changed

Lines changed: 48 additions & 17 deletions

File tree

‎core/src/codegen/from_variant_impl.rs‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use proc_macro2::TokenStream;
22
use quote::{quote, ToTokens};
3-
use syn::{parse_quote, Ident};
3+
use syn::spanned::Spanned;
4+
use syn::{parse_quote, parse_quote_spanned, Ident};
45

56
use crate::codegen::{ident_field, ExtractAttribute, ForwardAttrs, OuterFromImpl, TraitImpl};
67
use crate::options::{DataShape, ForwardedField};
@@ -18,7 +19,7 @@ pub struct FromVariantImpl<'a> {
1819
/// variant's fields should be placed.
1920
///
2021
/// This is one of `darling`'s "magic fields".
21-
pub fields: Option<&'a Ident>,
22+
pub fields: Option<&'a ForwardedField>,
2223
/// If set, the ident of the field into which the discriminant of the input variant
2324
/// should be placed. The receiving field must be an `Option` as not all enums have
2425
/// discriminants.
@@ -42,9 +43,7 @@ impl ToTokens for FromVariantImpl<'_> {
4243
|i| parse_quote!(#i: #input.discriminant.as_ref().map(|(_, expr)| expr.clone())),
4344
),
4445
self.forward_attrs.to_field_value(),
45-
self.fields
46-
.as_ref()
47-
.map(|i| parse_quote!(#i: _darling::ast::Fields::try_from(&#input.fields)?)),
46+
self.fields.as_ref().map(|i| i.to_field_value()),
4847
]
4948
.into_iter()
5049
.flatten();
@@ -58,11 +57,37 @@ impl ToTokens for FromVariantImpl<'_> {
5857
self.base.fallback_decl()
5958
};
6059

61-
let supports = self.supports.map(|i| {
60+
let read_fields = self
61+
.fields
62+
.as_ref()
63+
.map(|i| match &i.with {
64+
Some(p) => p.clone(),
65+
None => parse_quote_spanned!(i.ty.span()=> _darling::ast::Fields::try_from),
66+
})
67+
.unwrap_or_else(|| parse_quote!(_darling::export::Ok));
68+
69+
let supports = self
70+
.supports
71+
.map(|i| {
72+
quote! {
73+
#i.check
74+
}
75+
})
76+
.unwrap_or_else(|| quote!(_darling::export::Ok));
77+
let validate_and_read_fields = {
78+
// If the caller wants `fields` read into a field, we can use `fields` as the local variable name
79+
// because we know there are no other fields of that name.
80+
let let_binding = self.fields.map(|d| {
81+
let ident = &d.ident;
82+
quote!(let #ident = )
83+
});
84+
85+
// The awkward `map` here is to work around the fact that `ShapeSet::check` returns `()`,
86+
// but we need to return the fields for further processing.
6287
quote! {
63-
__errors.handle(#i.check(&#input.fields));
88+
#let_binding __errors.handle(#supports(&#input.fields).map(|_| &#input.fields).and_then(#read_fields));
6489
}
65-
});
90+
};
6691

6792
let error_declaration = self.base.declare_errors();
6893
let require_fields = self.base.require_fields();
@@ -75,7 +100,7 @@ impl ToTokens for FromVariantImpl<'_> {
75100

76101
#extractor
77102

78-
#supports
103+
#validate_and_read_fields
79104

80105
#require_fields
81106

‎core/src/options/from_variant.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,16 @@ use quote::ToTokens;
33
use syn::{DeriveInput, Field, Ident, Meta};
44

55
use crate::codegen::FromVariantImpl;
6-
use crate::options::{DataShape, OuterFrom, ParseAttribute, ParseData};
7-
use crate::{FromMeta, Result};
6+
use crate::options::{DataShape, ForwardedField, OuterFrom, ParseAttribute, ParseData};
7+
use crate::{FromField, FromMeta, Result};
88

99
#[derive(Debug, Clone)]
1010
pub struct FromVariantOptions {
1111
pub base: OuterFrom,
1212
/// The field on the deriving struct into which the discriminant expression
1313
/// should be placed by the derived `FromVariant` impl.
1414
pub discriminant: Option<Ident>,
15-
pub fields: Option<Ident>,
15+
pub fields: Option<ForwardedField>,
1616
pub supports: Option<DataShape>,
1717
}
1818

@@ -63,7 +63,7 @@ impl ParseData for FromVariantOptions {
6363
Ok(())
6464
}
6565
Some("fields") => {
66-
self.fields.clone_from(&field.ident);
66+
self.fields = ForwardedField::from_field(field).map(Some)?;
6767
Ok(())
6868
}
6969
_ => self.base.parse_field(field),

‎tests/supports.rs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ pub struct Container {
88
data: ast::Data<Variant, Panic>,
99
}
1010

11-
#[derive(Default, Debug, FromVariant)]
12-
#[darling(default, attributes(from_variants), supports(newtype, unit))]
11+
#[derive(Debug, FromVariant)]
12+
#[darling(attributes(from_variants), supports(newtype, unit))]
1313
pub struct Variant {
14-
into: Option<bool>,
15-
skip: Option<bool>,
14+
// Having this field tests compilation when both `supports` and forwarding are in use.
15+
fields: darling::ast::Fields<syn::Type>,
1616
}
1717

1818
#[derive(Debug, FromDeriveInput)]
@@ -91,6 +91,12 @@ fn enum_newtype_or_unit() {
9191
// Should pass
9292
let container = Container::from_derive_input(&source::newtype_enum()).unwrap();
9393
assert!(container.data.is_enum());
94+
let variants = container.data.take_enum().unwrap();
95+
assert!(variants[0].fields.is_tuple() && variants[0].fields.len() == 1);
96+
assert!(variants[1].fields.is_tuple() && variants[1].fields.len() == 1);
97+
98+
let empty = Container::from_derive_input(&source::empty_enum()).unwrap();
99+
assert!(empty.data.is_enum());
94100

95101
// Should error
96102
Container::from_derive_input(&source::named_field_enum()).unwrap_err();

0 commit comments

Comments
 (0)