Repository: TedDriggs/darling (rust)

Issue:
Flatten fields should combine unknown items from across multiple attributes
The current placement of the initializer for a `flatten`ed field has two problems:

1. If there are multiple attributes, only unknown items from the last attribute will be reflected in the returned value of the flattened field.
2. If there are no attributes, there will be a spurious error for the flattened field being uninitialized

Pull request:
Fix flattening to work with 0..n attributes
Fixes #271

Patch:
diff --git a/core/src/codegen/field.rs b/core/src/codegen/field.rs
index c17cc90..51020a9 100644
--- a/core/src/codegen/field.rs
+++ b/core/src/codegen/field.rs
@@ -96,6 +96,17 @@ impl<'a> ToTokens for Declaration<'a> {
         } else {
             quote!(let mut #ident: (bool, ::darling::export::Option<#ty>) = (false, None);)
         });
+
+        // The flatten field additionally needs a place to buffer meta items
+        // until attribute walking is done, so declare that now.
+        //
+        // We expect there can only be one field marked `flatten`, so it shouldn't
+        // be possible for this to shadow another declaration.
+        if field.flatten {
+            tokens.append_all(quote! {
+                let mut __flatten: Vec<::darling::ast::NestedMeta> = vec![];
+            });
+        }
     }
 }
 
@@ -123,13 +134,7 @@ impl<'a> ToTokens for FlattenInitializer<'a> {
         tokens.append_all(quote! {
             #ident = (true,
                 __errors.handle(
-                    ::darling::FromMeta::from_list(
-                        __flatten
-                            .into_iter()
-                            .cloned()
-                            .map(::darling::ast::NestedMeta::Meta)
-                            .collect::<Vec<_>>().as_slice()
-                        ) #add_parent_fields
+                    ::darling::FromMeta::from_list(&__flatten) #add_parent_fields
                     )
                 );
         });
diff --git a/core/src/codegen/trait_impl.rs b/core/src/codegen/trait_impl.rs
index 0bc7f7e..2f358db 100644
--- a/core/src/codegen/trait_impl.rs
+++ b/core/src/codegen/trait_impl.rs
@@ -112,7 +112,17 @@ impl<'a> TraitImpl<'a> {
         if let Data::Struct(ref vd) = self.data {
             let check_nones = vd.as_ref().map(Field::as_presence_check);
             let checks = check_nones.fields.as_slice();
-            quote!(#(#checks)*)
+
+            // If a field was marked `flatten`, now is the time to process any unclaimed meta items
+            // and mark the field as having been seen.
+            let flatten_field_init = vd.fields.iter().find(|f| f.flatten).map(|v| {
+                v.as_flatten_initializer(vd.fields.iter().filter_map(Field::as_name).collect())
+            });
+
+            quote! {
+                #flatten_field_init
+                #(#checks)*
+            }
         } else {
             quote!()
         }
diff --git a/core/src/codegen/variant_data.rs b/core/src/codegen/variant_data.rs
index 04c1416..9a8bffc 100644
--- a/core/src/codegen/variant_data.rs
+++ b/core/src/codegen/variant_data.rs
@@ -7,14 +7,12 @@ use crate::codegen::Field;
 pub struct FieldsGen<'a> {
     fields: &'a Fields<Field<'a>>,
     allow_unknown_fields: bool,
-    flatten_field: Option<&'a Field<'a>>,
 }
 
 impl<'a> FieldsGen<'a> {
     pub fn new(fields: &'a Fields<Field<'a>>, allow_unknown_fields: bool) -> Self {
         Self {
             fields,
-            flatten_field: fields.fields.iter().find(|f| f.flatten),
             allow_unknown_fields,
         }
     }
@@ -37,22 +35,11 @@ impl<'a> FieldsGen<'a> {
     /// Generate the loop which walks meta items looking for property matches.
     pub(in crate::codegen) fn core_loop(&self) -> TokenStream {
         let arms = self.fields.as_ref().map(Field::as_match);
-
-        let (flatten_buffer, flatten_declaration) = if let Some(flatten_field) = self.flatten_field
-        {
-            (
-                quote! { let mut __flatten = vec![]; },
-                Some(flatten_field.as_flatten_initializer(self.field_names().collect())),
-            )
-        } else {
-            (quote!(), None)
-        };
-
         // If there is a flatten field, buffer the unknown field so it can be passed
         // to the flatten function with all other unknown fields.
-        let handle_unknown = if self.flatten_field.is_some() {
+        let handle_unknown = if self.fields.iter().any(|f| f.flatten) {
             quote! {
-                __flatten.push(__inner);
+                __flatten.push(::darling::ast::NestedMeta::Meta(__inner.clone()));
             }
         }
         // If we're allowing unknown fields, then handling one is a no-op.
@@ -77,8 +64,6 @@ impl<'a> FieldsGen<'a> {
         let arms = arms.iter();
 
         quote!(
-            #flatten_buffer
-
             for __item in __items {
                 match *__item {
                     ::darling::export::NestedMeta::Meta(ref __inner) => {
@@ -94,8 +79,6 @@ impl<'a> FieldsGen<'a> {
                     }
                 }
             }
-
-            #flatten_declaration
         )
     }
 
@@ -119,8 +102,4 @@ impl<'a> FieldsGen<'a> {
 
         quote!(#(#inits),*)
     }
-
-    fn field_names(&self) -> impl Iterator<Item = &str> {
-        self.fields.iter().filter_map(Field::as_name)
-    }
 }


Test patch:
diff --git a/tests/flatten_from_field.rs b/tests/flatten_from_field.rs
index 44eebcf..3338f04 100644
--- a/tests/flatten_from_field.rs
+++ b/tests/flatten_from_field.rs
@@ -4,6 +4,7 @@ use syn::parse_quote;
 
 #[derive(FromMeta)]
 struct Vis {
+    #[darling(default)]
     public: bool,
     #[darling(default)]
     private: bool,
@@ -44,3 +45,40 @@ fn field_flattens() {
     assert!(!first_field.visibility.private);
     assert_eq!(first_field.example.unwrap(), "world");
 }
+
+#[test]
+fn field_flattens_with_no_field_level_attributes() {
+    let di = Input::from_derive_input(&parse_quote! {
+        struct Demo {
+            hello: String
+        }
+    })
+    .unwrap();
+
+    let fields = di.data.take_struct().unwrap();
+    let first_field = fields.into_iter().next().unwrap();
+    assert_eq!(
+        first_field.ident,
+        Some(Ident::new("hello", Span::call_site()))
+    );
+    assert!(!first_field.visibility.public);
+    assert!(!first_field.visibility.private);
+    assert_eq!(first_field.example, None);
+}
+
+#[test]
+fn field_flattens_across_attributes() {
+    let di = Input::from_derive_input(&parse_quote! {
+        struct Demo {
+            #[v(public)]
+            #[v(private)]
+            hello: String
+        }
+    })
+    .unwrap();
+
+    let fields = di.data.take_struct().unwrap();
+    let first_field = fields.into_iter().next().unwrap();
+    assert!(first_field.visibility.public);
+    assert!(first_field.visibility.private);
+}