From f6eb34a83059c35bb6c1e9104e39e7157dbe90bc Mon Sep 17 00:00:00 2001 From: Mingun Date: Thu, 22 Oct 2020 10:03:39 +0500 Subject: [PATCH 1/4] Assert that numeric field identifiers correctly deserialized (now failing) --- test_suite/tests/test_de.rs | 101 ++++++++++++++++++++++++++++ test_suite/tests/test_identifier.rs | 13 ++++ 2 files changed, 114 insertions(+) diff --git a/test_suite/tests/test_de.rs b/test_suite/tests/test_de.rs index 025d87b09..03b03941c 100644 --- a/test_suite/tests/test_de.rs +++ b/test_suite/tests/test_de.rs @@ -622,6 +622,24 @@ declare_tests! { Token::I32(2), Token::MapEnd, ], + Struct { a: 1, b: 2, c: 0 } => &[ + Token::Map { len: Some(3) }, + Token::U8(0), + Token::I32(1), + + Token::U8(1), + Token::I32(2), + Token::MapEnd, + ], + Struct { a: 1, b: 2, c: 0 } => &[ + Token::Map { len: Some(3) }, + Token::U16(0), + Token::I32(1), + + Token::U16(1), + Token::I32(2), + Token::MapEnd, + ], Struct { a: 1, b: 2, c: 0 } => &[ Token::Map { len: Some(3) }, Token::U32(0), @@ -631,6 +649,34 @@ declare_tests! { Token::I32(2), Token::MapEnd, ], + Struct { a: 1, b: 2, c: 0 } => &[ + Token::Map { len: Some(3) }, + Token::U64(0), + Token::I32(1), + + Token::U64(1), + Token::I32(2), + Token::MapEnd, + ], + // Mixed key types + Struct { a: 1, b: 2, c: 0 } => &[ + Token::Map { len: Some(3) }, + Token::U8(0), + Token::I32(1), + + Token::U64(1), + Token::I32(2), + Token::MapEnd, + ], + Struct { a: 1, b: 2, c: 0 } => &[ + Token::Map { len: Some(3) }, + Token::U8(0), + Token::I32(1), + + Token::Str("b"), + Token::I32(2), + Token::MapEnd, + ], Struct { a: 1, b: 2, c: 0 } => &[ Token::Struct { name: "Struct", len: 2 }, Token::Str("a"), @@ -663,6 +709,21 @@ declare_tests! { Token::I32(4), Token::MapEnd, ], + Struct { a: 1, b: 2, c: 0 } => &[ + Token::Map { len: Some(3) }, + Token::U8(0), + Token::I32(1), + + Token::U16(1), + Token::I32(2), + + Token::U32(2), + Token::I32(3), + + Token::U64(3), + Token::I32(4), + Token::MapEnd, + ], Struct { a: 1, b: 2, c: 0 } => &[ Token::Struct { name: "Struct", len: 2 }, Token::Str("a"), @@ -780,6 +841,26 @@ declare_tests! { Token::Str("Unit"), Token::Unit, ], + EnumOther::Unit => &[ + Token::Enum { name: "EnumOther" }, + Token::U8(0), + Token::Unit, + ], + EnumOther::Unit => &[ + Token::Enum { name: "EnumOther" }, + Token::U16(0), + Token::Unit, + ], + EnumOther::Unit => &[ + Token::Enum { name: "EnumOther" }, + Token::U32(0), + Token::Unit, + ], + EnumOther::Unit => &[ + Token::Enum { name: "EnumOther" }, + Token::U64(0), + Token::Unit, + ], } test_enum_other { EnumOther::Other => &[ @@ -787,6 +868,26 @@ declare_tests! { Token::Str("Foo"), Token::Unit, ], + EnumOther::Other => &[ + Token::Enum { name: "EnumOther" }, + Token::U8(42), + Token::Unit, + ], + EnumOther::Other => &[ + Token::Enum { name: "EnumOther" }, + Token::U16(42), + Token::Unit, + ], + EnumOther::Other => &[ + Token::Enum { name: "EnumOther" }, + Token::U32(42), + Token::Unit, + ], + EnumOther::Other => &[ + Token::Enum { name: "EnumOther" }, + Token::U64(42), + Token::Unit, + ], } test_box { Box::new(0i32) => &[Token::I32(0)], diff --git a/test_suite/tests/test_identifier.rs b/test_suite/tests/test_identifier.rs index eba7d4dac..12c6697e6 100644 --- a/test_suite/tests/test_identifier.rs +++ b/test_suite/tests/test_identifier.rs @@ -1,3 +1,4 @@ +//! Tests for `#[serde(field_identifier)]` and `#[serde(variant_identifier)]` use serde::Deserialize; use serde_test::{assert_de_tokens, Token}; @@ -27,6 +28,10 @@ fn test_field_identifier() { Bbb, } + assert_de_tokens(&F::Aaa, &[Token::U8(0)]); + assert_de_tokens(&F::Aaa, &[Token::U16(0)]); + assert_de_tokens(&F::Aaa, &[Token::U32(0)]); + assert_de_tokens(&F::Aaa, &[Token::U64(0)]); assert_de_tokens(&F::Aaa, &[Token::Str("aaa")]); assert_de_tokens(&F::Aaa, &[Token::Bytes(b"aaa")]); } @@ -42,6 +47,10 @@ fn test_unit_fallthrough() { Other, } + assert_de_tokens(&F::Other, &[Token::U8(42)]); + assert_de_tokens(&F::Other, &[Token::U16(42)]); + assert_de_tokens(&F::Other, &[Token::U32(42)]); + assert_de_tokens(&F::Other, &[Token::U64(42)]); assert_de_tokens(&F::Other, &[Token::Str("x")]); } @@ -68,5 +77,9 @@ fn test_newtype_fallthrough_generic() { Other(T), } + assert_de_tokens(&F::Other(42u8), &[Token::U8(42)]); + assert_de_tokens(&F::Other(42u16), &[Token::U16(42)]); + assert_de_tokens(&F::Other(42u32), &[Token::U32(42)]); + assert_de_tokens(&F::Other(42u64), &[Token::U64(42)]); assert_de_tokens(&F::Other("x".to_owned()), &[Token::Str("x")]); } From 34de1e00c85a9219f04a1259a0afa6e50e437f52 Mon Sep 17 00:00:00 2001 From: Mingun Date: Thu, 22 Oct 2020 10:10:32 +0500 Subject: [PATCH 2/4] Implement IdentifierDeserializer for u64 instead of u32 because all identifiers deserialized with visit_u64 --- 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 bcb964a9c..808d783d0 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2542,11 +2542,11 @@ pub trait IdentifierDeserializer<'de, E: Error> { fn from(self) -> Self::Deserializer; } -impl<'de, E> IdentifierDeserializer<'de, E> for u32 +impl<'de, E> IdentifierDeserializer<'de, E> for u64 where E: Error, { - type Deserializer = >::Deserializer; + type Deserializer = >::Deserializer; fn from(self) -> Self::Deserializer { self.into_deserializer() From 07374746405e350e2ddcf31d6711877d79e37183 Mon Sep 17 00:00:00 2001 From: Mingun Date: Thu, 22 Oct 2020 10:18:27 +0500 Subject: [PATCH 3/4] Allow field identifiers be any numbers if `#[serde(other)]` is used Thus behavior synchronized between string/bytes identifiers and numeric identifiers --- serde_derive/src/de.rs | 37 ++++++++++++++++--- test_suite/tests/expand/de_enum.expanded.rs | 10 +---- .../tests/expand/default_ty_param.expanded.rs | 10 +---- .../tests/expand/generic_enum.expanded.rs | 5 +-- .../tests/expand/generic_struct.expanded.rs | 10 +---- test_suite/tests/expand/lifetimes.expanded.rs | 10 +---- test_suite/tests/expand/named_map.expanded.rs | 10 +---- 7 files changed, 42 insertions(+), 50 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 1f5733a6d..3267a3e5f 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1940,6 +1940,15 @@ fn deserialize_generated_identifier( } } +/// Generates `Deserialize::deserialize` body for +/// ```ignore +/// #[serde(field_identifier)] +/// enum Field { +/// } +/// #[serde(variant_identifier)] +/// enum Variant { +/// } +/// ``` fn deserialize_custom_identifier( params: &Parameters, variants: &[Variant], @@ -1957,6 +1966,8 @@ fn deserialize_custom_identifier( let (ordinary, fallthrough) = if let Some(last) = variants.last() { let last_ident = &last.ident; if last.attrs.other() { + // Processes `#[serde(other)]` attribute. It always belongs to the last variant + // (checked in `check_identifier`), so all other are ordinal variants let ordinary = &variants[..variants.len() - 1]; let fallthrough = quote!(_serde::export::Ok(#this::#last_ident)); (ordinary, Some(fallthrough)) @@ -1976,6 +1987,10 @@ fn deserialize_custom_identifier( (variants, None) }; + // List of tuples: + // - field or variant name in the expected messages + // - information about field/variant + // - list of alternate names of the field/variant let names_idents: Vec<_> = ordinary .iter() .map(|variant| { @@ -2106,7 +2121,7 @@ fn deserialize_identifier( (None, None, None, None) }; - let fallthrough_arm = if let Some(fallthrough) = fallthrough { + let fallthrough_arm = if let Some(fallthrough) = fallthrough.clone() { fallthrough } else if is_variant { quote! { @@ -2118,8 +2133,19 @@ fn deserialize_identifier( } }; + let u64_fallthrough_arm = if let Some(fallthrough) = fallthrough { + fallthrough + } else { + let fallthrough_msg = format!("{} index 0 <= i < {}", index_expecting, fields.len()); + quote! { + _serde::export::Err(_serde::de::Error::invalid_value( + _serde::de::Unexpected::Unsigned(__value), + &#fallthrough_msg, + )) + } + }; + let variant_indices = 0_u64..; - let fallthrough_msg = format!("{} index 0 <= i < {}", index_expecting, fields.len()); let visit_other = if collect_other_fields { quote! { fn visit_bool<__E>(self, __value: bool) -> _serde::export::Result @@ -2254,10 +2280,9 @@ fn deserialize_identifier( #( #variant_indices => _serde::export::Ok(#main_constructors), )* - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &#fallthrough_msg, - )) + _ => { + #u64_fallthrough_arm + } } } } diff --git a/test_suite/tests/expand/de_enum.expanded.rs b/test_suite/tests/expand/de_enum.expanded.rs index a4ba24911..3fad0ded4 100644 --- a/test_suite/tests/expand/de_enum.expanded.rs +++ b/test_suite/tests/expand/de_enum.expanded.rs @@ -563,10 +563,7 @@ const _: () = { 1u64 => _serde::export::Ok(__Field::__field1), 2u64 => _serde::export::Ok(__Field::__field2), 3u64 => _serde::export::Ok(__Field::__field3), - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &"field index 0 <= i < 4", - )), + _ => _serde::export::Ok(__Field::__ignore), } } fn visit_str<__E>( @@ -1066,10 +1063,7 @@ const _: () = { 1u64 => _serde::export::Ok(__Field::__field1), 2u64 => _serde::export::Ok(__Field::__field2), 3u64 => _serde::export::Ok(__Field::__field3), - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &"field index 0 <= i < 4", - )), + _ => _serde::export::Ok(__Field::__ignore), } } fn visit_str<__E>( diff --git a/test_suite/tests/expand/default_ty_param.expanded.rs b/test_suite/tests/expand/default_ty_param.expanded.rs index 8aaa3ab43..ee0f0666f 100644 --- a/test_suite/tests/expand/default_ty_param.expanded.rs +++ b/test_suite/tests/expand/default_ty_param.expanded.rs @@ -74,10 +74,7 @@ const _: () = { { match __value { 0u64 => _serde::export::Ok(__Field::__field0), - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &"field index 0 <= i < 1", - )), + _ => _serde::export::Ok(__Field::__ignore), } } fn visit_str<__E>(self, __value: &str) -> _serde::export::Result @@ -250,10 +247,7 @@ const _: () = { { match __value { 0u64 => _serde::export::Ok(__Field::__field0), - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &"field index 0 <= i < 1", - )), + _ => _serde::export::Ok(__Field::__ignore), } } fn visit_str<__E>(self, __value: &str) -> _serde::export::Result diff --git a/test_suite/tests/expand/generic_enum.expanded.rs b/test_suite/tests/expand/generic_enum.expanded.rs index add7c510d..88f441b33 100644 --- a/test_suite/tests/expand/generic_enum.expanded.rs +++ b/test_suite/tests/expand/generic_enum.expanded.rs @@ -347,10 +347,7 @@ const _: () = { match __value { 0u64 => _serde::export::Ok(__Field::__field0), 1u64 => _serde::export::Ok(__Field::__field1), - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &"field index 0 <= i < 2", - )), + _ => _serde::export::Ok(__Field::__ignore), } } fn visit_str<__E>( diff --git a/test_suite/tests/expand/generic_struct.expanded.rs b/test_suite/tests/expand/generic_struct.expanded.rs index 59d2e51d7..ea3059de0 100644 --- a/test_suite/tests/expand/generic_struct.expanded.rs +++ b/test_suite/tests/expand/generic_struct.expanded.rs @@ -70,10 +70,7 @@ const _: () = { { match __value { 0u64 => _serde::export::Ok(__Field::__field0), - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &"field index 0 <= i < 1", - )), + _ => _serde::export::Ok(__Field::__ignore), } } fn visit_str<__E>(self, __value: &str) -> _serde::export::Result @@ -246,10 +243,7 @@ const _: () = { { match __value { 0u64 => _serde::export::Ok(__Field::__field0), - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &"field index 0 <= i < 1", - )), + _ => _serde::export::Ok(__Field::__ignore), } } fn visit_str<__E>(self, __value: &str) -> _serde::export::Result diff --git a/test_suite/tests/expand/lifetimes.expanded.rs b/test_suite/tests/expand/lifetimes.expanded.rs index 9840d4401..bab22ebe0 100644 --- a/test_suite/tests/expand/lifetimes.expanded.rs +++ b/test_suite/tests/expand/lifetimes.expanded.rs @@ -235,10 +235,7 @@ const _: () = { { match __value { 0u64 => _serde::export::Ok(__Field::__field0), - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &"field index 0 <= i < 1", - )), + _ => _serde::export::Ok(__Field::__ignore), } } fn visit_str<__E>( @@ -420,10 +417,7 @@ const _: () = { { match __value { 0u64 => _serde::export::Ok(__Field::__field0), - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &"field index 0 <= i < 1", - )), + _ => _serde::export::Ok(__Field::__ignore), } } fn visit_str<__E>( diff --git a/test_suite/tests/expand/named_map.expanded.rs b/test_suite/tests/expand/named_map.expanded.rs index ce43b6634..b35da6831 100644 --- a/test_suite/tests/expand/named_map.expanded.rs +++ b/test_suite/tests/expand/named_map.expanded.rs @@ -97,10 +97,7 @@ const _: () = { 0u64 => _serde::export::Ok(__Field::__field0), 1u64 => _serde::export::Ok(__Field::__field1), 2u64 => _serde::export::Ok(__Field::__field2), - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &"field index 0 <= i < 3", - )), + _ => _serde::export::Ok(__Field::__ignore), } } fn visit_str<__E>(self, __value: &str) -> _serde::export::Result @@ -373,10 +370,7 @@ const _: () = { 0u64 => _serde::export::Ok(__Field::__field0), 1u64 => _serde::export::Ok(__Field::__field1), 2u64 => _serde::export::Ok(__Field::__field2), - _ => _serde::export::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &"field index 0 <= i < 3", - )), + _ => _serde::export::Ok(__Field::__ignore), } } fn visit_str<__E>(self, __value: &str) -> _serde::export::Result From e80571751d52f33cee3ebc59110c1cea2d37371e Mon Sep 17 00:00:00 2001 From: Mingun Date: Thu, 22 Oct 2020 11:15:39 +0500 Subject: [PATCH 4/4] Allow borrowed and owned strings and bytes and u8, u16, u64 for variant keys in serde_test --- serde_test/src/de.rs | 28 ++++++++++++++++++++++++++ test_suite/tests/test_de.rs | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/serde_test/src/de.rs b/serde_test/src/de.rs index c9bcee5b3..5113b2910 100644 --- a/serde_test/src/de.rs +++ b/serde_test/src/de.rs @@ -168,14 +168,42 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> { self.next_token(); visitor.visit_str(variant) } + (Token::BorrowedStr(variant), Token::Unit) => { + self.next_token(); + visitor.visit_borrowed_str(variant) + } + (Token::String(variant), Token::Unit) => { + self.next_token(); + visitor.visit_string(variant.to_string()) + } (Token::Bytes(variant), Token::Unit) => { self.next_token(); visitor.visit_bytes(variant) } + (Token::BorrowedBytes(variant), Token::Unit) => { + self.next_token(); + visitor.visit_borrowed_bytes(variant) + } + (Token::ByteBuf(variant), Token::Unit) => { + self.next_token(); + visitor.visit_byte_buf(variant.to_vec()) + } + (Token::U8(variant), Token::Unit) => { + self.next_token(); + visitor.visit_u8(variant) + } + (Token::U16(variant), Token::Unit) => { + self.next_token(); + visitor.visit_u16(variant) + } (Token::U32(variant), Token::Unit) => { self.next_token(); visitor.visit_u32(variant) } + (Token::U64(variant), Token::Unit) => { + self.next_token(); + visitor.visit_u64(variant) + } (variant, Token::Unit) => unexpected!(variant), (variant, _) => { visitor.visit_map(EnumMapVisitor::new(self, variant, EnumFormat::Any)) diff --git a/test_suite/tests/test_de.rs b/test_suite/tests/test_de.rs index 03b03941c..3a41a6828 100644 --- a/test_suite/tests/test_de.rs +++ b/test_suite/tests/test_de.rs @@ -693,6 +693,46 @@ declare_tests! { Token::SeqEnd, ], } + test_struct_borrowed_keys { + Struct { a: 1, b: 2, c: 0 } => &[ + Token::Map { len: Some(3) }, + Token::BorrowedStr("a"), + Token::I32(1), + + Token::BorrowedStr("b"), + Token::I32(2), + Token::MapEnd, + ], + Struct { a: 1, b: 2, c: 0 } => &[ + Token::Struct { name: "Struct", len: 2 }, + Token::BorrowedStr("a"), + Token::I32(1), + + Token::BorrowedStr("b"), + Token::I32(2), + Token::StructEnd, + ], + } + test_struct_owned_keys { + Struct { a: 1, b: 2, c: 0 } => &[ + Token::Map { len: Some(3) }, + Token::String("a"), + Token::I32(1), + + Token::String("b"), + Token::I32(2), + Token::MapEnd, + ], + Struct { a: 1, b: 2, c: 0 } => &[ + Token::Struct { name: "Struct", len: 2 }, + Token::String("a"), + Token::I32(1), + + Token::String("b"), + Token::I32(2), + Token::StructEnd, + ], + } test_struct_with_skip { Struct { a: 1, b: 2, c: 0 } => &[ Token::Map { len: Some(3) },