Repository: pyfisch/cbor (rust)

Issue:
Change enum serialization
https://serde.rs/enum-representations.html

Serde CBOR should use the "Externally tagged" format by default in the next breaking version.
Also make sure that the other formats round-trip.
The current array based serialization should be kept as a legacy format so old data can be read.

Pull request description:
Serialize enums as maps by default
This matches other serde formats.
Directly set options on Serializer.
Remove rarely used to_* methods.

Closes #85 

Patch:
diff --git a/src/ser.rs b/src/ser.rs
index 6d62843..59b5a09 100644
--- a/src/ser.rs
+++ b/src/ser.rs
@@ -11,58 +11,6 @@ use serde::ser::{self, Serialize};
 #[cfg(feature = "std")]
 use std::io;
 
-/// Serializes a value to a writer.
-#[cfg(feature = "std")]
-pub fn to_writer<W, T>(writer: &mut W, value: &T) -> Result<()>
-where
-    W: io::Write,
-    T: ser::Serialize,
-{
-    value.serialize(&mut Serializer::new(&mut IoWrite::new(writer)))
-}
-
-/// Serializes a value to a writer and adds a CBOR self-describe tag.
-#[cfg(feature = "std")]
-pub fn to_writer_sd<W, T>(writer: &mut W, value: &T) -> Result<()>
-where
-    W: io::Write,
-    T: ser::Serialize,
-{
-    let mut writer = IoWrite::new(writer);
-    let mut ser = Serializer::new(&mut writer);
-    ser.self_describe()?;
-    value.serialize(&mut ser)
-}
-
-/// Serializes a value without names to a writer.
-///
-/// Struct fields and enum variants are identified by their numeric indices rather than names to
-/// save space.
-#[cfg(feature = "std")]
-pub fn to_writer_packed<W, T>(writer: &mut W, value: &T) -> Result<()>
-where
-    W: io::Write,
-    T: ser::Serialize,
-{
-    value.serialize(&mut Serializer::packed(&mut IoWrite::new(writer)))
-}
-
-/// Serializes a value without names to a writer and adds a CBOR self-describe tag.
-///
-/// Struct fields and enum variants are identified by their numeric indices rather than names to
-/// save space.
-#[cfg(feature = "std")]
-pub fn to_writer_packed_sd<W, T>(writer: &mut W, value: &T) -> Result<()>
-where
-    W: io::Write,
-    T: ser::Serialize,
-{
-    let mut writer = IoWrite::new(writer);
-    let mut ser = Serializer::packed(&mut writer);
-    ser.self_describe()?;
-    value.serialize(&mut ser)
-}
-
 /// Serializes a value to a vector.
 #[cfg(feature = "std")]
 pub fn to_vec<T>(value: &T) -> Result<Vec<u8>>
@@ -74,117 +22,25 @@ where
     Ok(vec)
 }
 
-/// Serializes a value to a vector and adds a CBOR self-describe tag.
-#[cfg(feature = "std")]
-pub fn to_vec_sd<T>(value: &T) -> Result<Vec<u8>>
-where
-    T: ser::Serialize,
-{
-    let mut vec = Vec::new();
-    to_writer_sd(&mut vec, value)?;
-    Ok(vec)
-}
-
-/// Serializes a value without names to a vector.
-///
-/// Struct fields and enum variants are identified by their numeric indices rather than names to
-/// save space.
+/// Serializes a value to a vector in packed format.
 #[cfg(feature = "std")]
 pub fn to_vec_packed<T>(value: &T) -> Result<Vec<u8>>
 where
     T: ser::Serialize,
 {
     let mut vec = Vec::new();
-    to_writer_packed(&mut vec, value)?;
+    value.serialize(&mut Serializer::new(&mut IoWrite::new(&mut vec)).packed_format())?;
     Ok(vec)
 }
 
-/// Serializes a value without names to a vector and adds a CBOR self-describe tag.
-///
-/// Struct fields and enum variants are identified by their numeric indices rather than names to
-/// save space.
-#[cfg(feature = "std")]
-pub fn to_vec_packed_sd<T>(value: &T) -> Result<Vec<u8>>
-where
-    T: ser::Serialize,
-{
-    let mut vec = Vec::new();
-    to_writer_packed_sd(&mut vec, value)?;
-    Ok(vec)
-}
-
-/// Serializes a value to a vector.
+/// Serializes a value to a writer.
 #[cfg(feature = "std")]
-pub fn to_vec_with_options<T>(value: &T, options: &SerializerOptions) -> Result<Vec<u8>>
+pub fn to_writer<W, T>(writer: &mut W, value: &T) -> Result<()>
 where
+    W: io::Write,
     T: ser::Serialize,
 {
-    let mut vec = Vec::new();
-    {
-        let mut ser = Serializer::new_with_options(&mut vec, options);
-        if options.self_describe {
-            ser.self_describe()?;
-        }
-        value.serialize(&mut ser)?;
-    }
-    Ok(vec)
-}
-
-/// Options for a CBOR serializer.
-///
-/// The `enum_as_map` option determines how enums are encoded.
-///
-/// This makes no difference when encoding and decoding enums using
-/// this crate, but it shows up when decoding to a `Value` or decoding
-/// in other languages.
-///
-/// With enum_as_map true, the encoding scheme matches the default encoding
-/// scheme used by `serde_json`.
-///
-/// # Examples
-///
-/// Given the following enum
-/// ```
-/// enum Enum {
-///     Unit,
-///     NewType(i32),
-///     Tuple(String, bool),
-///     Struct{ x: i32, y: i32 },
-/// }
-/// ```
-/// we will give the `Value` with the same encoding for each case using
-/// JSON notation.
-///
-/// ## Default encodings
-///
-/// * `Enum::Unit` encodes as `"Unit"`
-/// * `Enum::NewType(10)` encodes as `["NewType", 10]`
-/// * `Enum::Tuple("x", true)` encodes as `["Tuple", "x", true]`
-/// * `Enum::Struct{ x: 5, y: -5 }` encodes as `["Struct", {"x": 5, "y": -5}]`
-///
-/// ## Encodings with enum_as_map true
-///
-/// * `Enum::Unit` encodes as `"Unit"`
-/// * `Enum::NewType(10)` encodes as `{"NewType": 10}`
-/// * `Enum::Tuple("x", true)` encodes as `{"Tuple": ["x", true]}`
-/// * `Enum::Struct{ x: 5, y: -5 }` encodes as `{"Struct": {"x": 5, "y": -5}}`
-#[derive(Default)]
-pub struct SerializerOptions {
-    /// When set, struct fields and enum variants are identified by their numeric indices rather than names
-    /// to save space.
-    pub packed: bool,
-    /// When set, enums are encoded as maps rather than arrays.
-    pub enum_as_map: bool,
-    /// When set, `to_vec` will prepend the CBOR self-describe tag.
-    pub self_describe: bool,
-}
-
-#[cfg(feature = "std")]
-impl SerializerOptions {
-    /// Serializes a value to a vector.
-    pub fn to_vec<T: ser::Serialize>(&self, value: &T) -> Result<Vec<u8>> {
-        to_vec_with_options(value, self)
-    }
+    value.serialize(&mut Serializer::new(&mut IoWrite::new(writer)))
 }
 
 /// A structure for serializing Rust values to CBOR.
@@ -202,35 +58,61 @@ where
     ///
     /// `to_vec` and `to_writer` should normally be used instead of this method.
     #[inline]
-    pub fn new(writer: W) -> Serializer<W> {
+    pub fn new(writer: W) -> Self {
         Serializer {
             writer: writer,
             packed: false,
-            enum_as_map: false,
+            enum_as_map: true,
         }
     }
 
-    /// Creates a new "packed" CBOR serializer.
+    /// Choose concise/packed format for serializer.
     ///
-    /// Struct fields and enum variants are identified by their numeric indices rather than names
-    /// to save space.
-    #[inline]
-    pub fn packed(writer: W) -> Serializer<W> {
-        Serializer {
-            writer,
-            packed: true,
-            enum_as_map: false,
-        }
+    /// In the packed format enum variant names and field names
+    /// are replaced with numeric indizes to conserve space.
+    pub fn packed_format(mut self) -> Self {
+        self.packed = true;
+        self
     }
 
-    /// Creates a new CBOR serializer with the specified options.
-    #[inline]
-    pub fn new_with_options(writer: W, options: &SerializerOptions) -> Serializer<W> {
-        Serializer {
-            writer,
-            packed: options.packed,
-            enum_as_map: options.enum_as_map,
-        }
+    /// Enable old enum format used by `serde_cbor` versions <= v0.9.
+    ///
+    /// The `legacy_enums` option determines how enums are encoded.
+    ///
+    /// This makes no difference when encoding and decoding enums using
+    /// this crate, but it shows up when decoding to a `Value` or decoding
+    /// in other languages.
+    ///
+    /// # Examples
+    ///
+    /// Given the following enum
+    ///
+    /// ```rust
+    /// enum Enum {
+    ///     Unit,
+    ///     NewType(i32),
+    ///     Tuple(String, bool),
+    ///     Struct{ x: i32, y: i32 },
+    /// }
+    /// ```
+    /// we will give the `Value` with the same encoding for each case using
+    /// JSON notation.
+    ///
+    /// ## Default encodings
+    ///
+    /// * `Enum::Unit` encodes as `"Unit"`
+    /// * `Enum::NewType(10)` encodes as `{"NewType": 10}`
+    /// * `Enum::Tuple("x", true)` encodes as `{"Tuple": ["x", true]}`
+    ///
+    /// ## Legacy encodings
+    ///
+    /// * `Enum::Unit` encodes as `"Unit"`
+    /// * `Enum::NewType(10)` encodes as `["NewType", 10]`
+    /// * `Enum::Tuple("x", true)` encodes as `["Tuple", "x", true]`
+    /// * `Enum::Struct{ x: 5, y: -5 }` encodes as `["Struct", {"x": 5, "y": -5}]`
+    pub fn legacy_enums(mut self) -> Self {
+        self.enum_as_map = false;
+        self
     }
 
     /// Writes a CBOR self-describe tag to the stream.


Test patch:
diff --git a/tests/bennofs.rs b/tests/bennofs.rs
index 44212b5..1b289f4 100644
--- a/tests/bennofs.rs
+++ b/tests/bennofs.rs
@@ -49,7 +49,7 @@ mod std_tests {
 fn test() {
     let mut slice = [0u8; 64];
     let writer = SliceWrite::new(&mut slice);
-    let mut serializer = Serializer::packed(writer);
+    let mut serializer = Serializer::new(writer).packed_format();
     EXAMPLE.serialize(&mut serializer).unwrap();
     let writer = serializer.into_inner();
     let end = writer.bytes_written();
diff --git a/tests/enum.rs b/tests/enum.rs
index 36a0b09..630500d 100644
--- a/tests/enum.rs
+++ b/tests/enum.rs
@@ -31,9 +31,19 @@ fn test_simple_data_enum_roundtrip() {
 mod std_tests {
     use std::collections::BTreeMap;
 
+    use serde_cbor::ser::{IoWrite, Serializer};
     use serde_cbor::value::Value;
     use serde_cbor::{from_slice, to_vec};
 
+    pub fn to_vec_legacy<T>(value: &T) -> serde_cbor::Result<Vec<u8>>
+    where
+        T: serde::ser::Serialize,
+    {
+        let mut vec = Vec::new();
+        value.serialize(&mut Serializer::new(&mut IoWrite::new(&mut vec)).legacy_enums())?;
+        Ok(vec)
+    }
+
     #[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
     enum Enum {
         A,
@@ -99,11 +109,14 @@ mod std_tests {
 
     #[test]
     fn test_serialize() {
-        assert_eq!(to_vec(&Enum::A).unwrap(), &[97, 65]);
-        assert_eq!(to_vec(&Enum::B).unwrap(), &[97, 66]);
-        assert_eq!(to_vec(&DataEnum::A(42)).unwrap(), &[130, 97, 65, 24, 42]);
+        assert_eq!(to_vec_legacy(&Enum::A).unwrap(), &[97, 65]);
+        assert_eq!(to_vec_legacy(&Enum::B).unwrap(), &[97, 66]);
+        assert_eq!(
+            to_vec_legacy(&DataEnum::A(42)).unwrap(),
+            &[130, 97, 65, 24, 42]
+        );
         assert_eq!(
-            to_vec(&DataEnum::B(true, 9)).unwrap(),
+            to_vec_legacy(&DataEnum::B(true, 9)).unwrap(),
             &[131, 97, 66, 245, 9]
         );
     }
@@ -140,27 +153,27 @@ mod std_tests {
     #[test]
     fn test_enum_as_map() {
         // unit variants serialize like bare strings
-        let empty_s = to_vec(&Bar::Empty).unwrap();
-        let empty_str_s = to_vec(&"Empty").unwrap();
+        let empty_s = to_vec_legacy(&Bar::Empty).unwrap();
+        let empty_str_s = to_vec_legacy(&"Empty").unwrap();
         assert_eq!(empty_s, empty_str_s);
 
         // tuple-variants serialize like ["<variant>", values..]
-        let number_s = to_vec(&Bar::Number(42)).unwrap();
+        let number_s = to_vec_legacy(&Bar::Number(42)).unwrap();
         let number_vec = vec![Value::Text("Number".to_string()), Value::Integer(42)];
-        let number_vec_s = to_vec(&number_vec).unwrap();
+        let number_vec_s = to_vec_legacy(&number_vec).unwrap();
         assert_eq!(number_s, number_vec_s);
 
-        let flag_s = to_vec(&Bar::Flag("foo".to_string(), true)).unwrap();
+        let flag_s = to_vec_legacy(&Bar::Flag("foo".to_string(), true)).unwrap();
         let flag_vec = vec![
             Value::Text("Flag".to_string()),
             Value::Text("foo".to_string()),
             Value::Bool(true),
         ];
-        let flag_vec_s = to_vec(&flag_vec).unwrap();
+        let flag_vec_s = to_vec_legacy(&flag_vec).unwrap();
         assert_eq!(flag_s, flag_vec_s);
 
         // struct-variants serialize like ["<variant>", {struct..}]
-        let point_s = to_vec(&Bar::Point { x: 5, y: -5 }).unwrap();
+        let point_s = to_vec_legacy(&Bar::Point { x: 5, y: -5 }).unwrap();
         let mut struct_map = BTreeMap::new();
         struct_map.insert(Value::Text("x".to_string()), Value::Integer(5));
         struct_map.insert(Value::Text("y".to_string()), Value::Integer(-5));
@@ -168,28 +181,24 @@ mod std_tests {
             Value::Text("Point".to_string()),
             Value::Map(struct_map.clone()),
         ];
-        let point_vec_s = to_vec(&point_vec).unwrap();
+        let point_vec_s = to_vec_legacy(&point_vec).unwrap();
         assert_eq!(point_s, point_vec_s);
 
         // enum_as_map matches serde_json's default serialization for enums.
-        let opts = serde_cbor::ser::SerializerOptions {
-            enum_as_map: true,
-            ..Default::default()
-        };
 
         // unit variants still serialize like bare strings
-        let empty_s = opts.to_vec(&Bar::Empty).unwrap();
+        let empty_s = to_vec(&Bar::Empty).unwrap();
         assert_eq!(empty_s, empty_str_s);
 
         // 1-element tuple variants serialize like {"<variant>": value}
-        let number_s = opts.to_vec(&Bar::Number(42)).unwrap();
+        let number_s = to_vec(&Bar::Number(42)).unwrap();
         let mut number_map = BTreeMap::new();
         number_map.insert("Number", 42);
         let number_map_s = to_vec(&number_map).unwrap();
         assert_eq!(number_s, number_map_s);
 
         // multi-element tuple variants serialize like {"<variant>": [values..]}
-        let flag_s = opts.to_vec(&Bar::Flag("foo".to_string(), true)).unwrap();
+        let flag_s = to_vec(&Bar::Flag("foo".to_string(), true)).unwrap();
         let mut flag_map = BTreeMap::new();
         flag_map.insert(
             "Flag",
@@ -199,7 +208,7 @@ mod std_tests {
         assert_eq!(flag_s, flag_map_s);
 
         // struct-variants serialize like {"<variant>", {struct..}}
-        let point_s = opts.to_vec(&Bar::Point { x: 5, y: -5 }).unwrap();
+        let point_s = to_vec(&Bar::Point { x: 5, y: -5 }).unwrap();
         let mut point_map = BTreeMap::new();
         point_map.insert("Point", Value::Map(struct_map));
         let point_map_s = to_vec(&point_map).unwrap();
diff --git a/tests/ser.rs b/tests/ser.rs
index a9a1443..d374ce2 100644
--- a/tests/ser.rs
+++ b/tests/ser.rs
@@ -182,13 +182,6 @@ mod std_tests {
             serializer.serialize_u64(9).unwrap();
         }
         assert_eq!(vec, b"\xd9\xd9\xf7\x09");
-
-        let sd = ser::SerializerOptions {
-            self_describe: true,
-            ..Default::default()
-        };
-        let vec = sd.to_vec(&9).unwrap();
-        assert_eq!(vec, b"\xd9\xd9\xf7\x09");
     }
 
     #[test]
diff --git a/tests/std_types.rs b/tests/std_types.rs
index 3b85183..7a7ded4 100644
--- a/tests/std_types.rs
+++ b/tests/std_types.rs
@@ -167,13 +167,13 @@ mod std_tests {
         test_color_enum_transparent,
         Color,
         Color::Other(42),
-        "82654f74686572182a"
+        "a1654f74686572182a"
     );
     testcase!(
         test_color_enum_with_alpha,
         Color,
         Color::Alpha(234567, 60),
-        "8365416c7068611a00039447183c"
+        "a165416c706861821a00039447183c"
     );
     testcase!(test_i128_a, i128, -1i128, "20");
     testcase!(