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

serde >=1.0.181 has breaking changes that fails enum_::adjacently_tagged tests #630

Closed
Mingun opened this issue Aug 14, 2023 · 2 comments · Fixed by #669
Closed

serde >=1.0.181 has breaking changes that fails enum_::adjacently_tagged tests #630

Mingun opened this issue Aug 14, 2023 · 2 comments · Fixed by #669
Assignees
Labels
serde Issues related to mapping from Rust types to XML

Comments

@Mingun
Copy link
Collaborator

Mingun commented Aug 14, 2023

After merging serde-rs/serde#2505 the following tests are failing:

cargo +nightly-gnu test --all-features --test serde-de
...
failures:

---- enum_::adjacently_tagged::newtype::elements stdout ----
[tests/serde-de.rs:18] s = "<root><tag>Newtype</tag><content>true</content></root>"
thread 'enum_::adjacently_tagged::newtype::elements' panicked at 'called `Result::unwrap()` on an `Err` value: Custom("unknown variant `tag`, expected one of `Unit`, `Newtype`, `Struct`, `Holder`, `Flatten`")', tests/serde-de.rs:5734:18
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- enum_::adjacently_tagged::tuple_struct::elements stdout ----
[tests/serde-de.rs:18] s = "<root><tag>Tuple</tag><content>42</content><content>answer</content></root>"
thread 'enum_::adjacently_tagged::tuple_struct::elements' panicked at 'called `Result::unwrap()` on an `Err` value: Custom("unknown variant `tag`, expected `Tuple`")', tests/serde-de.rs:5757:19

---- enum_::adjacently_tagged::nested_struct::elements stdout ----
[tests/serde-de.rs:18] s = "<root>\n                        <tag>Holder</tag>\n                        <content>\n                            <string>answer</string>\n                            <nested>\n                                <float>42</float>\n
              </nested>\n                        </content>\n                    </root>"
thread 'enum_::adjacently_tagged::nested_struct::elements' panicked at 'called `Result::unwrap()` on an `Err` value: Custom("unknown variant `tag`, expected one of `Unit`, `Newtype`, `Struct`, `Holder`, `Flatten`")', tests/serde-de.rs:5826:18

---- enum_::adjacently_tagged::struct_::elements stdout ----
[tests/serde-de.rs:18] s = "<root><tag>Struct</tag><content><float>42</float><string>answer</string></content></root>"
thread 'enum_::adjacently_tagged::struct_::elements' panicked at 'called `Result::unwrap()` on an `Err` value: Custom("unknown variant `tag`, expected one of `Unit`, `Newtype`, `Struct`, `Holder`, `Flatten`")', tests/serde-de.rs:5782:18

---- enum_::adjacently_tagged::unit::elements stdout ----
[tests/serde-de.rs:18] s = "<root><tag>Unit</tag></root>"
thread 'enum_::adjacently_tagged::unit::elements' panicked at 'called `Result::unwrap()` on an `Err` value: Custom("unknown variant `tag`, expected one of `Unit`, `Newtype`, `Struct`, `Holder`, `Flatten`")', tests/serde-de.rs:5713:78


failures:
    enum_::adjacently_tagged::nested_struct::elements
    enum_::adjacently_tagged::newtype::elements
    enum_::adjacently_tagged::struct_::elements
    enum_::adjacently_tagged::tuple_struct::elements
    enum_::adjacently_tagged::unit::elements

test result: FAILED. 338 passed; 5 failed; 27 ignored; 0 measured; 0 filtered out; finished in 0.21s

error: test failed, to rerun pass `--test serde-de`

Needs investigate what's wrong and or adapt our tests, or report bug upstream

@Mingun Mingun added the serde Issues related to mapping from Rust types to XML label Aug 14, 2023
@Mingun
Copy link
Collaborator Author

Mingun commented Aug 14, 2023

The problem that serde-rs/serde#2505 change the deserialization strategy for the tag field.
Previously, it was deserialized using deserialize_identifier. Adjacently tagged enum

#[derive(Deserialize)]
#[serde(tag = "tag", content = "content")]
enum AdjacentlyTagged {
  Unit,
}

in serde model represented as a struct with two fields tag and content and deserialized accordingly:

struct AdjacentlyTagged {
  tag: &'static str,
  content: /* depends on a tag */,
}

In XML this struct could be represented as

<any-name>
  <tag>Unit</tag>
  <!-- When tag assumes content -->
  <!-- <content>...</content> -->
</any-name>

Field tag of such struct deserialized using deserialize_identifier.

However, after serde-rs/serde#2505 the equivalent struct looks as:

struct AdjacentlyTagged {
  tag: TagEnum,
  content: /* depends on a tag */,
}

enum TagEnum {
  Unit,
}

The tag field now deserialized using deserialize_enum. The problem, that in quick-xml deserialize_identifier significantly different from deserialize_enum.

According to our deserialization policy, we pass to the field type the whole XML fragment, together with the tag name, so the type of field tag would be deserialized from the fragment <tag>Unit</tag>.

deserialize_identifier knows, that it should skip <tag> and </tag> and take into account only the inner content. Because inner content is a Text("Unit"), deserializer will push string "Unit" to the visitor that adjacently tagged enum provide and that visitor will convert that string to the variant.

deserialize_enum, however, will threat <tag> as a name of variant, because it is designed to be used in situations like

<A>...</A>
<B>...</B>
<C>...</C>

which is a typical xs::choice and is mapped to

enum E {
  A,
  B,
  C,
}

So the new representation (since serde v1.0.181) of adjacently tagged enums follows the scenario that we considered incorrect, but unfortunately, we cannot change the definition of a virtual struct, used by derive macro.

Because without renaming tag field in such structure into a special name $value having the enum type of field has no sense, it seems that the best choice would be to change our policy and automatically map this:

struct Struct {
  enum_field: EnumType,
}
enum EnumType {
  Variant,
  // ...
}

to

<any-name>
  <enum_field>Variant</enum_field>
</any-name>

or

<any-name enum_field="Variant" />

Of course, this will support only unit variants. This, however, requires to check that our consistence rules keeps untouched. Especially need to check how this will work with flattened fields, because flattening is always a source of a pain in serde.

@Mingun
Copy link
Collaborator Author

Mingun commented Aug 14, 2023

Serialization also changed, from ElementSerializer::serialize_str to ElementSerializer::serialize_unit_variant

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

Successfully merging a pull request may close this issue.

1 participant