Apply change request

src/meta.rs

@@ -2,28 +2,115 @@ use std::collections::HashMap;
 
 use serde_derive::{Deserialize, Serialize};
 
+/// Path-based metadata to serialize with a value.
+///
+/// Path-based in this context means that the metadata is linked
+/// to the data in a relative and hierarchical fashion by tracking
+/// the current absolute path of the field being serialized.
+///
+/// # Example
+///
+/// ```
+/// # use ron::{ser::PrettyConfig, meta::Field};
+///
+/// #[derive(serde::Serialize)]
+/// struct Creature {
+///     seconds_since_existing: usize,
+///     linked: Option<Box<Self>>,
+/// }
+///
+/// let mut config = PrettyConfig::default();
+///
+/// config
+///     .meta
+///     .field
+///     // The path meta defaults to no root structure,
+///     // so we either provide a prebuilt one or initialize
+///     // an empty one to build.
+///     .get_or_insert_with(Field::empty)
+///     .build_fields(|fields| {
+///         fields
+///             // Get or insert the named field
+///             .field("seconds_since_existing")
+///             .with_doc("Outer seconds_since_existing");
+///         fields
+///             .field("linked")
+///             // Doc metadata is serialized preceded by three forward slashes and a space for each line
+///             .with_doc("Optional.\nProvide another creature to be wrapped.")
+///             // Even though it's another Creature, the fields have different paths, so they are addressed separately.
+///             .build_fields(|fields| {
+///                 fields
+///                     .field("seconds_since_existing")
+///                     .with_doc("Inner seconds_since_existing");
+///             });
+///     });
+///
+/// let value = Creature {
+///     seconds_since_existing: 0,
+///     linked: Some(Box::new(Creature {
+///         seconds_since_existing: 0,
+///         linked: None,
+///     })),
+/// };
+///
+/// let s = ron::ser::to_string_pretty(&value, config).unwrap();
+///
+/// assert_eq!(s, r#"(
+///     /// Outer seconds_since_existing
+///     seconds_since_existing: 0,
+///     /// Optional.
+///     /// Provide another creature to be wrapped.
+///     linked: Some((
+///         /// Inner seconds_since_existing
+///         seconds_since_existing: 0,
+///         linked: None,
+///     )),
+/// )"#);
+/// ```
+///
+/// # Identical paths
+///
+/// Especially in enums and tuples it's possible for fields
+/// to share a path, thus being unable to be addressed separately.
+///
+/// ```no_run
+/// enum Kind {
+///     A {
+///         field: (),
+///     },  // ^
+///         // cannot be addressed separately because they have the same path
+///     B { // v
+///         field: (),
+///     },
+/// }
+/// ```
+///
+/// ```no_run
+/// struct A {
+///     field: (),
+/// }
+///
+/// struct B {
+///     field: (),
+/// }
+///
+/// type Value = (
+///     A,
+///  // ^
+///  // These are different types, but they share two fields with the same path: `buf` and `len`
+///  // v
+///     B,
+/// );
+/// ```
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Default)]
-pub struct Meta {
-    fields: Fields,
+pub struct PathMeta {
+    pub field: Option<Field>,
 }
 
-impl Meta {
-    /// Get a reference to the named field position metadata.
-    #[must_use]
-    pub fn fields(&self) -> &Fields {
-        &self.fields
-    }
-
-    /// Get a mutable reference to the named field position metadata.
-    pub fn fields_mut(&mut self) -> &mut Fields {
-        &mut self.fields
-    }
-}
-
-/// The metadata and inner [Fields] of a field.
+/// The metadata and inner [`Fields`] of a field.
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Default)]
 pub struct Field {
-    meta: String,
+    doc: String,
     fields: Option<Fields>,
 }
 
@@ -32,62 +119,52 @@ impl Field {
     #[must_use]
     pub const fn empty() -> Self {
         Self {
-            meta: String::new(),
+            doc: String::new(),
             fields: None,
         }
     }
 
-    /// Create a new field metadata.
-    pub fn new(meta: impl Into<String>, fields: Option<Fields>) -> Self {
+    /// Create a new field metadata from parts.
+    pub fn new(doc: impl Into<String>, fields: Option<Fields>) -> Self {
         Self {
-            meta: meta.into(),
+            doc: doc.into(),
             fields,
         }
     }
 
-    /// Get the metadata of this field.
+    /// Get a shared reference to the documentation metadata of this field.
+    #[inline]
     #[must_use]
-    pub fn meta(&self) -> &str {
-        &self.meta
+    pub fn doc(&self) -> &str {
+        self.doc.as_str()
     }
 
-    /// Set the metadata of this field.
-    ///
-    /// ```
-    /// # use ron::meta::Field;
-    ///
-    /// let mut field = Field::empty();
-    ///
-    /// assert_eq!(field.meta(), "");
-    ///
-    /// field.with_meta("some meta");
-    ///
-    /// assert_eq!(field.meta(), "some meta");
-    /// ```
-    pub fn with_meta(&mut self, meta: impl Into<String>) -> &mut Self {
-        self.meta = meta.into();
-        self
+    /// Get a mutable reference to the documentation metadata of this field.
+    #[inline]
+    #[must_use]
+    pub fn doc_mut(&mut self) -> &mut String {
+        &mut self.doc
     }
 
-    /// Return whether the Field has metadata.
+    /// Set the documentation metadata of this field.
     ///
     /// ```
     /// # use ron::meta::Field;
     ///
     /// let mut field = Field::empty();
     ///
-    /// assert!(!field.has_meta());
+    /// assert_eq!(field.doc(), "");
     ///
-    /// field.with_meta("some");
+    /// field.with_doc("some meta");
     ///
-    /// assert!(field.has_meta());
+    /// assert_eq!(field.doc(), "some meta");
     /// ```
-    #[must_use]
-    pub fn has_meta(&self) -> bool {
-        !self.meta.is_empty()
+    pub fn with_doc(&mut self, doc: impl Into<String>) -> &mut Self {
+        self.doc = doc.into();
+        self
     }
 
-    /// Get a reference to the inner fields of this field, if it has any.
+    /// Get a shared reference to the inner fields of this field, if it has any.
     #[must_use]
     pub fn fields(&self) -> Option<&Fields> {
         self.fields.as_ref()
@@ -159,7 +236,7 @@ impl Field {
     }
 }
 
-/// Mapping of names to [Field]s.
+/// Mapping of names to [`Field`]s.
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Default)]
 pub struct Fields {
     fields: HashMap<String, Field>,
@@ -246,6 +323,20 @@ impl Fields {
         self.fields.insert(name.into(), field)
     }
 
+    /// Remove a field with the given name from the map.
+    ///
+    /// ```
+    /// # use ron::meta::{Fields, Field};
+    ///
+    /// let mut fields: Fields = [("a", Field::empty())].into_iter().collect();
+    ///
+    /// assert_eq!(fields.remove("a"), Some(Field::empty()));
+    /// assert_eq!(fields.remove("a"), None);
+    /// ```
+    pub fn remove(&mut self, name: impl AsRef<str>) -> Option<Field> {
+        self.fields.remove(name.as_ref())
+    }
+
     /// Get a mutable reference to the field with the provided `name`,
     /// inserting an empty [`Field`] if it didn't exist.
     ///

src/ser/mod.rs

@@ -7,7 +7,7 @@ use unicode_ident::is_xid_continue;
 use crate::{
     error::{Error, Result},
     extensions::Extensions,
-    meta::{Field, Meta},
+    meta::PathMeta,
     options::Options,
     parse::{is_ident_first_char, is_ident_raw_char, is_whitespace_char, LargeSInt, LargeUInt},
 };
@@ -111,7 +111,7 @@ pub struct PrettyConfig {
     /// Enable explicit number type suffixes like `1u16`
     pub number_suffixes: bool,
     /// Additional metadata to serialize
-    pub meta: Meta,
+    pub meta: PathMeta,
 }
 
 impl PrettyConfig {
@@ -362,7 +362,7 @@ impl Default for PrettyConfig {
             compact_structs: false,
             compact_maps: false,
             number_suffixes: false,
-            meta: Meta::default(),
+            meta: PathMeta::default(),
         }
     }
 }
@@ -380,7 +380,6 @@ pub struct Serializer<W: fmt::Write> {
     recursion_limit: Option<usize>,
     // Tracks the number of opened implicit `Some`s, set to 0 on backtracking
     implicit_some_depth: usize,
-    field_memory: Vec<&'static str>,
 }
 
 impl<W: fmt::Write> Serializer<W> {
@@ -433,7 +432,6 @@ impl<W: fmt::Write> Serializer<W> {
             newtype_variant: false,
             recursion_limit: options.recursion_limit,
             implicit_some_depth: 0,
-            field_memory: Vec::new(),
         })
     }
 
@@ -1319,7 +1317,14 @@ impl<'a, W: fmt::Write> ser::SerializeStruct for Compound<'a, W> {
     where
         T: ?Sized + Serialize,
     {
-        self.ser.field_memory.push(key);
+        let mut restore_field = self.ser.pretty.as_mut().and_then(|(config, _)| {
+            config.meta.field.take().map(|mut field| {
+                if let Some(fields) = field.fields_mut() {
+                    config.meta.field = fields.remove(key);
+                }
+                field
+            })
+        });
 
         if let State::First = self.state {
             self.state = State::Rest;
@@ -1339,24 +1344,23 @@ impl<'a, W: fmt::Write> ser::SerializeStruct for Compound<'a, W> {
             self.ser.indent()?;
 
             if let Some((ref config, _)) = self.ser.pretty {
-                let mut iter = self.ser.field_memory.iter();
-
-                let init = iter.next().and_then(|name| config.meta.fields().get(name));
-                let field = iter
-                    .try_fold(init, |field, name| {
-                        field.and_then(Field::fields).map(|fields| fields.get(name))
-                    })
-                    .flatten();
-
-                if let Some(field) = field {
-                    let lines: Vec<_> = field
-                        .meta()
+                if let Some(ref field) = config.meta.field {
+                    // TODO: `self.ser.indent()` borrows the entire serializer mutably,
+                    // consider constraining the signature in the future to avoid this heap allocation.
+                    let doc_lines: Vec<_> = field
+                        .doc()
                         .lines()
-                        .map(|line| format!("/// {line}\n"))
+                        .map(|line| {
+                            let mut buf = String::with_capacity(line.len() + 5);
+                            buf.push_str("/// ");
+                            buf.push_str(line);
+                            buf.push('\n');
+                            buf
+                        })
                         .collect();
 
-                    for line in lines {
-                        self.ser.output.write_str(&line)?;
+                    for doc_line in doc_lines {
+                        self.ser.output.write_str(&doc_line)?;
                         self.ser.indent()?;
                     }
                 }
@@ -1372,7 +1376,17 @@ impl<'a, W: fmt::Write> ser::SerializeStruct for Compound<'a, W> {
 
         guard_recursion! { self.ser => value.serialize(&mut *self.ser)? };
 
-        self.ser.field_memory.pop();
+        if let Some((ref mut config, _)) = self.ser.pretty {
+            std::mem::swap(&mut config.meta.field, &mut restore_field);
+
+            if let Some(ref mut field) = config.meta.field {
+                if let Some(fields) = field.fields_mut() {
+                    if let Some(restore_field) = restore_field {
+                        fields.insert(key, restore_field);
+                    }
+                }
+            }
+        };
 
         Ok(())
     }