diff --git a/serde/src/de/value.rs b/serde/src/de/value.rs index 1b154c3a8..731aa4ede 100644 --- a/serde/src/de/value.rs +++ b/serde/src/de/value.rs @@ -1439,9 +1439,21 @@ where visitor.visit_enum(self) } + fn deserialize_newtype_struct( + self, + _name: &'static str, + visitor: V, + ) -> Result + where + V: de::Visitor<'de>, + { + // For test_internally_tagged_enum + visitor.visit_newtype_struct(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 + bytes byte_buf option unit unit_struct seq tuple tuple_struct map struct identifier ignored_any } } diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index bcda9ab8b..d2af6f11d 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -220,8 +220,7 @@ mod content { /// # Parameters /// - `map`: map that will be drained /// - `tag_name`: name of tag in the `#[serde(tag = "tag_name")]` attribute - /// - `tag`: tag, if first value in map was a tag - /// - `vec`: placeholder for all other key-value pairs + /// - `first_key`: the first key already fetched from the map /// /// # Returns /// A tuple with two values: @@ -234,13 +233,16 @@ mod content { pub fn drain_map<'de, T, A>( mut map: A, tag_name: &'static str, - mut tag: Option, - mut vec: Vec<(Content<'de>, Content<'de>)>, + first_key: Content<'de>, ) -> Result<(T, ContentDeserializer<'de, A::Error>), A::Error> where T: Deserialize<'de>, A: MapAccess<'de>, { + let mut tag = None; + let mut vec = Vec::with_capacity(size_hint::cautious(map.size_hint())); + + vec.push((first_key, try!(map.next_value()))); while let Some(key) = try!(map.next_key_seed(TagOrContentVisitor::new(tag_name))) { match key { TagOrContent::Tag => { diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 9c1fc886f..99a4ba06c 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1470,9 +1470,7 @@ fn deserialize_internally_tagged_enum( { match try!(_serde::de::SeqAccess::next_element(&mut __seq)) { _serde::__private::Some(__tag) => { - let __rest = _serde::de::value::SeqAccessDeserializer::new(__seq); - let __content = try!(_serde::__private::de::Content::deserialize(__rest)); - let __deserializer = _serde::__private::de::ContentDeserializer::<__S::Error>::new(__content); + let __deserializer = _serde::de::value::SeqAccessDeserializer::new(__seq); match __tag { #(#variant_arms)* @@ -1486,28 +1484,25 @@ fn deserialize_internally_tagged_enum( where __M: _serde::de::MapAccess<#delife>, { - let mut __vec = _serde::__private::Vec::with_capacity( - _serde::de::MapAccess::size_hint(&__map).unwrap_or(0) - ); - + // Read the first field. If it is a tag, immediately deserialize the typed data. + // Otherwise, we collect everything until we find the tag, and then deserialize + // using ContentDeserializer. match try!(_serde::de::MapAccess::next_key_seed( &mut __map, _serde::__private::de::TagOrContentVisitor::new(#tag) )) { _serde::__private::Some(_serde::__private::de::TagOrContent::Tag) => { let __tag = try!(_serde::de::MapAccess::next_value(&mut __map)); - let (__tag, __deserializer) = try!(_serde::__private::de::drain_map( - __map, #tag, _serde::__private::Some(__tag), __vec - )); + let __deserializer = _serde::de::value::MapAccessDeserializer::new(__map); match __tag { #(#variant_arms)* } }, _serde::__private::Some(_serde::__private::de::TagOrContent::Content(__key)) => { - let __val = try!(_serde::de::MapAccess::next_value(&mut __map)); - __vec.push((__key, __val)); + // Drain map to Content::Map, convert it to ContentDeserializer + // Special handling for tag key -- search them and return as separate result let (__tag, __deserializer) = try!(_serde::__private::de::drain_map( - __map, #tag, _serde::__private::None, __vec + __map, #tag, __key )); match __tag { diff --git a/serde_test/src/de.rs b/serde_test/src/de.rs index 289cbfe31..107864ff5 100644 --- a/serde_test/src/de.rs +++ b/serde_test/src/de.rs @@ -524,12 +524,13 @@ impl<'de, 'a> VariantAccess<'de> for DeserializerEnumVisitor<'a, 'de> { Some(self.de.peek_token()) }; match token { + // See test_internally_tagged_struct_variant_containing_unit_variant + Some(Token::SeqEnd) | Some(Token::MapEnd) | Some(Token::StructEnd) | None => Ok(()), Some(Token::UnitVariant { .. }) => { self.de.next_token(); Ok(()) } Some(_) => Deserialize::deserialize(self.de), - None => Ok(()), } } diff --git a/test_suite/tests/test_macros.rs b/test_suite/tests/test_macros.rs index 47179edf7..d80ad771a 100644 --- a/test_suite/tests/test_macros.rs +++ b/test_suite/tests/test_macros.rs @@ -754,8 +754,6 @@ fn test_internally_tagged_enum() { Token::Seq { len: Some(2) }, Token::Str("C"), Token::Map { len: Some(0) }, - Token::MapEnd, - Token::SeqEnd, ], "invalid type: sequence, expected a map", ); @@ -785,6 +783,33 @@ fn test_internally_tagged_enum() { ], ); + // General case: tag field ("type") is not first + assert_de_tokens( + &InternallyTagged::E(Struct { f: 6 }), + &[ + Token::Struct { + name: "Struct", + len: 2, + }, + Token::Str("f"), + Token::U8(6), + Token::Str("type"), + Token::Str("E"), + Token::StructEnd, + ], + ); + assert_de_tokens( + &InternallyTagged::E(Struct { f: 6 }), + &[ + Token::Map { len: Some(2) }, + Token::Str("f"), + Token::U8(6), + Token::Str("type"), + Token::Str("E"), + Token::MapEnd, + ], + ); + assert_de_tokens( &InternallyTagged::E(Struct { f: 6 }), &[ @@ -984,6 +1009,7 @@ fn test_internally_tagged_struct_variant_containing_unit_variant() { Log { level: Level }, } + // Special case: tag field ("action") is the first field assert_de_tokens( &Message::Log { level: Level::Info }, &[ @@ -998,7 +1024,23 @@ fn test_internally_tagged_struct_variant_containing_unit_variant() { Token::StructEnd, ], ); + // General case: tag field ("action") is not the first field + assert_de_tokens( + &Message::Log { level: Level::Info }, + &[ + Token::Struct { + name: "Message", + len: 2, + }, + Token::Str("level"), + Token::BorrowedStr("Info"), + Token::Str("action"), + Token::Str("Log"), + Token::StructEnd, + ], + ); + // Special case: tag field ("action") is the first field assert_de_tokens( &Message::Log { level: Level::Info }, &[ @@ -1010,6 +1052,18 @@ fn test_internally_tagged_struct_variant_containing_unit_variant() { Token::MapEnd, ], ); + // General case: tag field ("action") is not the first field + assert_de_tokens( + &Message::Log { level: Level::Info }, + &[ + Token::Map { len: Some(2) }, + Token::Str("level"), + Token::BorrowedStr("Info"), + Token::Str("action"), + Token::Str("Log"), + Token::MapEnd, + ], + ); assert_de_tokens( &Message::Log { level: Level::Info },