Repository: tafia/quick-xml (rust)

Issue:
[Question]: Deserialize optional vector
Afaik the following line in `Cargo.toml` should mean that I'm using the latest git version of `quick-xml`:
```
quick-xml = { git = "https://github.com/tafia/quick-xml", features = ['serialize']}
```
Although I've put question in the title this issue might be a bug, but I can't tell whether I'm doing something wrong or if it is indeed a bug.

I've got the following xml:

```xml
<ENTRY>
  <CUE_V2 NAME="foo"></CUE_V2>
  <CUE_V2 NAME="bar"></CUE_V2>
</ENTRY>
```

And I've got the following struct to deserialize:
```rust
use serde::{Deserialize, Serialize};

#[derive(Debug, Serialize, Deserialize)]
#[serde(rename = "ENTRY")]
struct Entry {
    #[serde(rename = "CUE_V2")]
    cues: Vec<Cue>,
}

#[derive(Debug, Serialize, Deserialize)]
#[serde_with::serde_as]
struct Cue {
    #[serde(rename = "@NAME")]
    name: String,
}
```

This works but is incorrect since the cue vector should be optional like so:
```rust
#[derive(Debug, Serialize, Deserialize)]
#[serde(rename = "ENTRY")]
struct Entry {
    #[serde(rename = "CUE_V2")]
    cues: Option<Vec<Cue>>,
}
```

But this gives the error:
```
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: UnexpectedEnd([69, 78, 84, 82, 89])', src/main.rs:330:56
```

There might be a trivial solution, but after two days of using rust in total I can't seem to find it :)

If I may take the opportunity to ask another unrelated question, how do I build the documentation for the git version of the package locally? I've been looking through #490 but I think having the actual documentation would be a bit better :)

Pull request description:
 Fix using incorrect deserializer when deserialize `Option` values in maps and structs
Fixes #510. The reason for the error is that an incorrect deserializer was used after forwarding. The fix changes all methods that can forward actual deserialization to another deserializer, like that:
```rust
    fn deserialize_option<V>(self, visitor: V) -> Result<V::Value, DeError>
    where
        V: Visitor<'de>,
    {
        match self.peek()? {
            DeEvent::Text(t) if t.is_empty() => visitor.visit_none(),
            DeEvent::CData(t) if t.is_empty() => visitor.visit_none(),
            DeEvent::Eof => visitor.visit_none(),
            _ => visitor.visit_some(self), // <<< Forwarding deserialization to another deserializer
        }
    }
```

Patch:
diff --git a/Changelog.md b/Changelog.md
index d22f496..18a8d86 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -19,11 +19,14 @@
 
 - [#537]: Restore ability to deserialize attributes that represents XML namespace
   mappings (`xmlns:xxx`) that was broken since [#490]
+- [#510]: Fix an error of deserialization of `Option<T>` fields where `T` is some
+  sequence type (for example, `Vec` or tuple)
 
 ### Misc Changes
 
 [externally tagged]: https://serde.rs/enum-representations.html#externally-tagged
 [#490]: https://github.com/tafia/quick-xml/pull/490
+[#510]: https://github.com/tafia/quick-xml/issues/510
 [#537]: https://github.com/tafia/quick-xml/issues/537
 [#541]: https://github.com/tafia/quick-xml/pull/541
 
diff --git a/src/de/map.rs b/src/de/map.rs
index 75f7dbe..e0e3f69 100644
--- a/src/de/map.rs
+++ b/src/de/map.rs
@@ -480,10 +480,7 @@ where
 
     deserialize_primitives!(mut);
 
-    forward!(deserialize_option);
     forward!(deserialize_unit);
-    forward!(deserialize_unit_struct(name: &'static str));
-    forward!(deserialize_newtype_struct(name: &'static str));
 
     forward!(deserialize_map);
     forward!(deserialize_struct(
@@ -499,25 +496,11 @@ where
     forward!(deserialize_any);
     forward!(deserialize_ignored_any);
 
-    /// Tuple representation is the same as [sequences](#method.deserialize_seq).
-    fn deserialize_tuple<V>(self, _len: usize, visitor: V) -> Result<V::Value, DeError>
+    fn deserialize_option<V>(self, visitor: V) -> Result<V::Value, DeError>
     where
         V: Visitor<'de>,
     {
-        self.deserialize_seq(visitor)
-    }
-
-    /// Named tuple representation is the same as [unnamed tuples](#method.deserialize_tuple).
-    fn deserialize_tuple_struct<V>(
-        self,
-        _name: &'static str,
-        len: usize,
-        visitor: V,
-    ) -> Result<V::Value, DeError>
-    where
-        V: Visitor<'de>,
-    {
-        self.deserialize_tuple(len, visitor)
+        deserialize_option!(self.map.de, self, visitor)
     }
 
     /// Deserializes each `<tag>` in
@@ -758,10 +741,7 @@ where
 
     deserialize_primitives!(mut);
 
-    forward!(deserialize_option);
     forward!(deserialize_unit);
-    forward!(deserialize_unit_struct(name: &'static str));
-    forward!(deserialize_newtype_struct(name: &'static str));
 
     forward!(deserialize_map);
     forward!(deserialize_struct(
@@ -777,25 +757,11 @@ where
     forward!(deserialize_any);
     forward!(deserialize_ignored_any);
 
-    /// Representation of tuples the same as [sequences](#method.deserialize_seq).
-    fn deserialize_tuple<V>(self, _len: usize, visitor: V) -> Result<V::Value, DeError>
-    where
-        V: Visitor<'de>,
-    {
-        self.deserialize_seq(visitor)
-    }
-
-    /// Representation of named tuples the same as [unnamed tuples](#method.deserialize_tuple).
-    fn deserialize_tuple_struct<V>(
-        self,
-        _name: &'static str,
-        len: usize,
-        visitor: V,
-    ) -> Result<V::Value, DeError>
+    fn deserialize_option<V>(self, visitor: V) -> Result<V::Value, DeError>
     where
         V: Visitor<'de>,
     {
-        self.deserialize_tuple(len, visitor)
+        deserialize_option!(self.map.de, self, visitor)
     }
 
     /// This method deserializes a sequence inside of element that itself is a
diff --git a/src/de/mod.rs b/src/de/mod.rs
index 199bb99..09f9c59 100644
--- a/src/de/mod.rs
+++ b/src/de/mod.rs
@@ -912,8 +912,6 @@
 //! Use the [`Option`]. In that case inner array will always contains at least one
 //! element after deserialization:
 //! ```ignore
-//! // FIXME: #510,
-//! //        UnexpectedEnd([97, 110, 121, 45, 116, 97, 103])
 //! # use pretty_assertions::assert_eq;
 //! # use serde::Deserialize;
 //! # type Item = ();
@@ -935,10 +933,6 @@
 //! #   quick_xml::de::from_str(r#"<any-tag><item/><item/><item/></any-tag>"#).unwrap(),
 //! # );
 //! ```
-//! <div style="background:rgba(80, 240, 100, 0.20);padding:0.75em;">
-//!
-//! Currently not working. The bug is tracked in [#510].
-//! </div>
 //!
 //! See also [Frequently Used Patterns](#element-lists).
 //!
@@ -1728,7 +1722,6 @@
 //! [`deserialize_with`]: https://serde.rs/field-attrs.html#deserialize_with
 //! [#474]: https://github.com/tafia/quick-xml/issues/474
 //! [#497]: https://github.com/tafia/quick-xml/issues/497
-//! [#510]: https://github.com/tafia/quick-xml/issues/510
 
 // Macros should be defined before the modules that using them
 // Also, macros should be imported before using them
@@ -1822,6 +1815,50 @@ macro_rules! deserialize_primitives {
             self.deserialize_bytes(visitor)
         }
 
+        /// Representation of the named units the same as [unnamed units](#method.deserialize_unit)
+        fn deserialize_unit_struct<V>(
+            self,
+            _name: &'static str,
+            visitor: V,
+        ) -> Result<V::Value, DeError>
+        where
+            V: Visitor<'de>,
+        {
+            self.deserialize_unit(visitor)
+        }
+
+        fn deserialize_newtype_struct<V>(
+            self,
+            _name: &'static str,
+            visitor: V,
+        ) -> Result<V::Value, DeError>
+        where
+            V: Visitor<'de>,
+        {
+            self.deserialize_tuple(1, visitor)
+        }
+
+        /// Representation of tuples the same as [sequences](#method.deserialize_seq).
+        fn deserialize_tuple<V>(self, _len: usize, visitor: V) -> Result<V::Value, DeError>
+        where
+            V: Visitor<'de>,
+        {
+            self.deserialize_seq(visitor)
+        }
+
+        /// Representation of named tuples the same as [unnamed tuples](#method.deserialize_tuple).
+        fn deserialize_tuple_struct<V>(
+            self,
+            _name: &'static str,
+            len: usize,
+            visitor: V,
+        ) -> Result<V::Value, DeError>
+        where
+            V: Visitor<'de>,
+        {
+            self.deserialize_tuple(len, visitor)
+        }
+
         /// Identifiers represented as [strings](#method.deserialize_str).
         fn deserialize_identifier<V>(self, visitor: V) -> Result<V::Value, DeError>
         where
@@ -1832,6 +1869,17 @@ macro_rules! deserialize_primitives {
     };
 }
 
+macro_rules! deserialize_option {
+    ($de:expr, $deserializer:ident, $visitor:ident) => {
+        match $de.peek()? {
+            DeEvent::Text(t) if t.is_empty() => $visitor.visit_none(),
+            DeEvent::CData(t) if t.is_empty() => $visitor.visit_none(),
+            DeEvent::Eof => $visitor.visit_none(),
+            _ => $visitor.visit_some($deserializer),
+        }
+    };
+}
+
 mod key;
 mod map;
 mod simple_type;
@@ -2420,50 +2468,6 @@ where
         }
     }
 
-    /// Representation of the names units the same as [unnamed units](#method.deserialize_unit)
-    fn deserialize_unit_struct<V>(
-        self,
-        _name: &'static str,
-        visitor: V,
-    ) -> Result<V::Value, DeError>
-    where
-        V: Visitor<'de>,
-    {
-        self.deserialize_unit(visitor)
-    }
-
-    fn deserialize_newtype_struct<V>(
-        self,
-        _name: &'static str,
-        visitor: V,
-    ) -> Result<V::Value, DeError>
-    where
-        V: Visitor<'de>,
-    {
-        self.deserialize_tuple(1, visitor)
-    }
-
-    /// Representation of tuples the same as [sequences](#method.deserialize_seq).
-    fn deserialize_tuple<V>(self, _len: usize, visitor: V) -> Result<V::Value, DeError>
-    where
-        V: Visitor<'de>,
-    {
-        self.deserialize_seq(visitor)
-    }
-
-    /// Representation of named tuples the same as [unnamed tuples](#method.deserialize_tuple).
-    fn deserialize_tuple_struct<V>(
-        self,
-        _name: &'static str,
-        len: usize,
-        visitor: V,
-    ) -> Result<V::Value, DeError>
-    where
-        V: Visitor<'de>,
-    {
-        self.deserialize_tuple(len, visitor)
-    }
-
     fn deserialize_enum<V>(
         self,
         _name: &'static str,
@@ -2494,12 +2498,7 @@ where
     where
         V: Visitor<'de>,
     {
-        match self.peek()? {
-            DeEvent::Text(t) if t.is_empty() => visitor.visit_none(),
-            DeEvent::CData(t) if t.is_empty() => visitor.visit_none(),
-            DeEvent::Eof => visitor.visit_none(),
-            _ => visitor.visit_some(self),
-        }
+        deserialize_option!(self, self, visitor)
     }
 
     /// Always call `visitor.visit_unit()` because returned value ignored in any case.
diff --git a/src/escapei.rs b/src/escapei.rs
index f537233..8b959c1 100644
--- a/src/escapei.rs
+++ b/src/escapei.rs
@@ -1741,7 +1741,13 @@ fn parse_decimal(bytes: &str) -> Result<u32, EscapeError> {
 
 #[test]
 fn test_unescape() {
-    assert_eq!(unescape("test").unwrap(), Cow::Borrowed("test"));
+    let unchanged = unescape("test").unwrap();
+    // assert_eq does not check that Cow is borrowed, but we explicitly use Cow
+    // because it influences diff
+    // TODO: use assert_matches! when stabilized and other features will bump MSRV
+    assert_eq!(unchanged, Cow::Borrowed("test"));
+    assert!(matches!(unchanged, Cow::Borrowed(_)));
+
     assert_eq!(unescape("&lt;test&gt;").unwrap(), "<test>");
     assert_eq!(unescape("&#x30;").unwrap(), "0");
     assert_eq!(unescape("&#48;").unwrap(), "0");
@@ -1755,10 +1761,13 @@ fn test_unescape_with() {
         _ => None,
     };
 
-    assert_eq!(
-        unescape_with("test", custom_entities).unwrap(),
-        Cow::Borrowed("test")
-    );
+    let unchanged = unescape_with("test", custom_entities).unwrap();
+    // assert_eq does not check that Cow is borrowed, but we explicitly use Cow
+    // because it influences diff
+    // TODO: use assert_matches! when stabilized and other features will bump MSRV
+    assert_eq!(unchanged, Cow::Borrowed("test"));
+    assert!(matches!(unchanged, Cow::Borrowed(_)));
+
     assert_eq!(
         unescape_with("&lt;test&gt;", custom_entities).unwrap(),
         "<test>"
@@ -1771,7 +1780,13 @@ fn test_unescape_with() {
 
 #[test]
 fn test_escape() {
-    assert_eq!(escape("test"), Cow::Borrowed("test"));
+    let unchanged = escape("test");
+    // assert_eq does not check that Cow is borrowed, but we explicitly use Cow
+    // because it influences diff
+    // TODO: use assert_matches! when stabilized and other features will bump MSRV
+    assert_eq!(unchanged, Cow::Borrowed("test"));
+    assert!(matches!(unchanged, Cow::Borrowed(_)));
+
     assert_eq!(escape("<test>"), "&lt;test&gt;");
     assert_eq!(escape("\"a\"bc"), "&quot;a&quot;bc");
     assert_eq!(escape("\"a\"b&c"), "&quot;a&quot;b&amp;c");
@@ -1783,7 +1798,13 @@ fn test_escape() {
 
 #[test]
 fn test_partial_escape() {
-    assert_eq!(partial_escape("test"), Cow::Borrowed("test"));
+    let unchanged = partial_escape("test");
+    // assert_eq does not check that Cow is borrowed, but we explicitly use Cow
+    // because it influences diff
+    // TODO: use assert_matches! when stabilized and other features will bump MSRV
+    assert_eq!(unchanged, Cow::Borrowed("test"));
+    assert!(matches!(unchanged, Cow::Borrowed(_)));
+
     assert_eq!(partial_escape("<test>"), "&lt;test&gt;");
     assert_eq!(partial_escape("\"a\"bc"), "\"a\"bc");
     assert_eq!(partial_escape("\"a\"b&c"), "\"a\"b&amp;c");
diff --git a/src/events/mod.rs b/src/events/mod.rs
index 7ebed26..535032a 100644
--- a/src/events/mod.rs
+++ b/src/events/mod.rs
@@ -360,35 +360,28 @@ impl<'a> BytesDecl<'a> {
     /// # Examples
     ///
     /// ```
-    /// use std::borrow::Cow;
     /// use quick_xml::Error;
     /// use quick_xml::events::{BytesDecl, BytesStart};
     ///
     /// // <?xml version='1.1'?>
     /// let decl = BytesDecl::from_start(BytesStart::from_content(" version='1.1'", 0));
-    /// assert_eq!(
-    ///     decl.version().unwrap(),
-    ///     Cow::Borrowed(b"1.1".as_ref())
-    /// );
+    /// assert_eq!(decl.version().unwrap(), b"1.1".as_ref());
     ///
     /// // <?xml version='1.0' version='1.1'?>
     /// let decl = BytesDecl::from_start(BytesStart::from_content(" version='1.0' version='1.1'", 0));
-    /// assert_eq!(
-    ///     decl.version().unwrap(),
-    ///     Cow::Borrowed(b"1.0".as_ref())
-    /// );
+    /// assert_eq!(decl.version().unwrap(), b"1.0".as_ref());
     ///
     /// // <?xml encoding='utf-8'?>
     /// let decl = BytesDecl::from_start(BytesStart::from_content(" encoding='utf-8'", 0));
     /// match decl.version() {
-    ///     Err(Error::XmlDeclWithoutVersion(Some(key))) => assert_eq!(key, "encoding".to_string()),
+    ///     Err(Error::XmlDeclWithoutVersion(Some(key))) => assert_eq!(key, "encoding"),
     ///     _ => assert!(false),
     /// }
     ///
     /// // <?xml encoding='utf-8' version='1.1'?>
     /// let decl = BytesDecl::from_start(BytesStart::from_content(" encoding='utf-8' version='1.1'", 0));
     /// match decl.version() {
-    ///     Err(Error::XmlDeclWithoutVersion(Some(key))) => assert_eq!(key, "encoding".to_string()),
+    ///     Err(Error::XmlDeclWithoutVersion(Some(key))) => assert_eq!(key, "encoding"),
     ///     _ => assert!(false),
     /// }
     ///
diff --git a/src/reader/slice_reader.rs b/src/reader/slice_reader.rs
index fd4dbaa..f9ff503 100644
--- a/src/reader/slice_reader.rs
+++ b/src/reader/slice_reader.rs
@@ -212,6 +212,7 @@ impl<'a> Reader<&'a [u8]> {
     ///         <p>Usual XML rules does not apply inside it
     ///         <p>For example, elements not needed to be &quot;closed&quot;
     ///     "#));
+    /// assert!(matches!(text, Cow::Borrowed(_)));
     ///
     /// // Now we can enable checks again
     /// reader.check_end_names(true);
diff --git a/src/utils.rs b/src/utils.rs
index 185cff4..bd4ffe1 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -149,7 +149,7 @@ mod tests {
     #[test]
     fn write_byte_string0() {
         let bytes = ByteBuf(vec![10, 32, 32, 32, 32, 32, 32, 32, 32]);
-        assert_eq!(format!("{:?}", bytes), "\"0xA        \"".to_owned());
+        assert_eq!(format!("{:?}", bytes), "\"0xA        \"");
     }
 
     #[test]
@@ -160,7 +160,7 @@ mod tests {
         ]);
         assert_eq!(
             format!("{:?}", bytes),
-            r##""http://www.w3.org/2002/07/owl#""##.to_owned()
+            r##""http://www.w3.org/2002/07/owl#""##
         );
     }
 
@@ -169,6 +169,6 @@ mod tests {
         let bytes = ByteBuf(vec![
             67, 108, 97, 115, 115, 32, 73, 82, 73, 61, 34, 35, 66, 34,
         ]);
-        assert_eq!(format!("{:?}", bytes), r##""Class IRI=\"#B\"""##.to_owned());
+        assert_eq!(format!("{:?}", bytes), r##""Class IRI=\"#B\"""##);
     }
 }


Test patch:
diff --git a/tests/serde-issues.rs b/tests/serde-issues.rs
index 8e57df0..223183f 100644
--- a/tests/serde-issues.rs
+++ b/tests/serde-issues.rs
@@ -241,6 +241,48 @@ fn issue500() {
     );
 }
 
+/// Regression test for https://github.com/tafia/quick-xml/issues/510.
+#[test]
+fn issue510() {
+    #[derive(Debug, PartialEq, Serialize, Deserialize)]
+    #[serde(rename = "ENTRY")]
+    struct Entry {
+        #[serde(rename = "CUE_V2")]
+        cues: Option<Vec<Cue>>,
+    }
+
+    #[derive(Debug, PartialEq, Serialize, Deserialize)]
+    // #[serde_with::serde_as]
+    struct Cue {
+        #[serde(rename = "@NAME")]
+        name: String,
+    }
+
+    let data: Entry = from_str(
+        "\
+        <ENTRY>\
+            <CUE_V2 NAME='foo'></CUE_V2>\
+            <CUE_V2 NAME='bar'></CUE_V2>\
+        </ENTRY>\
+    ",
+    )
+    .unwrap();
+
+    assert_eq!(
+        data,
+        Entry {
+            cues: Some(vec![
+                Cue {
+                    name: "foo".to_string(),
+                },
+                Cue {
+                    name: "bar".to_string(),
+                },
+            ]),
+        }
+    );
+}
+
 /// Regression test for https://github.com/tafia/quick-xml/issues/537.
 ///
 /// This test checks that special `xmlns:xxx` attributes uses full name of
diff --git a/tests/unit_tests.rs b/tests/unit_tests.rs
index 4881ba0..502cd50 100644
--- a/tests/unit_tests.rs
+++ b/tests/unit_tests.rs
@@ -332,7 +332,7 @@ fn test_new_xml_decl_full() {
     let result = writer.into_inner();
     assert_eq!(
         String::from_utf8(result).expect("utf-8 output"),
-        "<?xml version=\"1.2\" encoding=\"utf-X\" standalone=\"yo\"?>".to_owned(),
+        "<?xml version=\"1.2\" encoding=\"utf-X\" standalone=\"yo\"?>",
         "writer output (LHS)"
     );
 }
@@ -347,7 +347,7 @@ fn test_new_xml_decl_standalone() {
     let result = writer.into_inner();
     assert_eq!(
         String::from_utf8(result).expect("utf-8 output"),
-        "<?xml version=\"1.2\" standalone=\"yo\"?>".to_owned(),
+        "<?xml version=\"1.2\" standalone=\"yo\"?>",
         "writer output (LHS)"
     );
 }
@@ -362,7 +362,7 @@ fn test_new_xml_decl_encoding() {
     let result = writer.into_inner();
     assert_eq!(
         String::from_utf8(result).expect("utf-8 output"),
-        "<?xml version=\"1.2\" encoding=\"utf-X\"?>".to_owned(),
+        "<?xml version=\"1.2\" encoding=\"utf-X\"?>",
         "writer output (LHS)"
     );
 }
@@ -377,7 +377,7 @@ fn test_new_xml_decl_version() {
     let result = writer.into_inner();
     assert_eq!(
         String::from_utf8(result).expect("utf-8 output"),
-        "<?xml version=\"1.2\"?>".to_owned(),
+        "<?xml version=\"1.2\"?>",
         "writer output (LHS)"
     );
 }
@@ -395,7 +395,7 @@ fn test_new_xml_decl_empty() {
     let result = writer.into_inner();
     assert_eq!(
         String::from_utf8(result).expect("utf-8 output"),
-        "<?xml version=\"\" encoding=\"\" standalone=\"\"?>".to_owned(),
+        "<?xml version=\"\" encoding=\"\" standalone=\"\"?>",
         "writer output (LHS)"
     );
 }
@@ -533,7 +533,7 @@ fn test_read_write_roundtrip_results_in_identity() -> Result<()> {
     }
 
     let result = writer.into_inner().into_inner();
-    assert_eq!(result, input.as_bytes());
+    assert_eq!(String::from_utf8(result).unwrap(), input);
     Ok(())
 }
 
@@ -559,7 +559,7 @@ fn test_read_write_roundtrip() -> Result<()> {
     }
 
     let result = writer.into_inner().into_inner();
-    assert_eq!(String::from_utf8(result).unwrap(), input.to_string());
+    assert_eq!(String::from_utf8(result).unwrap(), input);
     Ok(())
 }
 
@@ -589,7 +589,7 @@ fn test_read_write_roundtrip_escape_text() -> Result<()> {
     }
 
     let result = writer.into_inner().into_inner();
-    assert_eq!(String::from_utf8(result).unwrap(), input.to_string());
+    assert_eq!(String::from_utf8(result).unwrap(), input);
     Ok(())
 }