Skip to content

Commit

Permalink
Correctly calculate has_flatten attribute in all cases for deserial…
Browse files Browse the repository at this point in the history
…ization

Consequence: `FlattenSkipDeserializing[DenyUnknown]`
- does not collect data in Field, because do not read them anyway
- gets `deserialize_in_place` method
- gets ability to deserialize from sequence (visit_seq method)
- uses `deserialize_struct` instead of `deserialize_map`
  • Loading branch information
Mingun committed Aug 11, 2024
1 parent 0647a7c commit fd5b5e9
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 39 deletions.
30 changes: 13 additions & 17 deletions serde_derive/src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,7 @@ fn deserialize_body(cont: &Container, params: &Parameters) -> Fragment {
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),
}
Expand Down Expand Up @@ -465,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"
);

Expand Down Expand Up @@ -590,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"
);

Expand Down Expand Up @@ -972,9 +965,7 @@ fn deserialize_struct(
})
.collect();

let has_flatten = fields
.iter()
.any(|field| field.attrs.flatten() && !field.attrs.skip_deserializing());
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
Expand Down Expand Up @@ -1114,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;
}

Expand Down Expand Up @@ -1830,7 +1821,6 @@ fn deserialize_externally_tagged_variant(
params,
&variant.fields,
cattrs,
variant.attrs.has_flatten(),
TupleForm::ExternallyTagged(variant_ident),
),
Style::Struct => deserialize_struct(
Expand Down Expand Up @@ -1930,7 +1920,6 @@ fn deserialize_untagged_variant(
params,
&variant.fields,
cattrs,
variant.attrs.has_flatten(),
TupleForm::Untagged(variant_ident, deserializer),
),
Style::Struct => deserialize_struct(
Expand Down Expand Up @@ -2703,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"
);

Expand Down Expand Up @@ -3038,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);
Expand Down
1 change: 0 additions & 1 deletion serde_derive/src/internals/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ impl<'a> Container<'a> {
for field in &mut variant.fields {
if field.attrs.flatten() {
has_flatten = true;
variant.attrs.mark_has_flatten();
}
field.attrs.rename_by_rules(
variant
Expand Down
21 changes: 0 additions & 21 deletions serde_derive/src/internals/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -810,18 +810,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,
Expand Down Expand Up @@ -991,7 +979,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(),
Expand Down Expand Up @@ -1034,14 +1021,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
}
Expand Down

0 comments on commit fd5b5e9

Please sign in to comment.