minidom: Fix most issues reported by clippy

Emmanuel Gil Peyrot created

This improves some of our internals.

skip-changelog: These changes are purely internal.

Change summary

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(-)

Detailed changes

minidom/src/element.rs 🔗

@@ -73,13 +73,14 @@ pub type ItemWriter<W> = CustomItemWriter<W, rxml::writer::SimpleNamespaces>;
 /// 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"&apos;")),
             b'&' => escapes.push((loc, b"&amp;")),
             b'"' => escapes.push((loc, b"&quot;")),
-            _ => 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<S: Into<String>, V: IntoAttributeValue>(
         mut self,
         name: S,
@@ -917,12 +929,14 @@ impl ElementBuilder {
     }
 
     /// Appends anything implementing `Into<Node>` into the tree.
+    #[must_use]
     pub fn append<T: Into<Node>>(mut self, node: T) -> ElementBuilder {
         self.root.append_node(node.into());
         self
     }
 
     /// Appends an iterator of things implementing `Into<Node>` into the tree.
+    #[must_use]
     pub fn append_all<T: Into<Node>, I: IntoIterator<Item = T>>(
         mut self,
         iter: I,
@@ -934,6 +948,7 @@ impl ElementBuilder {
     }
 
     /// Builds the `Element`.
+    #[must_use]
     pub fn build(self) -> Element {
         self.root
     }

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<io::Error> 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")
             }

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<Element> {
         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<String> {
         match self {
             Node::Element(_) => None,

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<Prefixes>) -> 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<String>) -> 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()),
         }