Skip to content

Commit

Permalink
de: Make flattened structs and aliases interact correctly.
Browse files Browse the repository at this point in the history
The issue is that FlatStructAccess has no access to the aliases of the struct
it's deserializing.

Ideally we'd try to serialize the key rather than checking whether we're going
to use it before-hand, then actually take the value out, but that happens to be
tricky with the current seed API.

So we need to somehow get the aliased names back to FlatStructAccess. Introduce
a serialize_struct-like API that takes them in a backwards-compatible way. For
parallelism, and since we also support aliases on enum variants, also extend the
struct_variant API in a similar way. I'm open to better ways to fix it, but I
can't think of any other that isn't a breaking change...

Fixes #1504.
  • Loading branch information
emilio authored and Jesse Hoobergs committed Oct 2, 2022
1 parent 1f61acf commit 19d0c40
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 33 deletions.
46 changes: 46 additions & 0 deletions serde/src/de/mod.rs
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
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
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
1 change: 0 additions & 1 deletion test_suite/tests/test_annotations.rs
Expand Up @@ -627,7 +627,6 @@ fn test_rename_struct() {
}

#[test]
#[should_panic] // FIXME(emilio): It shouldn't!
fn test_alias_flattened() {
#[derive(Debug, PartialEq, Deserialize)]
struct Flattened {
Expand Down

0 comments on commit 19d0c40

Please sign in to comment.