Better errors for missing values

instant-xml-macros/src/de.rs

@@ -48,12 +48,13 @@ fn deserialize_scalar_enum(
         };
 
         let serialize_as = meta.serialize_as;
-        variants.extend(quote!(#serialize_as => #ident::#v_ident,));
+        variants.extend(quote!(Some(#serialize_as) => #ident::#v_ident,));
     }
 
     let generics = meta.xml_generics(BTreeSet::new());
     let (impl_generics, _, _) = generics.split_for_impl();
     let (_, ty_generics, where_clause) = input.generics.split_for_impl();
+    let type_str = ident.to_string();
 
     quote!(
         impl #impl_generics FromXml<'xml> for #ident #ty_generics #where_clause {
@@ -77,7 +78,10 @@ fn deserialize_scalar_enum(
 
                 let value = match deserializer.take_str()? {
                     #variants
-                    val => return Err(Error::UnexpectedValue(format!("enum variant not found for '{}'", val))),
+                    Some(val) => return Err(Error::UnexpectedValue(
+                        format!("enum variant not found for '{}'", val)
+                    )),
+                    None => return Err(Error::MissingValue(#type_str)),
                 };
 
                 *into = Some(value);
@@ -137,11 +141,13 @@ fn deserialize_forward_enum(
         }
 
         let v_ident = &variant.ident;
-        variants.extend(quote!(if <#no_lifetime_type as FromXml>::matches(id, None) {
+        variants.extend(
+            quote!(if <#no_lifetime_type as FromXml>::matches(id, None) {
                 let mut value = None;
                 <#no_lifetime_type as FromXml>::deserialize(deserializer, &mut value)?;
                 *into = value.map(#ident::#v_ident);
-        }));
+            }),
+        );
     }
 
     let generics = meta.xml_generics(borrowed);
@@ -226,6 +232,7 @@ fn deserialize_struct(
             &mut borrowed,
             &mut direct,
             field_meta,
+            &input.ident,
             &container_meta,
         );
 
@@ -337,6 +344,7 @@ fn named_field(
     borrowed: &mut BTreeSet<syn::Lifetime>,
     direct: &mut TokenStream,
     mut field_meta: FieldMeta,
+    type_name: &Ident,
     container_meta: &ContainerMeta,
 ) -> Result<(), syn::Error> {
     let field_name = field.ident.as_ref().unwrap();
@@ -462,10 +470,11 @@ fn named_field(
         }
     }
 
+    let field_str = format!("{type_name}::{field_name}");
     return_val.extend(quote!(
         #field_name: match #enum_name {
             Some(v) => v,
-            None => <#no_lifetime_type as FromXml>::missing_value()?,
+            None => <#no_lifetime_type as FromXml>::missing(#field_str)?,
         },
     ));
 
@@ -503,6 +512,7 @@ fn deserialize_tuple_struct(
             &mut declare_values,
             &mut return_val,
             &mut borrowed,
+            &input.ident,
         );
     }
 
@@ -547,11 +557,13 @@ fn unnamed_field(
     declare_values: &mut TokenStream,
     return_val: &mut TokenStream,
     borrowed: &mut BTreeSet<syn::Lifetime>,
+    type_name: &Ident,
 ) {
     let mut no_lifetime_type = field.ty.clone();
     discard_lifetimes(&mut no_lifetime_type, borrowed, false, true);
 
     let name = Ident::new(&format!("v{index}"), Span::call_site());
+    let field_str = format!("{type_name}::{index}");
     declare_values.extend(quote!(
         let #name = match <#no_lifetime_type as FromXml>::KIND {
             Kind::Element => match deserializer.next() {
@@ -564,7 +576,7 @@ fn unnamed_field(
                 }
                 Some(Ok(node)) => return Err(Error::UnexpectedNode(format!("{:?}", node))),
                 Some(Err(e)) => return Err(e),
-                None => return Err(Error::MissingValue(<#no_lifetime_type as FromXml>::KIND)),
+                None => return Err(Error::MissingValue(#field_str)),
             }
             Kind::Scalar => {
                 let mut value: Option<#no_lifetime_type> = None;
@@ -574,10 +586,11 @@ fn unnamed_field(
         };
     ));
 
+    let field_str = format!("{type_name}::{index}");
     return_val.extend(quote!(
         match #name {
             Some(v) => v,
-            None => <#no_lifetime_type as FromXml>::missing_value()?,
+            None => <#no_lifetime_type as FromXml>::missing(#field_str)?,
         },
     ));
 }

instant-xml/src/de.rs

@@ -4,7 +4,7 @@ use std::collections::{BTreeMap, VecDeque};
 use xmlparser::{ElementEnd, Token, Tokenizer};
 
 use crate::impls::decode;
-use crate::{Error, Id, Kind};
+use crate::{Error, Id};
 
 pub struct Deserializer<'cx, 'xml> {
     pub(crate) local: &'xml str,
@@ -30,13 +30,13 @@ impl<'cx, 'xml> Deserializer<'cx, 'xml> {
         }
     }
 
-    pub fn take_str(&mut self) -> Result<&'xml str, Error> {
+    pub fn take_str(&mut self) -> Result<Option<&'xml str>, Error> {
         match self.next() {
-            Some(Ok(Node::AttributeValue(s))) => Ok(s),
+            Some(Ok(Node::AttributeValue(s))) => Ok(Some(s)),
-            Some(Ok(Node::Text(s))) => Ok(s),
+            Some(Ok(Node::Text(s))) => Ok(Some(s)),
             Some(Ok(node)) => Err(Error::ExpectedScalar(format!("{node:?}"))),
             Some(Err(e)) => Err(e),
-            None => Err(Error::MissingValue(Kind::Scalar)),
+            None => Ok(None),
         }
     }
 
@@ -340,8 +340,10 @@ pub fn borrow_cow_str<'xml>(
         return Err(Error::DuplicateValue);
     }
 
-    let value = deserializer.take_str()?;
+    if let Some(value) = deserializer.take_str()? {
         *into = Some(decode(value));
+    };
+
     deserializer.ignore()?;
     Ok(())
 }
@@ -354,11 +356,12 @@ pub fn borrow_cow_slice_u8<'xml>(
         return Err(Error::DuplicateValue);
     }
 
-    let value = deserializer.take_str()?;
+    if let Some(value) = deserializer.take_str()? {
         *into = Some(match decode(value) {
             Cow::Borrowed(v) => Cow::Borrowed(v.as_bytes()),
             Cow::Owned(v) => Cow::Owned(v.into_bytes()),
         });
+    }
 
     deserializer.ignore()?;
     Ok(())

instant-xml/src/impls.rs

@@ -29,7 +29,11 @@ impl<'xml, T: FromStr> FromXml<'xml> for FromXmlStr<T> {
             return Err(Error::DuplicateValue);
         }
 
-        let value = deserializer.take_str()?;
+        let value = match deserializer.take_str()? {
+            Some(value) => value,
+            None => return Ok(()),
+        };
+
         match T::from_str(value) {
             Ok(value) => {
                 *into = Some(FromXmlStr(value));
@@ -63,6 +67,11 @@ impl<'xml> FromXml<'xml> for bool {
         }
 
         let value = match deserializer.take_str()? {
+            Some(value) => value,
+            None => return Ok(()),
+        };
+
+        let value = match value {
             "true" | "1" => true,
             "false" | "0" => false,
             val => {
@@ -215,8 +224,11 @@ impl<'xml> FromXml<'xml> for String {
             return Err(Error::DuplicateValue);
         }
 
-        let value = deserializer.take_str()?;
+        match deserializer.take_str()? {
-        *into = Some(decode(value).into_owned());
+            Some(value) => *into = Some(decode(value).into_owned()),
+            None => return Ok(()),
+        }
+
         Ok(())
     }
 
@@ -240,7 +252,11 @@ impl<'xml> FromXml<'xml> for &'xml str {
             return Err(Error::DuplicateValue);
         }
 
-        let value = deserializer.take_str()?;
+        let value = match deserializer.take_str()? {
+            Some(value) => value,
+            None => return Ok(()),
+        };
+
         match decode(value) {
             Cow::Borrowed(str) => *into = Some(str),
             Cow::Owned(_) => {
@@ -315,7 +331,7 @@ impl<'xml, T: FromXml<'xml>> FromXml<'xml> for Option<T> {
         Ok(())
     }
 
-    fn missing_value() -> Result<Self, Error> {
+    fn missing(_: &'static str) -> Result<Self, Error> {
         Ok(None)
     }
 
@@ -505,7 +521,7 @@ impl<'xml, T: FromXml<'xml>> FromXml<'xml> for Vec<T> {
         Ok(())
     }
 
-    fn missing_value() -> Result<Self, Error> {
+    fn missing(_: &'static str) -> Result<Self, Error> {
         Ok(Vec::new())
     }
 
@@ -579,8 +595,12 @@ impl<'xml> FromXml<'xml> for DateTime<Utc> {
             return Err(Error::DuplicateValue);
         }
 
-        let data = deserializer.take_str()?;
+        let value = match deserializer.take_str()? {
-        match DateTime::parse_from_rfc3339(data) {
+            Some(value) => value,
+            None => return Ok(()),
+        };
+
+        match DateTime::parse_from_rfc3339(value) {
             Ok(dt) if dt.timezone().utc_minus_local() == 0 => {
                 *into = Some(dt.with_timezone(&Utc));
                 Ok(())
@@ -635,8 +655,12 @@ impl<'xml> FromXml<'xml> for NaiveDate {
             return Err(Error::DuplicateValue);
         }
 
-        let data = deserializer.take_str()?;
+        let value = match deserializer.take_str()? {
-        match NaiveDate::parse_from_str(data, "%Y-%m-%d") {
+            Some(value) => value,
+            None => return Ok(()),
+        };
+
+        match NaiveDate::parse_from_str(value, "%Y-%m-%d") {
             Ok(d) => {
                 *into = Some(d);
                 Ok(())

instant-xml/src/lib.rs

@@ -41,8 +41,8 @@ pub trait FromXml<'xml>: Sized {
 
     // If the missing field is of type `Option<T>` then treat is as `None`,
     // otherwise it is an error.
-    fn missing_value() -> Result<Self, Error> {
+    fn missing(field: &'static str) -> Result<Self, Error> {
-        Err(Error::MissingValue(Self::KIND))
+        Err(Error::MissingValue(field))
     }
 
     const KIND: Kind;
@@ -63,7 +63,7 @@ pub fn from_str<'xml, T: FromXml<'xml>>(input: &'xml str) -> Result<T, Error> {
     T::deserialize(&mut Deserializer::new(root, &mut context), &mut value)?;
     match value {
         Some(value) => Ok(value),
-        None => T::missing_value(),
+        None => T::missing("<root>"),
     }
 }
 
@@ -100,8 +100,8 @@ pub enum Error {
     UnexpectedTag(String),
     #[error("missing tag")]
     MissingTag,
-    #[error("missing value")]
+    #[error("missing value: {0}")]
-    MissingValue(Kind),
+    MissingValue(&'static str),
     #[error("unexpected token: {0}")]
     UnexpectedToken(String),
     #[error("unknown prefix: {0}")]

instant-xml/tests/de-direct.rs

@@ -1,6 +1,6 @@
 use similar_asserts::assert_eq;
 
-use instant_xml::{from_str, Error, FromXml, Kind};
+use instant_xml::{from_str, Error, FromXml};
 
 #[derive(Debug, Eq, PartialEq, FromXml)]
 #[xml(ns("URI"))]
@@ -24,12 +24,12 @@ fn direct_namespaces() {
         from_str(
             "<StructDirectNamespace xmlns=\"URI\"><flag xmlns=\"WRONG\">true</flag></StructDirectNamespace>"
         ),
-        Err::<StructDirectNamespace, _>(Error::MissingValue(Kind::Scalar))
+        Err::<StructDirectNamespace, _>(Error::MissingValue("StructDirectNamespace::flag"))
     );
 
     // Wrong direct namespace - missing namespace
     assert_eq!(
         from_str("<StructDirectNamespace xmlns=\"URI\"><flag>true</flag></StructDirectNamespace>"),
-        Err::<StructDirectNamespace, _>(Error::MissingValue(Kind::Scalar))
+        Err::<StructDirectNamespace, _>(Error::MissingValue("StructDirectNamespace::flag"))
     );
 }

instant-xml/tests/de-ns.rs

@@ -1,6 +1,6 @@
 use similar_asserts::assert_eq;
 
-use instant_xml::{from_str, Error, FromXml, Kind};
+use instant_xml::{from_str, Error, FromXml};
 
 #[derive(Debug, Eq, PartialEq, FromXml)]
 struct NestedWrongNamespace {
@@ -39,7 +39,9 @@ fn default_namespaces() {
         from_str(
             "<NestedDe xmlns=\"WRONG\" xmlns:bar=\"BAZ\"><bar:flag>true</bar:flag></NestedDe>"
         ),
-        Err::<NestedDe, _>(Error::UnexpectedValue("unexpected root element NestedDe in namespace WRONG".to_owned()))
+        Err::<NestedDe, _>(Error::UnexpectedValue(
+            "unexpected root element NestedDe in namespace WRONG".to_owned()
+        ))
     );
 
     // Correct child namespace
@@ -72,7 +74,7 @@ fn default_namespaces() {
     assert_eq!(
         from_str("<StructWithWrongNestedNamespace xmlns=\"URI\" xmlns:dar=\"BAZ\"><NestedWrongNamespace><flag>true</flag></NestedWrongNamespace></StructWithWrongNestedNamespace>"),
         Err::<StructWithWrongNestedNamespace, _>(
-            Error::MissingValue(Kind::Element)
+            Error::MissingValue("StructWithWrongNestedNamespace::test")
         )
     );
 }
@@ -113,7 +115,7 @@ fn other_namespaces() {
         from_str(
             "<NestedOtherNamespace xmlns=\"URI\" xmlns:bar=\"WRONG\"><bar:flag>true</bar:flag></NestedOtherNamespace>"
         ),
-        Err::<NestedOtherNamespace, _>(Error::MissingValue(Kind::Scalar))
+        Err::<NestedOtherNamespace, _>(Error::MissingValue("NestedOtherNamespace::flag"))
     );
 
     // Other namespace not-nested - missing parser prefix
@@ -121,7 +123,7 @@ fn other_namespaces() {
         from_str(
             "<NestedOtherNamespace xmlns=\"URI\" xmlns:bar=\"BAR\"><flag>true</flag></NestedOtherNamespace>"
         ),
-        Err::<NestedOtherNamespace, _>(Error::MissingValue(Kind::Scalar))
+        Err::<NestedOtherNamespace, _>(Error::MissingValue("NestedOtherNamespace::flag"))
     );
 
     // Correct child other namespace