Repository: tafia/quick-xml (rust)

Issue:
Sequences of enums at root level are not parsed correctly
The following test is failed on second `assert_eq!` (`cargo test --all-features -- issue500`):
```rust
#[test]
fn issue500() {
    #[derive(Debug, Deserialize, Serialize, PartialEq)]
    struct TagOne {}

    #[derive(Debug, Deserialize, Serialize, PartialEq)]
    struct TagTwo {}

    #[derive(Debug, Deserialize, Serialize, PartialEq)]
    enum Tag {
        TagOne(TagOne),
        TagTwo(TagTwo),
    }

    #[derive(Debug, Deserialize, Serialize, PartialEq)]
    struct Root {
        #[serde(rename = "$value", default)]
        data: Vec<Tag>,
    }

    let data: Root = from_str("\
        <root>\
            <TagOne></TagOne>\
            <TagTwo></TagTwo>\
            <TagOne></TagOne>\
        </root>\
    ").unwrap();

    assert_eq!(
        data,
        Root {
            data: vec![
                Tag::TagOne(TagOne {}),
                Tag::TagTwo(TagTwo {}),
                Tag::TagOne(TagOne {}),
            ],
        }
    );

    let data: Vec<Tag> = from_str("\
        <TagOne></TagOne>\
        <TagTwo></TagTwo>\
        <TagOne></TagOne>\
    ").unwrap();

    assert_eq!(
        data,
        vec![
            Tag::TagOne(TagOne {}),
            Tag::TagTwo(TagTwo {}),
            Tag::TagOne(TagOne {}),
        ]
    );
}
```
Output:
```diff
---- issue500 stdout ----
thread 'issue500' panicked at 'assertion failed: `(left == right)`

Diff < left / right > :
 [
     TagOne(
         TagOne,
     ),
>    TagTwo(
>        TagTwo,
>    ),
     TagOne(
         TagOne,
     ),
 ]
```

Pull request description:
Fix deserialization of top-level sequences of enums
Fixes #500. The behavior of top-level sequence actually very simple -- try to deserialize until `Eof` is received, which is the only stop marker.

After the fix in the first commit it became obvious, that some refactoring could be done to put highly coupled code together, which is done in the next 2 commits.

Patch:
diff --git a/Changelog.md b/Changelog.md
index e16465a..0e66701 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -18,6 +18,14 @@
   In particular, that means that maps with numeric and numeric-like keys (for
   example, `"42"`) no longer can be serialized because [XML name] cannot start
   from a digit
+- [#500]: Fix deserialization of top-level sequences of enums, like
+  ```xml
+  <?xml version="1.0" encoding="UTF-8"?>
+  <!-- list of enum Enum { A, B, С } -->
+  <A/>
+  <B/>
+  <C/>
+  ```
 
 ### Misc Changes
 
@@ -44,6 +52,7 @@
   Refer to [documentation] for details.
 
 [#490]: https://github.com/tafia/quick-xml/pull/490
+[#500]: https://github.com/tafia/quick-xml/issues/500
 [XML name]: https://www.w3.org/TR/xml11/#NT-Name
 [documentation]: https://docs.rs/quick-xml/0.27.0/quick_xml/de/index.html#difference-between-text-and-value-special-names
 
diff --git a/src/de/map.rs b/src/de/map.rs
index 84bea43..b5c27f7 100644
--- a/src/de/map.rs
+++ b/src/de/map.rs
@@ -2,9 +2,9 @@
 
 use crate::{
     de::key::QNameDeserializer,
-    de::seq::{not_in, TagFilter},
     de::simple_type::SimpleTypeDeserializer,
     de::{str2bool, DeEvent, Deserializer, XmlRead, TEXT_KEY, VALUE_KEY},
+    encoding::Decoder,
     errors::serialize::DeError,
     events::attributes::IterState,
     events::BytesStart,
@@ -144,6 +144,8 @@ enum ValueSource {
     Nested,
 }
 
+////////////////////////////////////////////////////////////////////////////////////////////////////
+
 /// A deserializer that extracts map-like structures from an XML. This deserializer
 /// represents a one XML tag:
 ///
@@ -549,6 +551,72 @@ where
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 
+/// Check if tag `start` is included in the `fields` list. `decoder` is used to
+/// get a string representation of a tag.
+///
+/// Returns `true`, if `start` is not in the `fields` list and `false` otherwise.
+fn not_in(
+    fields: &'static [&'static str],
+    start: &BytesStart,
+    decoder: Decoder,
+) -> Result<bool, DeError> {
+    let tag = decoder.decode(start.name().into_inner())?;
+
+    Ok(fields.iter().all(|&field| field != tag.as_ref()))
+}
+
+/// A filter that determines, what tags should form a sequence.
+///
+/// There are two types of sequences:
+/// - sequence where each element represented by tags with the same name
+/// - sequence where each element can have a different tag
+///
+/// The first variant could represent a collection of structs, the second --
+/// a collection of enum variants.
+///
+/// In the second case we don't know what tag name should be expected as a
+/// sequence element, so we accept any element. Since the sequence are flattened
+/// into maps, we skip elements which have dedicated fields in a struct by using an
+/// `Exclude` filter that filters out elements with names matching field names
+/// from the struct.
+///
+/// # Lifetimes
+///
+/// `'de` represents a lifetime of the XML input, when filter stores the
+/// dedicated tag name
+#[derive(Debug)]
+enum TagFilter<'de> {
+    /// A `SeqAccess` interested only in tags with specified name to deserialize
+    /// an XML like this:
+    ///
+    /// ```xml
+    /// <...>
+    ///   <tag/>
+    ///   <tag/>
+    ///   <tag/>
+    ///   ...
+    /// </...>
+    /// ```
+    ///
+    /// The tag name is stored inside (`b"tag"` for that example)
+    Include(BytesStart<'de>), //TODO: Need to store only name instead of a whole tag
+    /// A `SeqAccess` interested in tags with any name, except explicitly listed.
+    /// Excluded tags are used as struct field names and therefore should not
+    /// fall into a `$value` category
+    Exclude(&'static [&'static str]),
+}
+
+impl<'de> TagFilter<'de> {
+    fn is_suitable(&self, start: &BytesStart, decoder: Decoder) -> Result<bool, DeError> {
+        match self {
+            Self::Include(n) => Ok(n.name() == start.name()),
+            Self::Exclude(fields) => not_in(fields, start, decoder),
+        }
+    }
+}
+
+////////////////////////////////////////////////////////////////////////////////////////////////////
+
 /// An accessor to sequence elements forming a value for struct field.
 /// Technically, this sequence is flattened out into structure and sequence
 /// elements are overlapped with other fields of a structure
@@ -750,3 +818,20 @@ where
         self.map.de.is_human_readable()
     }
 }
+
+////////////////////////////////////////////////////////////////////////////////////////////////////
+
+#[test]
+fn test_not_in() {
+    let tag = BytesStart::new("tag");
+
+    assert_eq!(not_in(&[], &tag, Decoder::utf8()).unwrap(), true);
+    assert_eq!(
+        not_in(&["no", "such", "tags"], &tag, Decoder::utf8()).unwrap(),
+        true
+    );
+    assert_eq!(
+        not_in(&["some", "tag", "included"], &tag, Decoder::utf8()).unwrap(),
+        false
+    );
+}
diff --git a/src/de/mod.rs b/src/de/mod.rs
index e3b50ca..e4efcba 100644
--- a/src/de/mod.rs
+++ b/src/de/mod.rs
@@ -333,7 +333,6 @@ macro_rules! deserialize_primitives {
 mod escape;
 mod key;
 mod map;
-mod seq;
 mod simple_type;
 mod var;
 
@@ -345,7 +344,7 @@ use crate::{
     name::QName,
     reader::Reader,
 };
-use serde::de::{self, Deserialize, DeserializeOwned, Visitor};
+use serde::de::{self, Deserialize, DeserializeOwned, DeserializeSeed, SeqAccess, Visitor};
 use std::borrow::Cow;
 #[cfg(feature = "overlapped-lists")]
 use std::collections::VecDeque;
@@ -967,7 +966,7 @@ where
     where
         V: Visitor<'de>,
     {
-        visitor.visit_seq(seq::TopLevelSeqAccess::new(self)?)
+        visitor.visit_seq(self)
     }
 
     fn deserialize_map<V>(self, visitor: V) -> Result<V::Value, DeError>
@@ -1022,6 +1021,32 @@ where
     }
 }
 
+/// An accessor to sequence elements forming a value for top-level sequence of XML
+/// elements.
+///
+/// Technically, multiple top-level elements violates XML rule of only one top-level
+/// element, but we consider this as several concatenated XML documents.
+impl<'de, 'a, R> SeqAccess<'de> for &'a mut Deserializer<'de, R>
+where
+    R: XmlRead<'de>,
+{
+    type Error = DeError;
+
+    fn next_element_seed<T>(&mut self, seed: T) -> Result<Option<T::Value>, Self::Error>
+    where
+        T: DeserializeSeed<'de>,
+    {
+        match self.peek()? {
+            DeEvent::Eof => Ok(None),
+
+            // Start(tag), End(tag), Text, CData
+            _ => seed.deserialize(&mut **self).map(Some),
+        }
+    }
+}
+
+////////////////////////////////////////////////////////////////////////////////////////////////////
+
 /// Trait used by the deserializer for iterating over input. This is manually
 /// "specialized" for iterating over `&[u8]`.
 ///
diff --git a/src/de/seq.rs b/src/de/seq.rs
deleted file mode 100644
index 00ad240..0000000
--- a/src/de/seq.rs
+++ /dev/null
@@ -1,170 +0,0 @@
-use crate::de::{DeError, DeEvent, Deserializer, XmlRead};
-use crate::encoding::Decoder;
-use crate::events::BytesStart;
-use serde::de::{DeserializeSeed, SeqAccess};
-
-/// Check if tag `start` is included in the `fields` list. `decoder` is used to
-/// get a string representation of a tag.
-///
-/// Returns `true`, if `start` is not in the `fields` list and `false` otherwise.
-pub fn not_in(
-    fields: &'static [&'static str],
-    start: &BytesStart,
-    decoder: Decoder,
-) -> Result<bool, DeError> {
-    let tag = decoder.decode(start.name().into_inner())?;
-
-    Ok(fields.iter().all(|&field| field != tag.as_ref()))
-}
-
-/// A filter that determines, what tags should form a sequence.
-///
-/// There are two types of sequences:
-/// - sequence where each element represented by tags with the same name
-/// - sequence where each element can have a different tag
-///
-/// The first variant could represent a collection of structs, the second --
-/// a collection of enum variants.
-///
-/// In the second case we don't know what tag name should be expected as a
-/// sequence element, so we accept any element. Since the sequence are flattened
-/// into maps, we skip elements which have dedicated fields in a struct by using an
-/// `Exclude` filter that filters out elements with names matching field names
-/// from the struct.
-///
-/// # Lifetimes
-///
-/// `'de` represents a lifetime of the XML input, when filter stores the
-/// dedicated tag name
-#[derive(Debug)]
-pub enum TagFilter<'de> {
-    /// A `SeqAccess` interested only in tags with specified name to deserialize
-    /// an XML like this:
-    ///
-    /// ```xml
-    /// <...>
-    ///   <tag/>
-    ///   <tag/>
-    ///   <tag/>
-    ///   ...
-    /// </...>
-    /// ```
-    ///
-    /// The tag name is stored inside (`b"tag"` for that example)
-    Include(BytesStart<'de>), //TODO: Need to store only name instead of a whole tag
-    /// A `SeqAccess` interested in tags with any name, except explicitly listed.
-    /// Excluded tags are used as struct field names and therefore should not
-    /// fall into a `$value` category
-    Exclude(&'static [&'static str]),
-}
-
-impl<'de> TagFilter<'de> {
-    pub fn is_suitable(&self, start: &BytesStart, decoder: Decoder) -> Result<bool, DeError> {
-        match self {
-            Self::Include(n) => Ok(n.name() == start.name()),
-            Self::Exclude(fields) => not_in(fields, start, decoder),
-        }
-    }
-}
-
-/// A SeqAccess
-pub struct TopLevelSeqAccess<'de, 'a, R>
-where
-    R: XmlRead<'de>,
-{
-    /// Deserializer used to deserialize sequence items
-    de: &'a mut Deserializer<'de, R>,
-    /// Filter that determines whether a tag is a part of this sequence.
-    ///
-    /// When feature `overlapped-lists` is not activated, iteration will stop
-    /// when found a tag that does not pass this filter.
-    ///
-    /// When feature `overlapped-lists` is activated, all tags, that not pass
-    /// this check, will be skipped.
-    filter: TagFilter<'de>,
-
-    /// Checkpoint after which all skipped events should be returned. All events,
-    /// that was skipped before creating this checkpoint, will still stay buffered
-    /// and will not be returned
-    #[cfg(feature = "overlapped-lists")]
-    checkpoint: usize,
-}
-
-impl<'a, 'de, R> TopLevelSeqAccess<'de, 'a, R>
-where
-    R: XmlRead<'de>,
-{
-    /// Creates a new accessor to a top-level sequence of XML elements.
-    pub fn new(de: &'a mut Deserializer<'de, R>) -> Result<Self, DeError> {
-        let filter = if let DeEvent::Start(e) = de.peek()? {
-            // Clone is cheap if event borrows from the input
-            TagFilter::Include(e.clone())
-        } else {
-            TagFilter::Exclude(&[])
-        };
-        Ok(Self {
-            #[cfg(feature = "overlapped-lists")]
-            checkpoint: de.skip_checkpoint(),
-
-            de,
-            filter,
-        })
-    }
-}
-
-#[cfg(feature = "overlapped-lists")]
-impl<'de, 'a, R> Drop for TopLevelSeqAccess<'de, 'a, R>
-where
-    R: XmlRead<'de>,
-{
-    fn drop(&mut self) {
-        self.de.start_replay(self.checkpoint);
-    }
-}
-
-impl<'de, 'a, R> SeqAccess<'de> for TopLevelSeqAccess<'de, 'a, R>
-where
-    R: XmlRead<'de>,
-{
-    type Error = DeError;
-
-    fn next_element_seed<T>(&mut self, seed: T) -> Result<Option<T::Value>, DeError>
-    where
-        T: DeserializeSeed<'de>,
-    {
-        let decoder = self.de.reader.decoder();
-        loop {
-            break match self.de.peek()? {
-                // If we see a tag that we not interested, skip it
-                #[cfg(feature = "overlapped-lists")]
-                DeEvent::Start(e) if !self.filter.is_suitable(e, decoder)? => {
-                    self.de.skip()?;
-                    continue;
-                }
-                // Stop iteration when list elements ends
-                #[cfg(not(feature = "overlapped-lists"))]
-                DeEvent::Start(e) if !self.filter.is_suitable(e, decoder)? => Ok(None),
-                DeEvent::End(_) => Ok(None),
-                DeEvent::Eof => Ok(None),
-
-                // Start(tag), Text, CData
-                _ => seed.deserialize(&mut *self.de).map(Some),
-            };
-        }
-    }
-}
-
-#[test]
-fn test_not_in() {
-    let tag = BytesStart::new("tag");
-
-    assert_eq!(not_in(&[], &tag, Decoder::utf8()).unwrap(), true);
-    assert_eq!(
-        not_in(&["no", "such", "tags"], &tag, Decoder::utf8()).unwrap(),
-        true
-    );
-    assert_eq!(
-        not_in(&["some", "tag", "included"], &tag, Decoder::utf8()).unwrap(),
-        false
-    );
-}


Test patch:
diff --git a/tests/serde-de.rs b/tests/serde-de.rs
index 25994ec..7ec1426 100644
--- a/tests/serde-de.rs
+++ b/tests/serde-de.rs
@@ -609,6 +609,26 @@ mod seq {
                 ]
             );
         }
+
+        /// Test for https://github.com/tafia/quick-xml/issues/500
+        #[test]
+        fn list_of_enum() {
+            #[derive(Debug, PartialEq, Deserialize)]
+            enum Enum {
+                One,
+                Two,
+            }
+
+            let data: Vec<Enum> = from_str(
+                r#"
+                <One/>
+                <Two/>
+                <One/>
+                "#,
+            )
+            .unwrap();
+            assert_eq!(data, vec![Enum::One, Enum::Two, Enum::One]);
+        }
     }
 
     /// Tests where each sequence item have an identical name in an XML.