Repository: tafia/quick-xml
Pull request #434 (2022-07-21 18:28:28)

Remove excessive serde tests and fix a bug
This PR removes tests which have better alternatives in `seq` submodule since #387.

Also, while working on namespaces support, I accidently found a bug in a deserializer, so I've added a test and a fix for it.

dralley: I used the merge conflict cleanup feature just to try it out - you can squash that commit if you want.
codecov-commenter: # [Codecov](https://codecov.io/gh/tafia/quick-xml/pull/434?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#434](https://codecov.io/gh/tafia/quick-xml/pull/434?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (acdb8c5) into [master](https://codecov.io/gh/tafia/quick-xml/commit/a5da6283349139dd59dbc64189d3093bbd0aacd6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (a5da628) will **decrease** coverage by `0.06%`.
> The diff coverage is `66.66%`.

```diff
@@            Coverage Diff             @@
##           master     #434      +/-   ##
==========================================
- Coverage   51.51%   51.44%   -0.07%     
==========================================
  Files          25       25              
  Lines       13310    13312       +2     
==========================================
- Hits         6856     6849       -7     
- Misses       6454     6463       +9     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `51.44% <66.66%> (-0.07%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/tafia/quick-xml/pull/434?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [src/de/map.rs](https://codecov.io/gh/tafia/quick-xml/pull/434/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2RlL21hcC5ycw==) | `72.35% <66.66%> (-0.21%)` | :arrow_down: |
| [src/de/var.rs](https://codecov.io/gh/tafia/quick-xml/pull/434/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2RlL3Zhci5ycw==) | `63.82% <0.00%> (-21.28%)` | :arrow_down: |
| [src/reader/mod.rs](https://codecov.io/gh/tafia/quick-xml/pull/434/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3JlYWRlci9tb2QucnM=) | `90.42% <0.00%> (ø)` | |
| [src/lib.rs](https://codecov.io/gh/tafia/quick-xml/pull/434/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2xpYi5ycw==) | `12.33% <0.00%> (+0.06%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/tafia/quick-xml/pull/434?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/tafia/quick-xml/pull/434?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Last update [a5da628...acdb8c5](https://codecov.io/gh/tafia/quick-xml/pull/434?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).


Patch:
diff --git a/Changelog.md b/Changelog.md
index 393b3bf..e9a74ec 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -58,6 +58,7 @@
 - [#421]: Fixed unknown bug in serde deserialization of externally tagged enums
   when an enum variant represented as a `Text` event (i.e. `<xml>tag</xml>`)
   and a document encoding is not an UTF-8
+- [#434]: Fixed incorrect error generated in some cases by serde deserializer
 
 ### Misc Changes
 
@@ -158,6 +159,7 @@
 - [#363]: Add tests for `Reader::read_event_impl` to ensure that proper events generated for corresponding inputs
 - [#407]: Improved benchmark suite to cover whole-document parsing, escaping and unescaping text
 - [#418]: Parameterized macrobenchmarks and comparative benchmarks, added throughput measurements via criterion
+- [#434]: Added more tests for serde deserialier
 
 [#8]: https://github.com/Mingun/fast-xml/pull/8
 [#9]: https://github.com/Mingun/fast-xml/pull/9
@@ -178,6 +180,7 @@
 [#418]: https://github.com/tafia/quick-xml/pull/418
 [#421]: https://github.com/tafia/quick-xml/pull/421
 [#423]: https://github.com/tafia/quick-xml/pull/423
+[#434]: https://github.com/tafia/quick-xml/pull/434
 [#437]: https://github.com/tafia/quick-xml/pull/437
 
 ## 0.23.0 -- 2022-05-08
diff --git a/src/de/map.rs b/src/de/map.rs
index c6261bf..2537457 100644
--- a/src/de/map.rs
+++ b/src/de/map.rs
@@ -298,7 +298,13 @@ where
                     };
                     key.map(Some)
                 }
-                _ => Ok(None),
+                // Stop iteration after reaching a closing tag
+                DeEvent::End(e) if e.name() == self.start.name() => Ok(None),
+                // This is a unmatched closing tag, so the XML is invalid
+                DeEvent::End(e) => Err(DeError::UnexpectedEnd(e.name().as_ref().to_owned())),
+                // We cannot get `Eof` legally, because we always inside of the
+                // opened tag `self.start`
+                DeEvent::Eof => Err(DeError::UnexpectedEof),
             }
         }
     }


Test patch:
diff --git a/tests/serde-de.rs b/tests/serde-de.rs
index 78ec5b7..dec5c1e 100644
--- a/tests/serde-de.rs
+++ b/tests/serde-de.rs
@@ -43,115 +43,6 @@ fn string_borrow() {
     assert_eq!(borrowed_item.text, "Hello world");
 }
 
-#[derive(Debug, Deserialize, PartialEq)]
-struct Item {
-    name: String,
-    source: String,
-}
-
-#[test]
-fn multiple_roots_attributes() {
-    let item: Vec<Item> = from_str(
-        r#"
-            <item name="hello1" source="world1.rs" />
-            <item name="hello2" source="world2.rs" />
-        "#,
-    )
-    .unwrap();
-    assert_eq!(
-        item,
-        vec![
-            Item {
-                name: "hello1".to_string(),
-                source: "world1.rs".to_string(),
-            },
-            Item {
-                name: "hello2".to_string(),
-                source: "world2.rs".to_string(),
-            },
-        ]
-    );
-}
-
-#[test]
-fn nested_collection() {
-    #[derive(Debug, Deserialize, PartialEq)]
-    struct Project {
-        name: String,
-
-        #[serde(rename = "item", default)]
-        items: Vec<Item>,
-    }
-
-    let project: Project = from_str(
-        r#"
-        <project name="my_project">
-            <item name="hello1" source="world1.rs" />
-            <item name="hello2" source="world2.rs" />
-        </project>
-        "#,
-    )
-    .unwrap();
-    assert_eq!(
-        project,
-        Project {
-            name: "my_project".to_string(),
-            items: vec![
-                Item {
-                    name: "hello1".to_string(),
-                    source: "world1.rs".to_string(),
-                },
-                Item {
-                    name: "hello2".to_string(),
-                    source: "world2.rs".to_string(),
-                },
-            ],
-        }
-    );
-}
-
-#[test]
-fn collection_of_enums() {
-    #[derive(Debug, Deserialize, PartialEq)]
-    enum MyEnum {
-        A(String),
-        B { name: String, flag: bool },
-        C,
-    }
-
-    #[derive(Debug, Deserialize, PartialEq)]
-    struct MyEnums {
-        // TODO: This should be #[serde(flatten)], but right now serde don't support flattening of sequences
-        // See https://github.com/serde-rs/serde/issues/1905
-        #[serde(rename = "$value")]
-        items: Vec<MyEnum>,
-    }
-
-    let s = r#"
-    <enums>
-        <A>test</A>
-        <B name="hello" flag="t" />
-        <C />
-    </enums>
-    "#;
-
-    let project: MyEnums = from_str(s).unwrap();
-
-    assert_eq!(
-        project,
-        MyEnums {
-            items: vec![
-                MyEnum::A("test".to_string()),
-                MyEnum::B {
-                    name: "hello".to_string(),
-                    flag: true,
-                },
-                MyEnum::C,
-            ],
-        }
-    );
-}
-
 /// Test for https://github.com/tafia/quick-xml/issues/231
 #[test]
 fn implicit_value() {
@@ -3589,6 +3480,18 @@ macro_rules! maplike_errors {
         mod non_closed {
             use super::*;
 
+            /// For struct we expect that error about not closed tag appears
+            /// earlier than error about missing fields
+            #[test]
+            fn missing_field() {
+                let data = from_str::<$type>(r#"<root>"#);
+
+                match data {
+                    Err(DeError::UnexpectedEof) => (),
+                    _ => panic!("Expected `UnexpectedEof`, found {:?}", data),
+                }
+            }
+
             #[test]
             fn attributes() {
                 let data = from_str::<$type>(r#"<root float="42" string="answer">"#);
@@ -3624,6 +3527,18 @@ macro_rules! maplike_errors {
             use super::*;
             use quick_xml::Error::EndEventMismatch;
 
+            /// For struct we expect that error about mismatched tag appears
+            /// earlier than error about missing fields
+            #[test]
+            fn missing_field() {
+                let data = from_str::<$type>(r#"<root></mismatched>"#);
+
+                match data {
+                    Err(DeError::InvalidXml(EndEventMismatch { .. })) => (),
+                    _ => panic!("Expected `InvalidXml(EndEventMismatch)`, found {:?}", data),
+                }
+            }
+
             #[test]
             fn attributes() {
                 let data = from_str::<$type>(
@@ -3922,6 +3837,12 @@ mod flatten_struct {
 mod enum_ {
     use super::*;
 
+    #[derive(Debug, Deserialize, PartialEq)]
+    struct Nested {
+        //TODO: change to f64 after fixing https://github.com/serde-rs/serde/issues/1183
+        float: String,
+    }
+
     mod externally_tagged {
         use super::*;
         use pretty_assertions::assert_eq;
@@ -3947,12 +3868,6 @@ mod enum_ {
             },
         }
 
-        #[derive(Debug, Deserialize, PartialEq)]
-        struct Nested {
-            //TODO: change to f64 after fixing https://github.com/serde-rs/serde/issues/1183
-            float: String,
-        }
-
         /// Workaround for serde bug https://github.com/serde-rs/serde/issues/1904
         #[derive(Debug, Deserialize, PartialEq)]
         enum Workaround {
@@ -4120,12 +4035,6 @@ mod enum_ {
             value: bool,
         }
 
-        #[derive(Debug, Deserialize, PartialEq)]
-        struct Nested {
-            //TODO: change to f64 after fixing https://github.com/serde-rs/serde/issues/1183
-            float: String,
-        }
-
         mod unit {
             use super::*;
             use pretty_assertions::assert_eq;
@@ -4306,12 +4215,6 @@ mod enum_ {
             },
         }
 
-        #[derive(Debug, Deserialize, PartialEq)]
-        struct Nested {
-            //TODO: change to f64 after fixing https://github.com/serde-rs/serde/issues/1183
-            float: String,
-        }
-
         /// Workaround for serde bug https://github.com/serde-rs/serde/issues/1904
         #[derive(Debug, Deserialize, PartialEq)]
         #[serde(tag = "tag", content = "content")]
@@ -4524,12 +4427,6 @@ mod enum_ {
             },
         }
 
-        #[derive(Debug, Deserialize, PartialEq)]
-        struct Nested {
-            //TODO: change to f64 after fixing https://github.com/serde-rs/serde/issues/1183
-            float: String,
-        }
-
         /// Workaround for serde bug https://github.com/serde-rs/serde/issues/1904
         #[derive(Debug, Deserialize, PartialEq)]
         #[serde(untagged)]