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

Fix deserialization of empty structs and tuples in untagged enums #2805

Merged
merged 9 commits into from
Aug 24, 2024
Merged

Fix deserialization of empty structs and tuples in untagged enums #2805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
98fb7d9
Move all untagged enum tests (except flatten) into a dedicated module
Mingun Aug 16, 2024
8514f41
Remove unnecessary generics
Mingun Aug 17, 2024
2dddc77
Cover ContentRefDeserializer::deserialize_option
Mingun Aug 17, 2024
171c6da
Complete coverage of ContentRefDeserializer::deserialize_newtype_struct
Mingun Aug 17, 2024
0093f74
Split test newtype_enum into four tests for each variant
Mingun Aug 17, 2024
6588b0a
Cover Content::Seq case in VariantRefDeserializer::struct_variant
Mingun Aug 17, 2024
4c5fec1
Test special cases that reaches SeqRefDeserializer::deserialize_any l…
Mingun Aug 17, 2024
da7fc79
Fix deserialization of empty struct variant in untagged enums
Mingun Aug 17, 2024
7bde100
Replace MapRefDeserializer with value::MapDeserializer
Mingun Aug 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 37 additions & 161 deletions serde/src/private/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1898,10 +1898,17 @@ mod content {
where
V: Visitor<'de>,
{
// Covered by tests/test_enum_untagged.rs
// with_optional_field::*
match *self.content {
Content::None => visitor.visit_none(),
Content::Some(ref v) => visitor.visit_some(ContentRefDeserializer::new(v)),
Content::Unit => visitor.visit_unit(),
// This case is necessary for formats which does not store marker of optionality of value,
// for example, JSON. When `deserialize_any` is requested from such formats, they will
// report value without using `Visitor::visit_some`, because they do not known in which
// contexts this value will be used.
// RON is example of format which preserve markers.
_ => visitor.visit_some(self),
}
}
Expand Down Expand Up @@ -1931,10 +1938,17 @@ mod content {
where
V: Visitor<'de>,
{
// Covered by tests/test_enum_untagged.rs
// newtype_struct
match *self.content {
Content::Newtype(ref v) => {
visitor.visit_newtype_struct(ContentRefDeserializer::new(v))
}
// This case is necessary for formats which does not store marker of a newtype,
// for example, JSON. When `deserialize_any` is requested from such formats, they will
// report value without using `Visitor::visit_newtype_struct`, because they do not
// known in which contexts this value will be used.
// RON is example of format which preserve markers.
_ => visitor.visit_newtype_struct(self),
}
}
Expand Down Expand Up @@ -2139,6 +2153,10 @@ mod content {
fn unit_variant(self) -> Result<(), E> {
match self.value {
Some(value) => de::Deserialize::deserialize(ContentRefDeserializer::new(value)),
// Covered by tests/test_annotations.rs
// test_partially_untagged_adjacently_tagged_enum
// Covered by tests/test_enum_untagged.rs
// newtype_enum::unit
None => Ok(()),
}
}
Expand All @@ -2148,6 +2166,11 @@ mod content {
T: de::DeserializeSeed<'de>,
{
match self.value {
// Covered by tests/test_annotations.rs
// test_partially_untagged_enum_desugared
// test_partially_untagged_enum_generic
// Covered by tests/test_enum_untagged.rs
// newtype_enum::newtype
Some(value) => seed.deserialize(ContentRefDeserializer::new(value)),
None => Err(de::Error::invalid_type(
de::Unexpected::UnitVariant,
Expand All @@ -2161,9 +2184,13 @@ mod content {
V: de::Visitor<'de>,
{
match self.value {
Some(Content::Seq(v)) => {
de::Deserializer::deserialize_any(SeqRefDeserializer::new(v), visitor)
}
// Covered by tests/test_annotations.rs
// test_partially_untagged_enum
// test_partially_untagged_enum_desugared
// Covered by tests/test_enum_untagged.rs
// newtype_enum::tuple0
// newtype_enum::tuple2
Some(Content::Seq(v)) => visit_content_seq_ref(v, visitor),
Some(other) => Err(de::Error::invalid_type(
other.unexpected(),
&"tuple variant",
Expand All @@ -2184,12 +2211,13 @@ mod content {
V: de::Visitor<'de>,
{
match self.value {
Some(Content::Map(v)) => {
de::Deserializer::deserialize_any(MapRefDeserializer::new(v), visitor)
}
Some(Content::Seq(v)) => {
de::Deserializer::deserialize_any(SeqRefDeserializer::new(v), visitor)
}
// Covered by tests/test_enum_untagged.rs
// newtype_enum::struct_from_map
Some(Content::Map(v)) => visit_content_map_ref(v, visitor),
// Covered by tests/test_enum_untagged.rs
// newtype_enum::struct_from_seq
// newtype_enum::empty_struct_from_seq
Some(Content::Seq(v)) => visit_content_seq_ref(v, visitor),
Some(other) => Err(de::Error::invalid_type(
other.unexpected(),
&"struct variant",
Expand All @@ -2202,158 +2230,6 @@ mod content {
}
}

struct SeqRefDeserializer<'a, 'de: 'a, E>
where
E: de::Error,
{
iter: <&'a [Content<'de>] as IntoIterator>::IntoIter,
err: PhantomData<E>,
}

impl<'a, 'de, E> SeqRefDeserializer<'a, 'de, E>
where
E: de::Error,
{
fn new(slice: &'a [Content<'de>]) -> Self {
SeqRefDeserializer {
iter: slice.iter(),
err: PhantomData,
}
}
}

impl<'de, 'a, E> de::Deserializer<'de> for SeqRefDeserializer<'a, 'de, E>
where
E: de::Error,
{
type Error = E;

#[inline]
fn deserialize_any<V>(mut self, visitor: V) -> Result<V::Value, Self::Error>
where
V: de::Visitor<'de>,
{
let len = self.iter.len();
if len == 0 {
visitor.visit_unit()
} else {
let ret = tri!(visitor.visit_seq(&mut self));
let remaining = self.iter.len();
if remaining == 0 {
Ok(ret)
} else {
Err(de::Error::invalid_length(len, &"fewer elements in array"))
}
}
}

forward_to_deserialize_any! {
bool i8 i16 i32 i64 i128 u8 u16 u32 u64 u128 f32 f64 char str string
bytes byte_buf option unit unit_struct newtype_struct seq tuple
tuple_struct map struct enum identifier ignored_any
}
}

impl<'de, 'a, E> de::SeqAccess<'de> for SeqRefDeserializer<'a, 'de, E>
where
E: de::Error,
{
type Error = E;

fn next_element_seed<T>(&mut self, seed: T) -> Result<Option<T::Value>, Self::Error>
where
T: de::DeserializeSeed<'de>,
{
match self.iter.next() {
Some(value) => seed
.deserialize(ContentRefDeserializer::new(value))
.map(Some),
None => Ok(None),
}
}

fn size_hint(&self) -> Option<usize> {
size_hint::from_bounds(&self.iter)
}
}

struct MapRefDeserializer<'a, 'de: 'a, E>
where
E: de::Error,
{
iter: <&'a [(Content<'de>, Content<'de>)] as IntoIterator>::IntoIter,
value: Option<&'a Content<'de>>,
err: PhantomData<E>,
}

impl<'a, 'de, E> MapRefDeserializer<'a, 'de, E>
where
E: de::Error,
{
fn new(map: &'a [(Content<'de>, Content<'de>)]) -> Self {
MapRefDeserializer {
iter: map.iter(),
value: None,
err: PhantomData,
}
}
}

impl<'de, 'a, E> de::MapAccess<'de> for MapRefDeserializer<'a, 'de, E>
where
E: de::Error,
{
type Error = E;

fn next_key_seed<T>(&mut self, seed: T) -> Result<Option<T::Value>, Self::Error>
where
T: de::DeserializeSeed<'de>,
{
match self.iter.next() {
Some((key, value)) => {
self.value = Some(value);
seed.deserialize(ContentRefDeserializer::new(key)).map(Some)
}
None => Ok(None),
}
}

fn next_value_seed<T>(&mut self, seed: T) -> Result<T::Value, Self::Error>
where
T: de::DeserializeSeed<'de>,
{
match self.value.take() {
Some(value) => seed.deserialize(ContentRefDeserializer::new(value)),
None => Err(de::Error::custom("value is missing")),
}
}

fn size_hint(&self) -> Option<usize> {
size_hint::from_bounds(&self.iter)
}
}

impl<'de, 'a, E> de::Deserializer<'de> for MapRefDeserializer<'a, 'de, E>
where
E: de::Error,
{
type Error = E;

#[inline]
fn deserialize_any<V>(self, visitor: V) -> Result<V::Value, Self::Error>
where
V: de::Visitor<'de>,
{
visitor.visit_map(self)
}

forward_to_deserialize_any! {
bool i8 i16 i32 i64 i128 u8 u16 u32 u64 u128 f32 f64 char str string
bytes byte_buf option unit unit_struct newtype_struct seq tuple
tuple_struct map struct enum identifier ignored_any
}
}

impl<'de, E> de::IntoDeserializer<'de, E> for ContentDeserializer<'de, E>
where
E: de::Error,
Expand Down
47 changes: 0 additions & 47 deletions test_suite/tests/test_annotations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1895,18 +1895,6 @@ fn test_expecting_message_externally_tagged_enum() {
);
}

#[test]
fn test_expecting_message_untagged_tagged_enum() {
#[derive(Deserialize)]
#[serde(untagged)]
#[serde(expecting = "something strange...")]
enum Enum {
Untagged,
}

assert_de_tokens_error::<Enum>(&[Token::Str("Untagged")], "something strange...");
}

#[test]
fn test_expecting_message_identifier_enum() {
#[derive(Deserialize)]
Expand Down Expand Up @@ -2958,41 +2946,6 @@ mod flatten {
mod untagged {
use super::*;

#[test]
fn straightforward() {
#[derive(Serialize, Deserialize, PartialEq, Debug)]
#[serde(untagged)]
enum Data {
A {
a: i32,
#[serde(flatten)]
flat: Flat,
},
}

#[derive(Serialize, Deserialize, PartialEq, Debug)]
struct Flat {
b: i32,
}

let data = Data::A {
a: 0,
flat: Flat { b: 0 },
};

assert_tokens(
&data,
&[
Token::Map { len: None },
Token::Str("a"),
Token::I32(0),
Token::Str("b"),
Token::I32(0),
Token::MapEnd,
],
);
}

#[derive(Debug, PartialEq, Serialize, Deserialize)]
struct Flatten {
#[serde(flatten)]
Expand Down
Loading