diff --git a/serde_codegen/src/de.rs b/serde_codegen/src/de.rs index b2d64a9b..ed4f9e0a 100644 --- a/serde_codegen/src/de.rs +++ b/serde_codegen/src/de.rs @@ -480,11 +480,14 @@ fn deserialize_item_enum( let type_name = item_attrs.name().deserialize_name(); + let variant_names_idents = variants.iter() + .enumerate() + .filter(|&(_, variant)| !variant.attrs.skip_deserializing()) + .map(|(i, variant)| (variant.attrs.name().deserialize_name(), field_i(i))) + .collect(); + let variant_visitor = deserialize_field_visitor( - variants.iter() - .filter(|variant| !variant.attrs.skip_deserializing()) - .map(|variant| variant.attrs.name().deserialize_name()) - .collect(), + variant_names_idents, item_attrs, true, ); @@ -496,25 +499,27 @@ fn deserialize_item_enum( }; // Match arms to extract a variant from a string - let mut variant_arms = vec![]; - for (i, variant) in variants.iter().filter(|variant| !variant.attrs.skip_deserializing()).enumerate() { - let variant_name = field_i(i); + let variant_arms = variants.iter() + .enumerate() + .filter(|&(_, variant)| !variant.attrs.skip_deserializing()) + .map(|(i, variant)| { + let variant_name = field_i(i); - let block = deserialize_variant( - type_ident, - impl_generics, - ty.clone(), - variant, - item_attrs, - ); + let block = deserialize_variant( + type_ident, + impl_generics, + ty.clone(), + variant, + item_attrs, + ); - let arm = quote! { - __Field::#variant_name => #block - }; - variant_arms.push(arm); - } + quote! { + __Field::#variant_name => #block + } + }); - let match_variant = if variant_arms.is_empty() { + let all_skipped = variants.iter().all(|variant| variant.attrs.skip_deserializing()); + let match_variant = if all_skipped { // This is an empty enum like `enum Impossible {}` or an enum in which // all variants have `#[serde(skip_deserializing)]`. quote! { @@ -628,12 +633,12 @@ fn deserialize_newtype_variant( } fn deserialize_field_visitor( - field_names: Vec, + fields: Vec<(String, Ident)>, item_attrs: &attr::Item, is_variant: bool, ) -> Tokens { - // Create the field names for the fields. - let field_idents: &Vec<_> = &(0 .. field_names.len()).map(field_i).collect(); + let field_names = fields.iter().map(|&(ref name, _)| name); + let field_idents: &Vec<_> = &fields.iter().map(|&(_, ref ident)| ident).collect(); let ignore_variant = if is_variant || item_attrs.deny_unknown_fields() { None @@ -697,13 +702,14 @@ fn deserialize_struct_visitor( fields: &[Field], item_attrs: &attr::Item, ) -> (Tokens, Tokens, Tokens) { - let field_exprs: Vec<_> = fields.iter() - .map(|field| field.attrs.name().deserialize_name()) + let field_names_idents = fields.iter() + .enumerate() + .filter(|&(_, field)| !field.attrs.skip_deserializing()) + .map(|(i, field)| (field.attrs.name().deserialize_name(), field_i(i))) .collect(); - let field_names = field_exprs.clone(); let field_visitor = deserialize_field_visitor( - field_exprs, + field_names_idents, item_attrs, false, ); @@ -716,6 +722,7 @@ fn deserialize_struct_visitor( item_attrs, ); + let field_names = fields.iter().map(|field| field.attrs.name().deserialize_name()); let fields_stmt = quote! { const FIELDS: &'static [&'static str] = &[ #(#field_names),* ]; }; @@ -730,16 +737,6 @@ fn deserialize_map( fields: &[Field], item_attrs: &attr::Item, ) -> Tokens { - if fields.is_empty() && item_attrs.deny_unknown_fields() { - return quote! { - // FIXME: Once we drop support for Rust 1.15: - // let None::<__Field> = try!(visitor.visit_key()); - try!(visitor.visit_key::<__Field>()).map(|impossible| match impossible {}); - try!(visitor.end()); - Ok(#struct_path {}) - }; - } - // Create the field names for the fields. let fields_names: Vec<_> = fields.iter() .enumerate() @@ -789,18 +786,6 @@ fn deserialize_map( } }); - // Match arms to ignore value for fields that have `skip_deserializing`. - // Ignored even if `deny_unknown_fields` is set. - let skipped_arms = fields_names.iter() - .filter(|&&(field, _)| field.attrs.skip_deserializing()) - .map(|&(_, ref name)| { - quote! { - __Field::#name => { - let _ = try!(visitor.visit_value::<_serde::de::impls::IgnoredAny>()); - } - } - }); - // Visit ignored values to consume them let ignored_arm = if item_attrs.deny_unknown_fields() { None @@ -810,6 +795,24 @@ fn deserialize_map( }) }; + let all_skipped = fields.iter().all(|field| field.attrs.skip_deserializing()); + let match_keys = if item_attrs.deny_unknown_fields() && all_skipped { + quote! { + // FIXME: Once we drop support for Rust 1.15: + // let None::<__Field> = try!(visitor.visit_key()); + try!(visitor.visit_key::<__Field>()).map(|impossible| match impossible {}); + } + } else { + quote! { + while let Some(key) = try!(visitor.visit_key::<__Field>()) { + match key { + #(#value_arms)* + #ignored_arm + } + } + } + }; + let extract_values = fields_names.iter() .filter(|&&(field, _)| !field.attrs.skip_deserializing()) .map(|&(field, ref name)| { @@ -837,13 +840,7 @@ fn deserialize_map( quote! { #(#let_values)* - while let Some(key) = try!(visitor.visit_key::<__Field>()) { - match key { - #(#value_arms)* - #(#skipped_arms)* - #ignored_arm - } - } + #match_keys try!(visitor.end()); diff --git a/testing/tests/test_de.rs b/testing/tests/test_de.rs index d1879fe8..3359492e 100644 --- a/testing/tests/test_de.rs +++ b/testing/tests/test_de.rs @@ -32,12 +32,40 @@ struct Struct { c: i32, } +#[derive(PartialEq, Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct StructDenyUnknown { + a: i32, + #[serde(skip_deserializing)] + b: i32, +} + +#[derive(PartialEq, Debug, Deserialize)] +struct StructSkipAll { + #[serde(skip_deserializing)] + a: i32, +} + +#[derive(PartialEq, Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct StructSkipAllDenyUnknown { + #[serde(skip_deserializing)] + a: i32, +} + #[derive(PartialEq, Debug, Deserialize)] enum Enum { + #[allow(dead_code)] + #[serde(skip_deserializing)] + Skipped, Unit, Simple(i32), Seq(i32, i32, i32), Map { a: i32, b: i32, c: i32 }, +} + +#[derive(PartialEq, Debug, Deserialize)] +enum EnumSkipAll { #[allow(dead_code)] #[serde(skip_deserializing)] Skipped, @@ -681,6 +709,29 @@ declare_tests! { Token::StructEnd, ], } + test_struct_skip_all { + StructSkipAll { a: 0 } => &[ + Token::StructStart("StructSkipAll", 0), + Token::StructEnd, + ], + StructSkipAll { a: 0 } => &[ + Token::StructStart("StructSkipAll", 1), + Token::StructSep, + Token::Str("a"), + Token::I32(1), + + Token::StructSep, + Token::Str("b"), + Token::I32(2), + Token::StructEnd, + ], + } + test_struct_skip_all_deny_unknown { + StructSkipAllDenyUnknown { a: 0 } => &[ + Token::StructStart("StructSkipAllDenyUnknown", 0), + Token::StructEnd, + ], + } test_enum_unit { Enum::Unit => &[ Token::EnumUnit("Enum", "Unit"), @@ -788,6 +839,34 @@ fn test_net_ipaddr() { } declare_error_tests! { + test_unknown_field { + &[ + Token::StructStart("StructDenyUnknown", 2), + Token::StructSep, + Token::Str("a"), + Token::I32(0), + + Token::StructSep, + Token::Str("d"), + ], + Error::UnknownField("d".to_owned()), + } + test_skipped_field_is_unknown { + &[ + Token::StructStart("StructDenyUnknown", 2), + Token::StructSep, + Token::Str("b"), + ], + Error::UnknownField("b".to_owned()), + } + test_skip_all_deny_unknown { + &[ + Token::StructStart("StructSkipAllDenyUnknown", 1), + Token::StructSep, + Token::Str("a"), + ], + Error::UnknownField("a".to_owned()), + } test_unknown_variant { &[ Token::EnumUnit("Enum", "Foo"), @@ -800,6 +879,12 @@ declare_error_tests! { ], Error::UnknownVariant("Skipped".to_owned()), } + test_enum_skip_all { + &[ + Token::EnumUnit("EnumSkipAll", "Skipped"), + ], + Error::UnknownVariant("Skipped".to_owned()), + } test_struct_seq_too_long { &[ Token::SeqStart(Some(4)),