From a1610f689ff06c51af725d22ca9b96e05ffe1fd6 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Sun, 2 Jun 2024 06:09:42 -0700 Subject: [PATCH] Allow direct String fields to match the empty string Empty is not the same as missing, and the empty string crops up a lot in upnp/dlna. --- instant-xml-macros/src/de.rs | 26 +++++++++++++ instant-xml/src/impls.rs | 17 ++++----- instant-xml/tests/de-direct.rs | 69 ++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 9 deletions(-) diff --git a/instant-xml-macros/src/de.rs b/instant-xml-macros/src/de.rs index 7146a71..81fa93c 100644 --- a/instant-xml-macros/src/de.rs +++ b/instant-xml-macros/src/de.rs @@ -234,6 +234,7 @@ fn deserialize_struct( let mut declare_values = TokenStream::new(); let mut return_val = TokenStream::new(); let mut direct = TokenStream::new(); + let mut after_loop = TokenStream::new(); let mut borrowed = BTreeSet::new(); for (index, field) in fields.named.iter().enumerate() { @@ -263,6 +264,7 @@ fn deserialize_struct( field_meta, &input.ident, &container_meta, + &mut after_loop, ); if let Err(err) = result { @@ -361,6 +363,7 @@ fn deserialize_struct( node => return Err(Error::UnexpectedNode(format!("{:?} in {}", node, #ident_str))), } } + #after_loop *into = Some(Self { #return_val }); Ok(()) @@ -401,6 +404,7 @@ fn deserialize_inline_struct( let mut declare_values = TokenStream::new(); let mut return_val = TokenStream::new(); let mut direct = TokenStream::new(); + let mut after_loop = TokenStream::new(); let mut borrowed = BTreeSet::new(); let mut matches = TokenStream::new(); @@ -433,6 +437,7 @@ fn deserialize_inline_struct( field_meta, &input.ident, &meta, + &mut after_loop, ); let data = match result { @@ -548,6 +553,7 @@ fn named_field<'a>( mut field_meta: FieldMeta, type_name: &Ident, container_meta: &ContainerMeta, + after_loop: &mut TokenStream, ) -> Result, syn::Error> { let field_name = field.ident.as_ref().unwrap(); let field_tag = field_meta.tag; @@ -598,6 +604,15 @@ fn named_field<'a>( let mut #val_name = <#no_lifetime_type as FromXml>::Accumulator::default(); )); + if field_meta.direct { + declare_values.extend(quote!( + // We will synthesize an empty text node when processing a direct + // element. Without this, we can miscategorize an empty tag as + // a missing tag + let mut seen_direct = false; + )); + } + let deserialize_with = field_meta .deserialize_with .map(|with| { @@ -630,10 +645,21 @@ fn named_field<'a>( } else if field_meta.direct { direct.extend(quote!( Node::Text(text) => { + seen_direct = true; let mut nested = deserializer.for_node(Node::Text(text)); <#no_lifetime_type>::deserialize(&mut #val_name, #field_str, &mut nested)?; } )); + // We can only enter this FromXml impl if the caller found the opening + // tag, so if we don't see the text node before the closing tag that is + // implied by terminating the loop, we need to populate the + // direct field with the implied empty text node. + after_loop.extend(quote!( + if !seen_direct { + let mut nested = deserializer.for_node(Node::Text("".into())); + <#no_lifetime_type>::deserialize(&mut #val_name, #field_str, &mut nested)?; + } + )); } else { tokens.r#match.extend(quote!( __Elements::#enum_name => match <#no_lifetime_type as FromXml>::KIND { diff --git a/instant-xml/src/impls.rs b/instant-xml/src/impls.rs index 270541b..7830ac3 100644 --- a/instant-xml/src/impls.rs +++ b/instant-xml/src/impls.rs @@ -270,10 +270,10 @@ impl<'xml> FromXml<'xml> for String { return Err(Error::DuplicateValue(field)); } - match deserializer.take_str()? { - Some(value) => *into = Some(value.into_owned()), - None => return Ok(()), - } + *into = Some(match deserializer.take_str()? { + Some(value) => value.into_owned(), + None => String::new(), + }); Ok(()) } @@ -300,12 +300,11 @@ impl<'xml, 'a> FromXml<'xml> for Cow<'a, str> { return Err(Error::DuplicateValue(field)); } - let value = match deserializer.take_str()? { - Some(value) => value, - None => return Ok(()), - }; + into.inner = Some(match deserializer.take_str()? { + Some(value) => value.into_owned().into(), + None => "".into(), + }); - into.inner = Some(value.into_owned().into()); Ok(()) } diff --git a/instant-xml/tests/de-direct.rs b/instant-xml/tests/de-direct.rs index c593a1e..e3ec9f0 100644 --- a/instant-xml/tests/de-direct.rs +++ b/instant-xml/tests/de-direct.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use similar_asserts::assert_eq; use instant_xml::{from_str, Error, FromXml}; @@ -33,3 +35,70 @@ fn direct_namespaces() { Err::(Error::MissingValue("StructDirectNamespace::flag")) ); } + +#[derive(Debug, Eq, PartialEq, FromXml)] +struct DirectString { + s: String, +} + +#[test] +fn direct_string() { + assert_eq!( + from_str("hello"), + Ok(DirectString { + s: "hello".to_string() + }) + ); +} + +#[derive(Debug, Eq, PartialEq, FromXml)] +struct DirectStr<'a> { + s: Cow<'a, str>, +} + +#[test] +fn direct_empty_str() { + assert_eq!( + from_str(""), + Ok(DirectStr { s: "".into() }) + ); +} + +#[test] +fn direct_missing_string() { + assert_eq!( + from_str(""), + Err::(Error::MissingValue("DirectString::s")) + ); +} + +#[derive(Debug, PartialEq, FromXml)] +struct ArtUri { + #[xml(direct)] + uri: String, +} + +#[derive(Debug, PartialEq, FromXml)] +struct Container { + art: Option, +} + +#[test] +fn container_empty_string() { + assert_eq!( + from_str(""), + Ok(Container { + art: Some(ArtUri { + uri: "".to_string() + }) + }) + ); + assert_eq!( + from_str(""), + Ok(Container { + art: Some(ArtUri { + uri: "".to_string() + }) + }) + ); +}