Skip to content
This repository has been archived by the owner on May 6, 2020. It is now read-only.

empty-element tag can't be parsed by codegen #32

Open
ibotty opened this issue Nov 1, 2016 · 19 comments
Open

empty-element tag can't be parsed by codegen #32

ibotty opened this issue Nov 1, 2016 · 19 comments
Labels

Comments

@ibotty
Copy link

ibotty commented Nov 1, 2016

#[derive(Deserialize, Debug)]
#[serde(rename="a")]
pub struct A {
    b1: String
}

compiled with serde_codegen::expand(&src, &dst), and

extern crate serde;
extern crate serde_xml;

include!(concat!(env!("OUT_DIR"), "/a.rs"));

fn main() {
    // let deserialized: Result<serde_xml::value::Element, serde_xml::Error> = serde_xml::de::from_str(r"<a><b1>v</b1><b2/></a>");
    let deserialized: Result<A, serde_xml::Error> = serde_xml::de::from_str(r"<a><b1>v</b1><b2/></a>");
    println!("deserialized: {:?}", deserialized);
}

Fails with 'SyntaxError(expected end tag, 1, 22))'. Removing the empty-element tag '' let's it work, as is parsing to serde_xml::value::Element.

That is with

"checksum serde 0.8.16 (registry+https://github.com/rust-lang/crates.io-index)" = "1105e65d0a0b212d2d735c8b5a4f6aba2adc501e8ad4497e9f1a39e4c4ac943e"
"checksum serde_codegen 0.8.16 (registry+https://github.com/rust-lang/crates.io-index)" = "446384bcfd7d9276a23b51e9dc14341909ad05377b68ac5da6f83a7a2094bcd0"
"checksum serde_codegen_internals 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)" = "318f7e77aa5187391d74aaf4553d2189f56b0ce25e963414c951b97877ffdcec"
"checksum serde_xml 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)" = "56346e526b0828da6b7a6a867076a6ae22d188ffd7a5511b4ffbd815def0ba95"
@ibotty
Copy link
Author

ibotty commented Nov 1, 2016

I forgot to mention: adding a unit field b2:() lets it parse it, i.e. using

pub struct A {
    b1: String,
    b2: ()
}

@oli-obk
Copy link

oli-obk commented Nov 1, 2016

Dupe of #19

@oli-obk oli-obk closed this as completed Nov 1, 2016
@oli-obk
Copy link

oli-obk commented Nov 1, 2016

This is simply a feature I haven't gotten around to implement.

@ibotty
Copy link
Author

ibotty commented Nov 1, 2016

I am not sure it's the same. I can ignore fields (don't mention them) without any problem, but not empty-element tags. I cannot reproduce issue #19, btw.

@oli-obk
Copy link

oli-obk commented Nov 1, 2016

Hmm ok. I'll investigate

@oli-obk oli-obk reopened this Nov 1, 2016
@oli-obk oli-obk added the bug label Nov 1, 2016
@indiv0
Copy link

indiv0 commented Dec 1, 2016

Any progress on this?

I'm not entirely sure if I'm having the same issue, but I have the following types:

pub struct Subpod {
    ...
    pub plaintext: Option<Plaintext>,
    ...
}

pub type Plaintext = String;

and a test which has:

...
<subpod title="">
    ...
    <plaintext/>
</subpod>
...

This test used to run successfully before, but not now. Which makes me think there's been a regression. The test has not been changed.

@oli-obk
Copy link

oli-obk commented Dec 1, 2016

Indeed there were breaking changes, that's why the version was updated.

If your "plaintext" field is always there, but sometimes empty and sometimes not, then you probably need it to be of a newtype type that contains an option.

So something along the lines of

pub struct Subpod {
    ...
    pub plaintext: Plaintext,
    ...
}

#[derive(Deserialize)]
pub struct Plaintext(Option<String>);

This change was necessary, because otherwise we could not have nested options.

@indiv0
Copy link

indiv0 commented Dec 1, 2016

Makes sense why you updated it. However I tried your fix and now I get:

running 6 tests
test model::tests::test_img_deserializer ... ok
test model::tests::test_plaintext_deserializer ... FAILED
test model::tests::test_pod_deserializer ... FAILED
test model::tests::test_infos_deserializer ... ok
test model::tests::test_query_result_deserializer ... FAILED
test model::tests::test_subpod_deserializer ... FAILED

failures:

---- model::tests::test_plaintext_deserializer stdout ----
        thread 'model::tests::test_plaintext_deserializer' panicked at 'called `Result::unwrap()` on an `Err` value: SyntaxError(serde expected some value: Invalid type. Expected `Str`, 0, 0)', ../src/libcore/result.rs:837
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- model::tests::test_pod_deserializer stdout ----
        thread 'model::tests::test_pod_deserializer' panicked at 'called `Result::unwrap()` on an `Err` value: SyntaxError(serde expected some value: Invalid type. Expected `Str`, 0, 0)', ../src/libcore/result.rs:837

---- model::tests::test_query_result_deserializer stdout ----
        thread 'model::tests::test_query_result_deserializer' panicked at 'called `Result::unwrap()` on an `Err` value: SyntaxError(serde expected some value: Invalid type. Expected `Str`, 0, 0)', ../src/libcore/result.rs:837

---- model::tests::test_subpod_deserializer stdout ----
        thread 'model::tests::test_subpod_deserializer' panicked at 'called `Result::unwrap()` on an `Err` value: SyntaxError(serde expected some value: Invalid type. Expected `Str`, 0, 0)', ../src/libcore/result.rs:837


failures:
    model::tests::test_plaintext_deserializer
    model::tests::test_pod_deserializer
    model::tests::test_query_result_deserializer
    model::tests::test_subpod_deserializer

test result: FAILED. 2 passed; 4 failed; 0 ignored; 0 measured

error: test failed

@oli-obk
Copy link

oli-obk commented Dec 1, 2016

ok, I'll investigate. Stay on an older serde_xml version for now, 0.8 should still work.

@indiv0
Copy link

indiv0 commented Dec 1, 2016

Alright

@oli-obk
Copy link

oli-obk commented Dec 1, 2016

@indiv0 I think the solution is to move away from Option<String> and to String entirely. So your <plaintext/> would deserialize to an empty string.

Although this depends on serde-rs/serde#635 .

Or do you need to be able to distinguish between <plaintext/>, <plaintext></plaintext> and the tag missing entirely?

@indiv0
Copy link

indiv0 commented Dec 1, 2016

Well I'm trying to maintain a wrapper for the WolframAlpha API.

I can't remember if the tag can be missing entirely, but I do need to be able to handle both <plaintext/> and <plaintext>ABC</plaintext> at least. I'll have to check if the tag is ever missing.

@ibotty
Copy link
Author

ibotty commented Dec 1, 2016

I just want to ignore the tag altogether, preferably without having a dummy field. I'll try the fix (or "fix") later.

@oli-obk
Copy link

oli-obk commented Dec 1, 2016

@indiv0 with the serde-pr you can do Option<String>:

  • missing tag => None
  • <plaintext/> => Some("")
  • <plaintext></plaintext> => Some("")
  • <plaintext>text</plaintext> => Some("text")

or if you just use a String:

  • missing tag => ""
  • <plaintext/> => ""
  • <plaintext></plaintext> => ""
  • <plaintext>text</plaintext> => "text"

@indiv0
Copy link

indiv0 commented Dec 1, 2016

OK awesome I think that'll work. I'll test it tomorrow.

@oli-obk
Copy link

oli-obk commented Dec 1, 2016

The branch has been merged. It's not been published yet, but you can use the serde github repo as a dependency in your code, serde-xml should then pull that in automatically.

@SamWhited
Copy link

Is there anything blocking this fix from being released? I'm also running into this issue, but would prefer not to depend on a GitHub dep if possible. I'd be happy to help unblock if possible.

@oli-obk
Copy link

oli-obk commented Jan 27, 2017

@SamWhited this is not a serde_xml issue, but a serde issue, serde has been released a while ago, all you need to do is update your serde dependency to the most recent 0.8 branch.

@SamWhited
Copy link

SamWhited commented Jan 27, 2017

I see; I thought I was on 0.9.1, but maybe not, I'll check when I'm next at my desk. Sorry for the noise and confusion.

EDIT: I was on 0.8.23 after all.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

4 participants