RReverser / serde-xml-rs

xml-rs based deserializer for Serde (compatible with 1.0+)
https://crates.io/crates/serde-xml-rs
MIT License
273 stars 92 forks source link

Decoding 0 elements in to a Vec gives "custom: missing field `$value`" error #188

Open nikclayton opened 2 years ago

nikclayton commented 2 years ago

tl;dr: If an element can contain 0-N child elements, it's not sufficient to specify the child elements in a struct field of type Vec. You have to wrap the Vec in an Option. I found this to be counterintuitive.

I'm not sure if this is a software bug or an issue that should be more clearly explained in the documentation.

Longer:

Consider the following (it's a snippet from an Android resources file):

<?xml version="1.0" encoding="utf-8"?>
<resources>
    <style name="Theme" parent="Theme.AppCompat" />

    <style name="Theme.Default">
        <item name="colorPrimary">@color/default_backgroundColorTouched</item>
        <item name="colorPrimaryDark">@color/default_backgroundColorHighlighted</item>
        ....
    </style>
</resources>

My initial attempt at modelling this was:

#[derive(Debug, Serialize, Deserialize, PartialEq)]
struct Resources {
    #[serde(rename = "$value")]
    styles: Vec<Style>,
}

#[derive(Debug, Serialize, Deserialize, PartialEq)]
struct Style {
    name: String,
    parent: Option<String>,
    #[serde(rename = "$value")]
    items: Vec<Item>,
}

#[derive(Debug, Serialize, Deserialize, PartialEq)]
struct Item {
    name: String,
}

My expectation was that any style elements with no content (i.e., the first one), would parse correctly, giving a Style.items field of length 0.

You get an error instead, "custom: missing field$value`", which is not very helpful.

The fix is to explicitly mark the items as optional, by changing the Style definition to:

#[derive(Debug, Serialize, Deserialize, PartialEq)]
struct Style {
    name: String,
    parent: Option<String>,
    #[serde(rename = "$value")]
    items: Option<Vec<Item>>,  // <-- Option added here
}

I can see arguments both ways for this, which is why I'm not sure this is a software bug, but either way, I do think this should be called out in the documentation.

pavelskipenes commented 2 years ago

a vector can have 0 elements so it should be a valid state

marcelbuesing commented 1 year ago

Another workaround is adding a default.

#[serde(default, rename = "$value")]
items: Vec<Item>,