Merge pull request #2795 from Mingun/has-flatten-rework

`has_flatten` rework

Author: dtolnay

serde_derive/src/de.rs

@@ -281,21 +281,11 @@ fn deserialize_body(cont: &Container, params: &Parameters) -> Fragment {
     } else if let attr::Identifier::No = cont.attrs.identifier() {
         match &cont.data {
             Data::Enum(variants) => deserialize_enum(params, variants, &cont.attrs),
-            Data::Struct(Style::Struct, fields) => deserialize_struct(
-                params,
-                fields,
-                &cont.attrs,
-                cont.attrs.has_flatten(),
-                StructForm::Struct,
-            ),
+            Data::Struct(Style::Struct, fields) => {
+                deserialize_struct(params, fields, &cont.attrs, StructForm::Struct)
+            }
             Data::Struct(Style::Tuple, fields) | Data::Struct(Style::Newtype, fields) => {
-                deserialize_tuple(
-                    params,
-                    fields,
-                    &cont.attrs,
-                    cont.attrs.has_flatten(),
-                    TupleForm::Tuple,
-                )
+                deserialize_tuple(params, fields, &cont.attrs, TupleForm::Tuple)
             }
             Data::Struct(Style::Unit, _) => deserialize_unit_struct(params, &cont.attrs),
         }
@@ -469,11 +459,10 @@ fn deserialize_tuple(
     params: &Parameters,
     fields: &[Field],
     cattrs: &attr::Container,
-    has_flatten: bool,
     form: TupleForm,
 ) -> Fragment {
     assert!(
-        !has_flatten,
+        !has_flatten(fields),
         "tuples and tuple variants cannot have flatten fields"
     );
 
@@ -594,7 +583,7 @@ fn deserialize_tuple_in_place(
     cattrs: &attr::Container,
 ) -> Fragment {
     assert!(
-        !cattrs.has_flatten(),
+        !has_flatten(fields),
         "tuples and tuple variants cannot have flatten fields"
     );
 
@@ -927,7 +916,6 @@ fn deserialize_struct(
     params: &Parameters,
     fields: &[Field],
     cattrs: &attr::Container,
-    has_flatten: bool,
     form: StructForm,
 ) -> Fragment {
     let this_type = &params.this_type;
@@ -976,6 +964,8 @@ fn deserialize_struct(
             )
         })
         .collect();
+
+    let has_flatten = has_flatten(fields);
     let field_visitor = deserialize_field_identifier(&field_names_idents, cattrs, has_flatten);
 
     // untagged struct variants do not get a visit_seq method. The same applies to
@@ -1115,7 +1105,7 @@ fn deserialize_struct_in_place(
 ) -> Option<Fragment> {
     // for now we do not support in_place deserialization for structs that
     // are represented as map.
-    if cattrs.has_flatten() {
+    if has_flatten(fields) {
         return None;
     }
 
@@ -1831,14 +1821,12 @@ fn deserialize_externally_tagged_variant(
             params,
             &variant.fields,
             cattrs,
-            variant.attrs.has_flatten(),
             TupleForm::ExternallyTagged(variant_ident),
         ),
         Style::Struct => deserialize_struct(
             params,
             &variant.fields,
             cattrs,
-            variant.attrs.has_flatten(),
             StructForm::ExternallyTagged(variant_ident),
         ),
     }
@@ -1882,7 +1870,6 @@ fn deserialize_internally_tagged_variant(
             params,
             &variant.fields,
             cattrs,
-            variant.attrs.has_flatten(),
             StructForm::InternallyTagged(variant_ident, deserializer),
         ),
         Style::Tuple => unreachable!("checked in serde_derive_internals"),
@@ -1933,14 +1920,12 @@ fn deserialize_untagged_variant(
             params,
             &variant.fields,
             cattrs,
-            variant.attrs.has_flatten(),
             TupleForm::Untagged(variant_ident, deserializer),
         ),
         Style::Struct => deserialize_struct(
             params,
             &variant.fields,
             cattrs,
-            variant.attrs.has_flatten(),
             StructForm::Untagged(variant_ident, deserializer),
         ),
     }
@@ -2707,7 +2692,7 @@ fn deserialize_map_in_place(
     cattrs: &attr::Container,
 ) -> Fragment {
     assert!(
-        !cattrs.has_flatten(),
+        !has_flatten(fields),
         "inplace deserialization of maps does not support flatten fields"
     );
 
@@ -3042,6 +3027,13 @@ fn effective_style(variant: &Variant) -> Style {
     }
 }
 
+/// True if there are fields that is not skipped and has a `#[serde(flatten)]` attribute.
+fn has_flatten(fields: &[Field]) -> bool {
+    fields
+        .iter()
+        .any(|field| field.attrs.flatten() && !field.attrs.skip_deserializing())
+}
+
 struct DeImplGenerics<'a>(&'a Parameters);
 #[cfg(feature = "deserialize_in_place")]
 struct InPlaceImplGenerics<'a>(&'a Parameters);

serde_derive/src/internals/ast.rs

@@ -63,7 +63,7 @@ impl<'a> Container<'a> {
         item: &'a syn::DeriveInput,
         derive: Derive,
     ) -> Option<Container<'a>> {
-        let mut attrs = attr::Container::from_ast(cx, item);
+        let attrs = attr::Container::from_ast(cx, item);
 
         let mut data = match &item.data {
             syn::Data::Enum(data) => Data::Enum(enum_from_ast(cx, &data.variants, attrs.default())),
@@ -77,16 +77,11 @@ impl<'a> Container<'a> {
             }
         };
 
-        let mut has_flatten = false;
         match &mut data {
             Data::Enum(variants) => {
                 for variant in variants {
                     variant.attrs.rename_by_rules(attrs.rename_all_rules());
                     for field in &mut variant.fields {
-                        if field.attrs.flatten() {
-                            has_flatten = true;
-                            variant.attrs.mark_has_flatten();
-                        }
                         field.attrs.rename_by_rules(
                             variant
                                 .attrs
@@ -98,18 +93,11 @@ impl<'a> Container<'a> {
             }
             Data::Struct(_, fields) => {
                 for field in fields {
-                    if field.attrs.flatten() {
-                        has_flatten = true;
-                    }
                     field.attrs.rename_by_rules(attrs.rename_all_rules());
                 }
             }
         }
 
-        if has_flatten {
-            attrs.mark_has_flatten();
-        }
-
         let mut item = Container {
             ident: item.ident.clone(),
             attrs,

serde_derive/src/internals/attr.rs

@@ -216,23 +216,6 @@ pub struct Container {
     type_into: Option<syn::Type>,
     remote: Option<syn::Path>,
     identifier: Identifier,
-    /// True if container is a struct and has a field with `#[serde(flatten)]`,
-    /// or is an enum with a struct variant which has a field with
-    /// `#[serde(flatten)]`.
-    ///
-    /// ```ignore
-    /// struct Container {
-    ///     #[serde(flatten)]
-    ///     some_field: (),
-    /// }
-    /// enum Container {
-    ///     Variant {
-    ///         #[serde(flatten)]
-    ///         some_field: (),
-    ///     },
-    /// }
-    /// ```
-    has_flatten: bool,
     serde_path: Option<syn::Path>,
     is_packed: bool,
     /// Error message generated when type can't be deserialized
@@ -603,7 +586,6 @@ impl Container {
             type_into: type_into.get(),
             remote: remote.get(),
             identifier: decide_identifier(cx, item, field_identifier, variant_identifier),
-            has_flatten: false,
             serde_path: serde_path.get(),
             is_packed,
             expecting: expecting.get(),
@@ -671,14 +653,6 @@ impl Container {
         self.identifier
     }
 
-    pub fn has_flatten(&self) -> bool {
-        self.has_flatten
-    }
-
-    pub fn mark_has_flatten(&mut self) {
-        self.has_flatten = true;
-    }
-
     pub fn custom_serde_path(&self) -> Option<&syn::Path> {
         self.serde_path.as_ref()
     }
@@ -810,18 +784,6 @@ pub struct Variant {
     rename_all_rules: RenameAllRules,
     ser_bound: Option<Vec<syn::WherePredicate>>,
     de_bound: Option<Vec<syn::WherePredicate>>,
-    /// True if variant is a struct variant which contains a field with
-    /// `#[serde(flatten)]`.
-    ///
-    /// ```ignore
-    /// enum Enum {
-    ///     Variant {
-    ///         #[serde(flatten)]
-    ///         some_field: (),
-    ///     },
-    /// }
-    /// ```
-    has_flatten: bool,
     skip_deserializing: bool,
     skip_serializing: bool,
     other: bool,
@@ -991,7 +953,6 @@ impl Variant {
             },
             ser_bound: ser_bound.get(),
             de_bound: de_bound.get(),
-            has_flatten: false,
             skip_deserializing: skip_deserializing.get(),
             skip_serializing: skip_serializing.get(),
             other: other.get(),
@@ -1034,14 +995,6 @@ impl Variant {
         self.de_bound.as_ref().map(|vec| &vec[..])
     }
 
-    pub fn has_flatten(&self) -> bool {
-        self.has_flatten
-    }
-
-    pub fn mark_has_flatten(&mut self) {
-        self.has_flatten = true;
-    }
-
     pub fn skip_deserializing(&self) -> bool {
         self.skip_deserializing
     }

serde_derive/src/ser.rs

@@ -289,9 +289,18 @@ fn serialize_tuple_struct(
 }
 
 fn serialize_struct(params: &Parameters, fields: &[Field], cattrs: &attr::Container) -> Fragment {
-    assert!(fields.len() as u64 <= u64::from(u32::MAX));
+    assert!(
+        fields.len() as u64 <= u64::from(u32::MAX),
+        "too many fields in {}: {}, maximum supported count is {}",
+        cattrs.name().serialize_name(),
+        fields.len(),
+        u32::MAX
+    );
 
-    if cattrs.has_flatten() {
+    let has_non_skipped_flatten = fields
+        .iter()
+        .any(|field| field.attrs.flatten() && !field.attrs.skip_serializing());
+    if has_non_skipped_flatten {
         serialize_struct_as_map(params, fields, cattrs)
     } else {
         serialize_struct_as_struct(params, fields, cattrs)
@@ -370,26 +379,8 @@ fn serialize_struct_as_map(
 
     let let_mut = mut_if(serialized_fields.peek().is_some() || tag_field_exists);
 
-    let len = if cattrs.has_flatten() {
-        quote!(_serde::__private::None)
-    } else {
-        let len = serialized_fields
-            .map(|field| match field.attrs.skip_serializing_if() {
-                None => quote!(1),
-                Some(path) => {
-                    let field_expr = get_member(params, field, &field.member);
-                    quote!(if #path(#field_expr) { 0 } else { 1 })
-                }
-            })
-            .fold(
-                quote!(#tag_field_exists as usize),
-                |sum, expr| quote!(#sum + #expr),
-            );
-        quote!(_serde::__private::Some(#len))
-    };
-
     quote_block! {
-        let #let_mut __serde_state = _serde::Serializer::serialize_map(__serializer, #len)?;
+        let #let_mut __serde_state = _serde::Serializer::serialize_map(__serializer, _serde::__private::None)?;
         #tag_field
         #(#serialize_fields)*
         _serde::ser::SerializeMap::end(__serde_state)

test_suite/tests/test_gen.rs

@@ -547,13 +547,32 @@ fn test_gen() {
     }
     assert::<FlattenWith>();
 
+    #[derive(Serialize, Deserialize)]
+    pub struct Flatten<T> {
+        #[serde(flatten)]
+        t: T,
+    }
+
     #[derive(Serialize, Deserialize)]
     #[serde(deny_unknown_fields)]
     pub struct FlattenDenyUnknown<T> {
         #[serde(flatten)]
         t: T,
     }
 
+    #[derive(Serialize, Deserialize)]
+    pub struct SkipDeserializing<T> {
+        #[serde(skip_deserializing)]
+        flat: T,
+    }
+
+    #[derive(Serialize, Deserialize)]
+    #[serde(deny_unknown_fields)]
+    pub struct SkipDeserializingDenyUnknown<T> {
+        #[serde(skip_deserializing)]
+        flat: T,
+    }
+
     #[derive(Serialize, Deserialize)]
     pub struct StaticStrStruct<'a> {
         a: &'a str,
@@ -720,14 +739,11 @@ fn test_gen() {
         flat: StdOption<T>,
     }
 
-    #[allow(clippy::collection_is_never_read)] // FIXME
-    const _: () = {
-        #[derive(Serialize, Deserialize)]
-        pub struct FlattenSkipDeserializing<T> {
-            #[serde(flatten, skip_deserializing)]
-            flat: T,
-        }
-    };
+    #[derive(Serialize, Deserialize)]
+    pub struct FlattenSkipDeserializing<T> {
+        #[serde(flatten, skip_deserializing)]
+        flat: T,
+    }
 
     #[derive(Serialize, Deserialize)]
     #[serde(untagged)]