Add minimal support for #[serde(flatten)] roundtrips through maps (ro…

…n-rs#455) * Detect flattened structs in `Deserializer::deserialize_map` * Add minimal and targeted support for `#[serde(flatten)]` * Fix the serde flatten canary for MSRV 1.57 * Added extra tests for missing quotation marks in flattened struct field names * Clean up the serde flatten canary capture * Document the #[serde(flatten)] hack * Final small cleanup --------- Co-authored-by: Clouds Flowing <[email protected]>
juntyr · Aug 16, 2023 · b96117b · b96117b
1 parent 48d72cf
commit b96117b
Show file tree

Hide file tree

Showing 5 changed files with 397 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add `escape_strings` option to `PrettyConfig` to allow serialising with or without escaping ([#426](https://github.com/ron-rs/ron/pull/426))
 - Add CIFuzz GitHub action
 - Add `compact_maps` and `compact_structs` options to `PrettyConfig` to allow serialising maps and structs on a single line ([#448](https://github.com/ron-rs/ron/pull/448))
+- Add minimal support for `#[serde(flatten)]` with roundtripping through RON maps ([#455](https://github.com/ron-rs/ron/pull/455))
 
 ## [0.8.1] - 2023-08-??
 - Fix issues [#277](https://github.com/ron-rs/ron/issues/277) and [#405](https://github.com/ron-rs/ron/issues/405) with `Value::Map` `IntoIter` and extraneous item check for `Value::Seq` ([#406](https://github.com/ron-rs/ron/pull/406))

diff --git a/README.md b/README.md
@@ -107,7 +107,10 @@ Note the following advantages of RON over JSON:
 RON is not designed to be a fully self-describing format (unlike JSON) and is thus not guaranteed to work when [`deserialize_any`](https://docs.rs/serde/latest/serde/trait.Deserializer.html#tymethod.deserialize_any) is used instead of its typed alternatives. In particular, the following Serde attributes are not yet supported:
 - `#[serde(tag = "type")]`, i.e. internally tagged enums
 - `#[serde(untagged)]`, i.e. untagged enums
-- `#[serde(flatten)]`, i.e. flattening an inner struct into its outer container
+
+Furthermore, `#[serde(flatten)]` only has limited support and relies on a small hack [^serde-flatten-hack]. Specifically, flattened structs are only serialised as maps and deserialised from maps. However, this limited implementation supports full roundtripping.
+
+[^serde-flatten-hack]: Deserialising a flattened struct from a map requires that the struct's [`Visitor::expecting`](https://docs.rs/serde/latest/serde/de/trait.Visitor.html#tymethod.expecting) implementation formats a string starting with `"struct "`. This is the case for automatically-derived [`Deserialize`](https://docs.rs/serde/latest/serde/de/trait.Deserialize.html) impls on structs.
 
 ## RON syntax overview
 

diff --git a/src/de/id.rs b/src/de/id.rs
@@ -4,11 +4,12 @@ use super::{Deserializer, Error, Result};
 
 pub struct IdDeserializer<'a, 'b: 'a> {
     d: &'a mut Deserializer<'b>,
+    map_as_struct: bool,
 }
 
 impl<'a, 'b: 'a> IdDeserializer<'a, 'b> {
-    pub fn new(d: &'a mut Deserializer<'b>) -> Self {
-        IdDeserializer { d }
+    pub fn new(map_as_struct: bool, d: &'a mut Deserializer<'b>) -> Self {
+        IdDeserializer { d, map_as_struct }
     }
 }
 
@@ -19,7 +20,14 @@ impl<'a, 'b: 'a, 'c> de::Deserializer<'b> for &'c mut IdDeserializer<'a, 'b> {
     where
         V: Visitor<'b>,
     {
-        self.d.deserialize_identifier(visitor)
+        if self.map_as_struct {
+            self.d.bytes.expect_byte(b'"', Error::ExpectedString)?;
+        }
+        let result = self.d.deserialize_identifier(visitor);
+        if self.map_as_struct {
+            self.d.bytes.expect_byte(b'"', Error::ExpectedStringEnd)?;
+        }
+        result
     }
 
     fn deserialize_str<V>(self, visitor: V) -> Result<V::Value>

diff --git a/src/de/mod.rs b/src/de/mod.rs
@@ -1,5 +1,9 @@
 /// Deserialization module.
-use std::{borrow::Cow, io, str};
+use std::{
+    borrow::Cow,
+    io::{self, Write},
+    str,
+};
 
 use base64::Engine;
 use serde::{
@@ -179,7 +183,7 @@ impl<'de> Deserializer<'de> {
 
             let value = guard_recursion! { self =>
                 visitor
-                    .visit_map(CommaSeparated::new(b')', self))
+                    .visit_map(CommaSeparated::new(Terminator::Struct, self))
                     .map_err(|err| {
                         struct_error_name(
                             err,
@@ -533,7 +537,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
 
         if self.bytes.consume("[") {
             let value = guard_recursion! { self =>
-                visitor.visit_seq(CommaSeparated::new(b']', self))?
+                visitor.visit_seq(CommaSeparated::new(Terminator::Seq, self))?
             };
             self.bytes.skip_ws()?;
 
@@ -556,7 +560,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
             self.newtype_variant = false;
 
             let value = guard_recursion! { self =>
-                visitor.visit_seq(CommaSeparated::new(b')', self))?
+                visitor.visit_seq(CommaSeparated::new(Terminator::Tuple, self))?
             };
             self.bytes.skip_ws()?;
 
@@ -593,11 +597,29 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
     where
         V: Visitor<'de>,
     {
+        // Detect `#[serde(flatten)]` as a struct deserialised as a map
+        const SERDE_FLATTEN_CANARY: &[u8] = b"struct ";
+
+        struct VisitorExpecting<V>(V);
+        impl<'de, V: Visitor<'de>> std::fmt::Display for VisitorExpecting<&'_ V> {
+            fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
+                self.0.expecting(fmt)
+            }
+        }
+
         self.newtype_variant = false;
 
+        let mut canary_buffer = [0u8; SERDE_FLATTEN_CANARY.len()];
+        let _ = write!(canary_buffer.as_mut(), "{}", VisitorExpecting(&visitor));
+        let terminator = if canary_buffer == SERDE_FLATTEN_CANARY {
+            Terminator::MapAsStruct
+        } else {
+            Terminator::Map
+        };
+
         if self.bytes.consume("{") {
             let value = guard_recursion! { self =>
-                visitor.visit_map(CommaSeparated::new(b'}', self))?
+                visitor.visit_map(CommaSeparated::new(terminator, self))?
             };
             self.bytes.skip_ws()?;
 
@@ -674,14 +696,33 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
     }
 }
 
+#[derive(Clone, Copy, PartialEq, Eq)]
+enum Terminator {
+    Map,
+    MapAsStruct,
+    Tuple,
+    Struct,
+    Seq,
+}
+
+impl Terminator {
+    fn as_byte(self) -> u8 {
+        match self {
+            Terminator::Map | Terminator::MapAsStruct => b'}',
+            Terminator::Tuple | Terminator::Struct => b')',
+            Terminator::Seq => b']',
+        }
+    }
+}
+
 struct CommaSeparated<'a, 'de: 'a> {
     de: &'a mut Deserializer<'de>,
-    terminator: u8,
+    terminator: Terminator,
     had_comma: bool,
 }
 
 impl<'a, 'de> CommaSeparated<'a, 'de> {
-    fn new(terminator: u8, de: &'a mut Deserializer<'de>) -> Self {
+    fn new(terminator: Terminator, de: &'a mut Deserializer<'de>) -> Self {
         CommaSeparated {
             de,
             terminator,
@@ -694,7 +735,7 @@ impl<'a, 'de> CommaSeparated<'a, 'de> {
 
         match (
             self.had_comma,
-            self.de.bytes.peek_or_eof()? != self.terminator,
+            self.de.bytes.peek_or_eof()? != self.terminator.as_byte(),
         ) {
             // Trailing comma, maybe has a next element
             (true, has_element) => Ok(has_element),
@@ -733,12 +774,14 @@ impl<'de, 'a> de::MapAccess<'de> for CommaSeparated<'a, 'de> {
         K: DeserializeSeed<'de>,
     {
         if self.has_element()? {
-            if self.terminator == b')' {
-                guard_recursion! { self.de =>
-                    seed.deserialize(&mut IdDeserializer::new(&mut *self.de)).map(Some)
-                }
-            } else {
-                guard_recursion! { self.de => seed.deserialize(&mut *self.de).map(Some) }
+            match self.terminator {
+                Terminator::Struct => guard_recursion! { self.de =>
+                    seed.deserialize(&mut IdDeserializer::new(false, &mut *self.de)).map(Some)
+                },
+                Terminator::MapAsStruct => guard_recursion! { self.de =>
+                    seed.deserialize(&mut IdDeserializer::new(true, &mut *self.de)).map(Some)
+                },
+                _ => guard_recursion! { self.de => seed.deserialize(&mut *self.de).map(Some) },
             }
         } else {
             Ok(None)