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

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

Open
fwcd opened this issue Jan 29, 2022 · 4 comments
Open

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

fwcd opened this issue Jan 29, 2022 · 4 comments
Labels
bug serde Issues related to mapping from Rust types to XML

Comments

@fwcd
Copy link
Contributor

fwcd commented Jan 29, 2022

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.

@Mingun Mingun added bug serde Issues related to mapping from Rust types to XML labels May 21, 2022
@rrichardson
Copy link

rrichardson commented Oct 7, 2022

@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
Copy link

I found #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.

@Mingun
Copy link
Collaborator

Mingun commented Oct 7, 2022

$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)

@xkr47
Copy link

xkr47 commented Dec 14, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug serde Issues related to mapping from Rust types to XML
Projects
None yet
Development

No branches or pull requests

4 participants