Serialise Option<T> struct fields as T if value if Some and ignore field if value is None.

3 files changed, 178 insertions, 44 deletions

diff --git a/src/serde/de.rs b/src/serde/de.rs
index 72a81bd..1b9838a 100644
--- a/src/serde/de.rs
+++ b/src/serde/de.rs
@@ -69,7 +69,7 @@ impl<'de, 'a, I> de::Deserializer<'de> for &'a mut Deserializer<I>
         match try_next!(self.events.next()) {
             PlistEvent::StartArray(len) => {
                 let len = try!(u64_option_to_usize(len));
-                let ret = visitor.visit_seq(MapAndSeqAccess::new(self, len))?;
+                let ret = visitor.visit_seq(MapAndSeqAccess::new(self, false, len))?;
                 expect!(self.events.next(), PlistEvent::EndArray);
                 Ok(ret)
             }
@@ -77,7 +77,7 @@ impl<'de, 'a, I> de::Deserializer<'de> for &'a mut Deserializer<I>
 
             PlistEvent::StartDictionary(len) => {
                 let len = try!(u64_option_to_usize(len));
-                let ret = visitor.visit_map(MapAndSeqAccess::new(self, len))?;
+                let ret = visitor.visit_map(MapAndSeqAccess::new(self, false, len))?;
                 expect!(self.events.next(), PlistEvent::EndDictionary);
                 Ok(ret)
             }
@@ -96,7 +96,7 @@ impl<'de, 'a, I> de::Deserializer<'de> for &'a mut Deserializer<I>
     forward_to_deserialize_any! {
         bool u8 u16 u32 u64 i8 i16 i32 i64 f32 f64 char str string
         seq bytes byte_buf map unit_struct
-        tuple_struct struct tuple ignored_any identifier
+        tuple_struct tuple ignored_any identifier
     }
 
     fn deserialize_unit<V>(self, visitor: V) -> Result<V::Value, Self::Error>
@@ -113,18 +113,11 @@ impl<'de, 'a, I> de::Deserializer<'de> for &'a mut Deserializer<I>
 
         let ret = match try_next!(self.events.next()) {
             PlistEvent::StringValue(ref s) if &s[..] == "None" => {
-                let ret = match visitor.visit_none() {
-                    Ok(ret) => ret,
-                    Err(e) => return Err(e),
-                };
-                // For some reason the try! below doesn't work - probably a macro hygene issue
-                // with Error and ::Error
-                // let ret = try!(visitor.visit_none());
                 expect!(self.events.next(), PlistEvent::StringValue(_));
-                ret
+                visitor.visit_none::<Self::Error>()?
             }
             PlistEvent::StringValue(ref s) if &s[..] == "Some" => {
-                try!(visitor.visit_some(&mut *self))
+                visitor.visit_some(&mut *self)?
             }
             _ => return Err(event_mismatch_error()),
         };
@@ -143,6 +136,19 @@ impl<'de, 'a, I> de::Deserializer<'de> for &'a mut Deserializer<I>
         visitor.visit_newtype_struct(self)
     }
 
+    fn deserialize_struct<V>(self,
+                             _name: &'static str,
+                             _fields: &'static [&'static str],
+                             visitor: V)
+                            -> Result<V::Value, Self::Error>
+        where V: de::Visitor<'de>
+    {
+        expect!(self.events.next(), PlistEvent::StartDictionary(_));
+        let ret = visitor.visit_map(MapAndSeqAccess::new(self, true, None))?;
+        expect!(self.events.next(), PlistEvent::EndDictionary);
+        Ok(ret)
+    }
+
     fn deserialize_enum<V>(self,
                            _enum: &'static str,
                            _variants: &'static [&'static str],
@@ -202,19 +208,87 @@ impl<'de, 'a, I> de::VariantAccess<'de> for &'a mut Deserializer<I>
     }
 }
 
+pub struct StructValueDeserializer<'a, I: 'a>
+    where I: IntoIterator<Item = Result<PlistEvent, Error>>
+{
+    de: &'a mut Deserializer<I>
+}
+
+impl<'de, 'a, I> de::Deserializer<'de> for StructValueDeserializer<'a, I>
+    where I: IntoIterator<Item = Result<PlistEvent, Error>>
+{
+    type Error = Error;
+
+    fn deserialize_any<V>(self, visitor: V) -> Result<V::Value, Self::Error>
+        where V: de::Visitor<'de>
+    {
+        self.de.deserialize_any(visitor)
+    }
+
+    forward_to_deserialize_any! {
+        bool u8 u16 u32 u64 i8 i16 i32 i64 f32 f64 char str string
+        seq bytes byte_buf map unit_struct
+        tuple_struct tuple ignored_any identifier
+    }
+
+    fn deserialize_unit<V>(self, visitor: V) -> Result<V::Value, Self::Error>
+        where V: de::Visitor<'de>
+    {
+        self.de.deserialize_unit(visitor)
+    }
+
+    fn deserialize_option<V>(self, visitor: V) -> Result<V::Value, Self::Error>
+        where V: de::Visitor<'de>
+    {
+        // None struct values are ignored so if we're here the value must be Some.
+        visitor.visit_some(self.de)
+    }
+
+    fn deserialize_newtype_struct<V>(self,
+                                     name: &'static str,
+                                     visitor: V)
+                                     -> Result<V::Value, Self::Error>
+        where V: de::Visitor<'de>
+    {
+        self.de.deserialize_newtype_struct(name, visitor)
+    }
+
+    fn deserialize_struct<V>(self,
+                             name: &'static str,
+                             fields: &'static [&'static str],
+                             visitor: V)
+                            -> Result<V::Value, Self::Error>
+        where V: de::Visitor<'de>
+    {
+        self.de.deserialize_struct(name, fields, visitor)
+    }
+
+    fn deserialize_enum<V>(self,
+                           enum_: &'static str,
+                           variants: &'static [&'static str],
+                           visitor: V)
+                           -> Result<V::Value, Self::Error>
+        where V: de::Visitor<'de>
+    {
+        self.de.deserialize_enum(enum_, variants, visitor)
+    }
+}
+
 struct MapAndSeqAccess<'a, I>
     where I: 'a + IntoIterator<Item = Result<PlistEvent, Error>>
 {
     de: &'a mut Deserializer<I>,
+    is_struct: bool,
     remaining: Option<usize>,
 }
 
 impl<'a, I> MapAndSeqAccess<'a, I>
     where I: 'a + IntoIterator<Item = Result<PlistEvent, Error>>
 {
-    fn new(de: &'a mut Deserializer<I>, len: Option<usize>) -> MapAndSeqAccess<'a, I> {
+    fn new(de: &'a mut Deserializer<I>, is_struct: bool, len: Option<usize>) -> MapAndSeqAccess<'a, I> {
         MapAndSeqAccess {
             de: de,
+            is_struct: is_struct,
             remaining: len,
         }
     }
@@ -228,14 +302,12 @@ impl<'de, 'a, I> de::SeqAccess<'de> for MapAndSeqAccess<'a, I>
     fn next_element_seed<T>(&mut self, seed: T) -> Result<Option<T::Value>, Self::Error>
         where T: de::DeserializeSeed<'de>
     {
-        match self.de.events.peek() {
-            Some(&Ok(PlistEvent::EndArray)) => Ok(None),
-            _ => {
-                let ret = seed.deserialize(&mut *self.de).map(|k| Some(k));
-                self.remaining = self.remaining.map(|r| r.saturating_sub(1));
-                ret
-            }
+        if let Some(&Ok(PlistEvent::EndArray)) = self.de.events.peek() {
+            return Ok(None);
         }
+
+        self.remaining = self.remaining.map(|r| r.saturating_sub(1));
+        seed.deserialize(&mut *self.de).map(Some)
     }
 
     fn size_hint(&self) -> Option<usize> {
@@ -251,20 +323,22 @@ impl<'de, 'a, I> de::MapAccess<'de> for MapAndSeqAccess<'a, I>
     fn next_key_seed<K>(&mut self, seed: K) -> Result<Option<K::Value>, Self::Error>
         where K: de::DeserializeSeed<'de>
     {
-        match self.de.events.peek() {
-            Some(&Ok(PlistEvent::EndDictionary)) => return Ok(None),
-            _ => {
-                let ret = seed.deserialize(&mut *self.de).map(|k| Some(k));
-                self.remaining = self.remaining.map(|r| r.saturating_sub(1));
-                ret
-            }
+        if let Some(&Ok(PlistEvent::EndDictionary)) = self.de.events.peek() {
+            return Ok(None);
         }
+
+        self.remaining = self.remaining.map(|r| r.saturating_sub(1));
+        seed.deserialize(&mut *self.de).map(Some)
     }
 
     fn next_value_seed<V>(&mut self, seed: V) -> Result<V::Value, Self::Error>
         where V: de::DeserializeSeed<'de>
     {
-        seed.deserialize(&mut *self.de)
+        if self.is_struct {
+            seed.deserialize(StructValueDeserializer { de: &mut *self.de })
+        } else {
+            seed.deserialize(&mut *self.de)
+        }
     }
 
     fn size_hint(&self) -> Option<usize> {
diff --git a/src/serde/ser.rs b/src/serde/ser.rs
index 48ad39f..3772d7f 100644
--- a/src/serde/ser.rs
+++ b/src/serde/ser.rs
@@ -14,14 +14,26 @@ impl ser::Error for Error {
 
 pub struct Serializer<W: EventWriter> {
     writer: W,
+    // We don't want to serialize None if the Option is in a struct field as this is how null
+    // fields are represented in plists. This is fragile but results in minimal code duplication.
+    // TODO: This is fragile. Use distinct types instead.
+    maybe_option_field_name: Option<&'static str>
 }
 
 impl<W: EventWriter> Serializer<W> {
     pub fn new(writer: W) -> Serializer<W> {
-        Serializer { writer: writer }
+        Serializer {
+            writer: writer,
+            maybe_option_field_name: None
+        }
     }
 
     fn emit(&mut self, event: PlistEvent) -> Result<(), Error> {
+        // Write a waiting struct field name.
+        // TODO: This is fragile. Use distinct types instead.
+        if let Some(field_name) = self.maybe_option_field_name.take() {
+            self.emit(PlistEvent::StringValue(field_name.to_owned()))?;
+        }
         Ok(self.writer.write(&event)?)
     }
 
@@ -111,15 +123,28 @@ impl<'a, W: EventWriter> ser::Serializer for &'a mut Serializer<W> {
     }
 
     fn serialize_none(self) -> Result<(), Self::Error> {
-        self.single_key_dict("None".to_owned())?;
-        self.serialize_unit()?;
-        self.single_key_dict_end()
+        // Don't write a dict for None if the Option is a struct field.
+        // TODO: This is fragile. Use distinct types instead.
+        if let None = self.maybe_option_field_name.take() {
+            self.single_key_dict("None".to_owned())?;
+            self.serialize_unit()?;
+            self.single_key_dict_end()?;
+        }
+        Ok(())
     }
 
     fn serialize_some<T: ?Sized + ser::Serialize>(self, value: &T) -> Result<(), Self::Error> {
-        self.single_key_dict("Some".to_owned())?;
-        value.serialize(&mut *self)?;
-        self.single_key_dict_end()
+        // Don't write a dict for None if the Option is a struct field.
+        // Can't use the write in emit here in case there is a Some(None).
+        // TODO: This is fragile. Use distinct types instead.
+        if let Some(field_name) = self.maybe_option_field_name.take() {
+            self.emit(PlistEvent::StringValue(field_name.to_owned()))?;
+            value.serialize(&mut *self)
+        } else {
+            self.single_key_dict("Some".to_owned())?;
+            value.serialize(&mut *self)?;
+            self.single_key_dict_end()
+        }
     }
 
     fn serialize_unit(self) -> Result<(), Self::Error> {
@@ -195,9 +220,10 @@ impl<'a, W: EventWriter> ser::Serializer for &'a mut Serializer<W> {
 
     fn serialize_struct(self,
                         _name: &'static str,
-                        len: usize)
+                        _len: usize)
                         -> Result<Self::SerializeStruct, Self::Error> {
-        self.serialize_map(Some(len))
+        // The number of struct fields is not known as fields with None values are ignored.
+        self.serialize_map(None)
     }
 
     fn serialize_struct_variant(self,
@@ -304,7 +330,10 @@ impl<'a, W: EventWriter> ser::SerializeStruct for Compound<'a, W> {
                                                    key: &'static str,
                                                    value: &T)
                                                    -> Result<(), Self::Error> {
-        <Self as ser::SerializeMap>::serialize_entry(self, key, value)
+        // Don't write a dict for None if the Option is a struct field.
+        // TODO: This is fragile. Use distinct types instead.
+        self.ser.maybe_option_field_name = Some(key);
+        value.serialize(&mut *self.ser)
     }
 
     fn end(self) -> Result<Self::Ok, Self::Error> {
diff --git a/tests/serde_tests/mod.rs b/tests/serde_tests/mod.rs
index 5235896..e490a2c 100644
--- a/tests/serde_tests/mod.rs
+++ b/tests/serde_tests/mod.rs
@@ -107,10 +107,10 @@ fn dog() {
 
     let comparison = &[StartDictionary(Some(1)),
                        StringValue("Dog".to_owned()),
-                       StartDictionary(Some(1)),
+                       StartDictionary(None),
                        StringValue("inner".to_owned()),
                        StartArray(Some(1)),
-                       StartDictionary(Some(3)),
+                       StartDictionary(None),
                        StringValue("a".to_owned()),
                        StringValue("".to_owned()),
                        StringValue("b".to_owned()),
@@ -165,14 +165,12 @@ fn cat() {
 
     let comparison = &[StartDictionary(Some(1)),
                        StringValue("Cat".to_owned()),
-                       StartDictionary(Some(3)),
+                       StartDictionary(None),
                        StringValue("age".to_owned()),
                        IntegerValue(12),
                        StringValue("name".to_owned()),
                        StringValue("Paws".to_owned()),
                        StringValue("firmware".to_owned()),
-                       StartDictionary(Some(1)),
-                       StringValue("Some".to_owned()),
                        StartArray(Some(9)),
                        IntegerValue(0),
                        IntegerValue(1),
@@ -185,7 +183,6 @@ fn cat() {
                        IntegerValue(8),
                        EndArray,
                        EndDictionary,
-                       EndDictionary,
                        EndDictionary];
 
     assert_roundtrip(cat, Some(comparison));
@@ -206,3 +203,37 @@ fn newtype_struct() {
 
     assert_roundtrip(newtype, Some(comparison));
 }
+
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+struct TypeWithOptions {
+    a: Option<String>,
+    b: Option<u32>,
+    c: Option<Box<TypeWithOptions>>
+}
+
+#[test]
+fn type_with_options() {
+    let inner = TypeWithOptions {
+        a: None,
+        b: Some(12),
+        c: None
+    };
+
+    let ty = TypeWithOptions {
+        a: Some("hello".to_owned()),
+        b: None,
+        c: Some(Box::new(inner))
+    };
+
+    let comparison = &[StartDictionary(None),
+                       StringValue("a".to_owned()),
+                       StringValue("hello".to_owned()),
+                       StringValue("c".to_owned()),
+                       StartDictionary(None),
+                       StringValue("b".to_owned()),
+                       IntegerValue(12),
+                       EndDictionary,
+                       EndDictionary];
+
+    assert_roundtrip(ty, Some(comparison));
+}