Skip to content

Commit

Permalink
Fix using incorrect deserializer when deserialize Option values in …
Browse files Browse the repository at this point in the history
…maps and structs
  • Loading branch information
Mingun authored and dralley committed Feb 6, 2023
1 parent 38fa3bc commit b3ebf7a
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 15 deletions.
3 changes: 3 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 14 additions & 2 deletions src/de/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,6 @@ where

deserialize_primitives!(mut);

forward!(deserialize_option);
forward!(deserialize_unit);
forward!(deserialize_unit_struct(name: &'static str));
forward!(deserialize_newtype_struct(name: &'static str));
Expand All @@ -499,6 +498,13 @@ where
forward!(deserialize_any);
forward!(deserialize_ignored_any);

fn deserialize_option<V>(self, visitor: V) -> Result<V::Value, DeError>
where
V: Visitor<'de>,
{
deserialize_option!(self.map.de, self, visitor)
}

/// Tuple representation is the same as [sequences](#method.deserialize_seq).
fn deserialize_tuple<V>(self, _len: usize, visitor: V) -> Result<V::Value, DeError>
where
Expand Down Expand Up @@ -758,7 +764,6 @@ where

deserialize_primitives!(mut);

forward!(deserialize_option);
forward!(deserialize_unit);
forward!(deserialize_unit_struct(name: &'static str));
forward!(deserialize_newtype_struct(name: &'static str));
Expand All @@ -777,6 +782,13 @@ where
forward!(deserialize_any);
forward!(deserialize_ignored_any);

fn deserialize_option<V>(self, visitor: V) -> Result<V::Value, DeError>
where
V: Visitor<'de>,
{
deserialize_option!(self.map.de, self, 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
Expand Down
25 changes: 12 additions & 13 deletions src/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ();
Expand All @@ -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).
//!
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1832,6 +1825,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;
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit b3ebf7a

Please sign in to comment.