Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

de: Make flattened structs and aliases interact correctly. #2160

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions serde/src/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,28 @@ pub trait Deserializer<'de>: Sized {
where
V: Visitor<'de>;

/// Hint that the `Deserialize` type is expecting a struct with a particular
/// name and fields.
///
/// `_fields_with_aliases` includes all valid field names, including
/// aliases. `fields` only includes the canonical struct fields.
///
/// Use this if you care about aliased fields. For backwards compatibility,
/// by default this calls `deserialize_struct`. If you implement this, you
/// probably want `deserialize_struct` to call this instead.
fn deserialize_struct_with_aliases<V>(
self,
name: &'static str,
fields: &'static [&'static str],
_fields_with_aliases: &'static [&'static str],
visitor: V,
) -> Result<V::Value, Self::Error>
where
V: Visitor<'de>
{
self.deserialize_struct(name, fields, visitor)
}

/// Hint that the `Deserialize` type is expecting an enum value with a
/// particular name and possible variants.
fn deserialize_enum<V>(
Expand Down Expand Up @@ -2222,6 +2244,30 @@ pub trait VariantAccess<'de>: Sized {
) -> Result<V::Value, Self::Error>
where
V: Visitor<'de>;

/// Called when deserializing a struct-like variant.
///
/// The `fields` are the names of the fields of the struct variant.
///
/// `_fields_with_aliases` includes all valid field names, including
/// aliases. `fields` only includes the canonical struct fields.
///
/// Use this if you care about aliased fields. For backwards compatibility,
/// by default this calls `struct_variant`. If you implement this, you
/// probably want `struct_variant` to call this instead.
///
/// Same constraints as `struct_vriant` applies.
fn struct_variant_with_aliases<V>(
self,
fields: &'static [&'static str],
_fields_with_aliases: &'static [&'static str],
visitor: V,
) -> Result<V::Value, Self::Error>
where
V: Visitor<'de>
{
self.struct_variant(fields, visitor)
}
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
17 changes: 15 additions & 2 deletions serde/src/private/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2766,14 +2766,27 @@ where

fn deserialize_struct<V>(
self,
_: &'static str,
name: &'static str,
fields: &'static [&'static str],
visitor: V,
) -> Result<V::Value, Self::Error>
where
V: Visitor<'de>,
{
visitor.visit_map(FlatStructAccess::new(self.0.iter_mut(), fields))
self.deserialize_struct_with_aliases(name, fields, fields, visitor)
}

fn deserialize_struct_with_aliases<V>(
self,
_: &'static str,
_: &'static [&'static str],
fields_with_aliases: &'static [&'static str],
visitor: V,
) -> Result<V::Value, Self::Error>
where
V: Visitor<'de>,
{
visitor.visit_map(FlatStructAccess::new(self.0.iter_mut(), fields_with_aliases))
}

fn deserialize_newtype_struct<V>(self, _name: &str, visitor: V) -> Result<V::Value, Self::Error>
Expand Down
95 changes: 65 additions & 30 deletions serde_derive/src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,12 @@ fn deserialize_struct(
}
} else if is_enum {
quote! {
_serde::de::VariantAccess::struct_variant(__variant, FIELDS, #visitor_expr)
_serde::de::VariantAccess::struct_variant_with_aliases(
__variant,
FIELDS,
FIELDS_WITH_ALIASES,
#visitor_expr,
)
}
} else if cattrs.has_flatten() {
quote! {
Expand All @@ -964,7 +969,13 @@ fn deserialize_struct(
} else {
let type_name = cattrs.name().deserialize_name();
quote! {
_serde::Deserializer::deserialize_struct(__deserializer, #type_name, FIELDS, #visitor_expr)
_serde::Deserializer::deserialize_struct_with_aliases(
__deserializer,
#type_name,
FIELDS,
FIELDS_WITH_ALIASES,
#visitor_expr,
)
}
};

Expand Down Expand Up @@ -1089,12 +1100,23 @@ fn deserialize_struct_in_place(
}
} else if is_enum {
quote! {
_serde::de::VariantAccess::struct_variant(__variant, FIELDS, #visitor_expr)
_serde::de::VariantAccess::struct_variant_with_aliases(
__variant,
FIELDS,
FIELDS_WITH_ALIASES,
#visitor_expr,
)
}
} else {
let type_name = cattrs.name().deserialize_name();
quote! {
_serde::Deserializer::deserialize_struct(__deserializer, #type_name, FIELDS, #visitor_expr)
_serde::Deserializer::deserialize_struct_with_aliases(
__deserializer,
#type_name,
FIELDS,
FIELDS_WITH_ALIASES,
#visitor_expr,
)
}
};

Expand Down Expand Up @@ -1633,10 +1655,11 @@ fn deserialize_adjacently_tagged_enum(
}

const FIELDS: &'static [&'static str] = &[#tag, #content];
_serde::Deserializer::deserialize_struct(
_serde::Deserializer::deserialize_struct_with_aliases(
__deserializer,
#type_name,
FIELDS,
FIELDS,
__Visitor {
marker: _serde::__private::PhantomData::<#this #ty_generics>,
lifetime: _serde::__private::PhantomData,
Expand Down Expand Up @@ -2029,21 +2052,18 @@ fn deserialize_custom_identifier(
)
})
.collect();

let names = names_idents.iter().map(|(name, _, _)| name);
let flat_fields = flatten_fields(&names_idents);

let names_const = if fallthrough.is_some() {
None
} else if is_variant {
let names = names_idents.iter().map(|&(ref name, _, _)| name);
let variants = quote! {
const VARIANTS: &'static [&'static str] = &[ #(#names),* ];
};
Some(variants)
} else {
let fields = quote! {
const FIELDS: &'static [&'static str] = &[ #(#names),* ];
};
Some(fields)
Some(decl_fields_consts(&names_idents, &flat_fields))
};

let (de_impl_generics, de_ty_generics, ty_generics, where_clause) =
Expand Down Expand Up @@ -2090,10 +2110,7 @@ fn deserialize_identifier(
collect_other_fields: bool,
expecting: Option<&str>,
) -> Fragment {
let mut flat_fields = Vec::new();
for (_, ident, aliases) in fields {
flat_fields.extend(aliases.iter().map(|alias| (alias, ident)));
}
let flat_fields = flatten_fields(&fields);

let field_strs: &Vec<_> = &flat_fields.iter().map(|(name, _)| name).collect();
let field_bytes: &Vec<_> = &flat_fields
Expand Down Expand Up @@ -2369,6 +2386,29 @@ fn deserialize_identifier(
}
}

fn flatten_fields(fields: &[(String, Ident, Vec<String>)]) -> Vec<(&String, &Ident)> {
let mut flat = vec![];
for &(_, ref ident, ref aliases) in fields {
flat.extend(aliases.iter().map(|alias| (alias, ident)))
}
flat
}

fn decl_fields_consts(
fields: &[(String, Ident, Vec<String>)],
flat_fields: &[(&String, &Ident)],
) -> TokenStream {
let block = {
let field_names = fields.iter().map(|&(ref name, _, _)| name);
let aliases = flat_fields.iter().map(|&(ref name, _)| name);
quote! {
const FIELDS: &'static [&'static str] = &[ #(#field_names),* ];
const FIELDS_WITH_ALIASES: &'static [&'static str] = &[ #(#aliases),* ];
}
};
block
}

fn deserialize_struct_as_struct_visitor(
struct_path: &TokenStream,
params: &Parameters,
Expand All @@ -2390,18 +2430,18 @@ fn deserialize_struct_as_struct_visitor(
})
.collect();

let fields_stmt = {
let field_names = field_names_idents.iter().map(|(name, _, _)| name);
quote_block! {
const FIELDS: &'static [&'static str] = &[ #(#field_names),* ];
}
};
let flat_fields = flatten_fields(&field_names_idents);
let fields_stmt = decl_fields_consts(&field_names_idents, &flat_fields);

let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, None);

let visit_map = deserialize_map(struct_path, params, fields, cattrs);

(field_visitor, Some(fields_stmt), visit_map)
(
field_visitor,
Some(quote_block! { #fields_stmt }),
visit_map,
)
}

fn deserialize_struct_as_map_visitor(
Expand Down Expand Up @@ -2667,18 +2707,13 @@ fn deserialize_struct_as_struct_in_place_visitor(
})
.collect();

let fields_stmt = {
let field_names = field_names_idents.iter().map(|(name, _, _)| name);
quote_block! {
const FIELDS: &'static [&'static str] = &[ #(#field_names),* ];
}
};

let flat_fields = flatten_fields(&field_names_idents);
let fields_stmt = decl_fields_consts(&field_names_idents, &flat_fields);
let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, None);

let visit_map = deserialize_map_in_place(params, fields, cattrs);

(field_visitor, fields_stmt, visit_map)
(field_visitor, quote_block! { #fields_stmt }, visit_map)
}

#[cfg(feature = "deserialize_in_place")]
Expand Down
57 changes: 57 additions & 0 deletions test_suite/tests/test_annotations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,63 @@ fn test_rename_struct() {
);
}

#[test]
fn test_alias_flattened() {
#[derive(Debug, PartialEq, Deserialize)]
struct Flattened {
#[serde(flatten)]
with_aliases: AliasStruct,
}

// First without the aliases.
assert_de_tokens(
&Flattened {
with_aliases: AliasStruct {
a1: 1,
a2: 2,
a4: 3,
},
},
&[
Token::Struct {
name: "Flattened",
len: 3,
},
Token::Str("a1"),
Token::I32(1),
Token::Str("a2"),
Token::I32(2),
Token::Str("a6"),
Token::I32(3),
Token::StructEnd,
],
);

// Now with them
assert_de_tokens(
&Flattened {
with_aliases: AliasStruct {
a1: 1,
a2: 2,
a4: 3,
},
},
&[
Token::Struct {
name: "Flattened",
len: 3,
},
Token::Str("a1"),
Token::I32(1),
Token::Str("a3"),
Token::I32(2),
Token::Str("a6"),
Token::I32(3),
Token::StructEnd,
],
);
}

#[test]
fn test_unknown_field_rename_struct() {
assert_de_tokens_error::<AliasStruct>(
Expand Down