Skip to content

Commit

Permalink
Rename IllFormed errors based on feedback
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel Alley <dalley@redhat.com>
  • Loading branch information
Mingun and dralley committed Nov 22, 2023
1 parent 55039e8 commit 2c55638
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 66 deletions.
10 changes: 5 additions & 5 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,21 @@ configuration is serializable.

- [#622]: Fix wrong disregarding of not closed markup, such as lone `<`.
- [#684]: Fix incorrect position reported for `Error::IllFormed(DoubleHyphenInComment)`.
- [#684]: Fix incorrect position reported for `Error::IllFormed(MissedDoctypeName)`.
- [#684]: Fix incorrect position reported for `Error::IllFormed(MissingDoctypeName)`.

### Misc Changes

- [#675]: Minimum supported version of serde raised to 1.0.139
- [#675]: Rework the `quick_xml::Error` type to provide more accurate information:
- `Error::EndEventMismatch` replaced by `IllFormedError::MismatchedEnd` in some cases
- `Error::EndEventMismatch` replaced by `IllFormedError::UnmatchedEnd` in some cases
- `Error::EndEventMismatch` replaced by `IllFormedError::MismatchedEndTag` in some cases
- `Error::EndEventMismatch` replaced by `IllFormedError::UnmatchedEndTag` in some cases
- `Error::TextNotFound` was removed because not used
- `Error::UnexpectedBang` replaced by `SyntaxError`
- `Error::UnexpectedEof` replaced by `SyntaxError` in some cases
- `Error::UnexpectedEof` replaced by `IllFormedError` in some cases
- `Error::UnexpectedToken` replaced by `IllFormedError::DoubleHyphenInComment`
- `Error::XmlDeclWithoutVersion` replaced by `IllFormedError::MissedVersion` (in [#684])
- `Error::EmptyDocType` replaced by `IllFormedError::MissedDoctypeName` (in [#684])
- `Error::XmlDeclWithoutVersion` replaced by `IllFormedError::MissingDeclVersion` (in [#684])
- `Error::EmptyDocType` replaced by `IllFormedError::MissingDoctypeName` (in [#684])
- [#684]: Changed positions reported for `SyntaxError`s: now they are always points
to the start of markup (i. e. to the `<` character) with error.
- [#684]: Now `<??>` parsed as `Event::PI` with empty content instead of raising
Expand Down
24 changes: 12 additions & 12 deletions src/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2649,7 +2649,7 @@ where
/// |[`DeEvent::Start`]|`<any-tag>...</any-tag>` |Emits [`UnexpectedStart("any-tag")`](DeError::UnexpectedStart)
/// |[`DeEvent::End`] |`</tag>` |Returns an empty slice. The reader guarantee that tag will match the open one
/// |[`DeEvent::Text`] |`text content` or `<![CDATA[cdata content]]>` (probably mixed)|Returns event content unchanged, expects the `</tag>` after that
/// |[`DeEvent::Eof`] | |Emits [`InvalidXml(IllFormed(MissedEnd))`](DeError::InvalidXml)
/// |[`DeEvent::Eof`] | |Emits [`InvalidXml(IllFormed(MissingEndTag))`](DeError::InvalidXml)
///
/// [`Text`]: Event::Text
/// [`CData`]: Event::CData
Expand Down Expand Up @@ -3642,7 +3642,7 @@ mod tests {

match de.read_to_end(QName(b"tag")) {
Err(DeError::InvalidXml(Error::IllFormed(cause))) => {
assert_eq!(cause, IllFormedError::MissedEnd("tag".into()))
assert_eq!(cause, IllFormedError::MissingEndTag("tag".into()))
}
x => panic!(
"Expected `Err(InvalidXml(IllFormed(_)))`, but got `{:?}`",
Expand All @@ -3661,7 +3661,7 @@ mod tests {

match de.read_to_end(QName(b"tag")) {
Err(DeError::InvalidXml(Error::IllFormed(cause))) => {
assert_eq!(cause, IllFormedError::MissedEnd("tag".into()))
assert_eq!(cause, IllFormedError::MissingEndTag("tag".into()))
}
x => panic!(
"Expected `Err(InvalidXml(IllFormed(_)))`, but got `{:?}`",
Expand Down Expand Up @@ -3756,7 +3756,7 @@ mod tests {
fn read_string() {
match from_str::<String>(r#"</root>"#) {
Err(DeError::InvalidXml(Error::IllFormed(cause))) => {
assert_eq!(cause, IllFormedError::UnmatchedEnd("root".into()));
assert_eq!(cause, IllFormedError::UnmatchedEndTag("root".into()));
}
x => panic!(
"Expected `Err(InvalidXml(IllFormed(_)))`, but got `{:?}`",
Expand All @@ -3770,7 +3770,7 @@ mod tests {
match from_str::<String>(r#"<root></other>"#) {
Err(DeError::InvalidXml(Error::IllFormed(cause))) => assert_eq!(
cause,
IllFormedError::MismatchedEnd {
IllFormedError::MismatchedEndTag {
expected: "root".into(),
found: "other".into(),
}
Expand Down Expand Up @@ -4098,7 +4098,7 @@ mod tests {
assert_eq!(de.next().unwrap(), DeEvent::End(BytesEnd::new("tag")));
match de.next() {
Err(DeError::InvalidXml(Error::IllFormed(cause))) => {
assert_eq!(cause, IllFormedError::UnmatchedEnd("tag2".into()));
assert_eq!(cause, IllFormedError::UnmatchedEndTag("tag2".into()));
}
x => panic!(
"Expected `Err(InvalidXml(IllFormed(_)))`, but got `{:?}`",
Expand Down Expand Up @@ -4241,7 +4241,7 @@ mod tests {
let mut de = make_de("</tag>");
match de.next() {
Err(DeError::InvalidXml(Error::IllFormed(cause))) => {
assert_eq!(cause, IllFormedError::UnmatchedEnd("tag".into()));
assert_eq!(cause, IllFormedError::UnmatchedEndTag("tag".into()));
}
x => panic!(
"Expected `Err(InvalidXml(IllFormed(_)))`, but got `{:?}`",
Expand Down Expand Up @@ -4320,7 +4320,7 @@ mod tests {
assert_eq!(de.next().unwrap(), DeEvent::Text("text".into()));
match de.next() {
Err(DeError::InvalidXml(Error::IllFormed(cause))) => {
assert_eq!(cause, IllFormedError::UnmatchedEnd("tag".into()));
assert_eq!(cause, IllFormedError::UnmatchedEndTag("tag".into()));
}
x => panic!(
"Expected `Err(InvalidXml(IllFormed(_)))`, but got `{:?}`",
Expand Down Expand Up @@ -4352,7 +4352,7 @@ mod tests {
assert_eq!(de.next().unwrap(), DeEvent::Text("text cdata ".into()));
match de.next() {
Err(DeError::InvalidXml(Error::IllFormed(cause))) => {
assert_eq!(cause, IllFormedError::UnmatchedEnd("tag".into()));
assert_eq!(cause, IllFormedError::UnmatchedEndTag("tag".into()));
}
x => panic!(
"Expected `Err(InvalidXml(IllFormed(_)))`, but got `{:?}`",
Expand Down Expand Up @@ -4458,7 +4458,7 @@ mod tests {
assert_eq!(de.next().unwrap(), DeEvent::Text(" cdata ".into()));
match de.next() {
Err(DeError::InvalidXml(Error::IllFormed(cause))) => {
assert_eq!(cause, IllFormedError::UnmatchedEnd("tag".into()));
assert_eq!(cause, IllFormedError::UnmatchedEndTag("tag".into()));
}
x => panic!(
"Expected `Err(InvalidXml(IllFormed(_)))`, but got `{:?}`",
Expand Down Expand Up @@ -4488,7 +4488,7 @@ mod tests {
assert_eq!(de.next().unwrap(), DeEvent::Text(" cdata text".into()));
match de.next() {
Err(DeError::InvalidXml(Error::IllFormed(cause))) => {
assert_eq!(cause, IllFormedError::UnmatchedEnd("tag".into()));
assert_eq!(cause, IllFormedError::UnmatchedEndTag("tag".into()));
}
x => panic!(
"Expected `Err(InvalidXml(IllFormed(_)))`, but got `{:?}`",
Expand Down Expand Up @@ -4538,7 +4538,7 @@ mod tests {
assert_eq!(de.next().unwrap(), DeEvent::Text(" cdata cdata2 ".into()));
match de.next() {
Err(DeError::InvalidXml(Error::IllFormed(cause))) => {
assert_eq!(cause, IllFormedError::UnmatchedEnd("tag".into()));
assert_eq!(cause, IllFormedError::UnmatchedEndTag("tag".into()));
}
x => panic!(
"Expected `Err(InvalidXml(IllFormed(_)))`, but got `{:?}`",
Expand Down
24 changes: 12 additions & 12 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,26 +81,26 @@ pub enum IllFormedError {
/// the declaration. In the last case it contains the name of the found attribute.
///
/// [specification]: https://www.w3.org/TR/xml11/#sec-prolog-dtd
MissedVersion(Option<String>),
MissingDeclVersion(Option<String>),
/// A document type definition (DTD) does not contain a name of a root element.
///
/// According to the [specification], document type definition (`<!DOCTYPE foo>`)
/// MUST contain a name which defines a document type (`foo`). If that name
/// is missed, this error is returned.
///
/// [specification]: https://www.w3.org/TR/xml11/#NT-doctypedecl
MissedDoctypeName,
MissingDoctypeName,
/// The end tag was not found during reading of a sub-tree of elements due to
/// encountering an EOF from the underlying reader. This error is returned from
/// [`Reader::read_to_end`].
///
/// [`Reader::read_to_end`]: crate::reader::Reader::read_to_end
MissedEnd(String),
MissingEndTag(String),
/// The specified end tag was encountered without corresponding open tag at the
/// same level of hierarchy
UnmatchedEnd(String),
UnmatchedEndTag(String),
/// The specified end tag does not match the start tag at that nesting level.
MismatchedEnd {
MismatchedEndTag {
/// Name of open tag, that is expected to be closed
expected: String,
/// Name of actually closed tag
Expand All @@ -122,25 +122,25 @@ pub enum IllFormedError {
impl fmt::Display for IllFormedError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Self::MissedVersion(None) => {
Self::MissingDeclVersion(None) => {
write!(f, "an XML declaration does not contain `version` attribute")
}
Self::MissedVersion(Some(attr)) => {
Self::MissingDeclVersion(Some(attr)) => {
write!(f, "an XML declaration must start with `version` attribute, but in starts with `{}`", attr)
}
Self::MissedDoctypeName => write!(
Self::MissingDoctypeName => write!(
f,
"`<!DOCTYPE>` declaration does not contain a name of a document type"
),
Self::MissedEnd(tag) => write!(
Self::MissingEndTag(tag) => write!(
f,
"start tag not closed: `</{}>` not found before end of input",
tag,
),
Self::UnmatchedEnd(tag) => {
Self::UnmatchedEndTag(tag) => {
write!(f, "close tag `</{}>` does not match any open tag", tag)
}
Self::MismatchedEnd { expected, found } => write!(
Self::MismatchedEndTag { expected, found } => write!(
f,
"expected `</{}>`, but `</{}>` was found",
expected, found,
Expand Down Expand Up @@ -199,7 +199,7 @@ pub enum Error {
impl Error {
pub(crate) fn missed_end(name: QName, decoder: Decoder) -> Self {
match decoder.decode(name.as_ref()) {
Ok(name) => IllFormedError::MissedEnd(name.into()).into(),
Ok(name) => IllFormedError::MissingEndTag(name.into()).into(),
Err(err) => err.into(),
}
}
Expand Down
14 changes: 8 additions & 6 deletions src/events/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ impl<'a> BytesDecl<'a> {
/// In case of multiple attributes value of the first one is returned.
///
/// If version is missed in the declaration, or the first thing is not a version,
/// [`IllFormedError::MissedVersion`] will be returned.
/// [`IllFormedError::MissingDeclVersion`] will be returned.
///
/// # Examples
///
Expand All @@ -410,21 +410,21 @@ impl<'a> BytesDecl<'a> {
/// // <?xml encoding='utf-8'?>
/// let decl = BytesDecl::from_start(BytesStart::from_content(" encoding='utf-8'", 0));
/// match decl.version() {
/// Err(Error::IllFormed(IllFormedError::MissedVersion(Some(key)))) => assert_eq!(key, "encoding"),
/// Err(Error::IllFormed(IllFormedError::MissingDeclVersion(Some(key)))) => assert_eq!(key, "encoding"),
/// _ => assert!(false),
/// }
///
/// // <?xml encoding='utf-8' version='1.1'?>
/// let decl = BytesDecl::from_start(BytesStart::from_content(" encoding='utf-8' version='1.1'", 0));
/// match decl.version() {
/// Err(Error::IllFormed(IllFormedError::MissedVersion(Some(key)))) => assert_eq!(key, "encoding"),
/// Err(Error::IllFormed(IllFormedError::MissingDeclVersion(Some(key)))) => assert_eq!(key, "encoding"),
/// _ => assert!(false),
/// }
///
/// // <?xml?>
/// let decl = BytesDecl::from_start(BytesStart::from_content("", 0));
/// match decl.version() {
/// Err(Error::IllFormed(IllFormedError::MissedVersion(None))) => {},
/// Err(Error::IllFormed(IllFormedError::MissingDeclVersion(None))) => {},
/// _ => assert!(false),
/// }
/// ```
Expand All @@ -437,12 +437,14 @@ impl<'a> BytesDecl<'a> {
// first attribute was not "version"
Some(Ok(a)) => {
let found = from_utf8(a.key.as_ref())?.to_string();
Err(Error::IllFormed(IllFormedError::MissedVersion(Some(found))))
Err(Error::IllFormed(IllFormedError::MissingDeclVersion(Some(
found,
))))
}
// error parsing attributes
Some(Err(e)) => Err(e.into()),
// no attributes
None => Err(Error::IllFormed(IllFormedError::MissedVersion(None))),
None => Err(Error::IllFormed(IllFormedError::MissingDeclVersion(None))),
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/reader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub struct Config {
pub check_comments: bool,

/// Whether mismatched closing tag names should be detected. If enabled, in
/// case of mismatch the [`Error::IllFormed(MismatchedEnd)`] is returned from
/// case of mismatch the [`Error::IllFormed(MismatchedEndTag)`] is returned from
/// read methods.
///
/// Note, that start and end tags [should match literally][spec], they cannot
Expand Down Expand Up @@ -71,7 +71,7 @@ pub struct Config {
///
/// Default: `true`
///
/// [`Error::IllFormed(MismatchedEnd)`]: crate::errors::IllFormedError::MismatchedEnd
/// [`Error::IllFormed(MismatchedEndTag)`]: crate::errors::IllFormedError::MismatchedEndTag
/// [spec]: https://www.w3.org/TR/xml11/#dt-etag
/// [`End`]: crate::events::Event::End
/// [`expand_empty_elements`]: Self::expand_empty_elements
Expand Down
6 changes: 3 additions & 3 deletions src/reader/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl ReaderState {
// We want report error at place where name is expected - this is just
// before `>`
self.offset -= 1;
return Err(Error::IllFormed(IllFormedError::MissedDoctypeName));
return Err(Error::IllFormed(IllFormedError::MissingDoctypeName));
}
}
}
Expand Down Expand Up @@ -170,7 +170,7 @@ impl ReaderState {
// Report error at start of the end tag at `<` character
// +2 for `<` and `>`
self.offset -= buf.len() + 2;
return Err(Error::IllFormed(IllFormedError::MismatchedEnd {
return Err(Error::IllFormed(IllFormedError::MismatchedEndTag {
expected,
found: decoder.decode(name).unwrap_or_default().into_owned(),
}));
Expand All @@ -183,7 +183,7 @@ impl ReaderState {
// Report error at start of the end tag at `<` character
// +2 for `<` and `>`
self.offset -= buf.len() + 2;
return Err(Error::IllFormed(IllFormedError::UnmatchedEnd(
return Err(Error::IllFormed(IllFormedError::UnmatchedEndTag(
decoder.decode(name).unwrap_or_default().into_owned(),
)));
}
Expand Down
2 changes: 1 addition & 1 deletion tests/fuzzing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fn fuzz_empty_doctype() {
let mut buf = Vec::new();
assert!(matches!(
reader.read_event_into(&mut buf).unwrap_err(),
Error::IllFormed(IllFormedError::MissedDoctypeName)
Error::IllFormed(IllFormedError::MissingDoctypeName)
));
assert_eq!(reader.read_event_into(&mut buf).unwrap(), Event::Eof);
}
2 changes: 1 addition & 1 deletion tests/issues.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ mod issue514 {
match reader.read_event() {
Err(Error::IllFormed(cause)) => assert_eq!(
cause,
IllFormedError::MismatchedEnd {
IllFormedError::MismatchedEndTag {
expected: "some-tag".into(),
found: "other-tag".into(),
}
Expand Down
4 changes: 2 additions & 2 deletions tests/reader-config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ mod check_end_names {
match reader.read_event() {
Err(Error::IllFormed(cause)) => assert_eq!(
cause,
IllFormedError::MismatchedEnd {
IllFormedError::MismatchedEndTag {
expected: "tag".into(),
found: "mismatched".into(),
}
Expand Down Expand Up @@ -421,7 +421,7 @@ mod trim_markup_names_in_closing_tags {
match reader.read_event() {
Err(Error::IllFormed(cause)) => assert_eq!(
cause,
IllFormedError::MismatchedEnd {
IllFormedError::MismatchedEndTag {
expected: "root".into(),
found: "root \t\r\n".into(),
}
Expand Down

0 comments on commit 2c55638

Please sign in to comment.