From af14717ddcefff15c215b6e41148e6f9838d0df5 Mon Sep 17 00:00:00 2001 From: Emmanuel Gil Peyrot Date: Tue, 25 Feb 2025 02:01:20 +0100 Subject: [PATCH] minidom: Fix most issues reported by clippy This improves some of our internals. skip-changelog: These changes are purely internal. --- minidom/src/element.rs | 25 ++++++++++++++++++++----- minidom/src/error.rs | 12 ++++++------ minidom/src/node.rs | 6 ++++++ minidom/src/tree_builder.rs | 19 +++++++++++-------- 4 files changed, 43 insertions(+), 19 deletions(-) diff --git a/minidom/src/element.rs b/minidom/src/element.rs index 1f0c78ca11e392e3cc97a15bac488d79417ef5ed..425dd4c30ef4242e1c03ef1fa449def8c574cd6d 100644 --- a/minidom/src/element.rs +++ b/minidom/src/element.rs @@ -73,13 +73,14 @@ pub type ItemWriter = CustomItemWriter; /// helper function to escape a `&[u8]` and replace all /// xml special characters (<, >, &, ', ") with their corresponding /// xml escaped value. +#[must_use] pub fn escape(raw: &[u8]) -> Cow<[u8]> { - let mut escapes: Vec<(usize, &'static [u8])> = Vec::new(); - let mut bytes = raw.iter(); fn to_escape(b: u8) -> bool { matches!(b, b'<' | b'>' | b'\'' | b'&' | b'"') } + let mut escapes: Vec<(usize, &'static [u8])> = Vec::new(); + let mut bytes = raw.iter(); let mut loc = 0; while let Some(i) = bytes.position(|&b| to_escape(b)) { loc += i; @@ -89,7 +90,7 @@ pub fn escape(raw: &[u8]) -> Cow<[u8]> { b'\'' => escapes.push((loc, b"'")), b'&' => escapes.push((loc, b"&")), b'"' => escapes.push((loc, b""")), - _ => unreachable!("Only '<', '>','\', '&' and '\"' are escaped"), + _ => unreachable!("Only '<', '>', '\'', '&' and '\"' are escaped"), } loc += 1; } @@ -228,16 +229,19 @@ impl Element { } /// Returns a reference to the local name of this element (that is, without a possible prefix). + #[must_use] pub fn name(&self) -> &str { &self.name } /// Returns a reference to the namespace of this element. + #[must_use] pub fn ns(&self) -> String { self.namespace.clone() } /// Returns a reference to the value of the given attribute, if it exists, else `None`. + #[must_use] pub fn attr(&self, name: &str) -> Option<&str> { if let Some(value) = self.attributes.get(name) { return Some(value); @@ -259,6 +263,7 @@ impl Element { /// assert_eq!(iter.next().unwrap(), ("a", "b")); /// assert_eq!(iter.next(), None); /// ``` + #[must_use] pub fn attrs(&self) -> Attrs { Attrs { iter: self.attributes.iter(), @@ -267,6 +272,7 @@ impl Element { /// Returns an iterator over the attributes of this element, with the value being a mutable /// reference. + #[must_use] pub fn attrs_mut(&mut self) -> AttrsMut { AttrsMut { iter: self.attributes.iter_mut(), @@ -401,7 +407,7 @@ impl Element { let namespace: RxmlNamespace = self.namespace.clone().into(); writer.write(Item::ElementHeadStart(&namespace, (*self.name).try_into()?))?; - for (key, value) in self.attributes.iter() { + for (key, value) in &self.attributes { let (prefix, name) = <&rxml::NameStr>::try_from(&**key) .unwrap() .split_name() @@ -418,7 +424,7 @@ impl Element { if !self.children.is_empty() { writer.write(Item::ElementHeadEnd)?; - for child in self.children.iter() { + for child in &self.children { child.write_to_inner(writer)?; } } @@ -477,6 +483,7 @@ impl Element { /// assert_eq!(iter.next(), None); /// ``` #[inline] + #[must_use] pub fn children(&self) -> Children { Children { iter: self.children.iter(), @@ -485,6 +492,7 @@ impl Element { /// Returns an iterator over mutable references to every child element of this element. #[inline] + #[must_use] pub fn children_mut(&mut self) -> ChildrenMut { ChildrenMut { iter: self.children.iter_mut(), @@ -522,6 +530,7 @@ impl Element { /// assert_eq!(iter.next(), None); /// ``` #[inline] + #[must_use] pub fn texts(&self) -> Texts { Texts { iter: self.children.iter(), @@ -530,6 +539,7 @@ impl Element { /// Returns an iterator over mutable references to every text node of this element. #[inline] + #[must_use] pub fn texts_mut(&mut self) -> TextsMut { TextsMut { iter: self.children.iter_mut(), @@ -643,6 +653,7 @@ impl Element { /// /// assert_eq!(elem.text(), "hello, world!"); /// ``` + #[must_use] pub fn text(&self) -> String { self.texts().fold(String::new(), |ret, new| ret + new) } @@ -907,6 +918,7 @@ impl ElementBuilder { } /// Sets an attribute. + #[must_use] pub fn attr, V: IntoAttributeValue>( mut self, name: S, @@ -917,12 +929,14 @@ impl ElementBuilder { } /// Appends anything implementing `Into` into the tree. + #[must_use] pub fn append>(mut self, node: T) -> ElementBuilder { self.root.append_node(node.into()); self } /// Appends an iterator of things implementing `Into` into the tree. + #[must_use] pub fn append_all, I: IntoIterator>( mut self, iter: I, @@ -934,6 +948,7 @@ impl ElementBuilder { } /// Builds the `Element`. + #[must_use] pub fn build(self) -> Element { self.root } diff --git a/minidom/src/error.rs b/minidom/src/error.rs index 8b15794b005bbb3cf14917e529dfd3b720b497fe..0ac47947105715926ab6f4fa63a1b920734590bf 100644 --- a/minidom/src/error.rs +++ b/minidom/src/error.rs @@ -48,10 +48,10 @@ impl StdError for Error { match self { Error::XmlError(e) => Some(e), Error::Io(e) => Some(e), - Error::EndOfDocument => None, - Error::InvalidPrefix => None, - Error::MissingNamespace => None, - Error::DuplicatePrefix => None, + Error::EndOfDocument + | Error::InvalidPrefix + | Error::MissingNamespace + | Error::DuplicatePrefix => None, } } } @@ -68,8 +68,8 @@ impl From for Error { impl fmt::Display for Error { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { match self { - Error::XmlError(e) => write!(fmt, "XML error: {}", e), - Error::Io(e) => write!(fmt, "I/O error: {}", e), + Error::XmlError(e) => write!(fmt, "XML error: {e}"), + Error::Io(e) => write!(fmt, "I/O error: {e}"), Error::EndOfDocument => { write!(fmt, "the end of the document has been reached prematurely") } diff --git a/minidom/src/node.rs b/minidom/src/node.rs index c5374cf65a9949a4d552f8b438ca483c8a5a678c..8fd2a6c6ff7dfb565cde12793f7596e02b4d9668 100644 --- a/minidom/src/node.rs +++ b/minidom/src/node.rs @@ -39,6 +39,7 @@ impl Node { /// assert_eq!(elm.as_element().unwrap().name(), "meow"); /// assert_eq!(txt.as_element(), None); /// ``` + #[must_use] pub fn as_element(&self) -> Option<&Element> { match *self { Node::Element(ref e) => Some(e), @@ -60,6 +61,7 @@ impl Node { /// assert_eq!(elm.as_element_mut().unwrap().name(), "meow"); /// assert_eq!(txt.as_element_mut(), None); /// ``` + #[must_use] pub fn as_element_mut(&mut self) -> Option<&mut Element> { match *self { Node::Element(ref mut e) => Some(e), @@ -81,6 +83,7 @@ impl Node { /// assert_eq!(elm.into_element().unwrap().name(), "meow"); /// assert_eq!(txt.into_element(), None); /// ``` + #[must_use] pub fn into_element(self) -> Option { match self { Node::Element(e) => Some(e), @@ -102,6 +105,7 @@ impl Node { /// assert_eq!(elm.as_text(), None); /// assert_eq!(txt.as_text().unwrap(), "meow"); /// ``` + #[must_use] pub fn as_text(&self) -> Option<&str> { match *self { Node::Element(_) => None, @@ -129,6 +133,7 @@ impl Node { /// } /// assert_eq!(txt.as_text().unwrap(), "meowzies"); /// ``` + #[must_use] pub fn as_text_mut(&mut self) -> Option<&mut String> { match *self { Node::Element(_) => None, @@ -150,6 +155,7 @@ impl Node { /// assert_eq!(elm.into_text(), None); /// assert_eq!(txt.into_text().unwrap(), "meow"); /// ``` + #[must_use] pub fn into_text(self) -> Option { match self { Node::Element(_) => None, diff --git a/minidom/src/tree_builder.rs b/minidom/src/tree_builder.rs index 5700d36af46bf63f388c19c2a301d9b2910c909a..73f60c99ad3bd115a6ec7ddedc1306c84c66f441 100644 --- a/minidom/src/tree_builder.rs +++ b/minidom/src/tree_builder.rs @@ -26,6 +26,7 @@ impl Default for TreeBuilder { impl TreeBuilder { /// Create a new one + #[must_use] pub fn new() -> Self { TreeBuilder { next_tag: None, @@ -39,17 +40,20 @@ impl TreeBuilder { /// /// Useful to provide knowledge of namespaces that would have been declared on parent elements /// not present in the reader. + #[must_use] pub fn with_prefixes_stack(mut self, prefixes_stack: Vec) -> Self { self.prefixes_stack = prefixes_stack; self } /// Stack depth + #[must_use] pub fn depth(&self) -> usize { self.stack.len() } /// Get the top-most element from the stack but don't remove it + #[must_use] pub fn top(&mut self) -> Option<&Element> { self.stack.last() } @@ -71,6 +75,7 @@ impl TreeBuilder { } /// Lookup XML namespace declaration for given prefix (or no prefix) + #[must_use] fn lookup_prefix(&self, prefix: &Option) -> Option<&str> { for nss in self.prefixes_stack.iter().rev() { if let Some(ns) = nss.get(prefix) { @@ -81,7 +86,7 @@ impl TreeBuilder { None } - fn process_end_tag(&mut self) -> Result<(), Error> { + fn process_end_tag(&mut self) { if let Some(el) = self.pop() { if self.depth() > 0 { let top = self.stack.len() - 1; @@ -90,8 +95,6 @@ impl TreeBuilder { self.root = Some(el); } } - - Ok(()) } fn process_text(&mut self, text: String) { @@ -101,7 +104,7 @@ impl TreeBuilder { } } - /// Process a Event that you got out of a RawParser + /// Process an event that you got out of a `RawParser`. pub fn process_event(&mut self, event: RawEvent) -> Result<(), Error> { match event { RawEvent::XmlDeclaration(_, _) => {} @@ -112,7 +115,7 @@ impl TreeBuilder { name.as_str().to_owned(), Prefixes::default(), BTreeMap::new(), - )) + )); } RawEvent::Attribute(_, (prefix, name), value) => { @@ -123,7 +126,7 @@ impl TreeBuilder { prefixes.insert(Some(prefix.as_str().to_owned()), value); } (Some(prefix), name) => { - attrs.insert(format!("{}:{}", prefix, name), value.as_str().to_owned()); + attrs.insert(format!("{prefix}:{name}"), value.as_str().to_owned()); } (None, name) => { attrs.insert(name.as_str().to_owned(), value.as_str().to_owned()); @@ -137,7 +140,7 @@ impl TreeBuilder { self.prefixes_stack.push(prefixes.clone()); let namespace = self - .lookup_prefix(&prefix.clone().map(|prefix| prefix.as_str().to_owned())) + .lookup_prefix(&prefix.map(|prefix| prefix.as_str().to_owned())) .ok_or(Error::MissingNamespace)? .to_owned(); let el = @@ -146,7 +149,7 @@ impl TreeBuilder { } } - RawEvent::ElementFoot(_) => self.process_end_tag()?, + RawEvent::ElementFoot(_) => self.process_end_tag(), RawEvent::Text(_, text) => self.process_text(text.as_str().to_owned()), }