From f4fb07fca6c8e4554d1a1b0f4584ff5627c9d72c Mon Sep 17 00:00:00 2001
From: Dirkjan Ochtman <dirkjan@ochtman.nl>
Date: Fri, 2 Dec 2022 09:08:36 +0100
Subject: [PATCH] Improve error reporting, allow 0/1 as bool values

---
 instant-xml-macros/src/de.rs  |  6 +++---
 instant-xml/src/impls.rs      | 31 +++++++++++++++++++------------
 instant-xml/src/lib.rs        |  9 ++++++---
 instant-xml/tests/de-ns.rs    |  2 +-
 instant-xml/tests/entities.rs |  2 +-
 5 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/instant-xml-macros/src/de.rs b/instant-xml-macros/src/de.rs
index ddca809..afb6934 100644
--- a/instant-xml-macros/src/de.rs
+++ b/instant-xml-macros/src/de.rs
@@ -48,7 +48,7 @@ fn deserialize_scalar_enum(
         };
 
         let serialize_as = meta.serialize_as;
-        variants.extend(quote!(Ok(#serialize_as) => #ident::#v_ident,));
+        variants.extend(quote!(#serialize_as => #ident::#v_ident,));
     }
 
     let generics = meta.xml_generics(BTreeSet::new());
@@ -75,9 +75,9 @@ fn deserialize_scalar_enum(
                     return Err(Error::DuplicateValue);
                 }
 
-                let value = match deserializer.take_str() {
+                let value = match deserializer.take_str()? {
                     #variants
-                    _ => return Err(Error::UnexpectedValue("enum variant not found")),
+                    val => return Err(Error::UnexpectedValue(format!("enum variant not found for '{}'", val))),
                 };
 
                 *into = Some(value);
diff --git a/instant-xml/src/impls.rs b/instant-xml/src/impls.rs
index f59679e..672d8bc 100644
--- a/instant-xml/src/impls.rs
+++ b/instant-xml/src/impls.rs
@@ -1,3 +1,4 @@
+use std::any::type_name;
 use std::borrow::Cow;
 use std::fmt;
 use std::net::IpAddr;
@@ -34,7 +35,10 @@ impl<'xml, T: FromStr> FromXml<'xml> for FromXmlStr<T> {
                 *into = Some(FromXmlStr(value));
                 Ok(())
             }
-            Err(_) => Err(Error::UnexpectedValue("unable to parse value")),
+            Err(_) => Err(Error::UnexpectedValue(format!(
+                "unable to parse {} from `{value}`",
+                type_name::<T>()
+            ))),
         }
     }
 
@@ -58,15 +62,18 @@ impl<'xml> FromXml<'xml> for bool {
             return Err(Error::DuplicateValue);
         }
 
-        let mut value = None;
-        FromXmlStr::<Self>::deserialize(deserializer, &mut value)?;
-        match value {
-            Some(value) => {
-                *into = Some(value.0);
-                Ok(())
+        let value = match deserializer.take_str()? {
+            "true" | "1" => true,
+            "false" | "0" => false,
+            val => {
+                return Err(Error::UnexpectedValue(format!(
+                    "unable to parse bool from '{val}'"
+                )))
             }
-            None => Err(Error::MissingValue(Kind::Scalar)),
-        }
+        };
+
+        *into = Some(value);
+        Ok(())
     }
 
     const KIND: Kind = Kind::Scalar;
@@ -241,9 +248,9 @@ impl<'xml> FromXml<'xml> for &'xml str {
         match decode(value) {
             Cow::Borrowed(str) => *into = Some(str),
             Cow::Owned(_) => {
-                return Err(Error::UnexpectedValue(
-                    "string with escape characters cannot be deserialized as &str",
-                ))
+                return Err(Error::UnexpectedValue(format!(
+                    "string with escape characters cannot be deserialized as &str: '{value}'",
+                )))
             }
         }
 
diff --git a/instant-xml/src/lib.rs b/instant-xml/src/lib.rs
index e1ba43f..e3fc617 100644
--- a/instant-xml/src/lib.rs
+++ b/instant-xml/src/lib.rs
@@ -53,7 +53,10 @@ pub fn from_str<'xml, T: FromXml<'xml>>(input: &'xml str) -> Result<T, Error> {
     let id = context.element_id(&root)?;
 
     if !T::matches(id, None) {
-        return Err(Error::UnexpectedValue("unexpected root"));
+        return Err(Error::UnexpectedValue(format!(
+            "unexpected root element {} in namespace {}",
+            id.name, id.ns
+        )));
     }
 
     let mut value = None;
@@ -91,8 +94,8 @@ pub enum Error {
     Other(std::string::String),
     #[error("unexpected end of stream")]
     UnexpectedEndOfStream,
-    #[error("unexpected value")]
-    UnexpectedValue(&'static str),
+    #[error("unexpected value: '{0}'")]
+    UnexpectedValue(String),
     #[error("unexpected tag: {0}")]
     UnexpectedTag(String),
     #[error("missing tag")]
diff --git a/instant-xml/tests/de-ns.rs b/instant-xml/tests/de-ns.rs
index 1f3125e..f6f30cf 100644
--- a/instant-xml/tests/de-ns.rs
+++ b/instant-xml/tests/de-ns.rs
@@ -39,7 +39,7 @@ fn default_namespaces() {
         from_str(
             "<NestedDe xmlns=\"WRONG\" xmlns:bar=\"BAZ\"><bar:flag>true</bar:flag></NestedDe>"
         ),
-        Err::<NestedDe, _>(Error::UnexpectedValue("unexpected root"))
+        Err::<NestedDe, _>(Error::UnexpectedValue("unexpected root element NestedDe in namespace WRONG".to_owned()))
     );
 
     // Correct child namespace
diff --git a/instant-xml/tests/entities.rs b/instant-xml/tests/entities.rs
index 3aa15a0..4bde05e 100644
--- a/instant-xml/tests/entities.rs
+++ b/instant-xml/tests/entities.rs
@@ -31,7 +31,7 @@ fn escape_back() {
         from_str(
             "<StructSpecialEntities xmlns=\"URI\"><string>&lt;&gt;&amp;&quot;&apos;adsad&quot;</string><str>str&amp;</str></StructSpecialEntities>"
         ),
-        Err::<StructSpecialEntities, _>(Error::UnexpectedValue("string with escape characters cannot be deserialized as &str"))
+        Err::<StructSpecialEntities, _>(Error::UnexpectedValue("string with escape characters cannot be deserialized as &str: 'str&amp;'".to_owned()))
     );
 
     // Borrowed