Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework errors #675

Merged
merged 24 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
e0adb66
Normalize failed text of all checks for expected errors
Mingun Oct 20, 2023
da2823d
cargo fmt
Mingun Oct 21, 2023
d9520c4
Remove `DeError::UnexpectedEnd`, because this error could be returned…
Mingun Oct 7, 2023
77e5a2c
Remove `DeError::ExpectedStart`, call `Visitor::visit_borrowed_str` o…
Mingun Oct 7, 2023
a5490f2
Do not return `DeError::Unsupported` in `SimpleTypeDeserializer`, cal…
Mingun Oct 20, 2023
f845369
Do not return `DeError::Unsupported` in attempt to deserialize byte data
Mingun Oct 20, 2023
77a2fad
Do not return `DeError::Unsupported` in attempt to deserialize newtyp…
Mingun Oct 20, 2023
29ca886
Replace `Error::UnexpectedEof` and `Error::UnexpectedBang` by `Error:…
Mingun Sep 28, 2023
0e5188d
Replace `Error::UnexpectedEof` by `Error::IllFormed(MissedEnd)` in re…
Mingun Sep 27, 2023
f7ee91f
Do not convert `DeError::InvalidXml(Error::IllFormed(MissedEnd))` to …
Mingun Oct 7, 2023
a91843e
Replace `DeError::UnexpectedEof` by `Error::IllFormed(MissedEnd)` whe…
Mingun Oct 28, 2023
7d20324
Replace `DeError::UnexpectedEof` by `Error::IllFormed(MissedEnd)` in …
Mingun Oct 21, 2023
4972d4f
Replace `DeError::UnexpectedEof` by `Error::IllFormed(MissedEnd)` and…
Mingun Oct 21, 2023
cee2725
`DeEvent::Eof` is impossible in `MapValueDeserializer::variant_seed`,…
Mingun Oct 21, 2023
d5489df
Remove unnecessary allocation
Mingun Oct 8, 2023
00de446
Report trimmed found name in EndEventMismatch error when end tag has …
Mingun Sep 28, 2023
ff28390
Report EndEventMismatch error at start of the end tag at `<` character
Mingun Sep 28, 2023
73fcf23
Inline mismatch_err closure because in the next commit it will genera…
Mingun Sep 28, 2023
6670800
Replace `Error::EndEventMismatch` by `Error::IllFormed(UnmatchedEnd)`…
Mingun Sep 28, 2023
4d887a2
Replace `Error::EndEventMismatch` by `Error::IllFormed(MismatchedEnd)`
Mingun Sep 28, 2023
a4febad
`IllFormed(UnmatchedEnd)` should be reported always because lone clos…
Mingun Sep 29, 2023
596edd6
Replace `Error::UnexpectedToken` by `Error::IllFormed(DoubleHyphenInC…
Mingun Oct 28, 2023
53c6b4e
Remove not used `Error::TextNotFound`
Mingun Oct 28, 2023
a049cd2
Better document `Error::XmlDeclWithoutVersion` and `Error::EmptyDocType`
Mingun Oct 28, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ include = ["src/*", "LICENSE-MIT.md", "README.md"]
[dependencies]
document-features = { version = "0.2", optional = true }
encoding_rs = { version = "0.8", optional = true }
serde = { version = ">=1.0.100", optional = true }
serde = { version = ">=1.0.139", optional = true }
tokio = { version = "1.10", optional = true, default-features = false, features = ["io-util"] }
memchr = "2.1"
arbitrary = { version = "1", features = ["derive"], optional = true }
Expand Down
4 changes: 4 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@

### Misc Changes

- [#675]: Minimum supported version of serde raised to 1.0.139

[#675]: https://github.com/tafia/quick-xml/pull/675


## 0.31.0 -- 2023-10-22

Expand Down
2 changes: 1 addition & 1 deletion src/de/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ mod tests {
match err {
DeError::$kind(e) => assert_eq!(e, $reason),
_ => panic!(
"Expected `{}({})`, found `{:?}`",
"Expected `Err({}({}))`, but got `{:?}`",
stringify!($kind),
$reason,
err
Expand Down
12 changes: 8 additions & 4 deletions src/de/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,9 @@ where
seed.deserialize(BorrowedStrDeserializer::<DeError>::new(TEXT_KEY))?,
true,
),
DeEvent::End(e) => return Err(DeError::UnexpectedEnd(e.name().into_inner().to_vec())),
// SAFETY: The reader is guaranteed that we don't have unmatched tags
// If we here, then out deserializer has a bug
DeEvent::End(e) => unreachable!("{:?}", e),
DeEvent::Eof => return Err(DeError::UnexpectedEof),
};
Ok((
Expand Down Expand Up @@ -927,9 +929,11 @@ where
DeEvent::Start(e) if !self.filter.is_suitable(e, decoder)? => Ok(None),

// Stop iteration after reaching a closing tag
DeEvent::End(e) if e.name() == self.map.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())),
// The matching tag name is guaranteed by the reader
DeEvent::End(e) => {
debug_assert_eq!(self.map.start.name(), e.name());
Ok(None)
}
// We cannot get `Eof` legally, because we always inside of the
// opened tag `self.map.start`
DeEvent::Eof => Err(DeError::UnexpectedEof),
Expand Down
92 changes: 59 additions & 33 deletions src/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,8 +509,9 @@
//! /// Use unit variant, if you do not care of a content.
//! /// You can use tuple variant if you want to parse
//! /// textual content as an xs:list.
//! /// Struct variants are not supported and will return
//! /// Err(Unsupported)
//! /// Struct variants are will pass a string to the
//! /// struct enum variant visitor, which typically
//! /// returns Err(Custom)
//! #[serde(rename = "$text")]
//! Text(String),
//! }
Expand Down Expand Up @@ -613,8 +614,9 @@
//! /// Use unit variant, if you do not care of a content.
//! /// You can use tuple variant if you want to parse
//! /// textual content as an xs:list.
//! /// Struct variants are not supported and will return
//! /// Err(Unsupported)
//! /// Struct variants are will pass a string to the
//! /// struct enum variant visitor, which typically
//! /// returns Err(Custom)
//! #[serde(rename = "$text")]
//! Text(String),
//! }
Expand Down Expand Up @@ -1377,19 +1379,23 @@
//! |Kind |Top-level and in `$value` field |In normal field |In `$text` field |
//! |-------|-----------------------------------------|---------------------|---------------------|
//! |Unit |`<Unit/>` |`<field>Unit</field>`|`Unit` |
//! |Newtype|`<Newtype>42</Newtype>` |Err(Unsupported) |Err(Unsupported) |
//! |Tuple |`<Tuple>42</Tuple><Tuple>answer</Tuple>` |Err(Unsupported) |Err(Unsupported) |
//! |Struct |`<Struct><q>42</q><a>answer</a></Struct>`|Err(Unsupported) |Err(Unsupported) |
//! |Newtype|`<Newtype>42</Newtype>` |Err(Custom) [^0] |Err(Custom) [^0] |
//! |Tuple |`<Tuple>42</Tuple><Tuple>answer</Tuple>` |Err(Custom) [^0] |Err(Custom) [^0] |
//! |Struct |`<Struct><q>42</q><a>answer</a></Struct>`|Err(Custom) [^0] |Err(Custom) [^0] |
//!
//! `$text` enum variant
//! --------------------
//!
//! |Kind |Top-level and in `$value` field |In normal field |In `$text` field |
//! |-------|-----------------------------------------|---------------------|---------------------|
//! |Unit |_(empty)_ |`<field/>` |_(empty)_ |
//! |Newtype|`42` |Err(Unsupported) [^1]|Err(Unsupported) [^2]|
//! |Tuple |`42 answer` |Err(Unsupported) [^3]|Err(Unsupported) [^4]|
//! |Struct |Err(Unsupported) |Err(Unsupported) |Err(Unsupported) |
//! |Newtype|`42` |Err(Custom) [^0] [^1]|Err(Custom) [^0] [^2]|
//! |Tuple |`42 answer` |Err(Custom) [^0] [^3]|Err(Custom) [^0] [^4]|
//! |Struct |Err(Custom) [^0] |Err(Custom) [^0] |Err(Custom) [^0] |
//!
//! [^0]: Error is returned by the deserialized type. In case of derived implementation a `Custom`
//! error will be returned, but custom deserialize implementation can successfully deserialize
//! value from a string which will be passed to it.
//!
//! [^1]: If this serialize as `<field>42</field>` then it will be ambiguity during deserialization,
//! because it clash with `Unit` representation in normal field.
Expand Down Expand Up @@ -1861,6 +1867,7 @@ macro_rules! deserialize_primitives {
}

/// Character represented as [strings](#method.deserialize_str).
#[inline]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you actually found this to make a difference?

The primary purpose of #[inline] is to allow inlining across compilation units. Within a compilation unit, the compiler is just going to decide on its own and mostly ignore this attribute unless you say #[inline(always)].

If these functions are not going to be called from any external library, I doubt it would make a difference.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, added just for consistence. Actually I want to run some profiler one day, just need time to learn how to do that and find a which one that could be run on Windows. Most tools supports only Linux. cargo flamegraph looks promising.

fn deserialize_char<V>(self, visitor: V) -> Result<V::Value, DeError>
where
V: Visitor<'de>,
Expand All @@ -1880,22 +1887,25 @@ macro_rules! deserialize_primitives {
}

/// Representation of owned strings the same as [non-owned](#method.deserialize_str).
#[inline]
fn deserialize_string<V>(self, visitor: V) -> Result<V::Value, DeError>
where
V: Visitor<'de>,
{
self.deserialize_str(visitor)
}

/// Returns [`DeError::Unsupported`]
fn deserialize_bytes<V>(self, _visitor: V) -> Result<V::Value, DeError>
/// Forwards deserialization to the [`deserialize_any`](#method.deserialize_any).
#[inline]
fn deserialize_bytes<V>(self, visitor: V) -> Result<V::Value, DeError>
where
V: Visitor<'de>,
{
Err(DeError::Unsupported("binary data content is not supported by XML format".into()))
self.deserialize_any(visitor)
}

/// Forwards deserialization to the [`deserialize_bytes`](#method.deserialize_bytes).
#[inline]
fn deserialize_byte_buf<V>(self, visitor: V) -> Result<V::Value, DeError>
where
V: Visitor<'de>,
Expand All @@ -1904,6 +1914,7 @@ macro_rules! deserialize_primitives {
}

/// Representation of the named units the same as [unnamed units](#method.deserialize_unit).
#[inline]
fn deserialize_unit_struct<V>(
self,
_name: &'static str,
Expand All @@ -1916,6 +1927,7 @@ macro_rules! deserialize_primitives {
}

/// Representation of tuples the same as [sequences](#method.deserialize_seq).
#[inline]
fn deserialize_tuple<V>(self, _len: usize, visitor: V) -> Result<V::Value, DeError>
where
V: Visitor<'de>,
Expand All @@ -1924,6 +1936,7 @@ macro_rules! deserialize_primitives {
}

/// Representation of named tuples the same as [unnamed tuples](#method.deserialize_tuple).
#[inline]
fn deserialize_tuple_struct<V>(
self,
_name: &'static str,
Expand All @@ -1947,6 +1960,7 @@ macro_rules! deserialize_primitives {
}

/// Identifiers represented as [strings](#method.deserialize_str).
#[inline]
fn deserialize_identifier<V>(self, visitor: V) -> Result<V::Value, DeError>
where
V: Visitor<'de>,
Expand Down Expand Up @@ -2619,7 +2633,7 @@ where
/// |Event |XML |Handling
/// |------------------|---------------------------|----------------------------------------
/// |[`DeEvent::Start`]|`<tag>...</tag>` |if `allow_start == true`, result determined by the second table, otherwise emits [`UnexpectedStart("tag")`](DeError::UnexpectedStart)
/// |[`DeEvent::End`] |`</any-tag>` |Emits [`UnexpectedEnd("any-tag")`](DeError::UnexpectedEnd)
/// |[`DeEvent::End`] |`</any-tag>` |This is impossible situation, the method will panic if it happens
/// |[`DeEvent::Text`] |`text content` or `<![CDATA[cdata content]]>` (probably mixed)|Returns event content unchanged
/// |[`DeEvent::Eof`] | |Emits [`UnexpectedEof`](DeError::UnexpectedEof)
///
Expand All @@ -2640,7 +2654,9 @@ where
// allow one nested level
DeEvent::Start(_) if allow_start => self.read_text(),
DeEvent::Start(e) => Err(DeError::UnexpectedStart(e.name().as_ref().to_owned())),
DeEvent::End(e) => Err(DeError::UnexpectedEnd(e.name().as_ref().to_owned())),
// SAFETY: The reader is guaranteed that we don't have unmatched tags
// If we here, then out deserializer has a bug
DeEvent::End(e) => unreachable!("{:?}", e),
DeEvent::Eof => Err(DeError::UnexpectedEof),
}
}
Expand Down Expand Up @@ -2816,8 +2832,16 @@ where
{
match self.next()? {
DeEvent::Start(e) => visitor.visit_map(ElementMapAccess::new(self, e, fields)?),
DeEvent::End(e) => Err(DeError::UnexpectedEnd(e.name().as_ref().to_owned())),
DeEvent::Text(_) => Err(DeError::ExpectedStart),
// SAFETY: The reader is guaranteed that we don't have unmatched tags
// If we here, then out deserializer has a bug
DeEvent::End(e) => unreachable!("{:?}", e),
// Deserializer methods are only hints, if deserializer could not satisfy
// request, it should return the data that it has. It is responsibility
// of a Visitor to return an error if it does not understand the data
DeEvent::Text(e) => match e.text {
Cow::Borrowed(s) => visitor.visit_borrowed_str(s),
Cow::Owned(s) => visitor.visit_string(s),
},
DeEvent::Eof => Err(DeError::UnexpectedEof),
}
}
Expand All @@ -2836,7 +2860,7 @@ where
/// |Event |XML |Handling
/// |------------------|---------------------------|-------------------------------------------
/// |[`DeEvent::Start`]|`<tag>...</tag>` |Calls `visitor.visit_unit()`, consumes all events up to and including corresponding `End` event
/// |[`DeEvent::End`] |`</tag>` |Emits [`UnexpectedEnd("tag")`](DeError::UnexpectedEnd)
/// |[`DeEvent::End`] |`</tag>` |This is impossible situation, the method will panic if it happens
/// |[`DeEvent::Text`] |`text content` or `<![CDATA[cdata content]]>` (probably mixed)|Calls `visitor.visit_unit()`. The content is ignored
/// |[`DeEvent::Eof`] | |Emits [`UnexpectedEof`](DeError::UnexpectedEof)
fn deserialize_unit<V>(self, visitor: V) -> Result<V::Value, DeError>
Expand All @@ -2849,7 +2873,9 @@ where
visitor.visit_unit()
}
DeEvent::Text(_) => visitor.visit_unit(),
DeEvent::End(e) => Err(DeError::UnexpectedEnd(e.name().as_ref().to_owned())),
// SAFETY: The reader is guaranteed that we don't have unmatched tags
// If we here, then out deserializer has a bug
DeEvent::End(e) => unreachable!("{:?}", e),
DeEvent::Eof => Err(DeError::UnexpectedEof),
}
}
Expand Down Expand Up @@ -3539,7 +3565,7 @@ mod tests {

match List::deserialize(&mut de) {
Err(DeError::TooManyEvents(count)) => assert_eq!(count.get(), 3),
e => panic!("Expected `Err(TooManyEvents(3))`, but found {:?}", e),
e => panic!("Expected `Err(TooManyEvents(3))`, but got `{:?}`", e),
}
}

Expand Down Expand Up @@ -3605,8 +3631,8 @@ mod tests {
assert_eq!(de.peek().unwrap(), &Start(BytesStart::new("tag")));

match de.read_to_end(QName(b"tag")) {
Err(DeError::UnexpectedEof) => (),
x => panic!("Expected `Err(UnexpectedEof)`, but found {:?}", x),
Err(DeError::UnexpectedEof) => {}
x => panic!("Expected `Err(UnexpectedEof)`, but got `{:?}`", x),
}
assert_eq!(de.next().unwrap(), Eof);
}
Expand All @@ -3619,8 +3645,8 @@ mod tests {
assert_eq!(de.peek().unwrap(), &Text("".into()));

match de.read_to_end(QName(b"tag")) {
Err(DeError::UnexpectedEof) => (),
x => panic!("Expected `Err(UnexpectedEof)`, but found {:?}", x),
Err(DeError::UnexpectedEof) => {}
x => panic!("Expected `Err(UnexpectedEof)`, but got `{:?}`", x),
}
assert_eq!(de.next().unwrap(), Eof);
}
Expand Down Expand Up @@ -3713,7 +3739,7 @@ mod tests {
assert_eq!(found, "root");
}
x => panic!(
r#"Expected `Err(InvalidXml(EndEventMismatch("", "root")))`, but found {:?}"#,
"Expected `Err(InvalidXml(EndEventMismatch {{ expected = '', found = 'root' }}))`, but got `{:?}`",
x
),
}
Expand All @@ -3727,7 +3753,7 @@ mod tests {
assert_eq!(found, "other");
}
x => panic!(
r#"Expected `Err(InvalidXml(EndEventMismatch("root", "other")))`, but found {:?}"#,
"Expected `Err(InvalidXml(EndEventMismatch {{ expected = 'root', found = 'other' }}))`, but got `{:?}`",
x
),
}
Expand Down Expand Up @@ -4055,7 +4081,7 @@ mod tests {
assert_eq!(expected, "");
assert_eq!(found, "tag2");
}
x => panic!("Expected `InvalidXml(EndEventMismatch {{ expected = '', found = 'tag2' }})`, but got {:?}", x),
x => panic!("Expected `Err(InvalidXml(EndEventMismatch {{ expected = '', found = 'tag2' }}))`, but got `{:?}`", x),
}
assert_eq!(de.next().unwrap(), DeEvent::Eof);
}
Expand Down Expand Up @@ -4196,7 +4222,7 @@ mod tests {
assert_eq!(expected, "");
assert_eq!(found, "tag");
}
x => panic!("Expected `InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }})`, but got {:?}", x),
x => panic!("Expected `Err(InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }}))`, but got `{:?}`", x),
}
assert_eq!(de.next().unwrap(), DeEvent::Eof);
}
Expand Down Expand Up @@ -4273,7 +4299,7 @@ mod tests {
assert_eq!(expected, "");
assert_eq!(found, "tag");
}
x => panic!("Expected `InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }})`, but got {:?}", x),
x => panic!("Expected `Err(InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }}))`, but got `{:?}`", x),
}
assert_eq!(de.next().unwrap(), DeEvent::Eof);
}
Expand Down Expand Up @@ -4303,7 +4329,7 @@ mod tests {
assert_eq!(expected, "");
assert_eq!(found, "tag");
}
x => panic!("Expected `InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }})`, but got {:?}", x),
x => panic!("Expected `Err(InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }}))`, but got `{:?}`", x),
}
assert_eq!(de.next().unwrap(), DeEvent::Eof);
}
Expand Down Expand Up @@ -4407,7 +4433,7 @@ mod tests {
assert_eq!(expected, "");
assert_eq!(found, "tag");
}
x => panic!("Expected `InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }})`, but got {:?}", x),
x => panic!("Expected `Err(InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }}))`, but got `{:?}`", x),
}
assert_eq!(de.next().unwrap(), DeEvent::Eof);
}
Expand Down Expand Up @@ -4435,7 +4461,7 @@ mod tests {
assert_eq!(expected, "");
assert_eq!(found, "tag");
}
x => panic!("Expected `InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }})`, but got {:?}", x),
x => panic!("Expected `Err(InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }}))`, but got `{:?}`", x),
}
assert_eq!(de.next().unwrap(), DeEvent::Eof);
}
Expand Down Expand Up @@ -4483,7 +4509,7 @@ mod tests {
assert_eq!(expected, "");
assert_eq!(found, "tag");
}
x => panic!("Expected `InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }})`, but got {:?}", x),
x => panic!("Expected `Err(InvalidXml(EndEventMismatch {{ expected = '', found = 'tag' }}))`, but got `{:?}`", x),
}
assert_eq!(de.next().unwrap(), DeEvent::Eof);
}
Expand Down
Loading