Fix clippy warnings for lib, examples, and tests#1585
Fix clippy warnings for lib, examples, and tests#1585cky wants to merge 3 commits intorust-bakery:mainfrom
Conversation
cky
force-pushed
the
fix-clippy-warnings
branch
from
f1690e9 to
3ee23fe
Compare
| #[inline(always)] | ||
| fn as_bytes(&self) -> &[u8] { | ||
| *self | ||
| self |
There was a problem hiding this comment.
This fixes clippy::explicit_auto_deref.
| /// Transforms common types to a char for basic token parsing | ||
| pub trait AsChar { | ||
| #[allow(clippy::len_without_is_empty)] | ||
| pub trait AsChar: Copy { |
There was a problem hiding this comment.
The Copy constraint addresses clippy::wrong_self_convention.
| #[inline] | ||
| pub fn is_hex_digit(chr: u8) -> bool { | ||
| (chr >= 0x30 && chr <= 0x39) || (chr >= 0x41 && chr <= 0x46) || (chr >= 0x61 && chr <= 0x66) | ||
| matches!(chr, 0x30..=0x39 | 0x41..=0x46 | 0x61..=0x66) |
There was a problem hiding this comment.
Note that Clippy's suggested fixes for clippy::manual_range_contains are unidiomatic for this use case (as well as other matches! fixes in this branch):
(0x30..=0x39).contains(&chr) || (0x41..=0x46).contains(&chr) || (0x61..=0x66).contains(&chr)| let larger = format!("{}", test); | ||
| assert_parse!(recognize_float(&larger[..]), Ok(("", test))); | ||
| assert_parse!(recognize_float(test), Ok(("", test))); | ||
|
|
||
| assert_parse!(float(larger.as_bytes()), Ok((&b""[..], expected32))); | ||
| assert_parse!(float(&larger[..]), Ok(("", expected32))); | ||
| assert_parse!(float(test.as_bytes()), Ok((&b""[..], expected32))); | ||
| assert_parse!(float(test), Ok(("", expected32))); | ||
|
|
||
| assert_parse!(double(larger.as_bytes()), Ok((&b""[..], expected64))); | ||
| assert_parse!(double(&larger[..]), Ok(("", expected64))); | ||
| assert_parse!(double(test.as_bytes()), Ok((&b""[..], expected64))); | ||
| assert_parse!(double(test), Ok(("", expected64))); |
There was a problem hiding this comment.
This addresses clippy::useless_format.
While it's easy to simply replace the format!("{}", test) with test.to_string() (as suggested by Clippy) or even test.to_owned(), neither of these address the fact that there's actually no reason to create a String here in the first place.
The reason a String is created for the streaming test is because the test needed to append a garbage character to avoid an Err::Incomplete result. There's no such need for the complete test.
| c == 'β' | ||
| || c == 'è' | ||
| || c == 'ƒ' | ||
| || c == 'ô' | ||
| || c == 'ř' | ||
| || c == 'è' | ||
| || c == 'Â' | ||
| || c == 'ß' | ||
| || c == 'Ç' |
There was a problem hiding this comment.
This change addresses clippy::nonminimal_bool. The expression is non-minimal because "è" is compared twice. While it's easy enough to just remove the duplicate, it seems a missed opportunity to optimise the comparison using matches!.
|
|
||
| for &byte in chunk { | ||
| if (byte >= 32 && byte <= 126) || byte >= 128 { | ||
| if matches!(byte, 32..=126 | 128..=255) { |
There was a problem hiding this comment.
Current versions of Rust supports using 128.. in patterns. But Rust 1.48 doesn't.
There was a problem hiding this comment.
and it fails on matches too: https://github.com/rust-bakery/nom/actions/runs/3807696627/jobs/6477623360#step:5:29
I'm releasing nom 7.2 soon, then for 8.0 I'll raise the minimum version.
Pull Request Test Coverage Report for Build 3818917772Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
merged in 12546b0 (had to edit it to set the minimal version in CI to 1.42), thanks! |
3 participants
Similar in concept to #1569, but:
lib,examples, andtests; note that I fix the main project only, notbenchmarks)I'll also add PR comments in places where the code changes are not immediately obvious.