Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Consistently handle numeric and string/bytes keys of field names #1914

Merged
merged 4 commits into from Jan 24, 2021
Merged

Consistently handle numeric and string/bytes keys of field names #1914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f6eb34a
Assert that numeric field identifiers correctly deserialized (now fai…
Mingun Oct 22, 2020
34de1e0
Implement IdentifierDeserializer for u64 instead of u32 because all i…
Mingun Oct 22, 2020
0737474
Allow field identifiers be any numbers if `#[serde(other)]` is used
Mingun Oct 22, 2020
e805717
Allow borrowed and owned strings and bytes and u8, u16, u64 for varia…
Mingun Oct 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions serde/src/private/de.rs
View file
Open in desktop
Expand Up @@ -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 = <u32 as IntoDeserializer<'de, E>>::Deserializer;
type Deserializer = <u64 as IntoDeserializer<'de, E>>::Deserializer;

fn from(self) -> Self::Deserializer {
self.into_deserializer()
Expand Down
37 changes: 31 additions & 6 deletions serde_derive/src/de.rs
View file
Open in desktop
Expand Up @@ -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],
Expand All @@ -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))
Expand All @@ -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| {
Expand Down Expand Up @@ -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! {
Expand All @@ -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<Self::Value, __E>
Expand Down Expand Up @@ -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
}
}
}
}
Expand Down
28 changes: 28 additions & 0 deletions serde_test/src/de.rs
View file
Open in desktop
Expand Up @@ -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))
Expand Down
10 changes: 2 additions & 8 deletions test_suite/tests/expand/de_enum.expanded.rs
View file
Open in desktop
Expand Up @@ -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>(
Expand Down Expand Up @@ -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>(
Expand Down
10 changes: 2 additions & 8 deletions test_suite/tests/expand/default_ty_param.expanded.rs
View file
Open in desktop
Expand Up @@ -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<Self::Value, __E>
Expand Down Expand Up @@ -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<Self::Value, __E>
Expand Down
5 changes: 1 addition & 4 deletions test_suite/tests/expand/generic_enum.expanded.rs
View file
Open in desktop
Expand Up @@ -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>(
Expand Down
10 changes: 2 additions & 8 deletions test_suite/tests/expand/generic_struct.expanded.rs
View file
Open in desktop
Expand Up @@ -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<Self::Value, __E>
Expand Down Expand Up @@ -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<Self::Value, __E>
Expand Down
10 changes: 2 additions & 8 deletions test_suite/tests/expand/lifetimes.expanded.rs
View file
Open in desktop
Expand Up @@ -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>(
Expand Down Expand Up @@ -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>(
Expand Down
10 changes: 2 additions & 8 deletions test_suite/tests/expand/named_map.expanded.rs
View file
Open in desktop
Expand Up @@ -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<Self::Value, __E>
Expand Down Expand Up @@ -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<Self::Value, __E>
Expand Down