From 5ee2fc0562a28c51483c2d80ca50801941dc91b2 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 1 Jun 2018 12:00:35 -0700 Subject: [PATCH] Allow multiple flattened maps to see the same fields Before this change, flattening anything after a flattened map was nonsensical because the later flattened field would always observe no input fields. #[derive(Deserialize)] struct S { #[serde(flatten)] map: Map, #[serde(flatten)] other: Other, // always empty } This change makes a flattened map not consume any of the input fields, leaving them available to later flattened fields in the same struct. The new behavior is useful when two flattened fields that both use deserialize_map care about disjoint subsets of the fields in the input. #[derive(Deserialize)] struct S { // Looks at fields with a "player1_" prefix. #[serde(flatten, with = "prefix_player1")] player1: Player, // Looks at fields with a "player2_" prefix. #[serde(flatten, with = "prefix_player2")] player2: Player, } --- serde/src/private/de.rs | 76 ++++++++++++++++++++++------ test_suite/tests/test_annotations.rs | 45 +++++++++++++++- 2 files changed, 105 insertions(+), 16 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 3c56e2f9..8fdf6c36 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -2723,7 +2723,7 @@ where where V: Visitor<'de>, { - visitor.visit_map(FlatMapAccess::new(self.0.iter_mut(), None)) + visitor.visit_map(FlatMapAccess::new(self.0.iter())) } fn deserialize_struct( @@ -2735,7 +2735,7 @@ where where V: Visitor<'de>, { - visitor.visit_map(FlatMapAccess::new(self.0.iter_mut(), Some(fields))) + visitor.visit_map(FlatStructAccess::new(self.0.iter_mut(), fields)) } fn deserialize_newtype_struct(self, _name: &str, visitor: V) -> Result @@ -2784,22 +2784,19 @@ where #[cfg(any(feature = "std", feature = "alloc"))] pub struct FlatMapAccess<'a, 'de: 'a, E> { - iter: slice::IterMut<'a, Option<(Content<'de>, Content<'de>)>>, - pending_content: Option>, - fields: Option<&'static [&'static str]>, + iter: slice::Iter<'a, Option<(Content<'de>, Content<'de>)>>, + pending_content: Option<&'a Content<'de>>, _marker: PhantomData, } #[cfg(any(feature = "std", feature = "alloc"))] impl<'a, 'de, E> FlatMapAccess<'a, 'de, E> { fn new( - iter: slice::IterMut<'a, Option<(Content<'de>, Content<'de>)>>, - fields: Option<&'static [&'static str]>, + iter: slice::Iter<'a, Option<(Content<'de>, Content<'de>)>>, ) -> FlatMapAccess<'a, 'de, E> { FlatMapAccess { iter: iter, pending_content: None, - fields: fields, _marker: PhantomData, } } @@ -2812,6 +2809,61 @@ where { type Error = E; + fn next_key_seed(&mut self, seed: T) -> Result, Self::Error> + where + T: DeserializeSeed<'de>, + { + while let Some(item) = self.iter.next() { + // Items in the vector are nulled out when used by a struct. + if let Some((ref key, ref content)) = *item { + self.pending_content = Some(content); + return seed.deserialize(ContentRefDeserializer::new(key)).map(Some); + } + } + Ok(None) + } + + fn next_value_seed(&mut self, seed: T) -> Result + where + T: DeserializeSeed<'de>, + { + match self.pending_content.take() { + Some(value) => seed.deserialize(ContentRefDeserializer::new(value)), + None => Err(Error::custom("value is missing")), + } + } +} + +#[cfg(any(feature = "std", feature = "alloc"))] +pub struct FlatStructAccess<'a, 'de: 'a, E> { + iter: slice::IterMut<'a, Option<(Content<'de>, Content<'de>)>>, + pending_content: Option>, + fields: &'static [&'static str], + _marker: PhantomData, +} + +#[cfg(any(feature = "std", feature = "alloc"))] +impl<'a, 'de, E> FlatStructAccess<'a, 'de, E> { + fn new( + iter: slice::IterMut<'a, Option<(Content<'de>, Content<'de>)>>, + fields: &'static [&'static str], + ) -> FlatStructAccess<'a, 'de, E> { + FlatStructAccess { + iter: iter, + pending_content: None, + fields: fields, + _marker: PhantomData, + } + } +} + +#[cfg(any(feature = "std", feature = "alloc"))] +impl<'a, 'de, E> MapAccess<'de> for FlatStructAccess<'a, 'de, E> +where + E: Error, +{ + type Error = E; + fn next_key_seed(&mut self, seed: T) -> Result, Self::Error> where T: DeserializeSeed<'de>, @@ -2822,13 +2874,7 @@ where // about. In case we do not know which fields we want, we take them all. let use_item = match *item { None => false, - Some((ref c, _)) => c.as_str().map_or(self.fields.is_none(), |key| { - match self.fields { - None => true, - Some(fields) if fields.contains(&key) => true, - _ => false, - } - }), + Some((ref c, _)) => c.as_str().map_or(false, |key| self.fields.contains(&key)), }; if use_item { diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index aea692cb..bc45f37c 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -14,7 +14,7 @@ extern crate serde_derive; extern crate serde; use self::serde::de::{self, Unexpected}; use self::serde::{Deserialize, Deserializer, Serialize, Serializer}; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::marker::PhantomData; extern crate serde_test; @@ -1683,6 +1683,49 @@ fn test_complex_flatten() { ); } +#[test] +fn test_flatten_map_twice() { + #[derive(Debug, PartialEq, Deserialize)] + struct Outer { + #[serde(flatten)] + first: BTreeMap, + #[serde(flatten)] + between: Inner, + #[serde(flatten)] + second: BTreeMap, + } + + #[derive(Debug, PartialEq, Deserialize)] + struct Inner { + y: String, + } + + assert_de_tokens( + &Outer { + first: { + let mut first = BTreeMap::new(); + first.insert("x".to_owned(), "X".to_owned()); + first.insert("y".to_owned(), "Y".to_owned()); + first + }, + between: Inner { y: "Y".to_owned() }, + second: { + let mut second = BTreeMap::new(); + second.insert("x".to_owned(), "X".to_owned()); + second + }, + }, + &[ + Token::Map { len: None }, + Token::Str("x"), + Token::Str("X"), + Token::Str("y"), + Token::Str("Y"), + Token::MapEnd, + ], + ); +} + #[test] fn test_flatten_unsupported_type() { #[derive(Debug, PartialEq, Serialize, Deserialize)]