diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index f3226f1c2..d01a5128a 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -2153,7 +2153,7 @@ fn deserialize_custom_identifier( }) .collect(); - let names = names_idents.iter().map(|(name, _, _)| name); + let names = names_idents.iter().flat_map(|(_, _, aliases)| aliases); let names_const = if fallthrough.is_some() { None @@ -2216,25 +2216,17 @@ fn deserialize_identifier( collect_other_fields: bool, expecting: Option<&str>, ) -> Fragment { - let mut flat_fields = Vec::new(); - for (_, ident, aliases) in fields { - flat_fields.extend(aliases.iter().map(|alias| (alias, ident))); - } - - let field_strs: &Vec<_> = &flat_fields.iter().map(|(name, _)| name).collect(); - let field_bytes: &Vec<_> = &flat_fields - .iter() - .map(|(name, _)| Literal::byte_string(name.as_bytes())) - .collect(); - - let constructors: &Vec<_> = &flat_fields - .iter() - .map(|(_, ident)| quote!(#this_value::#ident)) - .collect(); - let main_constructors: &Vec<_> = &fields - .iter() - .map(|(_, ident, _)| quote!(#this_value::#ident)) - .collect(); + let str_mapping = fields.iter().map(|(_, ident, aliases)| { + // `aliases` also contains a main name + quote!(#(#aliases)|* => _serde::__private::Ok(#this_value::#ident)) + }); + let bytes_mapping = fields.iter().map(|(_, ident, aliases)| { + // `aliases` also contains a main name + let aliases = aliases + .iter() + .map(|alias| Literal::byte_string(alias.as_bytes())); + quote!(#(#aliases)|* => _serde::__private::Ok(#this_value::#ident)) + }); let expecting = expecting.unwrap_or(if is_variant { "variant identifier" @@ -2242,8 +2234,6 @@ fn deserialize_identifier( "field identifier" }); - let index_expecting = if is_variant { "variant" } else { "field" }; - let bytes_to_str = if fallthrough.is_some() || collect_other_fields { None } else { @@ -2291,21 +2281,6 @@ fn deserialize_identifier( &fallthrough_arm_tokens }; - let u64_fallthrough_arm_tokens; - let u64_fallthrough_arm = if let Some(fallthrough) = &fallthrough { - fallthrough - } else { - let fallthrough_msg = format!("{} index 0 <= i < {}", index_expecting, fields.len()); - u64_fallthrough_arm_tokens = quote! { - _serde::__private::Err(_serde::de::Error::invalid_value( - _serde::de::Unexpected::Unsigned(__value), - &#fallthrough_msg, - )) - }; - &u64_fallthrough_arm_tokens - }; - - let variant_indices = 0_u64..; let visit_other = if collect_other_fields { quote! { fn visit_bool<__E>(self, __value: bool) -> _serde::__private::Result @@ -2400,15 +2375,33 @@ fn deserialize_identifier( } } } else { + let u64_mapping = fields.iter().enumerate().map(|(i, (_, ident, _))| { + let i = i as u64; + quote!(#i => _serde::__private::Ok(#this_value::#ident)) + }); + + let u64_fallthrough_arm_tokens; + let u64_fallthrough_arm = if let Some(fallthrough) = &fallthrough { + fallthrough + } else { + let index_expecting = if is_variant { "variant" } else { "field" }; + let fallthrough_msg = format!("{} index 0 <= i < {}", index_expecting, fields.len()); + u64_fallthrough_arm_tokens = quote! { + _serde::__private::Err(_serde::de::Error::invalid_value( + _serde::de::Unexpected::Unsigned(__value), + &#fallthrough_msg, + )) + }; + &u64_fallthrough_arm_tokens + }; + quote! { fn visit_u64<__E>(self, __value: u64) -> _serde::__private::Result where __E: _serde::de::Error, { match __value { - #( - #variant_indices => _serde::__private::Ok(#main_constructors), - )* + #(#u64_mapping,)* _ => #u64_fallthrough_arm, } } @@ -2416,6 +2409,8 @@ fn deserialize_identifier( }; let visit_borrowed = if fallthrough_borrowed.is_some() || collect_other_fields { + let str_mapping = str_mapping.clone(); + let bytes_mapping = bytes_mapping.clone(); let fallthrough_borrowed_arm = fallthrough_borrowed.as_ref().unwrap_or(fallthrough_arm); Some(quote! { fn visit_borrowed_str<__E>(self, __value: &'de str) -> _serde::__private::Result @@ -2423,9 +2418,7 @@ fn deserialize_identifier( __E: _serde::de::Error, { match __value { - #( - #field_strs => _serde::__private::Ok(#constructors), - )* + #(#str_mapping,)* _ => { #value_as_borrowed_str_content #fallthrough_borrowed_arm @@ -2438,9 +2431,7 @@ fn deserialize_identifier( __E: _serde::de::Error, { match __value { - #( - #field_bytes => _serde::__private::Ok(#constructors), - )* + #(#bytes_mapping,)* _ => { #bytes_to_str #value_as_borrowed_bytes_content @@ -2465,9 +2456,7 @@ fn deserialize_identifier( __E: _serde::de::Error, { match __value { - #( - #field_strs => _serde::__private::Ok(#constructors), - )* + #(#str_mapping,)* _ => { #value_as_str_content #fallthrough_arm @@ -2480,9 +2469,7 @@ fn deserialize_identifier( __E: _serde::de::Error, { match __value { - #( - #field_bytes => _serde::__private::Ok(#constructors), - )* + #(#bytes_mapping,)* _ => { #bytes_to_str #value_as_bytes_content diff --git a/serde_derive/src/internals/attr.rs b/serde_derive/src/internals/attr.rs index 42212a64d..1a8bd9d4d 100644 --- a/serde_derive/src/internals/attr.rs +++ b/serde_derive/src/internals/attr.rs @@ -184,12 +184,20 @@ impl Name { } fn deserialize_aliases(&self) -> Vec { - let mut aliases = self.deserialize_aliases.clone(); - let main_name = self.deserialize_name(); - if !aliases.contains(&main_name) { - aliases.push(main_name); + self.deserialize_aliases.clone() + } + + fn correct_aliases(&mut self) { + // `deserialize_aliases` got from a BTreeSet, so it sorted and does not + // contain duplicates. + // We cannot insert main name in `new` because rename_all rules not yet + // applied there. + match self.deserialize_aliases.binary_search(&self.deserialize) { + Ok(_) => {} // element already here + Err(pos) => self + .deserialize_aliases + .insert(pos, self.deserialize.clone()), } - aliases } } @@ -928,6 +936,7 @@ impl Variant { if !self.name.deserialize_renamed { self.name.deserialize = rules.deserialize.apply_to_variant(&self.name.deserialize); } + self.name.correct_aliases(); } pub fn rename_all_rules(&self) -> &RenameAllRules { @@ -1267,6 +1276,7 @@ impl Field { if !self.name.deserialize_renamed { self.name.deserialize = rules.deserialize.apply_to_field(&self.name.deserialize); } + self.name.correct_aliases(); } pub fn skip_serializing(&self) -> bool { diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 354f82c90..41cad2f3f 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -643,7 +643,7 @@ fn test_unknown_field_rename_struct() { Token::Str("a4"), Token::I32(3), ], - "unknown field `a4`, expected one of `a1`, `a3`, `a2`, `a5`, `a6`", + "unknown field `a4`, expected one of `a1`, `a2`, `a3`, `a5`, `a6`", ); } @@ -837,7 +837,7 @@ fn test_unknown_field_rename_enum() { Token::Str("d"), Token::I8(2), ], - "unknown field `d`, expected one of `a`, `c`, `b`, `e`, `f`", + "unknown field `d`, expected one of `a`, `b`, `c`, `e`, `f`", ); } diff --git a/test_suite/tests/test_identifier.rs b/test_suite/tests/test_identifier.rs index 6edb316cb..4e141414b 100644 --- a/test_suite/tests/test_identifier.rs +++ b/test_suite/tests/test_identifier.rs @@ -3,86 +3,186 @@ #![allow(clippy::derive_partial_eq_without_eq)] use serde_derive::Deserialize; -use serde_test::{assert_de_tokens, Token}; +use serde_test::{assert_de_tokens, assert_de_tokens_error, Token}; + +mod variant_identifier { + use super::*; -#[test] -fn test_variant_identifier() { #[derive(Deserialize, Debug, PartialEq)] #[serde(variant_identifier)] enum V { Aaa, + #[serde(alias = "Ccc", alias = "Ddd")] Bbb, } - assert_de_tokens(&V::Aaa, &[Token::U8(0)]); - assert_de_tokens(&V::Aaa, &[Token::U16(0)]); - assert_de_tokens(&V::Aaa, &[Token::U32(0)]); - assert_de_tokens(&V::Aaa, &[Token::U64(0)]); - assert_de_tokens(&V::Aaa, &[Token::Str("Aaa")]); - assert_de_tokens(&V::Aaa, &[Token::Bytes(b"Aaa")]); -} + #[test] + fn variant1() { + assert_de_tokens(&V::Aaa, &[Token::U8(0)]); + assert_de_tokens(&V::Aaa, &[Token::U16(0)]); + assert_de_tokens(&V::Aaa, &[Token::U32(0)]); + assert_de_tokens(&V::Aaa, &[Token::U64(0)]); + assert_de_tokens(&V::Aaa, &[Token::Str("Aaa")]); + assert_de_tokens(&V::Aaa, &[Token::Bytes(b"Aaa")]); + } -#[test] -fn test_field_identifier() { - #[derive(Deserialize, Debug, PartialEq)] - #[serde(field_identifier, rename_all = "snake_case")] - enum F { - Aaa, - Bbb, + #[test] + fn aliases() { + assert_de_tokens(&V::Bbb, &[Token::U8(1)]); + assert_de_tokens(&V::Bbb, &[Token::U16(1)]); + assert_de_tokens(&V::Bbb, &[Token::U32(1)]); + assert_de_tokens(&V::Bbb, &[Token::U64(1)]); + + assert_de_tokens(&V::Bbb, &[Token::Str("Bbb")]); + assert_de_tokens(&V::Bbb, &[Token::Bytes(b"Bbb")]); + + assert_de_tokens(&V::Bbb, &[Token::Str("Ccc")]); + assert_de_tokens(&V::Bbb, &[Token::Bytes(b"Ccc")]); + + assert_de_tokens(&V::Bbb, &[Token::Str("Ddd")]); + assert_de_tokens(&V::Bbb, &[Token::Bytes(b"Ddd")]); } - 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")]); + #[test] + fn unknown() { + assert_de_tokens_error::( + &[Token::U8(42)], + "invalid value: integer `42`, expected variant index 0 <= i < 2", + ); + assert_de_tokens_error::( + &[Token::U16(42)], + "invalid value: integer `42`, expected variant index 0 <= i < 2", + ); + assert_de_tokens_error::( + &[Token::U32(42)], + "invalid value: integer `42`, expected variant index 0 <= i < 2", + ); + assert_de_tokens_error::( + &[Token::U64(42)], + "invalid value: integer `42`, expected variant index 0 <= i < 2", + ); + assert_de_tokens_error::( + &[Token::Str("Unknown")], + "unknown variant `Unknown`, expected one of `Aaa`, `Bbb`, `Ccc`, `Ddd`", + ); + assert_de_tokens_error::( + &[Token::Bytes(b"Unknown")], + "unknown variant `Unknown`, expected one of `Aaa`, `Bbb`, `Ccc`, `Ddd`", + ); + } } -#[test] -fn test_unit_fallthrough() { +mod field_identifier { + use super::*; + #[derive(Deserialize, Debug, PartialEq)] #[serde(field_identifier, rename_all = "snake_case")] enum F { Aaa, + #[serde(alias = "ccc", alias = "ddd")] Bbb, - #[serde(other)] - 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")]); -} + #[test] + fn field1() { + 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")]); + } -#[test] -fn test_newtype_fallthrough() { - #[derive(Deserialize, Debug, PartialEq)] - #[serde(field_identifier, rename_all = "snake_case")] - enum F { - Aaa, - Bbb, - Other(String), + #[test] + fn aliases() { + assert_de_tokens(&F::Bbb, &[Token::U8(1)]); + assert_de_tokens(&F::Bbb, &[Token::U16(1)]); + assert_de_tokens(&F::Bbb, &[Token::U32(1)]); + assert_de_tokens(&F::Bbb, &[Token::U64(1)]); + + assert_de_tokens(&F::Bbb, &[Token::Str("bbb")]); + assert_de_tokens(&F::Bbb, &[Token::Bytes(b"bbb")]); + + assert_de_tokens(&F::Bbb, &[Token::Str("ccc")]); + assert_de_tokens(&F::Bbb, &[Token::Bytes(b"ccc")]); + + assert_de_tokens(&F::Bbb, &[Token::Str("ddd")]); + assert_de_tokens(&F::Bbb, &[Token::Bytes(b"ddd")]); } - assert_de_tokens(&F::Other("x".to_owned()), &[Token::Str("x")]); -} + #[test] + fn unknown() { + assert_de_tokens_error::( + &[Token::U8(42)], + "invalid value: integer `42`, expected field index 0 <= i < 2", + ); + assert_de_tokens_error::( + &[Token::U16(42)], + "invalid value: integer `42`, expected field index 0 <= i < 2", + ); + assert_de_tokens_error::( + &[Token::U32(42)], + "invalid value: integer `42`, expected field index 0 <= i < 2", + ); + assert_de_tokens_error::( + &[Token::U64(42)], + "invalid value: integer `42`, expected field index 0 <= i < 2", + ); + assert_de_tokens_error::( + &[Token::Str("unknown")], + "unknown field `unknown`, expected one of `aaa`, `bbb`, `ccc`, `ddd`", + ); + assert_de_tokens_error::( + &[Token::Bytes(b"unknown")], + "unknown field `unknown`, expected one of `aaa`, `bbb`, `ccc`, `ddd`", + ); + } -#[test] -fn test_newtype_fallthrough_generic() { - #[derive(Deserialize, Debug, PartialEq)] - #[serde(field_identifier, rename_all = "snake_case")] - enum F { - Aaa, - Bbb, - Other(T), + #[test] + fn unit_fallthrough() { + #[derive(Deserialize, Debug, PartialEq)] + #[serde(field_identifier, rename_all = "snake_case")] + enum F { + Aaa, + Bbb, + #[serde(other)] + 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")]); + } + + #[test] + fn newtype_fallthrough() { + #[derive(Deserialize, Debug, PartialEq)] + #[serde(field_identifier, rename_all = "snake_case")] + enum F { + Aaa, + Bbb, + Other(String), + } + + assert_de_tokens(&F::Other("x".to_owned()), &[Token::Str("x")]); } - 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")]); + #[test] + fn newtype_fallthrough_generic() { + #[derive(Deserialize, Debug, PartialEq)] + #[serde(field_identifier, rename_all = "snake_case")] + enum F { + Aaa, + Bbb, + 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")]); + } }