Capture other unrecognized fields

[dtolnay]

#[derive(Serialize, Deserialize)]
struct S {
    a: u32,
    b: String,
    #[serde(other)]
    other: Map<String, Value>,
}

{"a":0,"b":"","c":true}

This would deserialize into other containing "c" => true.

[TedDriggs]

I like this idea and might be able to submit a PR for it, but I have a few questions:

1. What are the type constraints on other? I'm guessing it works with any type that supports entries (so that it can report an error on duplicate fields and insert non-duplicate values).
2. Does the K type parameter of the map have to be String, or can it also be &'de str?
3. Does the V type parameter of the map have to be Value, or can it be any type? The latter seems more powerful, and I think it would work as long as local type inference can figure out what type to deserialize into.
4. Where is serde(other) valid?
   1. Named-field structs
   2. Named-field enum variants
   3. Anywhere else?
5. Should using serde(other) with serde(deny_unknown_fields) on a struct produce a warning?
6. What other attributes are valid with serde(other)?
   1. skip, skip_serializing, skip_serializing_if, and skip_deserializing
   2. rename_all
   3. borrow? (This is a new feature I'm not familiar with yet)
7. During serialization, should an error be generated if a key occurs on a named field and in other? Is name collision allowed if the named field's skip_serializing_if resolves to true?

[dtolnay]

1. It should support any type that implements visit_map (for deserialization) and uses serialize_map (for serialization). The handling of duplicate fields is baked into the Deserialize impl so doesn't need to be handled as part of this feature.

2. This would be handled by the Deserialize impl in visit_map.

3. This would be handled by the Deserialize impl in visit_map.

4. Just those two I think.

5. Yes, good idea.

6. For now I would just reject any other attributes. We can follow up later if anybody makes a strong case for a particular one.

7. Serde allows duplicate keys during serialization.

[clarfonthey]

I personally would prefer naming this something more verbose like other_fields. The extra typing will make the attribute more understandable in the long run, and right now, I wouldn't know what other means without having it explained.

[TedDriggs]

@dtolnay Right now, I only see errors in serde_derive_internals::Ctxt; is there a way to surface warnings?

[dtolnay]

No there is not. I guess make it an error.

[TedDriggs]

This is starting to feel suspiciously similar to #119, in that it will allow reading and writing of keys from a nested object into a flat namespace in the serialized object. If we had #[serde(flatten)], I feel like we could allow that attribute on a generic map type and avoid needing this separate attribute at all.

Assuming this is still desirable, I'm starting to wonder if this should be a container-level attribute rather than a field-level attribute.

#[derive(Serialize, Deserialize)]
#[serde(unknown_fields_in="other")]
pub struct Sample {
    hello: String,
    other: HashMap<String, Value>
}

The user experience is similar: The duplicated identifier is unfortunate, but semantically I think it's a little clearer. From an implementation perspective, a container-level approach would be marginally simpler:

1. Two fields can't both claim to be the recipient of unknown fields
2. Validation errors about deny_unknown_fields and unknown_fields_in are easier to surface and explain
3. Code generation in serde_derive::de::deserialize_map doesn't have to scan the fields to determine how to handle unknown values.

[daschl]

I'm interested in developing this feature, but I haven't dug into the serde codebase at all up until this point. Is it a "beginner" issue? If so and with a little guidance I'd like to take this one up.

[dtolnay]

Yes, go for it!

[daschl]

@dtolnay can you give me some pointers - like if the field or container level attribute is preferred

[dtolnay]

I would recommend handwriting some Deserialize impls for this and testing them against some JSON inputs to make sure they capture unrecognized fields the way you expected. I would recommend doing this before diving into attributes and proc macro stuff.

[daschl]

@dtolnay okay thanks I'll see how far I get with this and if I make progress I'll share it here.