Repository: tafia/quick-xml
Pull request #494 (2022-10-08 21:04:08)

Fix clippy issues
This PR fixes a few issues identified via `cargo clippy --all-features`. I interspersed a few `#[allow(clippy::...)]` bits where it seemed like readability was more important than the changes Clippy was suggesting or where performance might be affected (notably cases where `&Cow<>` is taken as a parameter instead of `Cow<>`, seemingly intentionally).

Might hold off on this one until the [serialization PR (#490)](https://github.com/tafia/quick-xml/pull/490) lands; I can fix the conflicts that will result afterward.

codecov-commenter: # [Codecov](https://codecov.io/gh/tafia/quick-xml/pull/494?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#494](https://codecov.io/gh/tafia/quick-xml/pull/494?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (4896f81) into [master](https://codecov.io/gh/tafia/quick-xml/commit/8fda0272109b64c302d08e804e5cea4a0344d311?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (8fda027) will **decrease** coverage by `0.06%`.
> The diff coverage is `43.75%`.

```diff
@@            Coverage Diff             @@
##           master     #494      +/-   ##
==========================================
- Coverage   54.05%   53.98%   -0.07%     
==========================================
  Files          30       30              
  Lines       12649    12638      -11     
==========================================
- Hits         6837     6823      -14     
- Misses       5812     5815       +3     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `53.98% <43.75%> (-0.07%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/tafia/quick-xml/pull/494?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [examples/custom\_entities.rs](https://codecov.io/gh/tafia/quick-xml/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZXhhbXBsZXMvY3VzdG9tX2VudGl0aWVzLnJz) | `0.00% <0.00%> (ø)` | |
| [examples/nested\_readers.rs](https://codecov.io/gh/tafia/quick-xml/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZXhhbXBsZXMvbmVzdGVkX3JlYWRlcnMucnM=) | `0.00% <0.00%> (ø)` | |
| [src/de/mod.rs](https://codecov.io/gh/tafia/quick-xml/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2RlL21vZC5ycw==) | `68.87% <0.00%> (-0.19%)` | :arrow_down: |
| [src/de/simple\_type.rs](https://codecov.io/gh/tafia/quick-xml/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2RlL3NpbXBsZV90eXBlLnJz) | `90.63% <ø> (ø)` | |
| [src/events/mod.rs](https://codecov.io/gh/tafia/quick-xml/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2V2ZW50cy9tb2QucnM=) | `72.88% <0.00%> (ø)` | |
| [src/reader/ns\_reader.rs](https://codecov.io/gh/tafia/quick-xml/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3JlYWRlci9uc19yZWFkZXIucnM=) | `63.71% <ø> (ø)` | |
| [src/reader/slice\_reader.rs](https://codecov.io/gh/tafia/quick-xml/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3JlYWRlci9zbGljZV9yZWFkZXIucnM=) | `95.31% <ø> (ø)` | |
| [src/utils.rs](https://codecov.io/gh/tafia/quick-xml/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3V0aWxzLnJz) | `65.35% <0.00%> (+0.51%)` | :arrow_up: |
| [src/events/attributes.rs](https://codecov.io/gh/tafia/quick-xml/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2V2ZW50cy9hdHRyaWJ1dGVzLnJz) | `93.90% <50.00%> (ø)` | |
| [src/escapei.rs](https://codecov.io/gh/tafia/quick-xml/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2VzY2FwZWkucnM=) | `12.04% <100.00%> (-0.06%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/tafia/quick-xml/pull/494/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

dralley: @dacut Could you rebase?
dacut: Ah, missed that the other issue was resolved. Will rebase and resolve.
dacut: Ok, rebased. `cargo clippy` is still (amazingly) showing clean, despite all the changes made to clippy in the meantime. (A lot of my projects with clippy checks broke recently.)

Patch:
diff --git a/examples/custom_entities.rs b/examples/custom_entities.rs
index 1405f3d..99c59c1 100644
--- a/examples/custom_entities.rs
+++ b/examples/custom_entities.rs
@@ -40,8 +40,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                     );
                 }
             }
-            Ok(Event::Start(ref e)) => match e.name().as_ref() {
-                b"test" => {
+            Ok(Event::Start(ref e)) => {
+                if let b"test" = e.name().as_ref() {
                     let attributes = e
                         .attributes()
                         .map(|a| {
@@ -55,8 +55,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                         .collect::<Vec<_>>();
                     println!("attributes values: {:?}", attributes);
                 }
-                _ => (),
-            },
+            }
             Ok(Event::Text(ref e)) => {
                 println!(
                     "text value: {}",
diff --git a/examples/nested_readers.rs b/examples/nested_readers.rs
index 3b30776..e00a175 100644
--- a/examples/nested_readers.rs
+++ b/examples/nested_readers.rs
@@ -21,8 +21,8 @@ fn main() -> Result<(), quick_xml::Error> {
     let mut found_tables = Vec::new();
     loop {
         match reader.read_event_into(&mut buf)? {
-            Event::Start(element) => match element.name().as_ref() {
-                b"w:tbl" => {
+            Event::Start(element) => {
+                if let b"w:tbl" = element.name().as_ref() {
                     count += 1;
                     let mut stats = TableStat {
                         index: count,
@@ -57,8 +57,7 @@ fn main() -> Result<(), quick_xml::Error> {
                         }
                     }
                 }
-                _ => {}
-            },
+            }
             Event::Eof => break,
             _ => {}
         }
diff --git a/src/de/mod.rs b/src/de/mod.rs
index 09f9c59..03b6648 100644
--- a/src/de/mod.rs
+++ b/src/de/mod.rs
@@ -2372,6 +2372,7 @@ where
 
 impl<'de> Deserializer<'de, SliceReader<'de>> {
     /// Create new deserializer that will borrow data from the specified string
+    #[allow(clippy::should_implement_trait)]
     pub fn from_str(s: &'de str) -> Self {
         let mut reader = Reader::from_str(s);
         reader
diff --git a/src/de/simple_type.rs b/src/de/simple_type.rs
index 24f4767..c3ea9c9 100644
--- a/src/de/simple_type.rs
+++ b/src/de/simple_type.rs
@@ -531,6 +531,7 @@ impl<'de, 'a> SimpleTypeDeserializer<'de, 'a> {
     }
 
     /// Creates a deserializer from a part of value at specified range
+    #[allow(clippy::ptr_arg)]
     pub fn from_part(
         value: &'a Cow<'de, [u8]>,
         range: Range<usize>,
diff --git a/src/escapei.rs b/src/escapei.rs
index 8b959c1..ce779f0 100644
--- a/src/escapei.rs
+++ b/src/escapei.rs
@@ -181,8 +181,7 @@ where
 
                 // search for character correctness
                 let pat = &raw[start + 1..end];
-                if pat.starts_with('#') {
-                    let entity = &pat[1..]; // starts after the #
+                if let Some(entity) = pat.strip_prefix('#') {
                     let codepoint = parse_number(entity, start..end)?;
                     unescaped.push_str(codepoint.encode_utf8(&mut [0u8; 4]));
                 } else if let Some(value) = named_entity(pat) {
@@ -1691,8 +1690,8 @@ fn named_entity(name: &str) -> Option<&str> {
 }
 
 fn parse_number(bytes: &str, range: Range<usize>) -> Result<char, EscapeError> {
-    let code = if bytes.starts_with('x') {
-        parse_hexadecimal(&bytes[1..])
+    let code = if let Some(hex_digits) = bytes.strip_prefix('x') {
+        parse_hexadecimal(hex_digits)
     } else {
         parse_decimal(bytes)
     }?;
diff --git a/src/events/attributes.rs b/src/events/attributes.rs
index e2cf8c6..8c99b8b 100644
--- a/src/events/attributes.rs
+++ b/src/events/attributes.rs
@@ -19,7 +19,7 @@ use std::{borrow::Cow, ops::Range};
 ///
 /// [`unescape_value`]: Self::unescape_value
 /// [`decode_and_unescape_value`]: Self::decode_and_unescape_value
-#[derive(Clone, PartialEq)]
+#[derive(Clone, Eq, PartialEq)]
 pub struct Attribute<'a> {
     /// The key to uniquely define the attribute.
     ///
@@ -537,6 +537,7 @@ impl IterState {
 
     /// Skip all characters up to first space symbol or end-of-input
     #[inline]
+    #[allow(clippy::manual_map)]
     fn skip_value(&self, slice: &[u8], offset: usize) -> Option<usize> {
         let mut iter = (offset..).zip(slice[offset..].iter());
 
@@ -776,7 +777,7 @@ impl IterState {
             None => {
                 // Because we reach end-of-input, stop iteration on next call
                 self.state = State::Done;
-                return Some(Err(AttrError::ExpectedQuote(slice.len(), quote)));
+                Some(Err(AttrError::ExpectedQuote(slice.len(), quote)))
             }
         }
     }
diff --git a/src/events/mod.rs b/src/events/mod.rs
index 535032a..1b813a8 100644
--- a/src/events/mod.rs
+++ b/src/events/mod.rs
@@ -119,7 +119,7 @@ impl<'a> BytesStart<'a> {
     /// Converts the event into an owned event without taking ownership of Event
     pub fn to_owned(&self) -> BytesStart<'static> {
         BytesStart {
-            buf: Cow::Owned(self.buf.to_owned().into()),
+            buf: Cow::Owned(self.buf.clone().into_owned()),
             name_len: self.name_len,
         }
     }
diff --git a/src/reader/mod.rs b/src/reader/mod.rs
index 77ed187..d928230 100644
--- a/src/reader/mod.rs
+++ b/src/reader/mod.rs
@@ -802,7 +802,7 @@ impl BangType {
         None
     }
     #[inline]
-    fn to_err(self) -> Error {
+    fn to_err(&self) -> Error {
         let bang_str = match self {
             Self::CData => "CData",
             Self::Comment => "Comment",
@@ -849,10 +849,7 @@ impl ReadElementState {
 /// A function to check whether the byte is a whitespace (blank, new line, carriage return or tab)
 #[inline]
 pub(crate) fn is_whitespace(b: u8) -> bool {
-    match b {
-        b' ' | b'\r' | b'\n' | b'\t' => true,
-        _ => false,
-    }
+    matches!(b, b' ' | b'\r' | b'\n' | b'\t')
 }
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
diff --git a/src/reader/ns_reader.rs b/src/reader/ns_reader.rs
index 4ce816f..09457f2 100644
--- a/src/reader/ns_reader.rs
+++ b/src/reader/ns_reader.rs
@@ -538,6 +538,7 @@ impl NsReader<BufReader<File>> {
 impl<'i> NsReader<&'i [u8]> {
     /// Creates an XML reader from a string slice.
     #[inline]
+    #[allow(clippy::should_implement_trait)]
     pub fn from_str(s: &'i str) -> Self {
         Self::new(Reader::from_str(s))
     }
diff --git a/src/reader/slice_reader.rs b/src/reader/slice_reader.rs
index f9ff503..dc0a5b8 100644
--- a/src/reader/slice_reader.rs
+++ b/src/reader/slice_reader.rs
@@ -21,6 +21,7 @@ use memchr;
 /// itself can be used to borrow from.
 impl<'a> Reader<&'a [u8]> {
     /// Creates an XML reader from a string slice.
+    #[allow(clippy::should_implement_trait)]
     pub fn from_str(s: &'a str) -> Self {
         // Rust strings are guaranteed to be UTF-8, so lock the encoding
         #[cfg(feature = "encoding")]
diff --git a/src/utils.rs b/src/utils.rs
index bd4ffe1..75f4ced 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -6,11 +6,12 @@ use serde::de::{Deserialize, Deserializer, Error, Visitor};
 #[cfg(feature = "serialize")]
 use serde::ser::{Serialize, Serializer};
 
+#[allow(clippy::ptr_arg)]
 pub fn write_cow_string(f: &mut Formatter, cow_string: &Cow<[u8]>) -> fmt::Result {
     match cow_string {
         Cow::Owned(s) => {
             write!(f, "Owned(")?;
-            write_byte_string(f, &s)?;
+            write_byte_string(f, s)?;
         }
         Cow::Borrowed(s) => {
             write!(f, "Borrowed(")?;


Test patch:
diff --git a/tests/xmlrs_reader_tests.rs b/tests/xmlrs_reader_tests.rs
index b5fc0bc..799bfcd 100644
--- a/tests/xmlrs_reader_tests.rs
+++ b/tests/xmlrs_reader_tests.rs
@@ -433,7 +433,7 @@ fn test_bytes(input: &[u8], output: &[u8], trim: bool) {
                 Ok(c) => format!("Characters({})", &c),
                 Err(err) => format!("FailedUnescape({:?}; {})", e.as_ref(), err),
             },
-            Ok((_, Event::Eof)) => format!("EndDocument"),
+            Ok((_, Event::Eof)) => "EndDocument".to_string(),
             Err(e) => format!("Error: {}", e),
         };
         if let Some((n, spec)) = spec_lines.next() {