From 5a91ac5ba514a1bc967eda723c4872914f44ad27 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 14 Mar 2018 11:36:01 +0100 Subject: [PATCH 01/51] Initial work on supporting structs as map with unknown field collection --- serde_derive_internals/src/ast.rs | 11 ++++ serde_derive_internals/src/attr.rs | 92 +++++++++++++++++++++++++++++- 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/serde_derive_internals/src/ast.rs b/serde_derive_internals/src/ast.rs index 2ba1f5d9..74fc56fe 100644 --- a/serde_derive_internals/src/ast.rs +++ b/serde_derive_internals/src/ast.rs @@ -63,6 +63,7 @@ impl<'a> Container<'a> { } }; + let mut have_collection_field = false; match data { Data::Enum(ref mut variants) => for variant in variants { variant.attrs.rename_by_rule(attrs.rename_all()); @@ -71,10 +72,20 @@ impl<'a> Container<'a> { } }, Data::Struct(_, ref mut fields) => for field in fields { + if field.ident.is_some() && field.ident.as_ref() == attrs.unknown_fields_into() { + field.attrs.mark_as_collection_field(); + have_collection_field = true; + } field.attrs.rename_by_rule(attrs.rename_all()); }, } + if attrs.unknown_fields_into().is_some() && !have_collection_field { + cx.error(format!("#[serde(unknown_fields_into)] was defined but target \ + field `{}` does not exist", + attrs.unknown_fields_into().unwrap())); + } + let item = Container { ident: item.ident, attrs: attrs, diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs index 7c6e3066..9d622386 100644 --- a/serde_derive_internals/src/attr.rs +++ b/serde_derive_internals/src/attr.rs @@ -15,6 +15,7 @@ use syn::punctuated::Punctuated; use syn::synom::{Synom, ParseError}; use std::collections::BTreeSet; use std::str::FromStr; +use std::fmt; use proc_macro2::{Span, TokenStream, TokenNode, TokenTree}; // This module handles parsing of `#[serde(...)]` attributes. The entrypoints @@ -27,6 +28,7 @@ use proc_macro2::{Span, TokenStream, TokenNode, TokenTree}; pub use case::RenameRule; +#[derive(Copy, Clone)] struct Attr<'c, T> { cx: &'c Ctxt, name: &'static str, @@ -66,6 +68,10 @@ impl<'c, T> Attr<'c, T> { fn get(self) -> Option { self.value } + + fn is_set(&self) -> bool { + self.value.is_some() + } } struct BoolAttr<'c>(Attr<'c, ()>); @@ -101,6 +107,35 @@ impl Name { } } +#[derive(Debug, PartialEq, Eq, Copy, Clone)] +pub enum ContainerRepr { + Auto, + Struct, + Map, +} + +impl fmt::Display for ContainerRepr { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", match *self { + ContainerRepr::Auto => "auto", + ContainerRepr::Struct => "struct", + ContainerRepr::Map => "map", + }) + } +} + +impl FromStr for ContainerRepr { + type Err = (); + fn from_str(s: &str) -> Result { + match s { + "auto" => Ok(ContainerRepr::Auto), + "struct" => Ok(ContainerRepr::Struct), + "map" => Ok(ContainerRepr::Map), + _ => Err(()), + } + } +} + /// Represents container (e.g. struct) attribute information pub struct Container { name: Name, @@ -114,6 +149,8 @@ pub struct Container { type_into: Option, remote: Option, identifier: Identifier, + repr: ContainerRepr, + unknown_fields_into: Option, } /// Styles of representing an enum. @@ -191,6 +228,8 @@ impl Container { let mut remote = Attr::none(cx, "remote"); let mut field_identifier = BoolAttr::none(cx, "field_identifier"); let mut variant_identifier = BoolAttr::none(cx, "variant_identifier"); + let mut repr = Attr::none(cx, "repr"); + let mut unknown_fields_into = Attr::none(cx, "unknown_fields_into"); for meta_items in item.attrs.iter().filter_map(get_serde_meta_items) { for meta_item in meta_items { @@ -228,6 +267,31 @@ impl Container { // Parse `#[serde(deny_unknown_fields)]` Meta(Word(word)) if word == "deny_unknown_fields" => { deny_unknown_fields.set_true(); + if unknown_fields_into.is_set() { + cx.error("#[serde(deny_unknown_fields)] cannot be combined \ + with #[serde(unknown_fields_into)]"); + } + } + + // Parse `#[serde(unknown_fields_into = "foo")]` + Meta(NameValue(ref m)) if m.ident == "unknown_fields_into" => { + if let Ok(s) = get_lit_str(cx, m.ident.as_ref(), m.ident.as_ref(), &m.lit) { + unknown_fields_into.set(Ident::new(&s.value(), Span::call_site()).into()); + if deny_unknown_fields.get() { + cx.error("#[serde(deny_unknown_fields)] cannot be combined \ + with #[serde(unknown_fields_into)]"); + } + } + } + + // Parse `#[serde(repr = "foo")]` + Meta(NameValue(ref m)) if m.ident == "repr" => { + if let Ok(s) = get_lit_str(cx, m.ident.as_ref(), m.ident.as_ref(), &m.lit) { + match ContainerRepr::from_str(&s.value()) { + Ok(value) => repr.set(value), + Err(()) => cx.error(format!("unknown value for #[serde(repr = {})]", s.value())) + } + } } // Parse `#[serde(default)]` @@ -358,6 +422,10 @@ impl Container { } } + if unknown_fields_into.get().is_some() && repr.get() != Some(ContainerRepr::Map) { + cx.error("#[serde(unknown_fields_into)] requires repr=\"map\""); + } + Container { name: Name { serialize: ser_name.get().unwrap_or_else(|| item.ident.to_string()), @@ -373,6 +441,8 @@ impl Container { type_into: type_into.get(), remote: remote.get(), identifier: decide_identifier(cx, item, &field_identifier, &variant_identifier), + repr: repr.get().unwrap_or(ContainerRepr::Auto), + unknown_fields_into: unknown_fields_into.get(), } } @@ -419,6 +489,14 @@ impl Container { pub fn identifier(&self) -> Identifier { self.identifier } + + pub fn repr(&self) -> ContainerRepr { + self.repr + } + + pub fn unknown_fields_into(&self) -> Option<&syn::Ident> { + self.unknown_fields_into.as_ref() + } } fn decide_tag( @@ -699,6 +777,7 @@ pub struct Field { de_bound: Option>, borrowed_lifetimes: BTreeSet, getter: Option, + collection_field: bool, } /// Represents the default to use for a field when deserializing. @@ -970,6 +1049,7 @@ impl Field { de_bound: de_bound.get(), borrowed_lifetimes: borrowed_lifetimes, getter: getter.get(), + collection_field: false, } } @@ -977,6 +1057,10 @@ impl Field { &self.name } + pub fn mark_as_collection_field(&mut self) { + self.collection_field = true; + } + pub fn rename_by_rule(&mut self, rule: &RenameRule) { if !self.ser_renamed { self.name.serialize = rule.apply_to_field(&self.name.serialize); @@ -987,11 +1071,15 @@ impl Field { } pub fn skip_serializing(&self) -> bool { - self.skip_serializing + self.skip_serializing || self.collection_field } pub fn skip_deserializing(&self) -> bool { - self.skip_deserializing + self.skip_deserializing || self.collection_field + } + + pub fn collection_field(&self) -> bool { + self.collection_field } pub fn skip_serializing_if(&self) -> Option<&syn::ExprPath> { From b4dbae250b74518262407d5a5aee7cbae409b2f7 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 14 Mar 2018 13:00:16 +0100 Subject: [PATCH 02/51] Added support for serialization of structs as maps --- serde_derive/src/ser.rs | 99 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 88 insertions(+), 11 deletions(-) diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 8da9e7c3..83720fea 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -245,6 +245,13 @@ fn serialize_tuple_struct( fn serialize_struct(params: &Parameters, fields: &[Field], cattrs: &attr::Container) -> Fragment { assert!(fields.len() as u64 <= u64::from(u32::MAX)); + match cattrs.repr() { + attr::ContainerRepr::Struct | attr::ContainerRepr::Auto => serialize_struct_as_struct(params, fields, cattrs), + attr::ContainerRepr::Map => serialize_struct_as_map(params, fields, cattrs), + } +} + +fn serialize_struct_as_struct(params: &Parameters, fields: &[Field], cattrs: &attr::Container) -> Fragment { let serialize_fields = serialize_struct_visitor( fields, params, @@ -279,6 +286,64 @@ fn serialize_struct(params: &Parameters, fields: &[Field], cattrs: &attr::Contai } } +fn serialize_struct_as_map(params: &Parameters, fields: &[Field], cattrs: &attr::Container) -> Fragment { + let serialize_fields = serialize_struct_visitor( + fields, + params, + false, + &StructTrait::SerializeMap, + ); + + let mut serialized_fields = fields + .iter() + .filter(|&field| !field.attrs.skip_serializing()) + .peekable(); + + let mut collect_extra = None; + if cattrs.unknown_fields_into().is_some() { + if let Some(field) = fields + .iter() + .filter(|field| field.attrs.collection_field()) + .next() + { + let ident = &field.ident; + collect_extra = Some(quote! { + for (ref __key, ref __value) in &self.#ident { + try!(_serde::ser::SerializeMap::serialize_entry( + &mut __serde_state, + __key, + __value)); + } + }); + } + } + + let let_mut = mut_if(serialized_fields.peek().is_some()); + + let len = if collect_extra.is_some() { + quote!(None) + } else { + let len = serialized_fields + .map(|field| match field.attrs.skip_serializing_if() { + None => quote!(1), + Some(path) => { + let ident = field.ident.expect("struct has unnamed fields"); + let field_expr = get_member(params, field, &Member::Named(ident)); + quote!(if #path(#field_expr) { 0 } else { 1 }) + } + }) + .fold(quote!(0), |sum, expr| quote!(#sum + #expr)); + quote!(Some(#len)) + }; + + quote_block! { + let #let_mut __serde_state = try!(_serde::Serializer::serialize_map(__serializer, #len)); + #(#serialize_fields)* + #collect_extra + _serde::ser::SerializeMap::end(__serde_state) + } +} + fn serialize_enum(params: &Parameters, variants: &[Variant], cattrs: &attr::Container) -> Fragment { assert!(variants.len() as u64 <= u64::from(u32::MAX)); @@ -885,12 +950,19 @@ fn serialize_struct_visitor( match skip { None => ser, Some(skip) => { - let skip_func = struct_trait.skip_field(span); - quote! { - if !#skip { - #ser - } else { - try!(#skip_func(&mut __serde_state, #key_expr)); + if let Some(skip_func) = struct_trait.skip_field(span) { + quote! { + if !#skip { + #ser + } else { + try!(#skip_func(&mut __serde_state, #key_expr)); + } + } + } else { + quote! { + if !#skip { + #ser + } } } } @@ -1011,6 +1083,7 @@ fn get_member(params: &Parameters, field: &Field, member: &Member) -> Tokens { } enum StructTrait { + SerializeMap, SerializeStruct, SerializeStructVariant, } @@ -1018,6 +1091,9 @@ enum StructTrait { impl StructTrait { fn serialize_field(&self, span: Span) -> Tokens { match *self { + StructTrait::SerializeMap => { + quote_spanned!(span=> _serde::ser::SerializeMap::serialize_entry) + } StructTrait::SerializeStruct => { quote_spanned!(span=> _serde::ser::SerializeStruct::serialize_field) } @@ -1027,14 +1103,15 @@ impl StructTrait { } } - fn skip_field(&self, span: Span) -> Tokens { + fn skip_field(&self, span: Span) -> Option { match *self { - StructTrait::SerializeStruct => { + StructTrait::SerializeMap => None, + StructTrait::SerializeStruct => Some({ quote_spanned!(span=> _serde::ser::SerializeStruct::skip_field) - } - StructTrait::SerializeStructVariant => { + }), + StructTrait::SerializeStructVariant => Some({ quote_spanned!(span=> _serde::ser::SerializeStructVariant::skip_field) - } + }) } } } From 39413c8ce7ec80107f6f799dc8c0ca8a93338388 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 14 Mar 2018 14:15:45 +0100 Subject: [PATCH 03/51] Implement deserializer for map mode and collection fields --- serde/src/export.rs | 1 + serde_derive/src/de.rs | 120 ++++++++++++++++++++++++++++++++--------- 2 files changed, 96 insertions(+), 25 deletions(-) diff --git a/serde/src/export.rs b/serde/src/export.rs index 772f3103..e92b647d 100644 --- a/serde/src/export.rs +++ b/serde/src/export.rs @@ -13,6 +13,7 @@ pub use lib::fmt::{self, Formatter}; pub use lib::marker::PhantomData; pub use lib::option::Option::{self, None, Some}; pub use lib::result::Result::{self, Err, Ok}; +pub use lib::iter::once; pub use self::string::from_utf8_lossy; diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 2f07d7e0..9dcb574e 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -766,6 +766,10 @@ fn deserialize_struct( untagged: &Untagged, ) -> Fragment { let is_enum = variant_ident.is_some(); + let as_map = deserializer.is_none() && !is_enum && match cattrs.repr() { + attr::ContainerRepr::Struct | attr::ContainerRepr::Auto => false, + attr::ContainerRepr::Map => true, + }; let this = ¶ms.this; let (de_impl_generics, de_ty_generics, ty_generics, where_clause) = @@ -793,10 +797,13 @@ fn deserialize_struct( let visit_seq = Stmts(deserialize_seq(&type_path, params, fields, true, cattrs)); - let (field_visitor, fields_stmt, visit_map) = - deserialize_struct_visitor(&type_path, params, fields, cattrs); + let (field_visitor, fields_stmt, visit_map) = if as_map { + deserialize_struct_as_map_visitor(&type_path, params, fields, cattrs) + } else { + deserialize_struct_as_struct_visitor(&type_path, params, fields, cattrs) + }; let field_visitor = Stmts(field_visitor); - let fields_stmt = Stmts(fields_stmt); + let fields_stmt = fields_stmt.map(Stmts); let visit_map = Stmts(visit_map); let visitor_expr = quote! { @@ -813,6 +820,10 @@ fn deserialize_struct( quote! { _serde::de::VariantAccess::struct_variant(__variant, FIELDS, #visitor_expr) } + } else if as_map { + quote! { + _serde::Deserializer::deserialize_map(__deserializer, #visitor_expr) + } } else { let type_name = cattrs.name().deserialize_name(); quote! { @@ -827,10 +838,10 @@ fn deserialize_struct( quote!(mut __seq) }; - // untagged struct variants do not get a visit_seq method + // untagged struct variants do not get a visit_seq method. The same applies to structs that + // only have a map representation. let visit_seq = match *untagged { - Untagged::Yes => None, - Untagged::No => Some(quote! { + Untagged::No if !as_map => Some(quote! { #[inline] fn visit_seq<__A>(self, #visitor_var: __A) -> _serde::export::Result where __A: _serde::de::SeqAccess<#delife> @@ -838,6 +849,7 @@ fn deserialize_struct( #visit_seq } }), + _ => None, }; quote_block! { @@ -1021,6 +1033,7 @@ fn deserialize_externally_tagged_enum( &variant_names_idents, cattrs, true, + false, )); // Match arms to extract a variant from a string @@ -1120,6 +1133,7 @@ fn deserialize_internally_tagged_enum( &variant_names_idents, cattrs, true, + false, )); // Match arms to extract a variant from a string @@ -1189,6 +1203,7 @@ fn deserialize_adjacently_tagged_enum( &variant_names_idents, cattrs, true, + false, )); let variant_arms: &Vec<_> = &variants @@ -1689,12 +1704,17 @@ fn deserialize_generated_identifier( fields: &[(String, Ident)], cattrs: &attr::Container, is_variant: bool, + struct_as_map_mode: bool, ) -> Fragment { let this = quote!(__Field); let field_idents: &Vec<_> = &fields.iter().map(|&(_, ref ident)| ident).collect(); let (ignore_variant, fallthrough) = if is_variant || cattrs.deny_unknown_fields() { (None, None) + } else if cattrs.unknown_fields_into().is_some() { + let ignore_variant = quote!(__other(String),); + let fallthrough = quote!(_serde::export::Ok(__Field::__other(__value.to_string()))); + (Some(ignore_variant), Some(fallthrough)) } else { let ignore_variant = quote!(__ignore,); let fallthrough = quote!(_serde::export::Ok(__Field::__ignore)); @@ -1706,6 +1726,7 @@ fn deserialize_generated_identifier( fields, is_variant, fallthrough, + struct_as_map_mode, )); quote_block! { @@ -1804,6 +1825,7 @@ fn deserialize_custom_identifier( &names_idents, is_variant, fallthrough, + false, )); quote_block! { @@ -1833,6 +1855,7 @@ fn deserialize_identifier( fields: &[(String, Ident)], is_variant: bool, fallthrough: Option, + struct_as_map_mode: bool, ) -> Fragment { let field_strs = fields.iter().map(|&(ref name, _)| name); let field_bytes = fields.iter().map(|&(ref name, _)| Literal::byte_string(name.as_bytes())); @@ -1852,22 +1875,26 @@ fn deserialize_identifier( let variant_indices = 0u64..; let fallthrough_msg = format!("{} index 0 <= i < {}", index_expecting, fields.len()); - let visit_index = quote! { - fn visit_u64<__E>(self, __value: u64) -> _serde::export::Result - where __E: _serde::de::Error - { - match __value { - #( - #variant_indices => _serde::export::Ok(#constructors), - )* - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &#fallthrough_msg)) + let visit_index = if struct_as_map_mode { + None + } else { + Some(quote! { + fn visit_u64<__E>(self, __value: u64) -> _serde::export::Result + where __E: _serde::de::Error + { + match __value { + #( + #variant_indices => _serde::export::Ok(#constructors), + )* + _ => _serde::export::Err(_serde::de::Error::invalid_value( + _serde::de::Unexpected::Unsigned(__value), + &#fallthrough_msg)) + } } - } + }) }; - let bytes_to_str = if fallthrough.is_some() { + let bytes_to_str = if fallthrough.is_some() && !struct_as_map_mode { None } else { let conversion = quote! { @@ -1922,12 +1949,12 @@ fn deserialize_identifier( } } -fn deserialize_struct_visitor( +fn deserialize_struct_as_struct_visitor( struct_path: &Tokens, params: &Parameters, fields: &[Field], cattrs: &attr::Container, -) -> (Fragment, Fragment, Fragment) { +) -> (Fragment, Option, Fragment) { let field_names_idents: Vec<_> = fields .iter() .enumerate() @@ -1942,11 +1969,31 @@ fn deserialize_struct_visitor( } }; - let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false); + let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, false); let visit_map = deserialize_map(struct_path, params, fields, cattrs); - (field_visitor, fields_stmt, visit_map) + (field_visitor, Some(fields_stmt), visit_map) +} + +fn deserialize_struct_as_map_visitor( + struct_path: &Tokens, + params: &Parameters, + fields: &[Field], + cattrs: &attr::Container, +) -> (Fragment, Option, Fragment) { + let field_names_idents: Vec<_> = fields + .iter() + .enumerate() + .filter(|&(_, field)| !field.attrs.skip_deserializing()) + .map(|(i, field)| (field.attrs.name().deserialize_name(), field_i(i))) + .collect(); + + let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, true); + + let visit_map = deserialize_map(struct_path, params, fields, cattrs); + + (field_visitor, None, visit_map) } fn deserialize_map( @@ -1973,6 +2020,19 @@ fn deserialize_map( } }); + // If we have a collect into, we also need a container that can + // hold the data we accumulate. + let mut let_collect = None; + for &(field, _) in fields_names.iter() { + if field.attrs.collection_field() { + let field_ty = &field.ty; + let_collect = Some(quote! { + let mut __collect: #field_ty = Default::default(); + }); + break; + } + } + // Match arms to extract a value for a field. let value_arms = fields_names .iter() @@ -2010,6 +2070,12 @@ fn deserialize_map( // Visit ignored values to consume them let ignored_arm = if cattrs.deny_unknown_fields() { None + } else if cattrs.unknown_fields_into().is_some() { + Some(quote! { + __Field::__other(__name) => { + __collect.extend(_serde::export::once((__name, try!(_serde::de::MapAccess::next_value(&mut __map))))); + } + }) } else { Some(quote! { _ => { let _ = try!(_serde::de::MapAccess::next_value::<_serde::de::IgnoredAny>(&mut __map)); } @@ -2052,7 +2118,9 @@ fn deserialize_map( let result = fields_names.iter().map(|&(field, ref name)| { let ident = field.ident.expect("struct contains unnamed fields"); - if field.attrs.skip_deserializing() { + if field.attrs.collection_field() { + quote_spanned!(Span::call_site()=> #ident: __collect) + } else if field.attrs.skip_deserializing() { let value = Expr(expr_is_missing(field, cattrs)); quote_spanned!(Span::call_site()=> #ident: #value) } else { @@ -2085,6 +2153,8 @@ fn deserialize_map( quote_block! { #(#let_values)* + #let_collect + #match_keys #let_default @@ -2115,7 +2185,7 @@ fn deserialize_struct_in_place_visitor( } }; - let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false); + let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, false); let visit_map = deserialize_map_in_place(params, fields, cattrs); From 1bd2c6129c35e44c80034089dd044ce04223be98 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 14 Mar 2018 18:46:13 +0100 Subject: [PATCH 04/51] Explicitly pass value requirements for the capture path --- serde_derive/src/de.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 9dcb574e..edc5a801 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1709,16 +1709,16 @@ fn deserialize_generated_identifier( let this = quote!(__Field); let field_idents: &Vec<_> = &fields.iter().map(|&(_, ref ident)| ident).collect(); - let (ignore_variant, fallthrough) = if is_variant || cattrs.deny_unknown_fields() { - (None, None) + let (ignore_variant, fallthrough, want_value) = if is_variant || cattrs.deny_unknown_fields() { + (None, None, false) } else if cattrs.unknown_fields_into().is_some() { let ignore_variant = quote!(__other(String),); let fallthrough = quote!(_serde::export::Ok(__Field::__other(__value.to_string()))); - (Some(ignore_variant), Some(fallthrough)) + (Some(ignore_variant), Some(fallthrough), true) } else { let ignore_variant = quote!(__ignore,); let fallthrough = quote!(_serde::export::Ok(__Field::__ignore)); - (Some(ignore_variant), Some(fallthrough)) + (Some(ignore_variant), Some(fallthrough), false) }; let visitor_impl = Stmts(deserialize_identifier( @@ -1727,6 +1727,7 @@ fn deserialize_generated_identifier( is_variant, fallthrough, struct_as_map_mode, + want_value, )); quote_block! { @@ -1826,6 +1827,7 @@ fn deserialize_custom_identifier( is_variant, fallthrough, false, + false, )); quote_block! { @@ -1856,6 +1858,7 @@ fn deserialize_identifier( is_variant: bool, fallthrough: Option, struct_as_map_mode: bool, + want_value: bool ) -> Fragment { let field_strs = fields.iter().map(|&(ref name, _)| name); let field_bytes = fields.iter().map(|&(ref name, _)| Literal::byte_string(name.as_bytes())); @@ -1894,7 +1897,7 @@ fn deserialize_identifier( }) }; - let bytes_to_str = if fallthrough.is_some() && !struct_as_map_mode { + let bytes_to_str = if fallthrough.is_some() && !want_value { None } else { let conversion = quote! { From 77b07f3fbf973c7292a391fa9eb07494baeede55 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 14 Mar 2018 18:46:47 +0100 Subject: [PATCH 05/51] Added tests for unknown_fields_into --- .../conflict/collect-into-wrong-type.rs | 19 +++++++++ .../conflict/repr-collect-struct.rs | 20 +++++++++ test_suite/tests/test_annotations.rs | 41 +++++++++++++++++++ 3 files changed, 80 insertions(+) create mode 100644 test_suite/tests/compile-fail/conflict/collect-into-wrong-type.rs create mode 100644 test_suite/tests/compile-fail/conflict/repr-collect-struct.rs diff --git a/test_suite/tests/compile-fail/conflict/collect-into-wrong-type.rs b/test_suite/tests/compile-fail/conflict/collect-into-wrong-type.rs new file mode 100644 index 00000000..4f55cb32 --- /dev/null +++ b/test_suite/tests/compile-fail/conflict/collect-into-wrong-type.rs @@ -0,0 +1,19 @@ +// Copyright 2018 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: the trait bound `&std::string::String: std::iter::Iterator` is not satisfied +#[serde(repr = "map", unknown_fields_into="other")] +struct X { + a: u32, + other: String, +} + +fn main() {} diff --git a/test_suite/tests/compile-fail/conflict/repr-collect-struct.rs b/test_suite/tests/compile-fail/conflict/repr-collect-struct.rs new file mode 100644 index 00000000..6396d32f --- /dev/null +++ b/test_suite/tests/compile-fail/conflict/repr-collect-struct.rs @@ -0,0 +1,20 @@ +// Copyright 2018 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +#[serde(repr = "struct", unknown_fields_into="other")] +//~^^ HELP: #[serde(unknown_fields_into)] requires repr="map" +struct X { + a: u32, + other: HashMap, +} + +fn main() {} diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 48a09a1a..a74ca55c 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -12,6 +12,7 @@ extern crate serde_derive; extern crate serde; +use std::collections::HashMap; use self::serde::{Deserialize, Deserializer, Serialize, Serializer}; use self::serde::de::{self, Unexpected}; @@ -94,6 +95,14 @@ where a5: E, } +#[derive(Debug, PartialEq, Serialize, Deserialize)] +#[serde(repr="map", unknown_fields_into="extra")] +struct CollectOther { + a: u32, + b: u32, + extra: HashMap, +} + #[test] fn test_default_struct() { assert_de_tokens( @@ -1267,3 +1276,35 @@ fn test_from_into_traits() { assert_ser_tokens::(&StructFromEnum(None), &[Token::None]); assert_de_tokens::(&StructFromEnum(Some(2)), &[Token::Some, Token::U32(2)]); } + +#[test] +fn test_collect_other() { + let mut extra = HashMap::new(); + extra.insert("c".into(), 3); + assert_de_tokens( + &CollectOther { a: 1, b: 2, extra: extra.clone() }, + &[ + Token::Map { len: None }, + Token::Str("a"), + Token::U32(1), + Token::Str("b"), + Token::U32(2), + Token::Str("c"), + Token::U32(3), + Token::MapEnd, + ], + ); + assert_ser_tokens( + &CollectOther { a: 1, b: 2, extra: extra }, + &[ + Token::Map { len: None }, + Token::Str("a"), + Token::U32(1), + Token::Str("b"), + Token::U32(2), + Token::Str("c"), + Token::U32(3), + Token::MapEnd, + ], + ); +} From 07d07347b31a95cf67b37b3cbb33301dc9593165 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 14 Mar 2018 21:16:48 +0100 Subject: [PATCH 06/51] Make clippy happy --- serde_derive/src/de.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index edc5a801..606b0f4e 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -2122,7 +2122,7 @@ fn deserialize_map( let result = fields_names.iter().map(|&(field, ref name)| { let ident = field.ident.expect("struct contains unnamed fields"); if field.attrs.collection_field() { - quote_spanned!(Span::call_site()=> #ident: __collect) + quote_spanned!(Span::def_site()=> #ident: __collect) } else if field.attrs.skip_deserializing() { let value = Expr(expr_is_missing(field, cattrs)); quote_spanned!(Span::call_site()=> #ident: #value) From 583c0d8d1465d76a483e0069e8fb8a4c2aee4678 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 14 Mar 2018 21:17:02 +0100 Subject: [PATCH 07/51] Make proc-macro2/nightly happy --- test_suite/tests/test_annotations.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index a74ca55c..3686041a 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -1295,7 +1295,7 @@ fn test_collect_other() { ], ); assert_ser_tokens( - &CollectOther { a: 1, b: 2, extra: extra }, + &CollectOther { a: 1, b: 2, extra }, &[ Token::Map { len: None }, Token::Str("a"), From 299cd2dbd094ff8e1f32f6b5c2e41cbf8715d929 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Thu, 15 Mar 2018 14:16:54 +0100 Subject: [PATCH 08/51] Replace unknown_fields_into with serde(flatten) --- serde_derive_internals/src/ast.rs | 15 ++++---- serde_derive_internals/src/attr.rs | 55 ++++++++---------------------- 2 files changed, 21 insertions(+), 49 deletions(-) diff --git a/serde_derive_internals/src/ast.rs b/serde_derive_internals/src/ast.rs index 74fc56fe..fcc7b924 100644 --- a/serde_derive_internals/src/ast.rs +++ b/serde_derive_internals/src/ast.rs @@ -63,7 +63,7 @@ impl<'a> Container<'a> { } }; - let mut have_collection_field = false; + let mut has_flatten = false; match data { Data::Enum(ref mut variants) => for variant in variants { variant.attrs.rename_by_rule(attrs.rename_all()); @@ -72,18 +72,17 @@ impl<'a> Container<'a> { } }, Data::Struct(_, ref mut fields) => for field in fields { - if field.ident.is_some() && field.ident.as_ref() == attrs.unknown_fields_into() { - field.attrs.mark_as_collection_field(); - have_collection_field = true; + if field.attrs.flatten() { + has_flatten = true; } field.attrs.rename_by_rule(attrs.rename_all()); }, } - if attrs.unknown_fields_into().is_some() && !have_collection_field { - cx.error(format!("#[serde(unknown_fields_into)] was defined but target \ - field `{}` does not exist", - attrs.unknown_fields_into().unwrap())); + if has_flatten && attrs.repr() != attr::ContainerRepr::Map { + cx.error(format!("#[serde(flatten)] requires \ + #[serde(repr = \"map\")] on the container, but \ + found #[serde(repr = \"{}\")]", attrs.repr())); } let item = Container { diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs index 9d622386..ad291fc4 100644 --- a/serde_derive_internals/src/attr.rs +++ b/serde_derive_internals/src/attr.rs @@ -68,10 +68,6 @@ impl<'c, T> Attr<'c, T> { fn get(self) -> Option { self.value } - - fn is_set(&self) -> bool { - self.value.is_some() - } } struct BoolAttr<'c>(Attr<'c, ()>); @@ -150,7 +146,6 @@ pub struct Container { remote: Option, identifier: Identifier, repr: ContainerRepr, - unknown_fields_into: Option, } /// Styles of representing an enum. @@ -229,7 +224,6 @@ impl Container { let mut field_identifier = BoolAttr::none(cx, "field_identifier"); let mut variant_identifier = BoolAttr::none(cx, "variant_identifier"); let mut repr = Attr::none(cx, "repr"); - let mut unknown_fields_into = Attr::none(cx, "unknown_fields_into"); for meta_items in item.attrs.iter().filter_map(get_serde_meta_items) { for meta_item in meta_items { @@ -267,21 +261,6 @@ impl Container { // Parse `#[serde(deny_unknown_fields)]` Meta(Word(word)) if word == "deny_unknown_fields" => { deny_unknown_fields.set_true(); - if unknown_fields_into.is_set() { - cx.error("#[serde(deny_unknown_fields)] cannot be combined \ - with #[serde(unknown_fields_into)]"); - } - } - - // Parse `#[serde(unknown_fields_into = "foo")]` - Meta(NameValue(ref m)) if m.ident == "unknown_fields_into" => { - if let Ok(s) = get_lit_str(cx, m.ident.as_ref(), m.ident.as_ref(), &m.lit) { - unknown_fields_into.set(Ident::new(&s.value(), Span::call_site()).into()); - if deny_unknown_fields.get() { - cx.error("#[serde(deny_unknown_fields)] cannot be combined \ - with #[serde(unknown_fields_into)]"); - } - } } // Parse `#[serde(repr = "foo")]` @@ -422,10 +401,6 @@ impl Container { } } - if unknown_fields_into.get().is_some() && repr.get() != Some(ContainerRepr::Map) { - cx.error("#[serde(unknown_fields_into)] requires repr=\"map\""); - } - Container { name: Name { serialize: ser_name.get().unwrap_or_else(|| item.ident.to_string()), @@ -442,7 +417,6 @@ impl Container { remote: remote.get(), identifier: decide_identifier(cx, item, &field_identifier, &variant_identifier), repr: repr.get().unwrap_or(ContainerRepr::Auto), - unknown_fields_into: unknown_fields_into.get(), } } @@ -493,10 +467,6 @@ impl Container { pub fn repr(&self) -> ContainerRepr { self.repr } - - pub fn unknown_fields_into(&self) -> Option<&syn::Ident> { - self.unknown_fields_into.as_ref() - } } fn decide_tag( @@ -778,6 +748,7 @@ pub struct Field { borrowed_lifetimes: BTreeSet, getter: Option, collection_field: bool, + flatten: bool, } /// Represents the default to use for a field when deserializing. @@ -811,6 +782,7 @@ impl Field { let mut de_bound = Attr::none(cx, "bound"); let mut borrowed_lifetimes = Attr::none(cx, "borrow"); let mut getter = Attr::none(cx, "getter"); + let mut flatten = BoolAttr::none(cx, "flatten"); let ident = match field.ident { Some(ref ident) => ident.to_string(), @@ -957,6 +929,11 @@ impl Field { } } + // Parse `#[serde(skip_deserializing)]` + Meta(Word(word)) if word == "flatten" => { + flatten.set_true(); + } + Meta(ref meta_item) => { cx.error(format!( "unknown serde field attribute `{}`", @@ -1049,7 +1026,7 @@ impl Field { de_bound: de_bound.get(), borrowed_lifetimes: borrowed_lifetimes, getter: getter.get(), - collection_field: false, + flatten: flatten.get(), } } @@ -1057,10 +1034,6 @@ impl Field { &self.name } - pub fn mark_as_collection_field(&mut self) { - self.collection_field = true; - } - pub fn rename_by_rule(&mut self, rule: &RenameRule) { if !self.ser_renamed { self.name.serialize = rule.apply_to_field(&self.name.serialize); @@ -1071,15 +1044,11 @@ impl Field { } pub fn skip_serializing(&self) -> bool { - self.skip_serializing || self.collection_field + self.skip_serializing } pub fn skip_deserializing(&self) -> bool { - self.skip_deserializing || self.collection_field - } - - pub fn collection_field(&self) -> bool { - self.collection_field + self.skip_deserializing || self.flatten } pub fn skip_serializing_if(&self) -> Option<&syn::ExprPath> { @@ -1113,6 +1082,10 @@ impl Field { pub fn getter(&self) -> Option<&syn::ExprPath> { self.getter.as_ref() } + + pub fn flatten(&self) -> bool { + self.flatten + } } type SerAndDe = (Option, Option); From 571bb8caedcb82de75448dbcc955456e845da62f Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Thu, 15 Mar 2018 14:17:48 +0100 Subject: [PATCH 09/51] Derive serialization for serde(flatten) --- serde_derive/src/ser.rs | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 83720fea..53d2ebaf 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -286,7 +286,7 @@ fn serialize_struct_as_struct(params: &Parameters, fields: &[Field], cattrs: &at } } -fn serialize_struct_as_map(params: &Parameters, fields: &[Field], cattrs: &attr::Container) -> Fragment { +fn serialize_struct_as_map(params: &Parameters, fields: &[Field], _cattrs: &attr::Container) -> Fragment { let serialize_fields = serialize_struct_visitor( fields, params, @@ -299,28 +299,13 @@ fn serialize_struct_as_map(params: &Parameters, fields: &[Field], cattrs: &attr: .filter(|&field| !field.attrs.skip_serializing()) .peekable(); - let mut collect_extra = None; - if cattrs.unknown_fields_into().is_some() { - if let Some(field) = fields - .iter() - .filter(|field| field.attrs.collection_field()) - .next() - { - let ident = &field.ident; - collect_extra = Some(quote! { - for (ref __key, ref __value) in &self.#ident { - try!(_serde::ser::SerializeMap::serialize_entry( - &mut __serde_state, - __key, - __value)); - } - }); - } - } - let let_mut = mut_if(serialized_fields.peek().is_some()); - let len = if collect_extra.is_some() { + let has_flatten = fields + .iter() + .any(|field| field.attrs.flatten()); + + let len = if has_flatten { quote!(None) } else { let len = serialized_fields @@ -339,7 +324,6 @@ fn serialize_struct_as_map(params: &Parameters, fields: &[Field], cattrs: &attr: quote_block! { let #let_mut __serde_state = try!(_serde::Serializer::serialize_map(__serializer, #len)); #(#serialize_fields)* - #collect_extra _serde::ser::SerializeMap::end(__serde_state) } } @@ -924,6 +908,7 @@ fn serialize_struct_visitor( .filter(|&field| !field.attrs.skip_serializing()) .map(|field| { let field_ident = field.ident.expect("struct has unnamed field"); + let mut field_expr = if is_enum { quote!(#field_ident) } else { @@ -942,9 +927,15 @@ fn serialize_struct_visitor( } let span = Span::def_site().located_at(field.original.span()); - let func = struct_trait.serialize_field(span); - let ser = quote! { - try!(#func(&mut __serde_state, #key_expr, #field_expr)); + let ser = if field.attrs.flatten() { + quote! { + try!((#field_expr).serialize(_serde::private::ser::FlatSerializer(&mut __serde_state))); + } + } else { + let func = struct_trait.serialize_field(span); + quote! { + try!(#func(&mut __serde_state, #key_expr, #field_expr)); + } }; match skip { From 5ae06bba49ab2760a9d73f157c0a37d1ec31fe78 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Thu, 15 Mar 2018 14:30:38 +0100 Subject: [PATCH 10/51] Store flatten flag in container attributes --- serde_derive/src/ser.rs | 8 ++------ serde_derive_internals/src/ast.rs | 6 +++++- serde_derive_internals/src/attr.rs | 10 ++++++++++ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 53d2ebaf..eb938816 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -286,7 +286,7 @@ fn serialize_struct_as_struct(params: &Parameters, fields: &[Field], cattrs: &at } } -fn serialize_struct_as_map(params: &Parameters, fields: &[Field], _cattrs: &attr::Container) -> Fragment { +fn serialize_struct_as_map(params: &Parameters, fields: &[Field], cattrs: &attr::Container) -> Fragment { let serialize_fields = serialize_struct_visitor( fields, params, @@ -301,11 +301,7 @@ fn serialize_struct_as_map(params: &Parameters, fields: &[Field], _cattrs: &attr let let_mut = mut_if(serialized_fields.peek().is_some()); - let has_flatten = fields - .iter() - .any(|field| field.attrs.flatten()); - - let len = if has_flatten { + let len = if cattrs.has_flatten() { quote!(None) } else { let len = serialized_fields diff --git a/serde_derive_internals/src/ast.rs b/serde_derive_internals/src/ast.rs index fcc7b924..916f647a 100644 --- a/serde_derive_internals/src/ast.rs +++ b/serde_derive_internals/src/ast.rs @@ -48,7 +48,7 @@ pub enum Style { impl<'a> Container<'a> { pub fn from_ast(cx: &Ctxt, item: &'a syn::DeriveInput) -> Container<'a> { - let attrs = attr::Container::from_ast(cx, item); + let mut attrs = attr::Container::from_ast(cx, item); let mut data = match item.data { syn::Data::Enum(ref data) => { @@ -85,6 +85,10 @@ impl<'a> Container<'a> { found #[serde(repr = \"{}\")]", attrs.repr())); } + if has_flatten { + attrs.mark_has_flatten(); + } + let item = Container { ident: item.ident, attrs: attrs, diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs index ad291fc4..b62e4748 100644 --- a/serde_derive_internals/src/attr.rs +++ b/serde_derive_internals/src/attr.rs @@ -146,6 +146,7 @@ pub struct Container { remote: Option, identifier: Identifier, repr: ContainerRepr, + has_flatten: bool, } /// Styles of representing an enum. @@ -417,6 +418,7 @@ impl Container { remote: remote.get(), identifier: decide_identifier(cx, item, &field_identifier, &variant_identifier), repr: repr.get().unwrap_or(ContainerRepr::Auto), + has_flatten: false, } } @@ -467,6 +469,14 @@ impl Container { pub fn repr(&self) -> ContainerRepr { self.repr } + + pub fn has_flatten(&self) -> bool { + self.has_flatten + } + + pub fn mark_has_flatten(&mut self) { + self.has_flatten = true; + } } fn decide_tag( From 6627540dd63f21800b7a0bc752d32b38f41c4c11 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Thu, 15 Mar 2018 15:49:40 +0100 Subject: [PATCH 11/51] Added support basic deserialization in derive --- serde/src/de/mod.rs | 7 +++++ serde/src/export.rs | 1 - serde_derive/src/de.rs | 58 ++++++++++++++++++++++++++++------------- serde_derive/src/ser.rs | 2 +- 4 files changed, 48 insertions(+), 20 deletions(-) diff --git a/serde/src/de/mod.rs b/serde/src/de/mod.rs index cc2c6502..162d4ad4 100644 --- a/serde/src/de/mod.rs +++ b/serde/src/de/mod.rs @@ -255,6 +255,13 @@ macro_rules! declare_error_trait { } } + /// Raised when a `Deserialize` struct type recieved a field with an + /// unrecognized name but the names are not actually known because of + /// flattening. + fn unknow_field_in_flattened_structure(field: &str) -> Self { + Error::custom(format_args!("unknown field `{}`", field)) + } + /// Raised when a `Deserialize` struct type expected to receive a required /// field with a particular name but that field was not present in the /// input. diff --git a/serde/src/export.rs b/serde/src/export.rs index e92b647d..772f3103 100644 --- a/serde/src/export.rs +++ b/serde/src/export.rs @@ -13,7 +13,6 @@ pub use lib::fmt::{self, Formatter}; pub use lib::marker::PhantomData; pub use lib::option::Option::{self, None, Some}; pub use lib::result::Result::{self, Err, Ok}; -pub use lib::iter::once; pub use self::string::from_utf8_lossy; diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 606b0f4e..7ced726a 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1711,7 +1711,7 @@ fn deserialize_generated_identifier( let (ignore_variant, fallthrough, want_value) = if is_variant || cattrs.deny_unknown_fields() { (None, None, false) - } else if cattrs.unknown_fields_into().is_some() { + } else if cattrs.has_flatten() { let ignore_variant = quote!(__other(String),); let fallthrough = quote!(_serde::export::Ok(__Field::__other(__value.to_string()))); (Some(ignore_variant), Some(fallthrough), true) @@ -2023,18 +2023,14 @@ fn deserialize_map( } }); - // If we have a collect into, we also need a container that can - // hold the data we accumulate. - let mut let_collect = None; - for &(field, _) in fields_names.iter() { - if field.attrs.collection_field() { - let field_ty = &field.ty; - let_collect = Some(quote! { - let mut __collect: #field_ty = Default::default(); - }); - break; - } - } + // Collect contents for flatten fields into a buffer + let let_collect = if cattrs.has_flatten() { + Some(quote! { + let mut __collect = Vec::<_serde::private::de::Content>::new(); + }) + } else { + None + }; // Match arms to extract a value for a field. let value_arms = fields_names @@ -2073,10 +2069,10 @@ fn deserialize_map( // Visit ignored values to consume them let ignored_arm = if cattrs.deny_unknown_fields() { None - } else if cattrs.unknown_fields_into().is_some() { + } else if cattrs.has_flatten() { Some(quote! { __Field::__other(__name) => { - __collect.extend(_serde::export::once((__name, try!(_serde::de::MapAccess::next_value(&mut __map))))); + __collect.push((__name, try!(_serde::de::MapAccess::next_value(&mut __map)))); } }) } else { @@ -2119,11 +2115,33 @@ fn deserialize_map( } }); + let extract_collected = fields_names + .iter() + .filter(|&&(field, _)| field.attrs.flatten()) + .map(|&(field, ref name)| { + let field_ty = field.ty; + quote! { + let #name: #field_ty = try!(_serde::de::Deserialize::deserialize( + _serde::private::de::FlatMapDeserializer::new( + &mut __collect, + _serde::export::PhantomData))); + } + }); + + let collected_deny_unknown_fields = if cattrs.deny_unknown_fields() { + Some(quote! { + if let Some((__key, _)) = __collect.into_iter().next() { + return _serde::export::Err( + _serde::de::Error::unknown_field_in_flattened_structure(__key)); + } + }) + } else { + None + }; + let result = fields_names.iter().map(|&(field, ref name)| { let ident = field.ident.expect("struct contains unnamed fields"); - if field.attrs.collection_field() { - quote_spanned!(Span::def_site()=> #ident: __collect) - } else if field.attrs.skip_deserializing() { + if field.attrs.skip_deserializing() && !field.attrs.flatten() { let value = Expr(expr_is_missing(field, cattrs)); quote_spanned!(Span::call_site()=> #ident: #value) } else { @@ -2164,6 +2182,10 @@ fn deserialize_map( #(#extract_values)* + #(#extract_collected)* + + #collected_deny_unknown_fields + _serde::export::Ok(#result) } } diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index eb938816..20d22ff6 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -925,7 +925,7 @@ fn serialize_struct_visitor( let span = Span::def_site().located_at(field.original.span()); let ser = if field.attrs.flatten() { quote! { - try!((#field_expr).serialize(_serde::private::ser::FlatSerializer(&mut __serde_state))); + try!((#field_expr).serialize(_serde::private::ser::FlatSerializer::new(&mut __serde_state))); } } else { let func = struct_trait.serialize_field(span); From 5457394f5be22d148ee42fe9982b7967c8468dcc Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Thu, 15 Mar 2018 16:17:54 +0100 Subject: [PATCH 12/51] Fixed various issues with combinding flatten and deny_unknown_fields --- serde/src/de/mod.rs | 2 +- serde_derive/src/de.rs | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/serde/src/de/mod.rs b/serde/src/de/mod.rs index 162d4ad4..f391a858 100644 --- a/serde/src/de/mod.rs +++ b/serde/src/de/mod.rs @@ -258,7 +258,7 @@ macro_rules! declare_error_trait { /// Raised when a `Deserialize` struct type recieved a field with an /// unrecognized name but the names are not actually known because of /// flattening. - fn unknow_field_in_flattened_structure(field: &str) -> Self { + fn unknown_field_in_flattened_structure(field: &str) -> Self { Error::custom(format_args!("unknown field `{}`", field)) } diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 7ced726a..ec2506d4 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1709,12 +1709,12 @@ fn deserialize_generated_identifier( let this = quote!(__Field); let field_idents: &Vec<_> = &fields.iter().map(|&(_, ref ident)| ident).collect(); - let (ignore_variant, fallthrough, want_value) = if is_variant || cattrs.deny_unknown_fields() { - (None, None, false) - } else if cattrs.has_flatten() { + let (ignore_variant, fallthrough, want_value) = if cattrs.has_flatten() { let ignore_variant = quote!(__other(String),); let fallthrough = quote!(_serde::export::Ok(__Field::__other(__value.to_string()))); (Some(ignore_variant), Some(fallthrough), true) + } else if is_variant || cattrs.deny_unknown_fields() { + (None, None, false) } else { let ignore_variant = quote!(__ignore,); let fallthrough = quote!(_serde::export::Ok(__Field::__ignore)); @@ -2026,7 +2026,7 @@ fn deserialize_map( // Collect contents for flatten fields into a buffer let let_collect = if cattrs.has_flatten() { Some(quote! { - let mut __collect = Vec::<_serde::private::de::Content>::new(); + let mut __collect = Vec::<(String, _serde::private::de::Content)>::new(); }) } else { None @@ -2067,14 +2067,14 @@ fn deserialize_map( }); // Visit ignored values to consume them - let ignored_arm = if cattrs.deny_unknown_fields() { - None - } else if cattrs.has_flatten() { + let ignored_arm = if cattrs.has_flatten() { Some(quote! { __Field::__other(__name) => { __collect.push((__name, try!(_serde::de::MapAccess::next_value(&mut __map)))); } }) + } else if cattrs.deny_unknown_fields() { + None } else { Some(quote! { _ => { let _ = try!(_serde::de::MapAccess::next_value::<_serde::de::IgnoredAny>(&mut __map)); } @@ -2128,11 +2128,11 @@ fn deserialize_map( } }); - let collected_deny_unknown_fields = if cattrs.deny_unknown_fields() { + let collected_deny_unknown_fields = if cattrs.has_flatten() && cattrs.deny_unknown_fields() { Some(quote! { if let Some((__key, _)) = __collect.into_iter().next() { return _serde::export::Err( - _serde::de::Error::unknown_field_in_flattened_structure(__key)); + _serde::de::Error::unknown_field_in_flattened_structure(&__key)); } }) } else { From 9e8cda4c3773dd1a7878a7af82533c0f441d8511 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 15 Mar 2018 21:11:35 +0100 Subject: [PATCH 13/51] Added basic not fully working FlatMapSerializer --- serde/src/private/ser.rs | 245 ++++++++++++++++++++++++++++++++++++++- serde_derive/src/de.rs | 2 +- serde_derive/src/ser.rs | 2 +- 3 files changed, 245 insertions(+), 4 deletions(-) diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index 932ebf4a..5b277852 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -58,10 +58,11 @@ enum Unsupported { ByteArray, Optional, Unit, + UnitStruct, + NewtypeStruct, Sequence, Tuple, TupleStruct, - #[cfg(not(any(feature = "std", feature = "alloc")))] Enum, } @@ -76,10 +77,11 @@ impl Display for Unsupported { Unsupported::ByteArray => formatter.write_str("a byte array"), Unsupported::Optional => formatter.write_str("an optional"), Unsupported::Unit => formatter.write_str("unit"), + Unsupported::UnitStruct => formatter.write_str("unit struct"), + Unsupported::NewtypeStruct => formatter.write_str("newtype struct"), Unsupported::Sequence => formatter.write_str("a sequence"), Unsupported::Tuple => formatter.write_str("a tuple"), Unsupported::TupleStruct => formatter.write_str("a tuple struct"), - #[cfg(not(any(feature = "std", feature = "alloc")))] Unsupported::Enum => formatter.write_str("an enum"), } } @@ -1028,3 +1030,242 @@ mod content { } } } + +#[cfg(any(feature = "std", feature = "alloc"))] +pub struct FlatMapSerializer(M); + +#[cfg(any(feature = "std", feature = "alloc"))] +impl FlatMapSerializer +where + M: SerializeMap +{ + fn bad_type(self, what: Unsupported) -> M::Error { + ser::Error::custom(format_args!( + "cannot flatten serialize {} values", what + )) + } +} + +#[cfg(any(feature = "std", feature = "alloc"))] +impl Serializer for FlatMapSerializer + where M: SerializeMap +{ + type Ok = M::Ok; + type Error = M::Error; + + type SerializeSeq = Impossible; + type SerializeTuple = Impossible; + type SerializeTupleStruct = Impossible; + type SerializeMap = FlatMapSerializeMap; + type SerializeStruct = FlatMapSerializeStruct; + type SerializeTupleVariant = Impossible; + type SerializeStructVariant = Impossible; + + fn serialize_bool(self, _: bool) -> Result { + Err(self.bad_type(Unsupported::Boolean)) + } + + fn serialize_i8(self, _: i8) -> Result { + Err(self.bad_type(Unsupported::Integer)) + } + + fn serialize_i16(self, _: i16) -> Result { + Err(self.bad_type(Unsupported::Integer)) + } + + fn serialize_i32(self, _: i32) -> Result { + Err(self.bad_type(Unsupported::Integer)) + } + + fn serialize_i64(self, _: i64) -> Result { + Err(self.bad_type(Unsupported::Integer)) + } + + fn serialize_u8(self, _: u8) -> Result { + Err(self.bad_type(Unsupported::Integer)) + } + + fn serialize_u16(self, _: u16) -> Result { + Err(self.bad_type(Unsupported::Integer)) + } + + fn serialize_u32(self, _: u32) -> Result { + Err(self.bad_type(Unsupported::Integer)) + } + + fn serialize_u64(self, _: u64) -> Result { + Err(self.bad_type(Unsupported::Integer)) + } + + fn serialize_f32(self, _: f32) -> Result { + Err(self.bad_type(Unsupported::Float)) + } + + fn serialize_f64(self, _: f64) -> Result { + Err(self.bad_type(Unsupported::Float)) + } + + fn serialize_char(self, _: char) -> Result { + Err(self.bad_type(Unsupported::Char)) + } + + fn serialize_str(self, _: &str) -> Result { + Err(self.bad_type(Unsupported::String)) + } + + fn serialize_bytes(self, _: &[u8]) -> Result { + Err(self.bad_type(Unsupported::ByteArray)) + } + + fn serialize_none(self) -> Result { + Err(self.bad_type(Unsupported::Optional)) + } + + fn serialize_some(self, _: &T) -> Result + where + T: Serialize, + { + Err(self.bad_type(Unsupported::Optional)) + } + + fn serialize_unit(self) -> Result { + Err(self.bad_type(Unsupported::Unit)) + } + + fn serialize_unit_struct(self, _: &'static str) -> Result { + Err(self.bad_type(Unsupported::UnitStruct)) + } + + fn serialize_unit_variant( + self, + _: &'static str, + _: u32, + _: &'static str, + ) -> Result { + Err(self.bad_type(Unsupported::Enum)) + } + + fn serialize_newtype_struct( + self, + _: &'static str, + _value: &T, + ) -> Result + where + T: Serialize, + { + // TODO: can we do better here? + Err(self.bad_type(Unsupported::NewtypeStruct)) + } + + fn serialize_newtype_variant( + self, + _: &'static str, + _: u32, + _: &'static str, + _: &T, + ) -> Result + where + T: Serialize, + { + Err(self.bad_type(Unsupported::Enum)) + } + + fn serialize_seq(self, _: Option) -> Result { + Err(self.bad_type(Unsupported::Sequence)) + } + + fn serialize_tuple(self, _: usize) -> Result { + Err(self.bad_type(Unsupported::Tuple)) + } + + fn serialize_tuple_struct( + self, + _: &'static str, + _: usize, + ) -> Result { + Err(self.bad_type(Unsupported::TupleStruct)) + } + + fn serialize_tuple_variant( + self, + _: &'static str, + _: u32, + _: &'static str, + _: usize, + ) -> Result { + Err(self.bad_type(Unsupported::Enum)) + } + + fn serialize_map(self, _: Option) -> Result { + Ok(FlatMapSerializeMap(self.0)) + } + + fn serialize_struct( + self, + _: &'static str, + _: usize, + ) -> Result { + Ok(FlatMapSerializeStruct(self.0)) + } + + fn serialize_struct_variant( + self, + _: &'static str, + _: u32, + _: &'static str, + _: usize, + ) -> Result { + Err(self.bad_type(Unsupported::Enum)) + } +} + +#[cfg(any(feature = "std", feature = "alloc"))] +pub struct FlatMapSerializeMap(M); + +#[cfg(any(feature = "std", feature = "alloc"))] +impl ser::SerializeMap for FlatMapSerializeMap + where M: SerializeMap +{ + type Ok = M::Ok; + type Error = M::Error; + + fn serialize_key(&mut self, key: &T) -> Result<(), Self::Error> + where + T: Serialize, + { + self.0.serialize_key(key) + } + + fn serialize_value(&mut self, value: &T) -> Result<(), Self::Error> + where + T: Serialize, + { + self.0.serialize_value(value) + } + + fn end(self) -> Result { + self.0.end() + } +} + +#[cfg(any(feature = "std", feature = "alloc"))] +pub struct FlatMapSerializeStruct(M); + +#[cfg(any(feature = "std", feature = "alloc"))] +impl ser::SerializeStruct for FlatMapSerializeStruct + where M: SerializeMap +{ + type Ok = M::Ok; + type Error = M::Error; + + fn serialize_field(&mut self, key: &'static str, value: &T) -> Result<(), Self::Error> + where + T: Serialize, + { + self.0.serialize_entry(key, value) + } + + fn end(self) -> Result { + self.0.end() + } +} diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index ec2506d4..01ea6bfe 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -2122,7 +2122,7 @@ fn deserialize_map( let field_ty = field.ty; quote! { let #name: #field_ty = try!(_serde::de::Deserialize::deserialize( - _serde::private::de::FlatMapDeserializer::new( + _serde::private::de::FlatMapDeserializer( &mut __collect, _serde::export::PhantomData))); } diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 20d22ff6..9211b144 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -925,7 +925,7 @@ fn serialize_struct_visitor( let span = Span::def_site().located_at(field.original.span()); let ser = if field.attrs.flatten() { quote! { - try!((#field_expr).serialize(_serde::private::ser::FlatSerializer::new(&mut __serde_state))); + try!((#field_expr).serialize(_serde::private::ser::FlatMapSerializer(&mut __serde_state))); } } else { let func = struct_trait.serialize_field(span); From b69292332146ef5f30b802ccefeb522aebb2501e Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 15 Mar 2018 21:37:52 +0100 Subject: [PATCH 14/51] Non working changes to the flatten serializer --- serde/src/private/ser.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index 5b277852..043bfb63 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -1032,12 +1032,12 @@ mod content { } #[cfg(any(feature = "std", feature = "alloc"))] -pub struct FlatMapSerializer(M); +pub struct FlatMapSerializer<'a, M: 'a>(pub &'a mut M); #[cfg(any(feature = "std", feature = "alloc"))] -impl FlatMapSerializer +impl<'a, M> FlatMapSerializer<'a, M> where - M: SerializeMap + M: SerializeMap + 'a { fn bad_type(self, what: Unsupported) -> M::Error { ser::Error::custom(format_args!( @@ -1047,8 +1047,8 @@ where } #[cfg(any(feature = "std", feature = "alloc"))] -impl Serializer for FlatMapSerializer - where M: SerializeMap +impl<'a, M> Serializer for FlatMapSerializer<'a, M> + where M: SerializeMap + 'a { type Ok = M::Ok; type Error = M::Error; @@ -1056,8 +1056,8 @@ impl Serializer for FlatMapSerializer type SerializeSeq = Impossible; type SerializeTuple = Impossible; type SerializeTupleStruct = Impossible; - type SerializeMap = FlatMapSerializeMap; - type SerializeStruct = FlatMapSerializeStruct; + type SerializeMap = FlatMapSerializeMap<'a, M>; + type SerializeStruct = FlatMapSerializeStruct<'a, M>; type SerializeTupleVariant = Impossible; type SerializeStructVariant = Impossible; @@ -1220,11 +1220,11 @@ impl Serializer for FlatMapSerializer } #[cfg(any(feature = "std", feature = "alloc"))] -pub struct FlatMapSerializeMap(M); +pub struct FlatMapSerializeMap<'a, M: 'a>(&'a mut M); #[cfg(any(feature = "std", feature = "alloc"))] -impl ser::SerializeMap for FlatMapSerializeMap - where M: SerializeMap +impl<'a, M> ser::SerializeMap for FlatMapSerializeMap<'a, M> + where M: SerializeMap + 'a { type Ok = M::Ok; type Error = M::Error; @@ -1249,11 +1249,11 @@ impl ser::SerializeMap for FlatMapSerializeMap } #[cfg(any(feature = "std", feature = "alloc"))] -pub struct FlatMapSerializeStruct(M); +pub struct FlatMapSerializeStruct<'a, M: 'a>(&'a mut M); #[cfg(any(feature = "std", feature = "alloc"))] -impl ser::SerializeStruct for FlatMapSerializeStruct - where M: SerializeMap +impl<'a, M> ser::SerializeStruct for FlatMapSerializeStruct<'a, M> + where M: SerializeMap + 'a { type Ok = M::Ok; type Error = M::Error; From 112dfd7428601ee75e203c177b1adb0b29c52d57 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 15 Mar 2018 22:21:11 +0100 Subject: [PATCH 15/51] Correctly suppress the end() call for flattened serialization --- serde/src/private/ser.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index 043bfb63..f19c4063 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -1050,16 +1050,16 @@ where impl<'a, M> Serializer for FlatMapSerializer<'a, M> where M: SerializeMap + 'a { - type Ok = M::Ok; + type Ok = (); type Error = M::Error; - type SerializeSeq = Impossible; - type SerializeTuple = Impossible; - type SerializeTupleStruct = Impossible; + type SerializeSeq = Impossible; + type SerializeTuple = Impossible; + type SerializeTupleStruct = Impossible; type SerializeMap = FlatMapSerializeMap<'a, M>; type SerializeStruct = FlatMapSerializeStruct<'a, M>; - type SerializeTupleVariant = Impossible; - type SerializeStructVariant = Impossible; + type SerializeTupleVariant = Impossible; + type SerializeStructVariant = Impossible; fn serialize_bool(self, _: bool) -> Result { Err(self.bad_type(Unsupported::Boolean)) @@ -1226,7 +1226,7 @@ pub struct FlatMapSerializeMap<'a, M: 'a>(&'a mut M); impl<'a, M> ser::SerializeMap for FlatMapSerializeMap<'a, M> where M: SerializeMap + 'a { - type Ok = M::Ok; + type Ok = (); type Error = M::Error; fn serialize_key(&mut self, key: &T) -> Result<(), Self::Error> @@ -1243,8 +1243,8 @@ impl<'a, M> ser::SerializeMap for FlatMapSerializeMap<'a, M> self.0.serialize_value(value) } - fn end(self) -> Result { - self.0.end() + fn end(self) -> Result<(), Self::Error> { + Ok(()) } } @@ -1255,7 +1255,7 @@ pub struct FlatMapSerializeStruct<'a, M: 'a>(&'a mut M); impl<'a, M> ser::SerializeStruct for FlatMapSerializeStruct<'a, M> where M: SerializeMap + 'a { - type Ok = M::Ok; + type Ok = (); type Error = M::Error; fn serialize_field(&mut self, key: &'static str, value: &T) -> Result<(), Self::Error> @@ -1265,7 +1265,7 @@ impl<'a, M> ser::SerializeStruct for FlatMapSerializeStruct<'a, M> self.0.serialize_entry(key, value) } - fn end(self) -> Result { - self.0.end() + fn end(self) -> Result<(), Self::Error> { + Ok(()) } } From ebf80ac9650561d6a87ad5d5646971469797e240 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 16 Mar 2018 01:23:57 +0100 Subject: [PATCH 16/51] Implement deserialization support for flatten --- serde/src/private/de.rs | 116 +++++++++++++++++++++++++++++++++++++++- serde_derive/src/de.rs | 6 +-- 2 files changed, 118 insertions(+), 4 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index a3a55d8e..74eda4da 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -8,7 +8,7 @@ use lib::*; -use de::{Deserialize, DeserializeSeed, Deserializer, Error, IntoDeserializer, Visitor}; +use de::{Deserialize, DeserializeSeed, Deserializer, Error, IntoDeserializer, Visitor, MapAccess}; #[cfg(any(feature = "std", feature = "alloc"))] use de::Unexpected; @@ -2062,3 +2062,117 @@ where T::deserialize_in_place(deserializer, self.0) } } + +#[cfg(any(feature = "std", feature = "alloc"))] +pub struct FlatMapDeserializer<'a, 'de: 'a, E>( + pub &'a mut Vec)>>, + pub PhantomData +); + +#[cfg(any(feature = "std", feature = "alloc"))] +impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> + where E: Error +{ + type Error = E; + + fn deserialize_any(self, _visitor: V) -> Result + where + V: Visitor<'de>, + { + Err(Error::custom("can only deserialize structs and maps in flatten mode")) + } + + fn deserialize_map(self, visitor: V) -> Result + where + V: Visitor<'de>, + { + visitor.visit_map(FlatMapAccess::new(self.0.iter_mut(), None)) + } + + fn deserialize_struct( + self, + _: &'static str, + fields: &'static [&'static str], + visitor: V + ) -> Result + where + V: Visitor<'de>, + { + visitor.visit_map(FlatMapAccess::new(self.0.iter_mut(), Some(fields))) + } + + forward_to_deserialize_any! { + bool i8 i16 i32 i64 u8 u16 u32 u64 f32 f64 char str string bytes + byte_buf option unit unit_struct newtype_struct seq tuple + tuple_struct enum identifier ignored_any + } +} + +#[cfg(any(feature = "std", feature = "alloc"))] +pub struct FlatMapAccess<'a, 'de: 'a, E> { + iter: slice::IterMut<'a, Option<(String, Content<'de>)>>, + pending_content: Option>, + fields: Option<&'static [&'static str]>, + _marker: PhantomData, +} + +impl<'a, 'de, E> FlatMapAccess<'a, 'de, E> { + fn new( + iter: slice::IterMut<'a, Option<(String, Content<'de>)>>, + fields: Option<&'static [&'static str]> + ) -> FlatMapAccess<'a, 'de, E> { + FlatMapAccess { + iter: iter, + pending_content: None, + fields: fields, + _marker: PhantomData, + } + } +} + +#[cfg(any(feature = "std", feature = "alloc"))] +impl<'a, 'de, E> MapAccess<'de> for FlatMapAccess<'a, 'de, E> + where E: Error +{ + type Error = E; + + fn next_key_seed(&mut self, seed: T) -> Result, Self::Error> + where + T: DeserializeSeed<'de>, + { + loop { + let item = match self.iter.next() { + Some(item) => { + if item.is_some() { + item + } else { + continue; + } + } + None => return Ok(None) + }; + + if match self.fields { + None => false, + Some(fields) if fields.contains(&item.as_ref().unwrap().0.as_str()) => false, + _ => true + } { + continue; + } + + let (key, content) = item.take().unwrap(); + self.pending_content = Some(content); + return seed.deserialize(ContentDeserializer::new(Content::String(key))).map(Some); + } + } + + fn next_value_seed(&mut self, seed: T) -> Result + where + T: DeserializeSeed<'de>, + { + match self.pending_content.take() { + Some(value) => seed.deserialize(ContentDeserializer::new(value)), + None => Err(Error::custom("value is missing")), + } + } +} diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 01ea6bfe..c9c6989f 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -2026,7 +2026,7 @@ fn deserialize_map( // Collect contents for flatten fields into a buffer let let_collect = if cattrs.has_flatten() { Some(quote! { - let mut __collect = Vec::<(String, _serde::private::de::Content)>::new(); + let mut __collect = Vec::>::new(); }) } else { None @@ -2070,7 +2070,7 @@ fn deserialize_map( let ignored_arm = if cattrs.has_flatten() { Some(quote! { __Field::__other(__name) => { - __collect.push((__name, try!(_serde::de::MapAccess::next_value(&mut __map)))); + __collect.push(Some((__name, try!(_serde::de::MapAccess::next_value(&mut __map))))); } }) } else if cattrs.deny_unknown_fields() { @@ -2130,7 +2130,7 @@ fn deserialize_map( let collected_deny_unknown_fields = if cattrs.has_flatten() && cattrs.deny_unknown_fields() { Some(quote! { - if let Some((__key, _)) = __collect.into_iter().next() { + if let Some(Some((__key, _))) = __collect.into_iter().filter(|x| x.is_some()).next() { return _serde::export::Err( _serde::de::Error::unknown_field_in_flattened_structure(&__key)); } From b4ef7ac32371b9df0a67b730b399ea1d749019e6 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 16 Mar 2018 01:26:57 +0100 Subject: [PATCH 17/51] Updated tests for flatten --- .../conflict/collect-into-wrong-type.rs | 19 ------------------ .../conflict/repr-collect-struct.rs | 20 ------------------- test_suite/tests/test_annotations.rs | 3 ++- 3 files changed, 2 insertions(+), 40 deletions(-) delete mode 100644 test_suite/tests/compile-fail/conflict/collect-into-wrong-type.rs delete mode 100644 test_suite/tests/compile-fail/conflict/repr-collect-struct.rs diff --git a/test_suite/tests/compile-fail/conflict/collect-into-wrong-type.rs b/test_suite/tests/compile-fail/conflict/collect-into-wrong-type.rs deleted file mode 100644 index 4f55cb32..00000000 --- a/test_suite/tests/compile-fail/conflict/collect-into-wrong-type.rs +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2018 Serde Developers -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -#[macro_use] -extern crate serde_derive; - -#[derive(Serialize)] //~ ERROR: the trait bound `&std::string::String: std::iter::Iterator` is not satisfied -#[serde(repr = "map", unknown_fields_into="other")] -struct X { - a: u32, - other: String, -} - -fn main() {} diff --git a/test_suite/tests/compile-fail/conflict/repr-collect-struct.rs b/test_suite/tests/compile-fail/conflict/repr-collect-struct.rs deleted file mode 100644 index 6396d32f..00000000 --- a/test_suite/tests/compile-fail/conflict/repr-collect-struct.rs +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright 2018 Serde Developers -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -#[macro_use] -extern crate serde_derive; - -#[derive(Serialize)] //~ ERROR: proc-macro derive panicked -#[serde(repr = "struct", unknown_fields_into="other")] -//~^^ HELP: #[serde(unknown_fields_into)] requires repr="map" -struct X { - a: u32, - other: HashMap, -} - -fn main() {} diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 3686041a..c11b02bb 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -96,10 +96,11 @@ where } #[derive(Debug, PartialEq, Serialize, Deserialize)] -#[serde(repr="map", unknown_fields_into="extra")] +#[serde(repr="map")] struct CollectOther { a: u32, b: u32, + #[serde(flatten)] extra: HashMap, } From d1833c56029bb588c274ce43b3b194d4527635d0 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 16 Mar 2018 12:39:15 +0100 Subject: [PATCH 18/51] Added support for basic enums in flatten --- serde/src/private/de.rs | 64 ++++++++++++++++++++++++++++++++++------- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 74eda4da..90049109 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -17,7 +17,7 @@ use de::Unexpected; pub use self::content::{Content, ContentDeserializer, ContentRefDeserializer, InternallyTaggedUnitVisitor, TagContentOtherField, TagContentOtherFieldVisitor, TagOrContentField, TagOrContentFieldVisitor, - TaggedContentVisitor, UntaggedUnitVisitor}; + TaggedContentVisitor, UntaggedUnitVisitor, EnumDeserializer}; /// If the missing field is of type `Option` then treat is as `None`, /// otherwise it is an error. @@ -1118,11 +1118,7 @@ mod content { } }; - visitor.visit_enum(EnumDeserializer { - variant: variant, - value: value, - err: PhantomData, - }) + visitor.visit_enum(EnumDeserializer::new(variant, value)) } fn deserialize_unit_struct( @@ -1170,7 +1166,7 @@ mod content { } } - struct EnumDeserializer<'de, E> + pub struct EnumDeserializer<'de, E> where E: de::Error, { @@ -1179,6 +1175,18 @@ mod content { err: PhantomData, } + impl<'de, E> EnumDeserializer<'de, E> + where E: de::Error + { + pub fn new(variant: Content<'de>, value: Option>) -> EnumDeserializer<'de, E> { + EnumDeserializer { + variant: variant, + value: value, + err: PhantomData, + } + } + } + impl<'de, E> de::EnumAccess<'de> for EnumDeserializer<'de, E> where E: de::Error, @@ -1199,7 +1207,7 @@ mod content { } } - struct VariantDeserializer<'de, E> + pub struct VariantDeserializer<'de, E> where E: de::Error, { @@ -2075,11 +2083,45 @@ impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> { type Error = E; - fn deserialize_any(self, _visitor: V) -> Result + fn deserialize_any(self, _: V) -> Result where V: Visitor<'de>, { - Err(Error::custom("can only deserialize structs and maps in flatten mode")) + Err(Error::custom("can only flatten structs, maps and basic enums")) + } + + fn deserialize_enum( + self, + name: &'static str, + variants: &'static [&'static str], + visitor: V + ) -> Result + where + V: Visitor<'de>, + { + let mut iter = self.0.iter_mut(); + loop { + let item = match iter.next() { + Some(item) => { + if item.is_some() { + item + } else { + continue; + } + } + None => return Err(Error::custom(format!( + "no variant of enum {} not found in flattened data", + name + ))) + }; + + if !variants.contains(&item.as_ref().unwrap().0.as_str()) { + continue; + } + + let (key, value) = item.take().unwrap(); + return visitor.visit_enum(EnumDeserializer::new(Content::String(key), Some(value))); + } } fn deserialize_map(self, visitor: V) -> Result @@ -2104,7 +2146,7 @@ impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> forward_to_deserialize_any! { bool i8 i16 i32 i64 u8 u16 u32 u64 f32 f64 char str string bytes byte_buf option unit unit_struct newtype_struct seq tuple - tuple_struct enum identifier ignored_any + tuple_struct identifier ignored_any } } From a8c8c2028ef153b25d859f450cceb9565441840b Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 16 Mar 2018 22:40:08 +0100 Subject: [PATCH 19/51] Added support for struct variant enum serialization --- serde/src/private/ser.rs | 75 ++++++++++++++++++++++------ test_suite/tests/test_annotations.rs | 68 ++++++++++++++++++++++++- 2 files changed, 128 insertions(+), 15 deletions(-) diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index f19c4063..948fc2fa 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -11,7 +11,12 @@ use lib::*; use ser::{self, Impossible, Serialize, SerializeMap, SerializeStruct, Serializer}; #[cfg(any(feature = "std", feature = "alloc"))] -use self::content::{SerializeStructVariantAsMapValue, SerializeTupleVariantAsMapValue}; +use self::content::{ + SerializeStructVariantAsMapValue, + SerializeTupleVariantAsMapValue, + ContentSerializer, + Content, +}; /// Used to check that serde(getter) attributes return the expected type. /// Not public API. @@ -461,7 +466,7 @@ mod content { } #[derive(Debug)] - enum Content { + pub enum Content { Bool(bool), U8(u8), @@ -586,12 +591,12 @@ mod content { } } - struct ContentSerializer { + pub struct ContentSerializer { error: PhantomData, } impl ContentSerializer { - fn new() -> Self { + pub fn new() -> Self { ContentSerializer { error: PhantomData } } } @@ -806,7 +811,7 @@ mod content { } } - struct SerializeSeq { + pub struct SerializeSeq { elements: Vec, error: PhantomData, } @@ -832,7 +837,7 @@ mod content { } } - struct SerializeTuple { + pub struct SerializeTuple { elements: Vec, error: PhantomData, } @@ -858,7 +863,7 @@ mod content { } } - struct SerializeTupleStruct { + pub struct SerializeTupleStruct { name: &'static str, fields: Vec, error: PhantomData, @@ -885,7 +890,7 @@ mod content { } } - struct SerializeTupleVariant { + pub struct SerializeTupleVariant { name: &'static str, variant_index: u32, variant: &'static str, @@ -919,7 +924,7 @@ mod content { } } - struct SerializeMap { + pub struct SerializeMap { entries: Vec<(Content, Content)>, key: Option, error: PhantomData, @@ -969,7 +974,7 @@ mod content { } } - struct SerializeStruct { + pub struct SerializeStruct { name: &'static str, fields: Vec<(&'static str, Content)>, error: PhantomData, @@ -996,7 +1001,7 @@ mod content { } } - struct SerializeStructVariant { + pub struct SerializeStructVariant { name: &'static str, variant_index: u32, variant: &'static str, @@ -1059,7 +1064,7 @@ impl<'a, M> Serializer for FlatMapSerializer<'a, M> type SerializeMap = FlatMapSerializeMap<'a, M>; type SerializeStruct = FlatMapSerializeStruct<'a, M>; type SerializeTupleVariant = Impossible; - type SerializeStructVariant = Impossible; + type SerializeStructVariant = FlatMapSerializeStructVariantAsMapValue<'a, M>; fn serialize_bool(self, _: bool) -> Result { Err(self.bad_type(Unsupported::Boolean)) @@ -1212,10 +1217,11 @@ impl<'a, M> Serializer for FlatMapSerializer<'a, M> self, _: &'static str, _: u32, - _: &'static str, + inner_variant: &'static str, _: usize, ) -> Result { - Err(self.bad_type(Unsupported::Enum)) + try!(self.0.serialize_key(inner_variant)); + Ok(FlatMapSerializeStructVariantAsMapValue::new(self.0, inner_variant)) } } @@ -1269,3 +1275,44 @@ impl<'a, M> ser::SerializeStruct for FlatMapSerializeStruct<'a, M> Ok(()) } } + +#[cfg(any(feature = "std", feature = "alloc"))] +pub struct FlatMapSerializeStructVariantAsMapValue<'a, M: 'a> { + map: &'a mut M, + name: &'static str, + fields: Vec<(&'static str, Content)>, +} + +impl<'a, M> FlatMapSerializeStructVariantAsMapValue<'a, M> + where M: SerializeMap + 'a +{ + fn new(map: &'a mut M, name: &'static str) -> FlatMapSerializeStructVariantAsMapValue<'a, M> { + FlatMapSerializeStructVariantAsMapValue { + map: map, + name: name, + fields: Vec::new(), + } + } +} + +#[cfg(any(feature = "std", feature = "alloc"))] +impl<'a, M> ser::SerializeStructVariant for FlatMapSerializeStructVariantAsMapValue<'a, M> + where M: SerializeMap + 'a +{ + type Ok = (); + type Error = M::Error; + + fn serialize_field(&mut self, key: &'static str, value: &T) -> Result<(), Self::Error> + where + T: Serialize, + { + let value = try!(value.serialize(ContentSerializer::::new())); + self.fields.push((key, value)); + Ok(()) + } + + fn end(self) -> Result<(), Self::Error> { + try!(self.map.serialize_value(&Content::Struct(self.name, self.fields))); + Ok(()) + } +} diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index c11b02bb..e3e91e1f 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -96,7 +96,7 @@ where } #[derive(Debug, PartialEq, Serialize, Deserialize)] -#[serde(repr="map")] +#[serde(repr = "map")] struct CollectOther { a: u32, b: u32, @@ -104,6 +104,27 @@ struct CollectOther { extra: HashMap, } +#[derive(Debug, PartialEq, Serialize, Deserialize)] +#[serde(repr = "map")] +struct ChangeRequest { + #[serde(flatten)] + data: ChangeAction, + #[serde(flatten)] + extra: HashMap, +} + +#[derive(Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +enum ChangeAction { + AppendInteger { + value: u32 + }, + InsertInteger { + index: u32, + value: u32 + }, +} + #[test] fn test_default_struct() { assert_de_tokens( @@ -1309,3 +1330,48 @@ fn test_collect_other() { ], ); } + +#[test] +fn test_flatten_struct_enum() { + let mut extra = HashMap::new(); + extra.insert("extra_key".into(), "extra value".into()); + let change_request = ChangeRequest { + data: ChangeAction::InsertInteger { + index: 0, + value: 42 + }, + extra: extra, + }; + assert_de_tokens( + &change_request, + &[ + Token::Map { len: None }, + Token::Str("insert_integer"), + Token::Map { len: None }, + Token::Str("index"), + Token::U32(0), + Token::Str("value"), + Token::U32(42), + Token::MapEnd, + Token::Str("extra_key"), + Token::String("extra value".into()), + Token::MapEnd + ], + ); + assert_ser_tokens( + &change_request, + &[ + Token::Map { len: None }, + Token::Str("insert_integer"), + Token::Struct { len: 2, name: "insert_integer" }, + Token::Str("index"), + Token::U32(0), + Token::Str("value"), + Token::U32(42), + Token::StructEnd, + Token::Str("extra_key"), + Token::String("extra value".into()), + Token::MapEnd + ], + ); +} From b8602a7e43d7369e7eae2c442b5225a249e65ca0 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 16 Mar 2018 22:58:16 +0100 Subject: [PATCH 20/51] Added test for tag/content enum flattening --- test_suite/tests/test_annotations.rs | 73 +++++++++++++++++++++++++--- 1 file changed, 67 insertions(+), 6 deletions(-) diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index e3e91e1f..3f9bfcb0 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -106,19 +106,33 @@ struct CollectOther { #[derive(Debug, PartialEq, Serialize, Deserialize)] #[serde(repr = "map")] -struct ChangeRequest { +struct FlattenStructEnumWrapper { #[serde(flatten)] - data: ChangeAction, + data: FlattenStructEnum, #[serde(flatten)] extra: HashMap, } #[derive(Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] -enum ChangeAction { - AppendInteger { +enum FlattenStructEnum { + InsertInteger { + index: u32, value: u32 }, +} + +#[derive(Debug, PartialEq, Serialize, Deserialize)] +#[serde(repr = "map")] +struct FlattenStructTagContentEnumWrapper { + outer: u32, + #[serde(flatten)] + data: FlattenStructTagContentEnum, +} + +#[derive(Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case", tag = "type", content = "value")] +enum FlattenStructTagContentEnum { InsertInteger { index: u32, value: u32 @@ -1335,8 +1349,8 @@ fn test_collect_other() { fn test_flatten_struct_enum() { let mut extra = HashMap::new(); extra.insert("extra_key".into(), "extra value".into()); - let change_request = ChangeRequest { - data: ChangeAction::InsertInteger { + let change_request = FlattenStructEnumWrapper { + data: FlattenStructEnum::InsertInteger { index: 0, value: 42 }, @@ -1375,3 +1389,50 @@ fn test_flatten_struct_enum() { ], ); } + +#[test] +fn test_flatten_struct_tag_content_enum() { + let change_request = FlattenStructTagContentEnumWrapper { + outer: 42, + data: FlattenStructTagContentEnum::InsertInteger { + index: 0, + value: 42 + }, + }; + assert_de_tokens( + &change_request, + &[ + Token::Map { len: None }, + Token::Str("outer"), + Token::U32(42), + Token::Str("type"), + Token::Str("insert_integer"), + Token::Str("value"), + Token::Map { len: None }, + Token::Str("index"), + Token::U32(0), + Token::Str("value"), + Token::U32(42), + Token::MapEnd, + Token::MapEnd, + ], + ); + assert_ser_tokens( + &change_request, + &[ + Token::Map { len: None }, + Token::Str("outer"), + Token::U32(42), + Token::Str("type"), + Token::Str("insert_integer"), + Token::Str("value"), + Token::Struct { len: 2, name: "insert_integer" }, + Token::Str("index"), + Token::U32(0), + Token::Str("value"), + Token::U32(42), + Token::StructEnd, + Token::MapEnd, + ], + ); +} From 49e302d17d467ea327432912ff8caaef6fc00154 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 16 Mar 2018 23:03:56 +0100 Subject: [PATCH 21/51] Improved error message for flattening on unsupported types --- serde/src/private/de.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 90049109..6fc2baee 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2087,7 +2087,7 @@ impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> where V: Visitor<'de>, { - Err(Error::custom("can only flatten structs, maps and basic enums")) + Err(Error::custom("can only flatten structs, maps and struct enum variants")) } fn deserialize_enum( From 352fe7b451d3a0df4aff521703235ceeddc2d9c1 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 16 Mar 2018 23:07:31 +0100 Subject: [PATCH 22/51] Removed an unused field that was left over from a merge conflict --- serde_derive_internals/src/attr.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs index b62e4748..04294df3 100644 --- a/serde_derive_internals/src/attr.rs +++ b/serde_derive_internals/src/attr.rs @@ -757,7 +757,6 @@ pub struct Field { de_bound: Option>, borrowed_lifetimes: BTreeSet, getter: Option, - collection_field: bool, flatten: bool, } From ca41e16e9282b640bf52bac74c7473da6d76fdbc Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 16 Mar 2018 23:20:14 +0100 Subject: [PATCH 23/51] Added some missing conditionals for feature compilation --- serde/src/private/de.rs | 1 + serde/src/private/ser.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 6fc2baee..d347c1c2 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2158,6 +2158,7 @@ pub struct FlatMapAccess<'a, 'de: 'a, E> { _marker: PhantomData, } +#[cfg(any(feature = "std", feature = "alloc"))] impl<'a, 'de, E> FlatMapAccess<'a, 'de, E> { fn new( iter: slice::IterMut<'a, Option<(String, Content<'de>)>>, diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index 948fc2fa..91dfcce7 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -1283,6 +1283,7 @@ pub struct FlatMapSerializeStructVariantAsMapValue<'a, M: 'a> { fields: Vec<(&'static str, Content)>, } +#[cfg(any(feature = "std", feature = "alloc"))] impl<'a, M> FlatMapSerializeStructVariantAsMapValue<'a, M> where M: SerializeMap + 'a { From bfdcbae9dbb09f2cfa65be2cd514dea362806527 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 16 Mar 2018 23:30:53 +0100 Subject: [PATCH 24/51] Fixed an unused import error --- serde/src/private/de.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index d347c1c2..7bc15947 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -8,10 +8,10 @@ use lib::*; -use de::{Deserialize, DeserializeSeed, Deserializer, Error, IntoDeserializer, Visitor, MapAccess}; +use de::{Deserialize, DeserializeSeed, Deserializer, Error, IntoDeserializer, Visitor}; #[cfg(any(feature = "std", feature = "alloc"))] -use de::Unexpected; +use de::{Unexpected, MapAccess}; #[cfg(any(feature = "std", feature = "alloc"))] pub use self::content::{Content, ContentDeserializer, ContentRefDeserializer, From 2f57cecf13b091448eb4dfc4c04952930e255273 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 16 Mar 2018 23:38:50 +0100 Subject: [PATCH 25/51] format! -> format_args! for an error message --- serde/src/private/de.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 7bc15947..80e7e8c7 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2109,7 +2109,7 @@ impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> continue; } } - None => return Err(Error::custom(format!( + None => return Err(Error::custom(format_args!( "no variant of enum {} not found in flattened data", name ))) From ffcde25b6edd074fc0be454958f874e73a2b621e Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sat, 17 Mar 2018 00:49:00 +0100 Subject: [PATCH 26/51] Fixed some clippy warnings --- test_suite/tests/test_annotations.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 3f9bfcb0..57b211a6 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -1354,7 +1354,7 @@ fn test_flatten_struct_enum() { index: 0, value: 42 }, - extra: extra, + extra, }; assert_de_tokens( &change_request, @@ -1368,7 +1368,7 @@ fn test_flatten_struct_enum() { Token::U32(42), Token::MapEnd, Token::Str("extra_key"), - Token::String("extra value".into()), + Token::Str("extra value"), Token::MapEnd ], ); @@ -1384,7 +1384,7 @@ fn test_flatten_struct_enum() { Token::U32(42), Token::StructEnd, Token::Str("extra_key"), - Token::String("extra value".into()), + Token::Str("extra value"), Token::MapEnd ], ); From ebc61baab2c6ad2ed334c2856c3efad7d4db1b5d Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 18 Mar 2018 13:02:00 +0100 Subject: [PATCH 27/51] Added newtype struct support for flattening --- serde/src/private/de.rs | 15 +++++++++++++-- serde/src/private/ser.rs | 7 ++----- test_suite/tests/test_annotations.rs | 15 ++++++++++----- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 80e7e8c7..7cb3152d 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2143,10 +2143,21 @@ impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> visitor.visit_map(FlatMapAccess::new(self.0.iter_mut(), Some(fields))) } + fn deserialize_newtype_struct( + self, + _name: &str, + visitor: V, + ) -> Result + where + V: Visitor<'de>, + { + visitor.visit_newtype_struct(self) + } + forward_to_deserialize_any! { bool i8 i16 i32 i64 u8 u16 u32 u64 f32 f64 char str string bytes - byte_buf option unit unit_struct newtype_struct seq tuple - tuple_struct identifier ignored_any + byte_buf option unit unit_struct seq tuple tuple_struct identifier + ignored_any } } diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index 91dfcce7..cac2bf41 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -64,7 +64,6 @@ enum Unsupported { Optional, Unit, UnitStruct, - NewtypeStruct, Sequence, Tuple, TupleStruct, @@ -83,7 +82,6 @@ impl Display for Unsupported { Unsupported::Optional => formatter.write_str("an optional"), Unsupported::Unit => formatter.write_str("unit"), Unsupported::UnitStruct => formatter.write_str("unit struct"), - Unsupported::NewtypeStruct => formatter.write_str("newtype struct"), Unsupported::Sequence => formatter.write_str("a sequence"), Unsupported::Tuple => formatter.write_str("a tuple"), Unsupported::TupleStruct => formatter.write_str("a tuple struct"), @@ -1153,13 +1151,12 @@ impl<'a, M> Serializer for FlatMapSerializer<'a, M> fn serialize_newtype_struct( self, _: &'static str, - _value: &T, + value: &T, ) -> Result where T: Serialize, { - // TODO: can we do better here? - Err(self.bad_type(Unsupported::NewtypeStruct)) + value.serialize(self) } fn serialize_newtype_variant( diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 57b211a6..be0b4e03 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -127,9 +127,12 @@ enum FlattenStructEnum { struct FlattenStructTagContentEnumWrapper { outer: u32, #[serde(flatten)] - data: FlattenStructTagContentEnum, + data: FlattenStructTagContentEnumNewtype, } +#[derive(Debug, PartialEq, Serialize, Deserialize)] +struct FlattenStructTagContentEnumNewtype(pub FlattenStructTagContentEnum); + #[derive(Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "snake_case", tag = "type", content = "value")] enum FlattenStructTagContentEnum { @@ -1394,10 +1397,12 @@ fn test_flatten_struct_enum() { fn test_flatten_struct_tag_content_enum() { let change_request = FlattenStructTagContentEnumWrapper { outer: 42, - data: FlattenStructTagContentEnum::InsertInteger { - index: 0, - value: 42 - }, + data: FlattenStructTagContentEnumNewtype( + FlattenStructTagContentEnum::InsertInteger { + index: 0, + value: 42 + } + ), }; assert_de_tokens( &change_request, From f1af2dc5abaf0492d318a04b034f0928e691cdc8 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 18 Mar 2018 13:10:54 +0100 Subject: [PATCH 28/51] Added support for newtype variant serialization --- serde/src/private/ser.rs | 4 +-- test_suite/tests/test_annotations.rs | 52 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index cac2bf41..bf78ecf4 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -1164,12 +1164,12 @@ impl<'a, M> Serializer for FlatMapSerializer<'a, M> _: &'static str, _: u32, _: &'static str, - _: &T, + value: &T, ) -> Result where T: Serialize, { - Err(self.bad_type(Unsupported::Enum)) + value.serialize(self) } fn serialize_seq(self, _: Option) -> Result { diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index be0b4e03..d72605e2 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -140,6 +140,12 @@ enum FlattenStructTagContentEnum { index: u32, value: u32 }, + NewtypeVariant(FlattenStructTagContentEnumNewtypeVariant), +} + +#[derive(Debug, PartialEq, Serialize, Deserialize)] +struct FlattenStructTagContentEnumNewtypeVariant { + value: u32, } #[test] @@ -1441,3 +1447,49 @@ fn test_flatten_struct_tag_content_enum() { ], ); } + +#[test] +fn test_flatten_struct_tag_content_enum_newtype() { + let change_request = FlattenStructTagContentEnumWrapper { + outer: 42, + data: FlattenStructTagContentEnumNewtype( + FlattenStructTagContentEnum::NewtypeVariant( + FlattenStructTagContentEnumNewtypeVariant { + value: 23 + } + ) + ), + }; + assert_de_tokens( + &change_request, + &[ + Token::Map { len: None }, + Token::Str("outer"), + Token::U32(42), + Token::Str("type"), + Token::Str("newtype_variant"), + Token::Str("value"), + Token::Map { len: None }, + Token::Str("value"), + Token::U32(23), + Token::MapEnd, + Token::MapEnd, + ], + ); + assert_ser_tokens( + &change_request, + &[ + Token::Map { len: None }, + Token::Str("outer"), + Token::U32(42), + Token::Str("type"), + Token::Str("newtype_variant"), + Token::Str("value"), + Token::Struct { len: 1, name: "FlattenStructTagContentEnumNewtypeVariant" }, + Token::Str("value"), + Token::U32(23), + Token::StructEnd, + Token::MapEnd, + ], + ); +} From 61b167be9adb14c7b551ea5790a001a0649f0d2f Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 18 Mar 2018 18:22:06 +0100 Subject: [PATCH 29/51] Attempted support for in_place deserialization for structs as map --- serde/src/de/mod.rs | 7 -- serde_derive/src/de.rs | 119 ++++++++++++++++++++++----- test_suite/tests/test_annotations.rs | 38 +++++++++ 3 files changed, 138 insertions(+), 26 deletions(-) diff --git a/serde/src/de/mod.rs b/serde/src/de/mod.rs index f391a858..cc2c6502 100644 --- a/serde/src/de/mod.rs +++ b/serde/src/de/mod.rs @@ -255,13 +255,6 @@ macro_rules! declare_error_trait { } } - /// Raised when a `Deserialize` struct type recieved a field with an - /// unrecognized name but the names are not actually known because of - /// flattening. - fn unknown_field_in_flattened_structure(field: &str) -> Self { - Error::custom(format_args!("unknown field `{}`", field)) - } - /// Raised when a `Deserialize` struct type expected to receive a required /// field with a particular name but that field was not present in the /// input. diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index c9c6989f..b8dab7d9 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -831,7 +831,7 @@ fn deserialize_struct( } }; - let all_skipped = fields.iter().all(|field| field.attrs.skip_deserializing()); + let all_skipped = !cattrs.has_flatten() && fields.iter().all(|field| field.attrs.skip_deserializing()); let visitor_var = if all_skipped { quote!(_) } else { @@ -892,6 +892,10 @@ fn deserialize_struct_in_place( deserializer: Option, ) -> Fragment { let is_enum = variant_ident.is_some(); + let as_map = deserializer.is_none() && !is_enum && match cattrs.repr() { + attr::ContainerRepr::Struct | attr::ContainerRepr::Auto => false, + attr::ContainerRepr::Map => true, + }; let this = ¶ms.this; let (de_impl_generics, de_ty_generics, ty_generics, where_clause) = @@ -905,10 +909,14 @@ fn deserialize_struct_in_place( let visit_seq = Stmts(deserialize_seq_in_place(params, fields, cattrs)); - let (field_visitor, fields_stmt, visit_map) = - deserialize_struct_in_place_visitor(params, fields, cattrs); + let (field_visitor, fields_stmt, visit_map) = if as_map { + deserialize_struct_as_map_in_place_visitor(params, fields, cattrs) + } else { + deserialize_struct_as_struct_in_place_visitor(params, fields, cattrs) + }; + let field_visitor = Stmts(field_visitor); - let fields_stmt = Stmts(fields_stmt); + let fields_stmt = fields_stmt.map(Stmts); let visit_map = Stmts(visit_map); let visitor_expr = quote! { @@ -925,6 +933,10 @@ fn deserialize_struct_in_place( quote! { _serde::de::VariantAccess::struct_variant(__variant, FIELDS, #visitor_expr) } + } else if as_map { + quote! { + _serde::Deserializer::deserialize_map(__deserializer, #visitor_expr) + } } else { let type_name = cattrs.name().deserialize_name(); quote! { @@ -932,20 +944,24 @@ fn deserialize_struct_in_place( } }; - let all_skipped = fields.iter().all(|field| field.attrs.skip_deserializing()); + let all_skipped = !cattrs.has_flatten() && fields.iter().all(|field| field.attrs.skip_deserializing()); let visitor_var = if all_skipped { quote!(_) } else { quote!(mut __seq) }; - let visit_seq = quote! { - #[inline] - fn visit_seq<__A>(self, #visitor_var: __A) -> _serde::export::Result - where __A: _serde::de::SeqAccess<#delife> - { - #visit_seq - } + let visit_seq = if as_map { + None + } else { + Some(quote! { + #[inline] + fn visit_seq<__A>(self, #visitor_var: __A) -> _serde::export::Result + where __A: _serde::de::SeqAccess<#delife> + { + #visit_seq + } + }) }; let in_place_impl_generics = de_impl_generics.in_place(); @@ -2081,7 +2097,7 @@ fn deserialize_map( }) }; - let all_skipped = fields.iter().all(|field| field.attrs.skip_deserializing()); + let all_skipped = !cattrs.has_flatten() && fields.iter().all(|field| field.attrs.skip_deserializing()); let match_keys = if cattrs.deny_unknown_fields() && all_skipped { quote! { // FIXME: Once we drop support for Rust 1.15: @@ -2132,7 +2148,7 @@ fn deserialize_map( Some(quote! { if let Some(Some((__key, _))) = __collect.into_iter().filter(|x| x.is_some()).next() { return _serde::export::Err( - _serde::de::Error::unknown_field_in_flattened_structure(&__key)); + _serde::de::Error::custom(format_args!("unknown field `{}`", &__key))); } }) } else { @@ -2191,11 +2207,11 @@ fn deserialize_map( } #[cfg(feature = "deserialize_in_place")] -fn deserialize_struct_in_place_visitor( +fn deserialize_struct_as_struct_in_place_visitor( params: &Parameters, fields: &[Field], cattrs: &attr::Container, -) -> (Fragment, Fragment, Fragment) { +) -> (Fragment, Option, Fragment) { let field_names_idents: Vec<_> = fields .iter() .enumerate() @@ -2214,7 +2230,27 @@ fn deserialize_struct_in_place_visitor( let visit_map = deserialize_map_in_place(params, fields, cattrs); - (field_visitor, fields_stmt, visit_map) + (field_visitor, Some(fields_stmt), visit_map) +} + +#[cfg(feature = "deserialize_in_place")] +fn deserialize_struct_as_map_in_place_visitor( + params: &Parameters, + fields: &[Field], + cattrs: &attr::Container, +) -> (Fragment, Option, Fragment) { + let field_names_idents: Vec<_> = fields + .iter() + .enumerate() + .filter(|&(_, field)| !field.attrs.skip_deserializing()) + .map(|(i, field)| (field.attrs.name().deserialize_name(), field_i(i))) + .collect(); + + let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, true); + + let visit_map = deserialize_map_in_place(params, fields, cattrs); + + (field_visitor, None, visit_map) } #[cfg(feature = "deserialize_in_place")] @@ -2240,6 +2276,15 @@ fn deserialize_map_in_place( } }); + // Collect contents for flatten fields into a buffer + let let_collect = if cattrs.has_flatten() { + Some(quote! { + let mut __collect = Vec::>::new(); + }) + } else { + None + }; + // Match arms to extract a value for a field. let value_arms_from = fields_names .iter() @@ -2274,7 +2319,13 @@ fn deserialize_map_in_place( }); // Visit ignored values to consume them - let ignored_arm = if cattrs.deny_unknown_fields() { + let ignored_arm = if cattrs.has_flatten() { + Some(quote! { + __Field::__other(__name) => { + __collect.push(Some((__name, try!(_serde::de::MapAccess::next_value(&mut __map))))); + } + }) + } else if cattrs.deny_unknown_fields() { None } else { Some(quote! { @@ -2282,7 +2333,7 @@ fn deserialize_map_in_place( }) }; - let all_skipped = fields.iter().all(|field| field.attrs.skip_deserializing()); + let all_skipped = !cattrs.has_flatten() && fields.iter().all(|field| field.attrs.skip_deserializing()); let match_keys = if cattrs.deny_unknown_fields() && all_skipped { quote! { @@ -2346,15 +2397,45 @@ fn deserialize_map_in_place( } }; + let extract_collected = fields_names + .iter() + .filter(|&&(field, _)| field.attrs.flatten()) + .map(|&(field, ref name)| { + let field_ty = field.ty; + quote! { + let #name: #field_ty = try!(_serde::de::Deserialize::deserialize( + _serde::private::de::FlatMapDeserializer( + &mut __collect, + _serde::export::PhantomData))); + } + }); + + let collected_deny_unknown_fields = if cattrs.has_flatten() && cattrs.deny_unknown_fields() { + Some(quote! { + if let Some(Some((__key, _))) = __collect.into_iter().filter(|x| x.is_some()).next() { + return _serde::export::Err( + _serde::de::Error::custom(format_args!("unknown field `{}`", &__key))); + } + }) + } else { + None + }; + quote_block! { #(#let_flags)* + #let_collect + #match_keys #let_default #(#check_flags)* + #(#extract_collected)* + + #collected_deny_unknown_fields + _serde::export::Ok(()) } } diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index d72605e2..5f6df68f 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -1493,3 +1493,41 @@ fn test_flatten_struct_tag_content_enum_newtype() { ], ); } + +#[test] +fn test_unknown_field_in_flatten() { + #[derive(Debug, PartialEq, Serialize, Deserialize)] + #[serde(repr = "map", deny_unknown_fields)] + struct Outer { + dummy: String, + #[serde(flatten)] + inner: Inner, + } + + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct Inner { + foo: HashMap, + } + + assert_de_tokens_error::( + &[ + Token::Struct { + name: "Outer", + len: 1, + }, + Token::Str("dummy"), + Token::Str("23"), + Token::Str("foo"), + Token::Map { len: None }, + Token::Str("a"), + Token::U32(1), + Token::Str("b"), + Token::U32(2), + Token::MapEnd, + Token::Str("bar"), + Token::U32(23), + Token::StructEnd, + ], + "unknown field `bar`", + ); +} From d44f12907bfa0fe6bb0835ba10d555fc45ac6ced Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 18 Mar 2018 18:27:35 +0100 Subject: [PATCH 30/51] Do not emit an in-place deserialization path for struct as map --- serde_derive/src/de.rs | 67 +++++++++++++++--------------------------- 1 file changed, 23 insertions(+), 44 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index b8dab7d9..d0b15a4a 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -268,7 +268,11 @@ fn deserialize_in_place_body(cont: &Container, params: &Parameters) -> Option { - deserialize_struct_in_place(None, params, fields, &cont.attrs, None) + if let Some(code) = deserialize_struct_in_place(None, params, fields, &cont.attrs, None) { + code + } else { + return None; + } } Data::Struct(Style::Tuple, ref fields) | Data::Struct(Style::Newtype, ref fields) => { deserialize_tuple_in_place(None, params, fields, &cont.attrs, None) @@ -890,13 +894,19 @@ fn deserialize_struct_in_place( fields: &[Field], cattrs: &attr::Container, deserializer: Option, -) -> Fragment { +) -> Option { let is_enum = variant_ident.is_some(); let as_map = deserializer.is_none() && !is_enum && match cattrs.repr() { attr::ContainerRepr::Struct | attr::ContainerRepr::Auto => false, attr::ContainerRepr::Map => true, }; + // for now we do not support in_place deserialization for structs that + // are represented as map. + if as_map { + return None; + } + let this = ¶ms.this; let (de_impl_generics, de_ty_generics, ty_generics, where_clause) = split_with_de_lifetime(params); @@ -909,11 +919,8 @@ fn deserialize_struct_in_place( let visit_seq = Stmts(deserialize_seq_in_place(params, fields, cattrs)); - let (field_visitor, fields_stmt, visit_map) = if as_map { - deserialize_struct_as_map_in_place_visitor(params, fields, cattrs) - } else { - deserialize_struct_as_struct_in_place_visitor(params, fields, cattrs) - }; + let (field_visitor, fields_stmt, visit_map) = deserialize_struct_as_struct_in_place_visitor( + params, fields, cattrs); let field_visitor = Stmts(field_visitor); let fields_stmt = fields_stmt.map(Stmts); @@ -933,10 +940,6 @@ fn deserialize_struct_in_place( quote! { _serde::de::VariantAccess::struct_variant(__variant, FIELDS, #visitor_expr) } - } else if as_map { - quote! { - _serde::Deserializer::deserialize_map(__deserializer, #visitor_expr) - } } else { let type_name = cattrs.name().deserialize_name(); quote! { @@ -951,24 +954,20 @@ fn deserialize_struct_in_place( quote!(mut __seq) }; - let visit_seq = if as_map { - None - } else { - Some(quote! { - #[inline] - fn visit_seq<__A>(self, #visitor_var: __A) -> _serde::export::Result - where __A: _serde::de::SeqAccess<#delife> - { - #visit_seq - } - }) + let visit_seq = quote! { + #[inline] + fn visit_seq<__A>(self, #visitor_var: __A) -> _serde::export::Result + where __A: _serde::de::SeqAccess<#delife> + { + #visit_seq + } }; let in_place_impl_generics = de_impl_generics.in_place(); let in_place_ty_generics = de_ty_generics.in_place(); let place_life = place_lifetime(); - quote_block! { + Some(quote_block! { #field_visitor struct __Visitor #in_place_impl_generics #where_clause { @@ -996,7 +995,7 @@ fn deserialize_struct_in_place( #fields_stmt #dispatch - } + }) } fn deserialize_enum( @@ -2233,26 +2232,6 @@ fn deserialize_struct_as_struct_in_place_visitor( (field_visitor, Some(fields_stmt), visit_map) } -#[cfg(feature = "deserialize_in_place")] -fn deserialize_struct_as_map_in_place_visitor( - params: &Parameters, - fields: &[Field], - cattrs: &attr::Container, -) -> (Fragment, Option, Fragment) { - let field_names_idents: Vec<_> = fields - .iter() - .enumerate() - .filter(|&(_, field)| !field.attrs.skip_deserializing()) - .map(|(i, field)| (field.attrs.name().deserialize_name(), field_i(i))) - .collect(); - - let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, true); - - let visit_map = deserialize_map_in_place(params, fields, cattrs); - - (field_visitor, None, visit_map) -} - #[cfg(feature = "deserialize_in_place")] fn deserialize_map_in_place( params: &Parameters, From 58d52e784b1a6eb71bbd05695fa97b24638ec094 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 18 Mar 2018 18:30:46 +0100 Subject: [PATCH 31/51] Remove #[serde(repr = "map")] --- serde_derive/src/de.rs | 10 ++---- serde_derive/src/ser.rs | 7 +++-- serde_derive_internals/src/ast.rs | 6 ---- serde_derive_internals/src/attr.rs | 47 ---------------------------- test_suite/tests/test_annotations.rs | 5 +-- 5 files changed, 7 insertions(+), 68 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index d0b15a4a..490d25ff 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -770,10 +770,7 @@ fn deserialize_struct( untagged: &Untagged, ) -> Fragment { let is_enum = variant_ident.is_some(); - let as_map = deserializer.is_none() && !is_enum && match cattrs.repr() { - attr::ContainerRepr::Struct | attr::ContainerRepr::Auto => false, - attr::ContainerRepr::Map => true, - }; + let as_map = deserializer.is_none() && !is_enum && cattrs.has_flatten(); let this = ¶ms.this; let (de_impl_generics, de_ty_generics, ty_generics, where_clause) = @@ -896,10 +893,7 @@ fn deserialize_struct_in_place( deserializer: Option, ) -> Option { let is_enum = variant_ident.is_some(); - let as_map = deserializer.is_none() && !is_enum && match cattrs.repr() { - attr::ContainerRepr::Struct | attr::ContainerRepr::Auto => false, - attr::ContainerRepr::Map => true, - }; + let as_map = deserializer.is_none() && !is_enum && cattrs.has_flatten(); // for now we do not support in_place deserialization for structs that // are represented as map. diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 9211b144..5df85d46 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -245,9 +245,10 @@ fn serialize_tuple_struct( fn serialize_struct(params: &Parameters, fields: &[Field], cattrs: &attr::Container) -> Fragment { assert!(fields.len() as u64 <= u64::from(u32::MAX)); - match cattrs.repr() { - attr::ContainerRepr::Struct | attr::ContainerRepr::Auto => serialize_struct_as_struct(params, fields, cattrs), - attr::ContainerRepr::Map => serialize_struct_as_map(params, fields, cattrs), + if cattrs.has_flatten() { + serialize_struct_as_map(params, fields, cattrs) + } else { + serialize_struct_as_struct(params, fields, cattrs) } } diff --git a/serde_derive_internals/src/ast.rs b/serde_derive_internals/src/ast.rs index 916f647a..fd531d8c 100644 --- a/serde_derive_internals/src/ast.rs +++ b/serde_derive_internals/src/ast.rs @@ -79,12 +79,6 @@ impl<'a> Container<'a> { }, } - if has_flatten && attrs.repr() != attr::ContainerRepr::Map { - cx.error(format!("#[serde(flatten)] requires \ - #[serde(repr = \"map\")] on the container, but \ - found #[serde(repr = \"{}\")]", attrs.repr())); - } - if has_flatten { attrs.mark_has_flatten(); } diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs index 04294df3..caf885f3 100644 --- a/serde_derive_internals/src/attr.rs +++ b/serde_derive_internals/src/attr.rs @@ -15,7 +15,6 @@ use syn::punctuated::Punctuated; use syn::synom::{Synom, ParseError}; use std::collections::BTreeSet; use std::str::FromStr; -use std::fmt; use proc_macro2::{Span, TokenStream, TokenNode, TokenTree}; // This module handles parsing of `#[serde(...)]` attributes. The entrypoints @@ -103,35 +102,6 @@ impl Name { } } -#[derive(Debug, PartialEq, Eq, Copy, Clone)] -pub enum ContainerRepr { - Auto, - Struct, - Map, -} - -impl fmt::Display for ContainerRepr { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", match *self { - ContainerRepr::Auto => "auto", - ContainerRepr::Struct => "struct", - ContainerRepr::Map => "map", - }) - } -} - -impl FromStr for ContainerRepr { - type Err = (); - fn from_str(s: &str) -> Result { - match s { - "auto" => Ok(ContainerRepr::Auto), - "struct" => Ok(ContainerRepr::Struct), - "map" => Ok(ContainerRepr::Map), - _ => Err(()), - } - } -} - /// Represents container (e.g. struct) attribute information pub struct Container { name: Name, @@ -145,7 +115,6 @@ pub struct Container { type_into: Option, remote: Option, identifier: Identifier, - repr: ContainerRepr, has_flatten: bool, } @@ -224,7 +193,6 @@ impl Container { let mut remote = Attr::none(cx, "remote"); let mut field_identifier = BoolAttr::none(cx, "field_identifier"); let mut variant_identifier = BoolAttr::none(cx, "variant_identifier"); - let mut repr = Attr::none(cx, "repr"); for meta_items in item.attrs.iter().filter_map(get_serde_meta_items) { for meta_item in meta_items { @@ -264,16 +232,6 @@ impl Container { deny_unknown_fields.set_true(); } - // Parse `#[serde(repr = "foo")]` - Meta(NameValue(ref m)) if m.ident == "repr" => { - if let Ok(s) = get_lit_str(cx, m.ident.as_ref(), m.ident.as_ref(), &m.lit) { - match ContainerRepr::from_str(&s.value()) { - Ok(value) => repr.set(value), - Err(()) => cx.error(format!("unknown value for #[serde(repr = {})]", s.value())) - } - } - } - // Parse `#[serde(default)]` Meta(Word(word)) if word == "default" => match item.data { syn::Data::Struct(syn::DataStruct { fields: syn::Fields::Named(_), .. }) => { @@ -417,7 +375,6 @@ impl Container { type_into: type_into.get(), remote: remote.get(), identifier: decide_identifier(cx, item, &field_identifier, &variant_identifier), - repr: repr.get().unwrap_or(ContainerRepr::Auto), has_flatten: false, } } @@ -466,10 +423,6 @@ impl Container { self.identifier } - pub fn repr(&self) -> ContainerRepr { - self.repr - } - pub fn has_flatten(&self) -> bool { self.has_flatten } diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 5f6df68f..b5fbd0ab 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -96,7 +96,6 @@ where } #[derive(Debug, PartialEq, Serialize, Deserialize)] -#[serde(repr = "map")] struct CollectOther { a: u32, b: u32, @@ -105,7 +104,6 @@ struct CollectOther { } #[derive(Debug, PartialEq, Serialize, Deserialize)] -#[serde(repr = "map")] struct FlattenStructEnumWrapper { #[serde(flatten)] data: FlattenStructEnum, @@ -123,7 +121,6 @@ enum FlattenStructEnum { } #[derive(Debug, PartialEq, Serialize, Deserialize)] -#[serde(repr = "map")] struct FlattenStructTagContentEnumWrapper { outer: u32, #[serde(flatten)] @@ -1497,7 +1494,7 @@ fn test_flatten_struct_tag_content_enum_newtype() { #[test] fn test_unknown_field_in_flatten() { #[derive(Debug, PartialEq, Serialize, Deserialize)] - #[serde(repr = "map", deny_unknown_fields)] + #[serde(deny_unknown_fields)] struct Outer { dummy: String, #[serde(flatten)] From ad40f976db3090b7cc66f50df822be949e984917 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 18 Mar 2018 21:07:08 +0100 Subject: [PATCH 32/51] Switch to using Content keys internally for flattening to later support arbitrary keys --- serde/src/private/de.rs | 72 ++++++++++++++++++++--------------------- serde_derive/src/de.rs | 71 +++++++++++++++++++++++----------------- 2 files changed, 78 insertions(+), 65 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 7cb3152d..10c2709b 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -269,6 +269,14 @@ mod content { } impl<'de> Content<'de> { + pub fn as_str(&self) -> Option<&str> { + match *self { + Content::Str(x) => Some(x), + Content::String(ref x) => Some(x.as_str()), + _ => None, + } + } + fn unexpected(&self) -> Unexpected { match *self { Content::Bool(b) => Unexpected::Bool(b), @@ -2073,7 +2081,7 @@ where #[cfg(any(feature = "std", feature = "alloc"))] pub struct FlatMapDeserializer<'a, 'de: 'a, E>( - pub &'a mut Vec)>>, + pub &'a mut Vec, Content<'de>)>>, pub PhantomData ); @@ -2100,28 +2108,23 @@ impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> V: Visitor<'de>, { let mut iter = self.0.iter_mut(); - loop { - let item = match iter.next() { - Some(item) => { - if item.is_some() { - item - } else { - continue; - } - } - None => return Err(Error::custom(format_args!( - "no variant of enum {} not found in flattened data", - name - ))) - }; - - if !variants.contains(&item.as_ref().unwrap().0.as_str()) { + while let Some(item) = iter.next() { + if item.is_none() || !item.as_ref().unwrap().0.as_str() + .map(|x| variants.contains(&x)).unwrap_or(false) { continue; } let (key, value) = item.take().unwrap(); - return visitor.visit_enum(EnumDeserializer::new(Content::String(key), Some(value))); + return visitor.visit_enum(EnumDeserializer::new( + key, + Some(value) + )); } + + Err(Error::custom(format_args!( + "no variant of enum {} not found in flattened data", + name + ))) } fn deserialize_map(self, visitor: V) -> Result @@ -2163,7 +2166,7 @@ impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> #[cfg(any(feature = "std", feature = "alloc"))] pub struct FlatMapAccess<'a, 'de: 'a, E> { - iter: slice::IterMut<'a, Option<(String, Content<'de>)>>, + iter: slice::IterMut<'a, Option<(Content<'de>, Content<'de>)>>, pending_content: Option>, fields: Option<&'static [&'static str]>, _marker: PhantomData, @@ -2172,7 +2175,7 @@ pub struct FlatMapAccess<'a, 'de: 'a, E> { #[cfg(any(feature = "std", feature = "alloc"))] impl<'a, 'de, E> FlatMapAccess<'a, 'de, E> { fn new( - iter: slice::IterMut<'a, Option<(String, Content<'de>)>>, + iter: slice::IterMut<'a, Option<(Content<'de>, Content<'de>)>>, fields: Option<&'static [&'static str]> ) -> FlatMapAccess<'a, 'de, E> { FlatMapAccess { @@ -2194,30 +2197,27 @@ impl<'a, 'de, E> MapAccess<'de> for FlatMapAccess<'a, 'de, E> where T: DeserializeSeed<'de>, { - loop { - let item = match self.iter.next() { - Some(item) => { - if item.is_some() { - item - } else { - continue; - } - } - None => return Ok(None) + while let Some(item) = self.iter.next() { + if item.is_none() { + continue; }; - if match self.fields { - None => false, - Some(fields) if fields.contains(&item.as_ref().unwrap().0.as_str()) => false, - _ => true - } { + if !item.as_ref().unwrap().0.as_str() + .map(|key| { + match self.fields { + None => true, + Some(fields) if fields.contains(&key) => true, + _ => false + } + }).unwrap_or(true) { continue; } let (key, content) = item.take().unwrap(); self.pending_content = Some(content); - return seed.deserialize(ContentDeserializer::new(Content::String(key))).map(Some); + return seed.deserialize(ContentDeserializer::new(key)).map(Some); } + Ok(None) } fn next_value_seed(&mut self, seed: T) -> Result diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 490d25ff..ae7a9f97 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1042,7 +1042,6 @@ fn deserialize_externally_tagged_enum( &variant_names_idents, cattrs, true, - false, )); // Match arms to extract a variant from a string @@ -1142,7 +1141,6 @@ fn deserialize_internally_tagged_enum( &variant_names_idents, cattrs, true, - false, )); // Match arms to extract a variant from a string @@ -1212,7 +1210,6 @@ fn deserialize_adjacently_tagged_enum( &variant_names_idents, cattrs, true, - false, )); let variant_arms: &Vec<_> = &variants @@ -1712,22 +1709,22 @@ fn deserialize_untagged_newtype_variant( fn deserialize_generated_identifier( fields: &[(String, Ident)], cattrs: &attr::Container, - is_variant: bool, - struct_as_map_mode: bool, + is_variant: bool ) -> Fragment { let this = quote!(__Field); let field_idents: &Vec<_> = &fields.iter().map(|&(_, ref ident)| ident).collect(); - let (ignore_variant, fallthrough, want_value) = if cattrs.has_flatten() { - let ignore_variant = quote!(__other(String),); - let fallthrough = quote!(_serde::export::Ok(__Field::__other(__value.to_string()))); - (Some(ignore_variant), Some(fallthrough), true) + let (ignore_variant, fallthrough) = if cattrs.has_flatten() { + let ignore_variant = quote!(__other(_serde::private::de::Content<'de>),); + let fallthrough = quote!(_serde::export::Ok(__Field::__other( + _serde::private::de::Content::String(__value.to_string())))); + (Some(ignore_variant), Some(fallthrough)) } else if is_variant || cattrs.deny_unknown_fields() { - (None, None, false) + (None, None) } else { let ignore_variant = quote!(__ignore,); let fallthrough = quote!(_serde::export::Ok(__Field::__ignore)); - (Some(ignore_variant), Some(fallthrough), false) + (Some(ignore_variant), Some(fallthrough)) }; let visitor_impl = Stmts(deserialize_identifier( @@ -1735,13 +1732,18 @@ fn deserialize_generated_identifier( fields, is_variant, fallthrough, - struct_as_map_mode, - want_value, + cattrs.has_flatten(), )); + let lifetime = if cattrs.has_flatten() { + Some(quote!(<'de>)) + } else { + None + }; + quote_block! { #[allow(non_camel_case_types)] - enum __Field { + enum __Field #lifetime { #(#field_idents,)* #ignore_variant } @@ -1749,12 +1751,12 @@ fn deserialize_generated_identifier( struct __FieldVisitor; impl<'de> _serde::de::Visitor<'de> for __FieldVisitor { - type Value = __Field; + type Value = __Field #lifetime; #visitor_impl } - impl<'de> _serde::Deserialize<'de> for __Field { + impl<'de> _serde::Deserialize<'de> for __Field #lifetime { #[inline] fn deserialize<__D>(__deserializer: __D) -> _serde::export::Result where __D: _serde::Deserializer<'de> @@ -1836,7 +1838,6 @@ fn deserialize_custom_identifier( is_variant, fallthrough, false, - false, )); quote_block! { @@ -1866,8 +1867,7 @@ fn deserialize_identifier( fields: &[(String, Ident)], is_variant: bool, fallthrough: Option, - struct_as_map_mode: bool, - want_value: bool + collect_other_fields: bool ) -> Fragment { let field_strs = fields.iter().map(|&(ref name, _)| name); let field_bytes = fields.iter().map(|&(ref name, _)| Literal::byte_string(name.as_bytes())); @@ -1887,7 +1887,7 @@ fn deserialize_identifier( let variant_indices = 0u64..; let fallthrough_msg = format!("{} index 0 <= i < {}", index_expecting, fields.len()); - let visit_index = if struct_as_map_mode { + let visit_index = if collect_other_fields { None } else { Some(quote! { @@ -1906,7 +1906,7 @@ fn deserialize_identifier( }) }; - let bytes_to_str = if fallthrough.is_some() && !want_value { + let bytes_to_str = if fallthrough.is_some() && !collect_other_fields { None } else { let conversion = quote! { @@ -1981,7 +1981,7 @@ fn deserialize_struct_as_struct_visitor( } }; - let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, false); + let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false); let visit_map = deserialize_map(struct_path, params, fields, cattrs); @@ -2001,7 +2001,7 @@ fn deserialize_struct_as_map_visitor( .map(|(i, field)| (field.attrs.name().deserialize_name(), field_i(i))) .collect(); - let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, true); + let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false); let visit_map = deserialize_map(struct_path, params, fields, cattrs); @@ -2035,7 +2035,10 @@ fn deserialize_map( // Collect contents for flatten fields into a buffer let let_collect = if cattrs.has_flatten() { Some(quote! { - let mut __collect = Vec::>::new(); + let mut __collect = Vec::>::new(); }) } else { None @@ -2079,7 +2082,9 @@ fn deserialize_map( let ignored_arm = if cattrs.has_flatten() { Some(quote! { __Field::__other(__name) => { - __collect.push(Some((__name, try!(_serde::de::MapAccess::next_value(&mut __map))))); + __collect.push(Some(( + __name, + try!(_serde::de::MapAccess::next_value(&mut __map))))); } }) } else if cattrs.deny_unknown_fields() { @@ -2140,8 +2145,13 @@ fn deserialize_map( let collected_deny_unknown_fields = if cattrs.has_flatten() && cattrs.deny_unknown_fields() { Some(quote! { if let Some(Some((__key, _))) = __collect.into_iter().filter(|x| x.is_some()).next() { - return _serde::export::Err( - _serde::de::Error::custom(format_args!("unknown field `{}`", &__key))); + if let Some(__key) = __key.as_str() { + return _serde::export::Err( + _serde::de::Error::custom(format_args!("unknown field `{}`", &__key))); + } else { + return _serde::export::Err( + _serde::de::Error::custom(format_args!("unexpected map key"))); + } } }) } else { @@ -2219,7 +2229,7 @@ fn deserialize_struct_as_struct_in_place_visitor( } }; - let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false, false); + let field_visitor = deserialize_generated_identifier(&field_names_idents, cattrs, false); let visit_map = deserialize_map_in_place(params, fields, cattrs); @@ -2252,7 +2262,10 @@ fn deserialize_map_in_place( // Collect contents for flatten fields into a buffer let let_collect = if cattrs.has_flatten() { Some(quote! { - let mut __collect = Vec::>::new(); + let mut __collect = Vec::>::new(); }) } else { None From 205f606962c52b62a6de359ce14e07a290a71dc9 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 18 Mar 2018 22:59:27 +0100 Subject: [PATCH 33/51] Various clippy fixes --- serde/src/private/de.rs | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 10c2709b..7de0f04d 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2107,10 +2107,9 @@ impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> where V: Visitor<'de>, { - let mut iter = self.0.iter_mut(); - while let Some(item) = iter.next() { + for item in self.0.iter_mut() { if item.is_none() || !item.as_ref().unwrap().0.as_str() - .map(|x| variants.contains(&x)).unwrap_or(false) { + .map_or(false, |x| variants.contains(&x)) { continue; } @@ -2198,24 +2197,20 @@ impl<'a, 'de, E> MapAccess<'de> for FlatMapAccess<'a, 'de, E> T: DeserializeSeed<'de>, { while let Some(item) = self.iter.next() { - if item.is_none() { - continue; - }; - - if !item.as_ref().unwrap().0.as_str() - .map(|key| { + let use_item = item.is_some() && item.as_ref().unwrap().0.as_str() + .map_or(self.fields.is_none(), |key| { match self.fields { None => true, Some(fields) if fields.contains(&key) => true, _ => false } - }).unwrap_or(true) { - continue; - } + }); - let (key, content) = item.take().unwrap(); - self.pending_content = Some(content); - return seed.deserialize(ContentDeserializer::new(key)).map(Some); + if use_item { + let (key, content) = item.take().unwrap(); + self.pending_content = Some(content); + return seed.deserialize(ContentDeserializer::new(key)).map(Some); + } } Ok(None) } From c5a3128492325aba513e27ad7de6466d287bf548 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 18 Mar 2018 23:01:13 +0100 Subject: [PATCH 34/51] Added a more complex flattening test --- test_suite/tests/test_annotations.rs | 108 +++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index b5fbd0ab..2a81fbe2 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -1528,3 +1528,111 @@ fn test_unknown_field_in_flatten() { "unknown field `bar`", ); } + +#[test] +fn test_complex_flatten() { + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct Outer { + y: u32, + #[serde(flatten)] + first: First, + #[serde(flatten)] + second: Second, + z: u32 + } + + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct First { + a: u32, + b: bool, + c: Vec, + d: String, + e: Option, + } + + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct Second { + f: u32, + } + + assert_de_tokens( + &Outer { + y: 0, + first: First { + a: 1, + b: true, + c: vec!["a".into(), "b".into()], + d: "c".into(), + e: Some(2), + }, + second: Second { + f: 3 + }, + z: 4 + }, + &[ + Token::Map { len: None }, + Token::Str("y"), + Token::U32(0), + Token::Str("a"), + Token::U32(1), + Token::Str("b"), + Token::Bool(true), + Token::Str("c"), + Token::Seq { len: Some(2) }, + Token::Str("a"), + Token::Str("b"), + Token::SeqEnd, + Token::Str("d"), + Token::Str("c"), + Token::Str("e"), + Token::U64(2), + Token::Str("f"), + Token::U32(3), + Token::Str("z"), + Token::U32(4), + Token::MapEnd, + ], + ); + + assert_ser_tokens( + &Outer { + y: 0, + first: First { + a: 1, + b: true, + c: vec!["a".into(), "b".into()], + d: "c".into(), + e: Some(2), + }, + second: Second { + f: 3 + }, + z: 4 + }, + &[ + Token::Map { len: None }, + Token::Str("y"), + Token::U32(0), + Token::Str("a"), + Token::U32(1), + Token::Str("b"), + Token::Bool(true), + Token::Str("c"), + Token::Seq { len: Some(2) }, + Token::Str("a"), + Token::Str("b"), + Token::SeqEnd, + Token::Str("d"), + Token::Str("c"), + Token::Str("e"), + Token::Some, + Token::U64(2), + Token::Str("f"), + Token::U32(3), + Token::Str("z"), + Token::U32(4), + Token::MapEnd, + ], + ); +} From 7cf184624a5e5b048cdee22dc7dcd321f6971c31 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 18 Mar 2018 23:46:28 +0100 Subject: [PATCH 35/51] Use more consistent error messages for bad flattening --- serde/src/private/de.rs | 2 +- serde/src/private/ser.rs | 4 +-- test_suite/tests/test_annotations.rs | 38 ++++++++++++++++++++++++++-- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 7de0f04d..09d07af8 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2095,7 +2095,7 @@ impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> where V: Visitor<'de>, { - Err(Error::custom("can only flatten structs, maps and struct enum variants")) + Err(Error::custom("can only flatten structs and maps")) } fn deserialize_enum( diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index bf78ecf4..98dda162 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -1043,9 +1043,7 @@ where M: SerializeMap + 'a { fn bad_type(self, what: Unsupported) -> M::Error { - ser::Error::custom(format_args!( - "cannot flatten serialize {} values", what - )) + ser::Error::custom(format_args!("can only flatten structs and maps (got {})", what)) } } diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 2a81fbe2..1fd449d4 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -17,8 +17,8 @@ use self::serde::{Deserialize, Deserializer, Serialize, Serializer}; use self::serde::de::{self, Unexpected}; extern crate serde_test; -use self::serde_test::{assert_de_tokens, assert_de_tokens_error, assert_ser_tokens, assert_tokens, - Token}; +use self::serde_test::{assert_de_tokens, assert_de_tokens_error, assert_ser_tokens, + assert_ser_tokens_error, assert_tokens, Token}; trait MyDefault: Sized { fn my_default() -> Self; @@ -1636,3 +1636,37 @@ fn test_complex_flatten() { ], ); } + +#[test] +fn test_flatten_unsupported_type() { + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct Outer { + outer: String, + #[serde(flatten)] + inner: String, + } + + assert_ser_tokens_error( + &Outer { + outer: "foo".into(), + inner: "bar".into(), + }, + &[ + Token::Map { len: None }, + Token::Str("outer"), + Token::Str("foo"), + ], + "can only flatten structs and maps (got a string)", + ); + assert_de_tokens_error::( + &[ + Token::Map { len: None }, + Token::Str("outer"), + Token::Str("foo"), + Token::Str("a"), + Token::Str("b"), + Token::MapEnd + ], + "can only flatten structs and maps", + ); +} From f02dbf381b369bffc21909cf11181c490d30a318 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Mon, 19 Mar 2018 00:57:58 +0100 Subject: [PATCH 36/51] Added non string key support for flattening --- serde_derive/src/de.rs | 113 ++++++++++++++++++++++++--- serde_derive/src/lib.rs | 2 +- test_suite/tests/test_annotations.rs | 27 +++++++ 3 files changed, 131 insertions(+), 11 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index ae7a9f97..acc1086b 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1716,8 +1716,7 @@ fn deserialize_generated_identifier( let (ignore_variant, fallthrough) = if cattrs.has_flatten() { let ignore_variant = quote!(__other(_serde::private::de::Content<'de>),); - let fallthrough = quote!(_serde::export::Ok(__Field::__other( - _serde::private::de::Content::String(__value.to_string())))); + let fallthrough = quote!(_serde::export::Ok(__Field::__other(__value))); (Some(ignore_variant), Some(fallthrough)) } else if is_variant || cattrs.deny_unknown_fields() { (None, None) @@ -1887,8 +1886,86 @@ fn deserialize_identifier( let variant_indices = 0u64..; let fallthrough_msg = format!("{} index 0 <= i < {}", index_expecting, fields.len()); - let visit_index = if collect_other_fields { - None + let visit_other = if collect_other_fields { + Some(quote! { + fn visit_bool<__E>(self, __value: bool) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::Bool(__value))) + } + + fn visit_i8<__E>(self, __value: i8) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::I8(__value))) + } + + fn visit_i16<__E>(self, __value: i16) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::I16(__value))) + } + + fn visit_i32<__E>(self, __value: i32) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::I32(__value))) + } + + fn visit_i64<__E>(self, __value: i64) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::I64(__value))) + } + + fn visit_u8<__E>(self, __value: u8) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::U8(__value))) + } + + fn visit_u16<__E>(self, __value: u16) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::U16(__value))) + } + + fn visit_u32<__E>(self, __value: u32) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::U32(__value))) + } + + fn visit_u64<__E>(self, __value: u64) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::U64(__value))) + } + + fn visit_f32<__E>(self, __value: f32) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::F32(__value))) + } + + fn visit_f64<__E>(self, __value: f64) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::F64(__value))) + } + + fn visit_char<__E>(self, __value: char) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::Char(__value))) + } + + fn visit_unit<__E>(self) -> Result + where __E: _serde::de::Error + { + Ok(__Field::__other(_serde::private::de::Content::Unit)) + } + }) } else { Some(quote! { fn visit_u64<__E>(self, __value: u64) -> _serde::export::Result @@ -1906,13 +1983,25 @@ fn deserialize_identifier( }) }; - let bytes_to_str = if fallthrough.is_some() && !collect_other_fields { + let bytes_to_str = if fallthrough.is_some() || collect_other_fields { None } else { - let conversion = quote! { + Some(quote! { let __value = &_serde::export::from_utf8_lossy(__value); - }; - Some(conversion) + }) + }; + + let (value_as_str_content, value_as_bytes_content) = if !collect_other_fields { + (None, None) + } else { + ( + Some(quote! { + let __value = _serde::private::de::Content::String(__value.to_string()); + }), + Some(quote! { + let __value = _serde::private::de::Content::ByteBuf(__value.to_vec()); + }) + ) }; let fallthrough_arm = if let Some(fallthrough) = fallthrough { @@ -1932,7 +2021,7 @@ fn deserialize_identifier( _serde::export::Formatter::write_str(formatter, #expecting) } - #visit_index + #visit_other fn visit_str<__E>(self, __value: &str) -> _serde::export::Result where __E: _serde::de::Error @@ -1941,7 +2030,10 @@ fn deserialize_identifier( #( #field_strs => _serde::export::Ok(#constructors), )* - _ => #fallthrough_arm + _ => { + #value_as_str_content + #fallthrough_arm + } } } @@ -1954,6 +2046,7 @@ fn deserialize_identifier( )* _ => { #bytes_to_str + #value_as_bytes_content #fallthrough_arm } } diff --git a/serde_derive/src/lib.rs b/serde_derive/src/lib.rs index 5cc2e1f8..faf0f1af 100644 --- a/serde_derive/src/lib.rs +++ b/serde_derive/src/lib.rs @@ -26,7 +26,7 @@ #![cfg_attr(feature = "cargo-clippy", allow(enum_variant_names, redundant_field_names, too_many_arguments, used_underscore_binding))] // The `quote!` macro requires deep recursion. -#![recursion_limit = "256"] +#![recursion_limit = "512"] #[macro_use] extern crate quote; diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 1fd449d4..7ff073cf 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -1670,3 +1670,30 @@ fn test_flatten_unsupported_type() { "can only flatten structs and maps", ); } + +#[test] +fn test_non_string_keys() { + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct TestStruct { + name: String, + age: u32, + #[serde(flatten)] + mapping: HashMap, + } + + let mut mapping = HashMap::new(); + mapping.insert(0, 42); + assert_tokens( + &TestStruct { name: "peter".into(), age: 3, mapping }, + &[ + Token::Map { len: None }, + Token::Str("name"), + Token::Str("peter"), + Token::Str("age"), + Token::U32(3), + Token::U32(0), + Token::U32(42), + Token::MapEnd, + ], + ); +} From 7c596c71361391138e083884d5a4cc11f55b6905 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 13:11:17 +0100 Subject: [PATCH 37/51] Remove unnecessary as_str --- serde/src/private/de.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 09d07af8..13de204f 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -272,7 +272,7 @@ mod content { pub fn as_str(&self) -> Option<&str> { match *self { Content::Str(x) => Some(x), - Content::String(ref x) => Some(x.as_str()), + Content::String(ref x) => Some(x), _ => None, } } From 6e324e887de6103ae1a36eb620dda37b760caba1 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 13:20:56 +0100 Subject: [PATCH 38/51] Some refactoring to use a bit less unwrap() --- serde/src/private/de.rs | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 13de204f..8958c55e 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2108,16 +2108,18 @@ impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> V: Visitor<'de>, { for item in self.0.iter_mut() { - if item.is_none() || !item.as_ref().unwrap().0.as_str() - .map_or(false, |x| variants.contains(&x)) { - continue; - } + let use_item = match *item { + None => false, + Some((ref c, _)) => c.as_str().map_or(false, |x| variants.contains(&x)) + }; - let (key, value) = item.take().unwrap(); - return visitor.visit_enum(EnumDeserializer::new( - key, - Some(value) - )); + if use_item { + let (key, value) = item.take().unwrap(); + return visitor.visit_enum(EnumDeserializer::new( + key, + Some(value) + )); + } } Err(Error::custom(format_args!( @@ -2197,14 +2199,18 @@ impl<'a, 'de, E> MapAccess<'de> for FlatMapAccess<'a, 'de, E> T: DeserializeSeed<'de>, { while let Some(item) = self.iter.next() { - let use_item = item.is_some() && item.as_ref().unwrap().0.as_str() - .map_or(self.fields.is_none(), |key| { - match self.fields { - None => true, - Some(fields) if fields.contains(&key) => true, - _ => false - } - }); + let use_item = match *item { + None => false, + Some((ref c, _)) => { + c.as_str().map_or(self.fields.is_none(), |key| { + match self.fields { + None => true, + Some(fields) if fields.contains(&key) => true, + _ => false + } + }) + } + }; if use_item { let (key, content) = item.take().unwrap(); From abeea8914739f46dcace4797780fb25f8f0ed606 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 13:35:16 +0100 Subject: [PATCH 39/51] Fully qualify some calls in generated code and fix a bad comment --- serde_derive/src/ser.rs | 6 +++--- serde_derive_internals/src/attr.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 5df85d46..2b85c125 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -303,7 +303,7 @@ fn serialize_struct_as_map(params: &Parameters, fields: &[Field], cattrs: &attr: let let_mut = mut_if(serialized_fields.peek().is_some()); let len = if cattrs.has_flatten() { - quote!(None) + quote!(_serde::export::None) } else { let len = serialized_fields .map(|field| match field.attrs.skip_serializing_if() { @@ -315,7 +315,7 @@ fn serialize_struct_as_map(params: &Parameters, fields: &[Field], cattrs: &attr: } }) .fold(quote!(0), |sum, expr| quote!(#sum + #expr)); - quote!(Some(#len)) + quote!(_serde::export::Some(#len)) }; quote_block! { @@ -926,7 +926,7 @@ fn serialize_struct_visitor( let span = Span::def_site().located_at(field.original.span()); let ser = if field.attrs.flatten() { quote! { - try!((#field_expr).serialize(_serde::private::ser::FlatMapSerializer(&mut __serde_state))); + try!(_serde::Serialize::serialize(&#field_expr, _serde::private::ser::FlatMapSerializer(&mut __serde_state))); } } else { let func = struct_trait.serialize_field(span); diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs index caf885f3..9f3640aa 100644 --- a/serde_derive_internals/src/attr.rs +++ b/serde_derive_internals/src/attr.rs @@ -891,7 +891,7 @@ impl Field { } } - // Parse `#[serde(skip_deserializing)]` + // Parse `#[serde(flatten)]` Meta(Word(word)) if word == "flatten" => { flatten.set_true(); } From 8637dda60fa782b1168a652c501259feb3768410 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 13:38:08 +0100 Subject: [PATCH 40/51] Refactored a test --- test_suite/tests/test_annotations.rs | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 7ff073cf..531b4eec 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -1323,20 +1323,7 @@ fn test_from_into_traits() { fn test_collect_other() { let mut extra = HashMap::new(); extra.insert("c".into(), 3); - assert_de_tokens( - &CollectOther { a: 1, b: 2, extra: extra.clone() }, - &[ - Token::Map { len: None }, - Token::Str("a"), - Token::U32(1), - Token::Str("b"), - Token::U32(2), - Token::Str("c"), - Token::U32(3), - Token::MapEnd, - ], - ); - assert_ser_tokens( + assert_tokens( &CollectOther { a: 1, b: 2, extra }, &[ Token::Map { len: None }, From 5b884b5bf9f3bc35b112cacb774be95171676a58 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 13:38:22 +0100 Subject: [PATCH 41/51] Added some missing UFCs --- serde_derive/src/de.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index acc1086b..dae7176e 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -2491,7 +2491,9 @@ fn deserialize_map_in_place( let collected_deny_unknown_fields = if cattrs.has_flatten() && cattrs.deny_unknown_fields() { Some(quote! { - if let Some(Some((__key, _))) = __collect.into_iter().filter(|x| x.is_some()).next() { + if let _serde::export::Some(_serde::export::Some((__key, _))) = + __collect.into_iter().filter(|x| x.is_some()).next() + { return _serde::export::Err( _serde::de::Error::custom(format_args!("unknown field `{}`", &__key))); } From 50c636a923bef3e36d45764b15dfa78426e9dcab Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 13:43:23 +0100 Subject: [PATCH 42/51] Remove now dead as_map detection (can be cattrs.has_flatten) --- serde_derive/src/de.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index dae7176e..9d32e15d 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -770,7 +770,6 @@ fn deserialize_struct( untagged: &Untagged, ) -> Fragment { let is_enum = variant_ident.is_some(); - let as_map = deserializer.is_none() && !is_enum && cattrs.has_flatten(); let this = ¶ms.this; let (de_impl_generics, de_ty_generics, ty_generics, where_clause) = @@ -798,7 +797,7 @@ fn deserialize_struct( let visit_seq = Stmts(deserialize_seq(&type_path, params, fields, true, cattrs)); - let (field_visitor, fields_stmt, visit_map) = if as_map { + let (field_visitor, fields_stmt, visit_map) = if cattrs.has_flatten() { deserialize_struct_as_map_visitor(&type_path, params, fields, cattrs) } else { deserialize_struct_as_struct_visitor(&type_path, params, fields, cattrs) @@ -821,7 +820,7 @@ fn deserialize_struct( quote! { _serde::de::VariantAccess::struct_variant(__variant, FIELDS, #visitor_expr) } - } else if as_map { + } else if cattrs.has_flatten() { quote! { _serde::Deserializer::deserialize_map(__deserializer, #visitor_expr) } @@ -842,7 +841,7 @@ fn deserialize_struct( // untagged struct variants do not get a visit_seq method. The same applies to structs that // only have a map representation. let visit_seq = match *untagged { - Untagged::No if !as_map => Some(quote! { + Untagged::No if !cattrs.has_flatten() => Some(quote! { #[inline] fn visit_seq<__A>(self, #visitor_var: __A) -> _serde::export::Result where __A: _serde::de::SeqAccess<#delife> @@ -893,11 +892,10 @@ fn deserialize_struct_in_place( deserializer: Option, ) -> Option { let is_enum = variant_ident.is_some(); - let as_map = deserializer.is_none() && !is_enum && cattrs.has_flatten(); // for now we do not support in_place deserialization for structs that // are represented as map. - if as_map { + if cattrs.has_flatten() { return None; } From 695c3eedcbdbbb9449ebd092988ae5107f8aacfc Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 14:45:14 +0100 Subject: [PATCH 43/51] Do not imply flatten from skip_serialize --- serde_derive/src/de.rs | 30 ++++++++++++++++++++++-------- serde_derive_internals/src/attr.rs | 2 +- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 9d32e15d..59b7e9f2 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -349,6 +349,8 @@ fn deserialize_tuple( split_with_de_lifetime(params); let delife = params.borrowed.de_lifetime(); + debug_assert!(!cattrs.has_flatten()); + // If there are getters (implying private fields), construct the local type // and use an `Into` conversion to get the remote type. If there are no // getters then construct the target type directly. @@ -444,6 +446,8 @@ fn deserialize_tuple_in_place( split_with_de_lifetime(params); let delife = params.borrowed.de_lifetime(); + debug_assert!(!cattrs.has_flatten()); + let is_enum = variant_ident.is_some(); let expecting = match variant_ident { Some(variant_ident) => format!("tuple variant {}::{}", params.type_name(), variant_ident), @@ -526,6 +530,8 @@ fn deserialize_seq( ) -> Fragment { let vars = (0..fields.len()).map(field_i as fn(_) -> _); + // XXX: do we need an error for flattening here? + let deserialized_count = fields .iter() .filter(|field| !field.attrs.skip_deserializing()) @@ -617,6 +623,8 @@ fn deserialize_seq_in_place( ) -> Fragment { let vars = (0..fields.len()).map(field_i as fn(_) -> _); + // XXX: do we need an error for flattening here? + let deserialized_count = fields .iter() .filter(|field| !field.attrs.skip_deserializing()) @@ -831,7 +839,7 @@ fn deserialize_struct( } }; - let all_skipped = !cattrs.has_flatten() && fields.iter().all(|field| field.attrs.skip_deserializing()); + let all_skipped = fields.iter().all(|field| field.attrs.skip_deserializing()); let visitor_var = if all_skipped { quote!(_) } else { @@ -939,7 +947,7 @@ fn deserialize_struct_in_place( } }; - let all_skipped = !cattrs.has_flatten() && fields.iter().all(|field| field.attrs.skip_deserializing()); + let all_skipped = fields.iter().all(|field| field.attrs.skip_deserializing()); let visitor_var = if all_skipped { quote!(_) } else { @@ -2058,6 +2066,8 @@ fn deserialize_struct_as_struct_visitor( fields: &[Field], cattrs: &attr::Container, ) -> (Fragment, Option, Fragment) { + debug_assert!(!cattrs.has_flatten()); + let field_names_idents: Vec<_> = fields .iter() .enumerate() @@ -2088,7 +2098,7 @@ fn deserialize_struct_as_map_visitor( let field_names_idents: Vec<_> = fields .iter() .enumerate() - .filter(|&(_, field)| !field.attrs.skip_deserializing()) + .filter(|&(_, field)| !field.attrs.skip_deserializing() && !field.attrs.flatten()) .map(|(i, field)| (field.attrs.name().deserialize_name(), field_i(i))) .collect(); @@ -2115,7 +2125,7 @@ fn deserialize_map( // Declare each field that will be deserialized. let let_values = fields_names .iter() - .filter(|&&(field, _)| !field.attrs.skip_deserializing()) + .filter(|&&(field, _)| !field.attrs.skip_deserializing() && !field.attrs.flatten()) .map(|&(field, ref name)| { let field_ty = &field.ty; quote! { @@ -2138,7 +2148,7 @@ fn deserialize_map( // Match arms to extract a value for a field. let value_arms = fields_names .iter() - .filter(|&&(field, _)| !field.attrs.skip_deserializing()) + .filter(|&&(field, _)| !field.attrs.skip_deserializing() && !field.attrs.flatten()) .map(|&(field, ref name)| { let deser_name = field.attrs.name().deserialize_name(); @@ -2186,7 +2196,7 @@ fn deserialize_map( }) }; - let all_skipped = !cattrs.has_flatten() && fields.iter().all(|field| field.attrs.skip_deserializing()); + let all_skipped = fields.iter().all(|field| field.attrs.skip_deserializing()); let match_keys = if cattrs.deny_unknown_fields() && all_skipped { quote! { // FIXME: Once we drop support for Rust 1.15: @@ -2208,7 +2218,7 @@ fn deserialize_map( let extract_values = fields_names .iter() - .filter(|&&(field, _)| !field.attrs.skip_deserializing()) + .filter(|&&(field, _)| !field.attrs.skip_deserializing() && !field.attrs.flatten()) .map(|&(field, ref name)| { let missing_expr = Match(expr_is_missing(field, cattrs)); @@ -2251,7 +2261,7 @@ fn deserialize_map( let result = fields_names.iter().map(|&(field, ref name)| { let ident = field.ident.expect("struct contains unnamed fields"); - if field.attrs.skip_deserializing() && !field.attrs.flatten() { + if field.attrs.skip_deserializing() { let value = Expr(expr_is_missing(field, cattrs)); quote_spanned!(Span::call_site()=> #ident: #value) } else { @@ -2306,6 +2316,8 @@ fn deserialize_struct_as_struct_in_place_visitor( fields: &[Field], cattrs: &attr::Container, ) -> (Fragment, Option, Fragment) { + debug_assert!(!cattrs.has_flatten()); + let field_names_idents: Vec<_> = fields .iter() .enumerate() @@ -2333,6 +2345,8 @@ fn deserialize_map_in_place( fields: &[Field], cattrs: &attr::Container, ) -> Fragment { + debug_assert!(!cattrs.has_flatten()); + // Create the field names for the fields. let fields_names: Vec<_> = fields .iter() diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs index 9f3640aa..96e3a012 100644 --- a/serde_derive_internals/src/attr.rs +++ b/serde_derive_internals/src/attr.rs @@ -1010,7 +1010,7 @@ impl Field { } pub fn skip_deserializing(&self) -> bool { - self.skip_deserializing || self.flatten + self.skip_deserializing } pub fn skip_serializing_if(&self) -> Option<&syn::ExprPath> { From e4ef087735e312650aa381a76b7126a02572c2ed Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 15:19:36 +0100 Subject: [PATCH 44/51] Added support for borrowing when flattening --- serde_derive/src/de.rs | 42 ++++++++++++++- test_suite/tests/test_annotations.rs | 79 ++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 59b7e9f2..456773a8 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1875,7 +1875,9 @@ fn deserialize_identifier( collect_other_fields: bool ) -> Fragment { let field_strs = fields.iter().map(|&(ref name, _)| name); + let field_borrowed_strs = fields.iter().map(|&(ref name, _)| name); let field_bytes = fields.iter().map(|&(ref name, _)| Literal::byte_string(name.as_bytes())); + let field_borrowed_bytes = fields.iter().map(|&(ref name, _)| Literal::byte_string(name.as_bytes())); let constructors: &Vec<_> = &fields .iter() @@ -1997,15 +1999,22 @@ fn deserialize_identifier( }) }; - let (value_as_str_content, value_as_bytes_content) = if !collect_other_fields { - (None, None) + let (value_as_str_content, value_as_borrowed_str_content, + value_as_bytes_content, value_as_borrowed_bytes_content) = if !collect_other_fields { + (None, None, None, None) } else { ( Some(quote! { let __value = _serde::private::de::Content::String(__value.to_string()); }), + Some(quote! { + let __value = _serde::private::de::Content::Str(__value); + }), Some(quote! { let __value = _serde::private::de::Content::ByteBuf(__value.to_vec()); + }), + Some(quote! { + let __value = _serde::private::de::Content::Bytes(__value); }) ) }; @@ -2043,6 +2052,35 @@ fn deserialize_identifier( } } + fn visit_borrowed_str<__E>(self, __value: &'de str) -> _serde::export::Result + where __E: _serde::de::Error + { + match __value { + #( + #field_borrowed_strs => _serde::export::Ok(#constructors), + )* + _ => { + #value_as_borrowed_str_content + #fallthrough_arm + } + } + } + + fn visit_borrowed_bytes<__E>(self, __value: &'de [u8]) -> _serde::export::Result + where __E: _serde::de::Error + { + match __value { + #( + #field_borrowed_bytes => _serde::export::Ok(#constructors), + )* + _ => { + #bytes_to_str + #value_as_borrowed_bytes_content + #fallthrough_arm + } + } + } + fn visit_bytes<__E>(self, __value: &[u8]) -> _serde::export::Result where __E: _serde::de::Error { diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 531b4eec..9eccf697 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -1684,3 +1684,82 @@ fn test_non_string_keys() { ], ); } + +#[test] +fn test_lifetime_propagation_for_flatten() { + #[derive(Deserialize, Serialize, Debug, PartialEq)] + struct A { + #[serde(flatten)] + t: T, + } + + #[derive(Deserialize, Serialize, Debug, PartialEq)] + struct B<'a> { + #[serde(flatten, borrow)] + t: HashMap<&'a str, u32>, + } + + #[derive(Deserialize, Serialize, Debug, PartialEq)] + struct C<'a> { + #[serde(flatten, borrow)] + t: HashMap<&'a [u8], u32>, + } + + let mut owned_map = HashMap::new(); + owned_map.insert("x".to_string(), 42u32); + assert_tokens( + &A { t: owned_map }, + &[ + Token::Map { len: None }, + Token::Str("x"), + Token::U32(42), + Token::MapEnd, + ], + ); + + let mut borrowed_map = HashMap::new(); + borrowed_map.insert("x", 42u32); + assert_ser_tokens( + &B { t: borrowed_map.clone() }, + &[ + Token::Map { len: None }, + Token::BorrowedStr("x"), + Token::U32(42), + Token::MapEnd, + ], + ); + + assert_de_tokens( + &B { t: borrowed_map }, + &[ + Token::Map { len: None }, + Token::BorrowedStr("x"), + Token::U32(42), + Token::MapEnd, + ], + ); + + let mut borrowed_map = HashMap::new(); + borrowed_map.insert(&b"x"[..], 42u32); + assert_ser_tokens( + &C { t: borrowed_map.clone() }, + &[ + Token::Map { len: None }, + Token::Seq { len: Some(1) }, + Token::U8(120), + Token::SeqEnd, + Token::U32(42), + Token::MapEnd, + ], + ); + + assert_de_tokens( + &C { t: borrowed_map }, + &[ + Token::Map { len: None }, + Token::BorrowedBytes(b"x"), + Token::U32(42), + Token::MapEnd, + ], + ); +} From 1d92569abca0187aeb548feb8c5f02a9e1fd8e74 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 21:24:00 +0100 Subject: [PATCH 45/51] Added explanatory comment about fetching data from buffered content --- serde/src/private/de.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 8958c55e..6eae7311 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2108,6 +2108,9 @@ impl<'a, 'de, E> Deserializer<'de> for FlatMapDeserializer<'a, 'de, E> V: Visitor<'de>, { for item in self.0.iter_mut() { + // items in the vector are nulled out when used. So we can only use + // an item if it's still filled in and if the field is one we care + // about. let use_item = match *item { None => false, Some((ref c, _)) => c.as_str().map_or(false, |x| variants.contains(&x)) @@ -2199,6 +2202,9 @@ impl<'a, 'de, E> MapAccess<'de> for FlatMapAccess<'a, 'de, E> T: DeserializeSeed<'de>, { while let Some(item) = self.iter.next() { + // items in the vector are nulled out when used. So we can only use + // an item if it's still filled in and if the field is one we care + // about. In case we do not know which fields we want, we take them all. let use_item = match *item { None => false, Some((ref c, _)) => { From 96393bfcc7324eca877002157ca6f44d066013b6 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 21:48:25 +0100 Subject: [PATCH 46/51] Added checks for flatten attribute --- serde_derive_internals/src/check.rs | 35 +++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/serde_derive_internals/src/check.rs b/serde_derive_internals/src/check.rs index c3df861b..7afe42ca 100644 --- a/serde_derive_internals/src/check.rs +++ b/serde_derive_internals/src/check.rs @@ -14,6 +14,7 @@ use Ctxt; /// object. Simpler checks should happen when parsing and building the attrs. pub fn check(cx: &Ctxt, cont: &Container) { check_getter(cx, cont); + check_flatten(cx, cont); check_identifier(cx, cont); check_variant_skip_attrs(cx, cont); check_internal_tag_field_name_conflict(cx, cont); @@ -40,6 +41,40 @@ fn check_getter(cx: &Ctxt, cont: &Container) { } } +/// Flattening has some restrictions we can test. +fn check_flatten(cx: &Ctxt, cont: &Container) { + match cont.data { + Data::Enum(_) => { + if cont.attrs.has_flatten() { + cx.error("#[serde(flatten)] is not allowed in an enum"); + } + } + Data::Struct(_, _) => { + for field in cont.data.all_fields() { + if !field.attrs.flatten() { + continue; + } + if field.attrs.skip_serializing() { + cx.error( + "#[serde(flatten] can not be combined with \ + #[serde(skip_serializing)]" + ); + } else if field.attrs.skip_serializing_if().is_some() { + cx.error( + "#[serde(flatten] can not be combined with \ + #[serde(skip_serializing_if = \"...\")]" + ); + } else if field.attrs.skip_deserializing() { + cx.error( + "#[serde(flatten] can not be combined with \ + #[serde(skip_deserializing)]" + ); + } + } + } + } +} + /// The `other` attribute must be used at most once and it must be the last /// variant of an enum that has the `field_identifier` attribute. /// From bb2ecb3bc4c71dc2027859e8679a86a9aa48c978 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 22:04:12 +0100 Subject: [PATCH 47/51] Added compilefail tests for flatten conflicts --- .../conflict/flatten-skip-deserializing.rs | 24 +++++++++++++++++++ .../conflict/flatten-skip-serializing-if.rs | 24 +++++++++++++++++++ .../conflict/flatten-skip-serializing.rs | 24 +++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 test_suite/tests/compile-fail/conflict/flatten-skip-deserializing.rs create mode 100644 test_suite/tests/compile-fail/conflict/flatten-skip-serializing-if.rs create mode 100644 test_suite/tests/compile-fail/conflict/flatten-skip-serializing.rs diff --git a/test_suite/tests/compile-fail/conflict/flatten-skip-deserializing.rs b/test_suite/tests/compile-fail/conflict/flatten-skip-deserializing.rs new file mode 100644 index 00000000..3ef4d38f --- /dev/null +++ b/test_suite/tests/compile-fail/conflict/flatten-skip-deserializing.rs @@ -0,0 +1,24 @@ +// Copyright 2018 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Deserialize)] //~ ERROR: proc-macro derive panicked +//~^ HELP: #[serde(flatten] can not be combined with #[serde(skip_deserializing)] +struct Foo { + #[serde(flatten, skip_deserializing)] + other: Other, +} + +#[derive(Deserialize)] +struct Other { + x: u32 +} + +fn main() {} diff --git a/test_suite/tests/compile-fail/conflict/flatten-skip-serializing-if.rs b/test_suite/tests/compile-fail/conflict/flatten-skip-serializing-if.rs new file mode 100644 index 00000000..6a5fa9d6 --- /dev/null +++ b/test_suite/tests/compile-fail/conflict/flatten-skip-serializing-if.rs @@ -0,0 +1,24 @@ +// Copyright 2018 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +//~^ HELP: #[serde(flatten] can not be combined with #[serde(skip_serializing_if = "...")] +struct Foo { + #[serde(flatten, skip_serializing_if="Option::is_none")] + other: Option, +} + +#[derive(Serialize)] +struct Other { + x: u32 +} + +fn main() {} diff --git a/test_suite/tests/compile-fail/conflict/flatten-skip-serializing.rs b/test_suite/tests/compile-fail/conflict/flatten-skip-serializing.rs new file mode 100644 index 00000000..204f9bf3 --- /dev/null +++ b/test_suite/tests/compile-fail/conflict/flatten-skip-serializing.rs @@ -0,0 +1,24 @@ +// Copyright 2018 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +//~^ HELP: #[serde(flatten] can not be combined with #[serde(skip_serializing)] +struct Foo { + #[serde(flatten, skip_serializing)] + other: Other, +} + +#[derive(Serialize)] +struct Other { + x: u32 +} + +fn main() {} From 99614c72664d887ac586aa0d30831ff3e614f004 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 22:15:47 +0100 Subject: [PATCH 48/51] Added flatten on enum compile fail test --- serde_derive_internals/src/check.rs | 2 +- .../enum-representation/flatten-enum.rs | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 test_suite/tests/compile-fail/enum-representation/flatten-enum.rs diff --git a/serde_derive_internals/src/check.rs b/serde_derive_internals/src/check.rs index 7afe42ca..0ed8afe6 100644 --- a/serde_derive_internals/src/check.rs +++ b/serde_derive_internals/src/check.rs @@ -46,7 +46,7 @@ fn check_flatten(cx: &Ctxt, cont: &Container) { match cont.data { Data::Enum(_) => { if cont.attrs.has_flatten() { - cx.error("#[serde(flatten)] is not allowed in an enum"); + cx.error("#[serde(flatten)] is not supported on enums"); } } Data::Struct(_, _) => { diff --git a/test_suite/tests/compile-fail/enum-representation/flatten-enum.rs b/test_suite/tests/compile-fail/enum-representation/flatten-enum.rs new file mode 100644 index 00000000..87fba41c --- /dev/null +++ b/test_suite/tests/compile-fail/enum-representation/flatten-enum.rs @@ -0,0 +1,21 @@ +// Copyright 2018 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +//~^ HELP: #[serde(flatten] is not supported on enums +enum Foo { + #[serde(flatten)] + Foo { + x: u32, + } +} + +fn main() {} From 27f935f036f3c8bc97775574c7182ce8458ee2b5 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 23:05:05 +0100 Subject: [PATCH 49/51] Correctly serialize newtype variants for flatten --- serde/src/private/ser.rs | 5 ++-- test_suite/tests/test_annotations.rs | 34 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index 98dda162..8af1283a 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -1161,13 +1161,14 @@ impl<'a, M> Serializer for FlatMapSerializer<'a, M> self, _: &'static str, _: u32, - _: &'static str, + variant: &'static str, value: &T, ) -> Result where T: Serialize, { - value.serialize(self) + try!(self.0.serialize_key(variant)); + self.0.serialize_value(value) } fn serialize_seq(self, _: Option) -> Result { diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 9eccf697..6ce2c3ef 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -1763,3 +1763,37 @@ fn test_lifetime_propagation_for_flatten() { ], ); } + +#[test] +fn test_flatten_enum_newtype() { + #[derive(Serialize, Deserialize, PartialEq, Debug)] + struct S { + #[serde(flatten)] + flat: E, + } + + #[derive(Serialize, Deserialize, PartialEq, Debug)] + enum E { + Q(HashMap), + } + + let e = E::Q({ + let mut map = HashMap::new(); + map.insert("k".to_owned(), "v".to_owned()); + map + }); + let s = S { flat: e }; + + assert_tokens( + &s, + &[ + Token::Map { len: None }, + Token::Str("Q"), + Token::Map { len: Some(1) }, + Token::Str("k"), + Token::Str("v"), + Token::MapEnd, + Token::MapEnd, + ], + ); +} From 0fde3c2ee8cb1c4a6e6b81c34426169edf7fdffa Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 23:06:55 +0100 Subject: [PATCH 50/51] Fix a warning caused by no-default-features --- serde/src/private/ser.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index 8af1283a..5d743808 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -63,6 +63,7 @@ enum Unsupported { ByteArray, Optional, Unit, + #[cfg(any(feature = "std", feature = "alloc"))] UnitStruct, Sequence, Tuple, @@ -81,6 +82,7 @@ impl Display for Unsupported { Unsupported::ByteArray => formatter.write_str("a byte array"), Unsupported::Optional => formatter.write_str("an optional"), Unsupported::Unit => formatter.write_str("unit"), + #[cfg(any(feature = "std", feature = "alloc"))] Unsupported::UnitStruct => formatter.write_str("unit struct"), Unsupported::Sequence => formatter.write_str("a sequence"), Unsupported::Tuple => formatter.write_str("a tuple"), From 3d647f4063d67c461326521f06abe66697f6a7f3 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 20 Mar 2018 23:26:22 +0100 Subject: [PATCH 51/51] Fixed a compilefail test for flatten on enums --- serde_derive_internals/src/check.rs | 4 +--- .../tests/compile-fail/enum-representation/flatten-enum.rs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/serde_derive_internals/src/check.rs b/serde_derive_internals/src/check.rs index 0ed8afe6..dd37085d 100644 --- a/serde_derive_internals/src/check.rs +++ b/serde_derive_internals/src/check.rs @@ -45,9 +45,7 @@ fn check_getter(cx: &Ctxt, cont: &Container) { fn check_flatten(cx: &Ctxt, cont: &Container) { match cont.data { Data::Enum(_) => { - if cont.attrs.has_flatten() { - cx.error("#[serde(flatten)] is not supported on enums"); - } + debug_assert!(!cont.attrs.has_flatten()); } Data::Struct(_, _) => { for field in cont.data.all_fields() { diff --git a/test_suite/tests/compile-fail/enum-representation/flatten-enum.rs b/test_suite/tests/compile-fail/enum-representation/flatten-enum.rs index 87fba41c..8444860f 100644 --- a/test_suite/tests/compile-fail/enum-representation/flatten-enum.rs +++ b/test_suite/tests/compile-fail/enum-representation/flatten-enum.rs @@ -10,7 +10,7 @@ extern crate serde_derive; #[derive(Serialize)] //~ ERROR: proc-macro derive panicked -//~^ HELP: #[serde(flatten] is not supported on enums +//~^ HELP: unknown serde variant attribute `flatten` enum Foo { #[serde(flatten)] Foo {