Merge pull request #672 from Mingun/fix-671

Always deserialize simpleTypes (for example, attribute values) as Some(…)

Author: Mingun

Changelog.md

@@ -30,6 +30,8 @@ serde >= 1.0.181
   unit structs, unit variants). `<int>123<something-else/></int>` is no longer valid
   content. Previously all data after `123` up to closing tag would be silently skipped.
 - [#567]: Fixed incorrect deserialization of vectors of enums from sequences of tags.
+- [#671]: Fixed deserialization of empty `simpleType`s (for example, attributes) into
+  `Option` fields: now they are always deserialized as `Some("")`.
 
 ### Misc Changes
 
@@ -58,6 +60,7 @@ serde >= 1.0.181
 [#661]: https://github.com/tafia/quick-xml/pull/661
 [#662]: https://github.com/tafia/quick-xml/pull/662
 [#665]: https://github.com/tafia/quick-xml/pull/665
+[#671]: https://github.com/tafia/quick-xml/issues/671
 
 
 ## 0.30.0 -- 2023-07-23

src/de/simple_type.rs

@@ -481,7 +481,32 @@ impl<'de, 'a> SeqAccess<'de> for ListIter<'de, 'a> {
 /// - attribute values (`<... ...="value" ...>`)
 /// - mixed text / CDATA content (`<...>text<![CDATA[cdata]]></...>`)
 ///
+/// This deserializer processes items as following:
+/// - numbers are parsed from a text content using [`FromStr`];
+/// - booleans converted from the text according to the XML [specification]:
+///   - `"true"` and `"1"` converted to `true`;
+///   - `"false"` and `"0"` converted to `false`;
+/// - strings returned as is;
+/// - characters also returned as strings. If string contain more than one character
+///   or empty, it is responsibility of a type to return an error;
+/// - `Option` always deserialized as `Some` using the same deserializer.
+///   If attribute or text content is missed, then the deserializer even wouldn't
+///   be used, so if it is used, then the value should be;
+/// - units (`()`) and unit structs always deserialized successfully;
+/// - newtype structs forwards deserialization to the inner type using the same
+///   deserializer;
+/// - sequences, tuples and tuple structs are deserialized as `xs:list`s. Only
+///   sequences of primitive types is possible to deserialize this way and they
+///   should be delimited by a space (` `, `\t`, `\r`, or `\n`);
+/// - structs and maps returns [`DeError::Unsupported`];
+/// - enums:
+///   - unit variants: just return `()`;
+///   - all other variants returns [`DeError::Unsupported`];
+/// - identifiers are deserialized as strings.
+///
 /// [simple types]: https://www.w3.org/TR/xmlschema11-1/#Simple_Type_Definition
+/// [`FromStr`]: std::str::FromStr
+/// [specification]: https://www.w3.org/TR/xmlschema11-2/#boolean
 pub struct SimpleTypeDeserializer<'de, 'a> {
     /// - In case of attribute contains escaped attribute value
     /// - In case of text contains unescaped text value
@@ -642,11 +667,7 @@ impl<'de, 'a> Deserializer<'de> for SimpleTypeDeserializer<'de, 'a> {
     where
         V: Visitor<'de>,
     {
-        if self.content.is_empty() {
-            visitor.visit_none()
-        } else {
-            visitor.visit_some(self)
-        }
+        visitor.visit_some(self)
     }
 
     fn deserialize_unit<V>(self, visitor: V) -> Result<V::Value, Self::Error>
@@ -1225,7 +1246,7 @@ mod tests {
         err!(utf8, borrowed_bytes: Bytes = "&lt;escaped&#32;string"
              => Unsupported("binary data content is not supported by XML format"));
 
-        simple!(utf8, option_none: Option<&str> = "" => None);
+        simple!(utf8, option_none: Option<&str> = "" => Some(""));
         simple!(utf8, option_some: Option<&str> = "non-escaped string" => Some("non-escaped string"));
 
         simple_only!(utf8, unit: () = "any data" => ());
@@ -1311,7 +1332,7 @@ mod tests {
         unsupported!(borrowed_bytes: Bytes = "&lt;escaped&#32;string"
                      => "binary data content is not supported by XML format");
 
-        utf16!(option_none: Option<()> = "" => None);
+        utf16!(option_none: Option<()> = "" => Some(()));
         utf16!(option_some: Option<()> = "any data" => Some(()));
 
         utf16!(unit: () = "any data" => ());

tests/serde-issues.rs

@@ -16,7 +16,7 @@ mod issue252 {
 
     #[test]
     fn attributes() {
-        #[derive(Serialize, Debug, PartialEq)]
+        #[derive(Debug, Deserialize, Serialize, PartialEq)]
         struct OptionalAttributes {
             #[serde(rename = "@a")]
             a: Option<&'static str>,
@@ -26,58 +26,78 @@ mod issue252 {
             b: Option<&'static str>,
         }
 
+        // Writing `a=""` for a `None` we reflects serde_json behavior which also
+        // writes `a: null` for `None`, and reflect they deserialization asymmetry
+        let xml = r#"<OptionalAttributes a=""/>"#;
         assert_eq!(
             to_string(&OptionalAttributes { a: None, b: None }).unwrap(),
-            r#"<OptionalAttributes a=""/>"#
+            xml
         );
         assert_eq!(
-            to_string(&OptionalAttributes {
+            from_str::<OptionalAttributes>(xml).unwrap(),
+            OptionalAttributes {
                 a: Some(""),
-                b: Some("")
-            })
-            .unwrap(),
-            r#"<OptionalAttributes a="" b=""/>"#
-        );
-        assert_eq!(
-            to_string(&OptionalAttributes {
-                a: Some("a"),
-                b: Some("b")
-            })
-            .unwrap(),
-            r#"<OptionalAttributes a="a" b="b"/>"#
+                b: None
+            }
         );
+
+        let value = OptionalAttributes {
+            a: Some(""),
+            b: Some(""),
+        };
+        let xml = r#"<OptionalAttributes a="" b=""/>"#;
+        assert_eq!(to_string(&value).unwrap(), xml);
+        assert_eq!(from_str::<OptionalAttributes>(xml).unwrap(), value);
+
+        let value = OptionalAttributes {
+            a: Some("a"),
+            b: Some("b"),
+        };
+        let xml = r#"<OptionalAttributes a="a" b="b"/>"#;
+        assert_eq!(to_string(&value).unwrap(), xml);
+        assert_eq!(from_str::<OptionalAttributes>(xml).unwrap(), value);
     }
 
     #[test]
     fn elements() {
-        #[derive(Serialize, Debug, PartialEq)]
+        #[derive(Debug, Deserialize, Serialize, PartialEq)]
         struct OptionalElements {
             a: Option<&'static str>,
 
             #[serde(skip_serializing_if = "Option::is_none")]
             b: Option<&'static str>,
         }
 
+        // Writing `<a/>` for a `None` we reflects serde_json behavior which also
+        // writes `a: null` for `None`, and reflect they deserialization asymmetry
+        let xml = "<OptionalElements><a/></OptionalElements>";
         assert_eq!(
             to_string(&OptionalElements { a: None, b: None }).unwrap(),
-            r#"<OptionalElements><a/></OptionalElements>"#
+            xml
         );
         assert_eq!(
-            to_string(&OptionalElements {
+            from_str::<OptionalElements>(xml).unwrap(),
+            OptionalElements {
                 a: Some(""),
-                b: Some("")
-            })
-            .unwrap(),
-            r#"<OptionalElements><a/><b/></OptionalElements>"#
-        );
-        assert_eq!(
-            to_string(&OptionalElements {
-                a: Some("a"),
-                b: Some("b")
-            })
-            .unwrap(),
-            r#"<OptionalElements><a>a</a><b>b</b></OptionalElements>"#
+                b: None
+            }
         );
+
+        let value = OptionalElements {
+            a: Some(""),
+            b: Some(""),
+        };
+        let xml = "<OptionalElements><a/><b/></OptionalElements>";
+        assert_eq!(to_string(&value).unwrap(), xml);
+        assert_eq!(from_str::<OptionalElements>(xml).unwrap(), value);
+
+        let value = OptionalElements {
+            a: Some("a"),
+            b: Some("b"),
+        };
+        let xml = "<OptionalElements><a>a</a><b>b</b></OptionalElements>";
+        assert_eq!(to_string(&value).unwrap(), xml);
+        assert_eq!(from_str::<OptionalElements>(xml).unwrap(), value);
     }
 }