From d2eea870012a0d04c5608d18bd0a156aac9b3a4a Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 11 Jan 2017 11:02:24 -0800 Subject: [PATCH 1/2] Handle various degenerate cases --- serde_codegen/src/de.rs | 39 ++++++++++++------- testing/tests/test_gen.rs | 79 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 13 deletions(-) diff --git a/serde_codegen/src/de.rs b/serde_codegen/src/de.rs index e75a7c96..af2af257 100644 --- a/serde_codegen/src/de.rs +++ b/serde_codegen/src/de.rs @@ -509,14 +509,6 @@ fn deserialize_item_enum( const VARIANTS: &'static [&'static str] = &[ #(#variant_names),* ]; }; - let ignored_arm = if item_attrs.deny_unknown_fields() { - None - } else { - Some(quote! { - __Field::__ignore => { Err(_serde::de::Error::end_of_stream()) } - }) - }; - // 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() { @@ -536,7 +528,20 @@ fn deserialize_item_enum( }; variant_arms.push(arm); } - variant_arms.extend(ignored_arm.into_iter()); + + let match_variant = if variant_arms.is_empty() { + // This is an empty enum like `enum Impossible {}` or an enum in which + // all variants have `#[serde(skip_deserializing)]`. + quote! { + visitor.visit_variant::<__Field>().map(|impossible| match impossible {}) + } + } else { + quote! { + match try!(visitor.visit_variant()) { + #(#variant_arms)* + } + } + }; let (visitor_item, visitor_ty, visitor_expr) = deserialize_visitor(impl_generics); @@ -551,9 +556,7 @@ fn deserialize_item_enum( fn visit_enum<__V>(&mut self, mut visitor: __V) -> ::std::result::Result<#ty, __V::Error> where __V: _serde::de::VariantVisitor, { - match try!(visitor.visit_variant()) { - #(#variant_arms)* - } + #match_variant } } @@ -646,7 +649,7 @@ fn deserialize_field_visitor( .map(|i| aster::id(format!("__field{}", i))) .collect(); - let ignore_variant = if item_attrs.deny_unknown_fields() { + let ignore_variant = if is_variant || item_attrs.deny_unknown_fields() { None } else { Some(quote!(__ignore,)) @@ -744,6 +747,16 @@ fn deserialize_map( fields: &[Field], item_attrs: &attr::Item, ) -> Tokens { + if fields.is_empty() && item_attrs.deny_unknown_fields() { + return quote! { + // 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 = fields.iter() .enumerate() diff --git a/testing/tests/test_gen.rs b/testing/tests/test_gen.rs index 660eb52e..24338312 100644 --- a/testing/tests/test_gen.rs +++ b/testing/tests/test_gen.rs @@ -208,6 +208,85 @@ fn test_gen() { struct NonAsciiIdents { σ: f64 } + + #[derive(Serialize, Deserialize)] + struct EmptyBraced {} + + #[derive(Serialize, Deserialize)] + #[serde(deny_unknown_fields)] + struct EmptyBracedDenyUnknown {} + + #[derive(Serialize, Deserialize)] + struct BracedSkipAll { + #[serde(skip_deserializing)] + f: u8, + } + + #[derive(Serialize, Deserialize)] + #[serde(deny_unknown_fields)] + struct BracedSkipAllDenyUnknown { + #[serde(skip_deserializing)] + f: u8, + } + + #[cfg(feature = "unstable-testing")] + #[cfg_attr(feature = "unstable-testing", derive(Serialize, Deserialize))] + struct EmptyTuple(); + + #[cfg(feature = "unstable-testing")] + #[cfg_attr(feature = "unstable-testing", derive(Serialize, Deserialize))] + #[serde(deny_unknown_fields)] + struct EmptyTupleDenyUnknown(); + + #[derive(Serialize, Deserialize)] + struct TupleSkipAll(#[serde(skip_deserializing)] u8); + + #[derive(Serialize, Deserialize)] + #[serde(deny_unknown_fields)] + struct TupleSkipAllDenyUnknown(#[serde(skip_deserializing)] u8); + + #[derive(Serialize, Deserialize)] + enum EmptyEnum {} + + #[derive(Serialize, Deserialize)] + #[serde(deny_unknown_fields)] + enum EmptyEnumDenyUnknown {} + + #[derive(Serialize, Deserialize)] + enum EnumSkipAll { + #[serde(skip_deserializing)] + #[allow(dead_code)] + Variant, + } + + #[cfg(feature = "unstable-testing")] + #[cfg_attr(feature = "unstable-testing", derive(Serialize, Deserialize))] + enum EmptyVariants { + Braced {}, + Tuple(), + BracedSkip { + #[serde(skip_deserializing)] + f: u8, + }, + TupleSkip(#[serde(skip_deserializing)] u8), + } + + #[cfg(feature = "unstable-testing")] + #[cfg_attr(feature = "unstable-testing", derive(Serialize, Deserialize))] + #[serde(deny_unknown_fields)] + enum EmptyVariantsDenyUnknown { + Braced {}, + Tuple(), + BracedSkip { + #[serde(skip_deserializing)] + f: u8, + }, + TupleSkip(#[serde(skip_deserializing)] u8), + } + + #[derive(Serialize, Deserialize)] + #[serde(deny_unknown_fields)] + struct UnitDenyUnknown; } ////////////////////////////////////////////////////////////////////////// From b01c23b5ee3ba96c286b22fb8f958ea6b2576fde Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 11 Jan 2017 11:34:47 -0800 Subject: [PATCH 2/2] Also provide a smarter alternative for the other impossible case --- serde_codegen/src/de.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/serde_codegen/src/de.rs b/serde_codegen/src/de.rs index af2af257..9e0b6bee 100644 --- a/serde_codegen/src/de.rs +++ b/serde_codegen/src/de.rs @@ -533,6 +533,9 @@ fn deserialize_item_enum( // This is an empty enum like `enum Impossible {}` or an enum in which // all variants have `#[serde(skip_deserializing)]`. quote! { + // FIXME: Once we drop support for Rust 1.15: + // let Err(err) = visitor.visit_variant::<__Field>(); + // Err(err) visitor.visit_variant::<__Field>().map(|impossible| match impossible {}) } } else { @@ -749,7 +752,7 @@ fn deserialize_map( ) -> Tokens { if fields.is_empty() && item_attrs.deny_unknown_fields() { return quote! { - // Once we drop support for Rust 1.15: + // 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());