Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add serialize_map_struct and deserialize_map_struct #2431

Closed
wants to merge 4 commits into from

Conversation

clouds56
Copy link

This might fixes #2350, #1346 and #1584.

This PR should not break any exist crates/code.

Problem

When a struct S has a field with attr flatten, the struct would be treated as map

struct S {
  name: String,
  #[serde(flatten)]
  others: HashMap<String, i32>,
}

This approach works well widely in format that doesn't care with type name (like serde_json), while sometimes it's annoying that we would like to treat map and struct differently (like ron, envy).

This PR

add serialize_map_struct and deserialize_map_struct

    fn deserialize_map_struct<V>(
        self,
        name: &'static str,
        fields: &'static [&'static str],
        visitor: V,
    ) -> Result<V::Value, Self::Error>
    where
        V: Visitor<'de>,
    {
        let _ = name;
        let _ = fields;
        self.deserialize_map(visitor)
    }

    fn serialize_map_struct(
        self,
        name: &'static str,
        len: Option<usize>,
    ) -> Result<Self::SerializeMap, Self::Error> {
        let _ = name;
        let _ = len;
        self.serialize_map(len)
    }
  1. which simply delegates to serialize_map and deserialize_map, so nothing would be broken
  2. pass name (as well as fields in deserialize), so your serializer could tell it is a struct other than map
  3. add serialize_field to SerializeMap in order to get field names if you interest.
  4. consist with other interface
    • we have serialize_tuple, serialize_tuple_struct and serialize_tuple_variant all handle with tuple, but with different parameters to indicate different semantics,
    • the same story goes with serialize_unit, serialize_newtype_struct, serialize_struct,
    • now it went to serialize_map. serialize_map_struct is also a map, just come with additional information for struct.

Alternatives

ron-rs/ron#115 (comment) shows a workaround that used in serde-starlark

#[derive(Serialize)]
pub struct RustLibrary {
    // ...
    #[serde(flatten)]
    pub common: CommonAttrs,
}

#[derive(Serialize)]
#[serde(untagged)]
pub enum Starlark {
    // ...
    #[serde(serialize_with = "serialize::rust_library")]
    RustLibrary(RustLibrary),
}

// serialize::rust_library
pub fn rust_library<S>(rule: &RustLibrary, serializer: S) -> Result<S::Ok, S::Error>
where
    S: Serializer,
{
    FunctionCall::new("rust_library", rule).serialize(serializer)
}

It wrapped with FunctionCall which gives RustLibrary struct name, as well as force child struct to be parsed as struct,

impl<'a, S> Serializer for FunctionCallSerializer<'a, S>
where
    S: Serializer,
{
    // ...
    fn serialize_map(self, len: Option<usize>) -> Result<Self::SerializeMap, Self::Error> {
        let len = len.unwrap_or(2);
        let mut delegate = self.delegate.serialize_struct("(", len)?;
        delegate.serialize_field("", self.function)?;
        Ok(FunctionCallArgs { delegate })
    }
}

This approach could handle things well for custom struct, but it is common that the flatten structs are import from some crates (and maybe deeply nested), so users had no control on this.

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Overall I don't think the new functionality that would be unlocked by this for data formats is compelling enough to warrant having new methods on Deserializer, Serializer, SerializeMap for this. I would prefer if the issue with RON were addressed in some other way (like the way which works well for serde-starlark that was pointed out in the RON issue).

In general it is not a goal for the serde traits to natively support all functionality in arbitrary formats, just some of the most ubiquitous commonalities across a range of formats.

@dtolnay dtolnay closed this Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

serde_derive: flatten causes struct serializer to lose access to struct name
2 participants