quick-xml icon indicating copy to clipboard operation
quick-xml copied to clipboard

Deserializing `$primitive=` unit variants doesn't work correctly yet

Open fwcd opened this issue 3 years ago • 4 comments
trafficstars

Currently, the deserializer doesn't recognize a primitive unit variant (i.e. one that uses the mechanism introduced in #304) correctly. Consider the following example (simplified from the unit tests):

enum Node {
  Unit,
  #[serde(rename = "$primitive=PrimitiveUnit")]
  PrimitiveUnit
}

Deserializing it with from_str("PrimitiveUnit").unwrap() throws

thread ... panicked at 'called `Result::unwrap()` on an `Err` value: Custom("unknown variant `PrimitiveUnit`, expected one of `Unit`, `$primitive=PrimitiveUnit`")`

Experimenting a bit with EnumAccess, tracking the primitive variants in a similar fashion to unflattened fields seems to do the trick on this test (see this branch and this implementation). However, it doesn't fix the error when the enum is nested in a struct, e.g. like this:

struct Wrapper {
  node: Node
}

In this case, deserializing from_str(r#"<Wrapper node="PrimitiveUnit"/>"#) yields us the same error. The problem is that the deserializer attempts to deserialize it as a string (apparently?) in the context of a MapAccess. I am not entirely sure how the Serde machinery works here, but we might have to keep track of primitive variants even outside of direct enum contexts, which could be quite a bit harder to do.

The corresponding unit tests can be found here.

fwcd avatar Jan 29 '22 14:01 fwcd

@fwcd thanks for investigating this.

I would like to have a go at fixing this. Has anyone started researching it? Is there a simple fix, or would there likely have to be some major machinery changes? It seems like that a node would just have to "remember" its primitive designation.

Edit misunderstood the original example.

Should we just add a new Node type to represent primitive Units?

rrichardson avatar Oct 07 '22 15:10 rrichardson

I found https://github.com/tafia/quick-xml/pull/304

It appears that we just need to execute the opposite step for de, perhaps in a similar fashion as what de does for $unflatten in structs.

I'll take a stab at implementing it.

rrichardson avatar Oct 07 '22 16:10 rrichardson

$primitive= prefix will be removed in #490, so the problem disappears. The current course for mapping:

enum E {
  Unit
}
struct AnyName {
  element: E,

  #[serde(rename = "@attribute")] // prepend @
  attribute: E,

  #[serde(rename = "#any")] // special name
  any_content: E,

  #[serde(rename = "#text")] // special name
  text_content: E,
}

assert_eq!(
  from_str("\
    <any-name attribute=\"Unit\">\
      <Unit/>\
      <!-- #any content -->\
      <Unit/>\
      <!-- #text content -->\
      Unit\
    </any-name>\
  ").unwrap(),
  AnyName {
    element: E::Unit,
    attribute: E::Unit,
    any_content: E::Unit,
    text_content: E::Unit,
  }
);

Note, that this is only illustration, #any and #text fields does not supported together in one struct (even when one is flattened from another struct)

Feel free to investigate, what of that not yet working (assuming that #490 is merged)

Mingun avatar Oct 07 '22 16:10 Mingun

FYI you can work around this issue like this:

enum Node {
  Unit,
  #[serde(rename(serialize = "$primitive=PrimitiveUnit"))]
  PrimitiveUnit
}

In case you'd need to rename it on deserialize also, you could use:

  #[serde(rename(serialize = "$primitive=primitive-unit", deserialize = "primitive-unit"))]

Source: Serde Field attributes documentation

xkr47 avatar Dec 14 '22 10:12 xkr47