rust-bakery / nom

Rust parser combinator framework
MIT License
9.18k stars 792 forks source link

clippy rust 1.72.1 #1678

Closed thatmarkenglishguy closed 8 months ago

thatmarkenglishguy commented 11 months ago

Issue 1677 Clippy warnings building on the command line. Changes in this PR address the warnings in the test case below. Additionally ignore Intellij project files.

Test case

git clone git@github.com:rust-bakery/nom.git
cd nom
git checkout 90d78d65a10821272ce8856570605b07a917a6c1
cargo clippy

Build output

warning: unneeded `return` statement
   --> src/multi/mod.rs:817:9
    |
817 | /         return Err(Err::Error(OM::Error::bind(move || {
818 | |           <F as Parser<I>>::Error::from_error_kind(input, ErrorKind::Many1Count)
819 | |         })))
    | |____________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
    = note: `#[warn(clippy::needless_return)]` on by default
    = help: remove `return`

error: using `clone` on a double-reference; this will copy the reference of type `&[u8]` instead of cloning the inner type
   --> src/traits.rs:336:28
    |
336 |         E::from_error_kind(self.clone(), e)
    |                            ^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_double_ref
    = note: `#[deny(clippy::clone_double_ref)]` on by default
help: try dereferencing it
    |
336 |         E::from_error_kind(&(*self).clone(), e)
    |                            ~~~~~~~~~~~~~~~~
help: or try being explicit if you are sure, that you want to clone a reference
    |
336 |         E::from_error_kind(<&[u8]>::clone(self), e)
    |                            ~~~~~~~~~~~~~~~~~~~~

error: using `clone` on a double-reference; this will copy the reference of type `&[u8]` instead of cloning the inner type
   --> src/traits.rs:344:32
    |
344 |             E::from_error_kind(self.clone(), e)
    |                                ^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_double_ref
help: try dereferencing it
    |
344 |             E::from_error_kind(&(*self).clone(), e)
    |                                ~~~~~~~~~~~~~~~~
help: or try being explicit if you are sure, that you want to clone a reference
    |
344 |             E::from_error_kind(<&[u8]>::clone(self), e)
    |                                ~~~~~~~~~~~~~~~~~~~~

error: using `clone` on a double-reference; this will copy the reference of type `&str` instead of cloning the inner type
   --> src/traits.rs:533:28
    |
533 |         E::from_error_kind(self.clone(), e)
    |                            ^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_double_ref
help: try dereferencing it
    |
533 |         E::from_error_kind(&(*self).clone(), e)
    |                            ~~~~~~~~~~~~~~~~
help: or try being explicit if you are sure, that you want to clone a reference
    |
533 |         E::from_error_kind(<&str>::clone(self), e)
    |                            ~~~~~~~~~~~~~~~~~~~

warning: length comparison to zero
   --> src/traits.rs:545:19
    |
545 |         } else if self.len() == 0 {
    |                   ^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `self.is_empty()`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
    = note: `#[warn(clippy::len_zero)]` on by default

error: using `clone` on a double-reference; this will copy the reference of type `&str` instead of cloning the inner type
   --> src/traits.rs:547:32
    |
547 |             E::from_error_kind(self.clone(), e)
    |                                ^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_double_ref
help: try dereferencing it
    |
547 |             E::from_error_kind(&(*self).clone(), e)
    |                                ~~~~~~~~~~~~~~~~
help: or try being explicit if you are sure, that you want to clone a reference
    |
547 |             E::from_error_kind(<&str>::clone(self), e)
    |                                ~~~~~~~~~~~~~~~~~~~

warning: deref which would be done by auto-deref
   --> src/traits.rs:672:5
    |
672 |     *self
    |     ^^^^^ help: try this: `self`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref
    = note: `#[warn(clippy::explicit_auto_deref)]` on by default

warning: this boolean expression can be simplified
    --> src/traits.rs:1457:5
     |
1457 |     !(self.start < self.end)
     |     ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `self.start >= self.end`
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
     = note: `#[warn(clippy::nonminimal_bool)]` on by default

error: this range is empty so it will yield no values
    --> src/traits.rs:1462:7
     |
1462 |       1..0
     |       ^^^^
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
     = note: `#[deny(clippy::reversed_empty_ranges)]` on by default
help: consider using the following if you are attempting to iterate over this range in reverse
     |
1462 |       (0..1).rev()
     |

error: this range is empty so it will yield no values
    --> src/traits.rs:1470:7
     |
1470 |       1..0
     |       ^^^^
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
help: consider using the following if you are attempting to iterate over this range in reverse
     |
1470 |       (0..1).rev()
     |

error: this range is empty so it will yield no values
    --> src/traits.rs:1545:7
     |
1545 |       1..0
     |       ^^^^
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
help: consider using the following if you are attempting to iterate over this range in reverse
     |
1545 |       (0..1).rev()
     |

error: this range is empty so it will yield no values
    --> src/traits.rs:1553:7
     |
1553 |       1..0
     |       ^^^^
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
help: consider using the following if you are attempting to iterate over this range in reverse
     |
1553 |       (0..1).rev()
     |

warning: redundant clone
   --> src/bytes/mod.rs:958:18
    |
958 |     let i = input.clone();
    |                  ^^^^^^^^ help: remove this
    |
note: cloned value is neither consumed nor mutated
   --> src/bytes/mod.rs:958:13
    |
958 |     let i = input.clone();
    |             ^^^^^^^^^^^^^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone
    = note: `#[warn(clippy::redundant_clone)]` on by default

warning: `nom` (lib) generated 5 warnings
error: could not compile `nom` (lib) due to 8 previous errors; 5 warnings emitted

Left each individual file's changes as a separate commit. Should be straightforward to squash. Tests still pass running cargo test.

UPDATE 13/09/2023

Merged from main and ran:

cargo clippy --all-targets

See comment below for details

thatmarkenglishguy commented 9 months ago

Merged from main, updated from rust 1.70.0 to 1.72.1, and ran:

cargo clippy --all-targets

which yielded the following build errors, now also addressed in this PR.

warning: using `.deref()` on a double reference, which returns `&[u8]` instead of dereferencing the inner type
   --> src/traits.rs:665:9
    |
665 |     self.deref()
    |         ^^^^^^^^
    |
    = note: `#[warn(suspicious_double_ref_op)]` on by default

warning: `nom` (lib) generated 1 warning
warning: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
   --> examples/json_iterator.rs:97:13
    |
97  | /             match value(v.data()) {
98  | |               Ok((i, _)) => {
99  | |                 v.offset(i);
100 | |               }
101 | |               Err(_) => {}
102 | |             }
    | |_____________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match
    = note: `#[warn(clippy::single_match)]` on by default
help: try this
    |
97  ~             if let Ok((i, _)) = value(v.data()) {
98  +               v.offset(i);
99  +             }
    |

warning: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
   --> examples/json_iterator.rs:105:11
    |
105 | /           match tag::<_, _, ()>("]")(v.data()) {
106 | |             Ok((i, _)) => {
107 | |               println!("]");
108 | |               v.offset(i);
...   |
112 | |             Err(_) => {}
113 | |           };
    | |___________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match
help: try this
    |
105 ~           if let Ok((i, _)) = tag::<_, _, ()>("]")(v.data()) {
106 +             println!("]");
107 +             v.offset(i);
108 +             done = true;
109 +             return None;
110 ~           };
    |

warning: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
   --> examples/json_iterator.rs:161:13
    |
161 | /             match value(v.data()) {
162 | |               Ok((i, _)) => {
163 | |                 v.offset(i);
164 | |               }
165 | |               Err(_) => {}
166 | |             }
    | |_____________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match
help: try this
    |
161 ~             if let Ok((i, _)) = value(v.data()) {
162 +               v.offset(i);
163 +             }
    |

warning: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
   --> examples/json_iterator.rs:169:11
    |
169 | /           match tag::<_, _, ()>("}")(v.data()) {
170 | |             Ok((i, _)) => {
171 | |               println!("}}");
172 | |               v.offset(i);
...   |
176 | |             Err(_) => {}
177 | |           };
    | |___________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match
help: try this
    |
169 ~           if let Ok((i, _)) = tag::<_, _, ()>("}")(v.data()) {
170 +             println!("}}");
171 +             v.offset(i);
172 +             done = true;
173 +             return None;
174 ~           };
    |

warning: the following explicit lifetimes could be elided: 'a
   --> examples/json_iterator.rs:228:1
    |
228 | fn string<'a>(i: &'a str) -> IResult<&'a str, &'a str> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
    = note: `#[warn(clippy::needless_lifetimes)]` on by default
help: elide the lifetimes
    |
228 - fn string<'a>(i: &'a str) -> IResult<&'a str, &'a str> {
228 + fn string(i: &str) -> IResult<&str, &str> {
    |

warning: the following explicit lifetimes could be elided: 'a
   --> examples/json_iterator.rs:235:1
    |
235 | fn boolean<'a>(input: &'a str) -> IResult<&'a str, bool> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
help: elide the lifetimes
    |
235 - fn boolean<'a>(input: &'a str) -> IResult<&'a str, bool> {
235 + fn boolean(input: &str) -> IResult<&str, bool> {
    |

warning: the following explicit lifetimes could be elided: 'a
   --> examples/json_iterator.rs:239:1
    |
239 | fn array<'a>(i: &'a str) -> IResult<&'a str, ()> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
help: elide the lifetimes
    |
239 - fn array<'a>(i: &'a str) -> IResult<&'a str, ()> {
239 + fn array(i: &str) -> IResult<&str, ()> {
    |

warning: the following explicit lifetimes could be elided: 'a
   --> examples/json_iterator.rs:252:1
    |
252 | fn key_value<'a>(i: &'a str) -> IResult<&'a str, (&'a str, ())> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
help: elide the lifetimes
    |
252 - fn key_value<'a>(i: &'a str) -> IResult<&'a str, (&'a str, ())> {
252 + fn key_value(i: &str) -> IResult<&str, (&str, ())> {
    |

warning: the following explicit lifetimes could be elided: 'a
   --> examples/json_iterator.rs:256:1
    |
256 | fn hash<'a>(i: &'a str) -> IResult<&'a str, ()> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
help: elide the lifetimes
    |
256 - fn hash<'a>(i: &'a str) -> IResult<&'a str, ()> {
256 + fn hash(i: &str) -> IResult<&str, ()> {
    |

warning: the following explicit lifetimes could be elided: 'a
   --> examples/json_iterator.rs:269:1
    |
269 | fn value<'a>(i: &'a str) -> IResult<&'a str, ()> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
help: elide the lifetimes
    |
269 - fn value<'a>(i: &'a str) -> IResult<&'a str, ()> {
269 + fn value(i: &str) -> IResult<&str, ()> {
    |

warning: called `map(..).flatten()` on `Iterator`
   --> examples/json_iterator.rs:311:10
    |
311 |           .map(|(user, o)| {
    |  __________^
312 | |           o.filter(|(k, _)| *k == "city")
313 | |             .filter_map(move |(_, v)| v.string().map(|s| (user, s)))
314 | |         })
315 | |         .flatten()
    | |__________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
    = note: `#[warn(clippy::map_flatten)]` on by default
help: try replacing `map` with `flat_map` and remove the `.flatten()`
    |
311 ~         .flat_map(|(user, o)| {
312 +           o.filter(|(k, _)| *k == "city")
313 +             .filter_map(move |(_, v)| v.string().map(|s| (user, s)))
314 +         })
    |

warning: the following explicit lifetimes could be elided: 'b
   --> tests/issues.rs:126:3
    |
126 | /   fn list<'a, 'b>(
127 | |     input: Input<'a>,
128 | |     _cs: &'b f64,
129 | |   ) -> Result<(Input<'a>, Vec<f64>), Err<Error<&'a [u8]>>> {
    | |__________________________________________________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
    = note: `#[warn(clippy::needless_lifetimes)]` on by default
help: elide the lifetimes
    |
126 ~   fn list<'a>(
127 |     input: Input<'a>,
128 ~     _cs: &f64,
    |

warning: `nom` (example "json_iterator") generated 11 warnings (run `cargo clippy --fix --example "json_iterator"` to apply 7 suggestions)
warning: `nom` (test "issues") generated 1 warning (run `cargo clippy --fix --test "issues"` to apply 1 suggestion)
warning: this lifetime isn't used in the function definition
   --> examples/json2.rs:161:15
    |
161 | fn json_value<'a>() -> JsonParser {
    |               ^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_lifetimes
    = note: `#[warn(clippy::extra_unused_lifetimes)]` on by default

    Checking nom v7.1.2 (/Users/mark.english/code/thirdparty/nom/nom)
warning: `nom` (example "json2") generated 1 warning
error: this range is empty so it will yield no values
   --> src/multi/tests.rs:575:10
    |
575 |     many(2..=1, tag("a")).parse(i)
    |          ^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
    = note: `#[deny(clippy::reversed_empty_ranges)]` on by default
help: consider using the following if you are attempting to iterate over this range in reverse
    |
575 |     many((1..=2).rev(), tag("a")).parse(i)
    |          ~~~~~~~~~~~~~

error: this range is empty so it will yield no values
   --> src/multi/tests.rs:730:10
    |
730 |     fold(2..=1, tag("a"), Vec::new, fold_into_vec).parse(i)
    |          ^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
help: consider using the following if you are attempting to iterate over this range in reverse
    |
730 |     fold((1..=2).rev(), tag("a"), Vec::new, fold_into_vec).parse(i)
    |          ~~~~~~~~~~~~~

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:553:11
    |
553 |       Ok((&" latin"[..], &"€"[..]))
    |           ^^^^^^^^^^^^^ help: use the original value instead: `" latin"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing
    = note: `#[warn(clippy::redundant_slicing)]` on by default

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:553:26
    |
553 |       Ok((&" latin"[..], &"€"[..]))
    |                          ^^^^^^^^ help: use the original value instead: `"€"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:557:11
    |
557 |       Ok((&" latin"[..], &"𝄠"[..]))
    |           ^^^^^^^^^^^^^ help: use the original value instead: `" latin"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:557:26
    |
557 |       Ok((&" latin"[..], &"𝄠"[..]))
    |                          ^^^^^^^^ help: use the original value instead: `"𝄠"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:561:11
    |
561 |       Ok((&" latin"[..], &"باب"[..]))
    |           ^^^^^^^^^^^^^ help: use the original value instead: `" latin"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:561:26
    |
561 |       Ok((&" latin"[..], &"باب"[..]))
    |                          ^^^^^^^^^^ help: use the original value instead: `"باب"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:565:11
    |
565 |       Ok((&" latin"[..], &"💣💢ᾠ"[..]))
    |           ^^^^^^^^^^^^^ help: use the original value instead: `" latin"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:565:26
    |
565 |       Ok((&" latin"[..], &"💣💢ᾠ"[..]))
    |                          ^^^^^^^^^^^^ help: use the original value instead: `"💣💢ᾠ"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:569:11
    |
569 |       Ok((&"latin"[..], &""[..]))
    |           ^^^^^^^^^^^^ help: use the original value instead: `"latin"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:569:25
    |
569 |       Ok((&"latin"[..], &""[..]))
    |                         ^^^^^^^ help: use the original value instead: `""`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:571:51
    |
571 |     assert_eq!(multi_byte_chars("باب", 1, 3), Ok((&""[..], &"باب"[..])));
    |                                                   ^^^^^^^ help: use the original value instead: `""`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:571:60
    |
571 |     assert_eq!(multi_byte_chars("باب", 1, 3), Ok((&""[..], &"باب"[..])));
    |                                                            ^^^^^^^^^^ help: use the original value instead: `"باب"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:572:51
    |
572 |     assert_eq!(multi_byte_chars("باب", 1, 2), Ok((&"ب"[..], &"با"[..])));
    |                                                   ^^^^^^^^ help: use the original value instead: `"ب"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:572:61
    |
572 |     assert_eq!(multi_byte_chars("باب", 1, 2), Ok((&"ب"[..], &"با"[..])));
    |                                                             ^^^^^^^^^ help: use the original value instead: `"با"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: redundant slicing of the whole range
   --> src/bytes/complete.rs:575:33
    |
575 |       Err(Err::Error(Error::new(&"latin"[..], ErrorKind::TakeWhileMN)))
    |                                 ^^^^^^^^^^^^ help: use the original value instead: `"latin"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing

warning: `nom` (lib test) generated 16 warnings (1 duplicate)
error: could not compile `nom` (lib test) due to 2 previous errors; 16 warnings emitted
Geal commented 8 months ago

ok, I don't know what happend here :sweat_smile: I tried to push a commit to the branch to fix the build, git complained that my local branch had missing commit, even though it was right at th same level (with one more commit). And force pushing just closed the PR o_O

Geal commented 8 months ago

reopening now, the build should pass

Geal commented 8 months ago

ok, can't reopen apparently, I'll make another PR integrating your changes

Geal commented 8 months ago

following up in #1702