Merge pull request #2805 from Mingun/untagged-tests

Fix deserialization of empty structs and tuples in untagged enums

Author: dtolnay

serde/src/private/de.rs

@@ -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),
             }
         }
@@ -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),
             }
         }
@@ -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(()),
             }
         }
@@ -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,
@@ -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",
@@ -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",
@@ -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,

test_suite/tests/test_annotations.rs

@@ -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)]
@@ -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)]