From 19d0c40c4b82bb0f9e5c7ce1d83fa388e6b0f1f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 4 May 2019 20:31:11 +0200 Subject: [PATCH] de: Make flattened structs and aliases interact correctly. 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. --- serde/src/de/mod.rs | 46 ++++++++++++++ serde/src/private/de.rs | 17 ++++- serde_derive/src/de.rs | 95 +++++++++++++++++++--------- test_suite/tests/test_annotations.rs | 1 - 4 files changed, 126 insertions(+), 33 deletions(-) diff --git a/serde/src/de/mod.rs b/serde/src/de/mod.rs index d9dafbe1e..01b1b42b4 100644 --- a/serde/src/de/mod.rs +++ b/serde/src/de/mod.rs @@ -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( + self, + name: &'static str, + fields: &'static [&'static str], + _fields_with_aliases: &'static [&'static str], + visitor: V, + ) -> Result + 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( @@ -2222,6 +2244,30 @@ pub trait VariantAccess<'de>: Sized { ) -> Result 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( + self, + fields: &'static [&'static str], + _fields_with_aliases: &'static [&'static str], + visitor: V, + ) -> Result + where + V: Visitor<'de> + { + self.struct_variant(fields, visitor) + } } //////////////////////////////////////////////////////////////////////////////// diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index f0697d64f..66c2d362e 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2766,14 +2766,27 @@ where fn deserialize_struct( self, - _: &'static str, + name: &'static str, fields: &'static [&'static str], visitor: V, ) -> Result 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( + self, + _: &'static str, + _: &'static [&'static str], + fields_with_aliases: &'static [&'static str], + visitor: V, + ) -> Result + where + V: Visitor<'de>, + { + visitor.visit_map(FlatStructAccess::new(self.0.iter_mut(), fields_with_aliases)) } fn deserialize_newtype_struct(self, _name: &str, visitor: V) -> Result diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index ff7bc42f4..735d5444d 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -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! { @@ -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, + ) } }; @@ -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, + ) } }; @@ -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, @@ -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) = @@ -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 @@ -2369,6 +2386,29 @@ fn deserialize_identifier( } } +fn flatten_fields(fields: &[(String, Ident, Vec)]) -> 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)], + 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, @@ -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( @@ -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")] diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index cec57e90a..b827c886c 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -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 {