Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address clippy lints #160

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a3e76e6
Address `clippy::deprecated_cfg_attr` lint violations
lopopolo May 21, 2023
8e629d5
Address `clippy::get_first` lint violations
lopopolo May 21, 2023
ab08a45
Address `clippy::needless_borrow` lint violations
lopopolo May 21, 2023
584674d
Address `clippy::op_ref` lint violations
lopopolo May 21, 2023
f46caa1
Address `clippy::needless_lifetimes` lint violations
lopopolo May 21, 2023
9843ef7
Address `clippy::useless_conversion` lint violations
lopopolo May 21, 2023
6a367f6
Address `clippy::map_clone` lint violations
lopopolo May 21, 2023
d9d7b88
Simplify `ByteSlice::last_byte`
lopopolo May 21, 2023
5627aeb
Address `clippy::redundant_static_lifetimes` lint violations
lopopolo May 21, 2023
948142f
Address `clippy::while_let_on_iterator` lint violations
lopopolo May 21, 2023
14db391
Address `clippy::needless_return` lint violations
lopopolo May 21, 2023
9b7e161
Address `clippy::identity_op` lint violations
lopopolo May 21, 2023
906eb4b
Address `clippy::ptr_offset_with_cast` lint violations
lopopolo May 21, 2023
5b379ed
Address `clippy::derived_hash_with_manual_eq` lint violations
lopopolo May 21, 2023
8bd170b
Allow `clippy::single_char_add_str` lint crate-wide
lopopolo May 21, 2023
76f590d
Address `clippy::manual_find` lint violations
lopopolo May 21, 2023
246b823
Address `clippy::wildcard_in_or_patterns` lint violations
lopopolo May 21, 2023
7b16270
use ptr.sub instead of ptr.offset with cast and assert
lopopolo May 22, 2023
39a749e
Address `clippy::transmute_ptr_to_ptr` lint violations
lopopolo May 22, 2023
af7dba9
Address `clippy::range_plus_one` lint violations
lopopolo May 22, 2023
5c7c495
Address `clippy::cloned_instead_of_copied` lint violation
lopopolo May 22, 2023
319b31a
Address `clippy::redundant_closure_for_method_calls` lint violation
lopopolo May 22, 2023
9800151
Address `clippy::cast_lossless` and `clippy::unreadable_literal` lints
lopopolo May 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/ascii.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,12 @@ fn first_non_ascii_byte_mask(mask: usize) -> usize {

/// Increment the given pointer by the given amount.
unsafe fn ptr_add(ptr: *const u8, amt: usize) -> *const u8 {
debug_assert!(amt < ::core::isize::MAX as usize);
ptr.offset(amt as isize)
ptr.add(amt)
}

/// Decrement the given pointer by the given amount.
unsafe fn ptr_sub(ptr: *const u8, amt: usize) -> *const u8 {
debug_assert!(amt < ::core::isize::MAX as usize);
ptr.offset((amt as isize).wrapping_neg())
ptr.sub(amt)
}
Comment on lines 235 to 242
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these two functions can probably be nuked and the routines from core slotted into place where they are used.

I believe these two functions on the pointer primitive were added during the great pointer reform of std.


#[cfg(any(test, miri, not(target_arch = "x86_64")))]
Expand Down
9 changes: 3 additions & 6 deletions src/bstr.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use core::mem;

#[cfg(feature = "alloc")]
use alloc::boxed::Box;

Expand Down Expand Up @@ -29,7 +27,6 @@ use alloc::boxed::Box;
/// The `Display` implementation behaves as if `BStr` were first lossily
/// converted to a `str`. Invalid UTF-8 bytes are substituted with the Unicode
/// replacement codepoint, which looks like this: �.
#[derive(Hash)]
#[repr(transparent)]
pub struct BStr {
pub(crate) bytes: [u8],
Expand Down Expand Up @@ -60,7 +57,7 @@ impl BStr {
/// assert_eq!(a, c);
/// ```
#[inline]
pub fn new<'a, B: ?Sized + AsRef<[u8]>>(bytes: &'a B) -> &'a BStr {
pub fn new<B: ?Sized + AsRef<[u8]>>(bytes: &B) -> &BStr {
BStr::from_bytes(bytes.as_ref())
}

Expand All @@ -73,12 +70,12 @@ impl BStr {

#[inline]
pub(crate) fn from_bytes(slice: &[u8]) -> &BStr {
unsafe { mem::transmute(slice) }
unsafe { &*(slice as *const [u8] as *const BStr) }
}

#[inline]
pub(crate) fn from_bytes_mut(slice: &mut [u8]) -> &mut BStr {
unsafe { mem::transmute(slice) }
unsafe { &mut *(slice as *mut [u8] as *mut BStr) }
Comment on lines -76 to +78
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getting rid of the transmute seems like a win here. This cast soup could probably use a SAFETY comment that refers to the fact that BStr is repr(transparent) wrapper around [u8].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a clippy restriction lint that can be used to enforce that all unsafe blocks have safety comments: https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks

}

#[inline]
Expand Down
2 changes: 1 addition & 1 deletion src/bstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use crate::bstr::BStr;
/// A `BString` has the same representation as a `Vec<u8>` and a `String`.
/// That is, it is made up of three word sized components: a pointer to a
/// region of memory containing the bytes, a length and a capacity.
#[derive(Clone, Hash)]
#[derive(Clone)]
pub struct BString {
bytes: Vec<u8>,
}
Expand Down
8 changes: 4 additions & 4 deletions src/byteset/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn build_table(byteset: &[u8]) -> [u8; 256] {
#[inline]
pub(crate) fn find(haystack: &[u8], byteset: &[u8]) -> Option<usize> {
match byteset.len() {
0 => return None,
0 => None,
1 => memchr(byteset[0], haystack),
2 => memchr2(byteset[0], byteset[1], haystack),
3 => memchr3(byteset[0], byteset[1], byteset[2], haystack),
Expand All @@ -28,7 +28,7 @@ pub(crate) fn find(haystack: &[u8], byteset: &[u8]) -> Option<usize> {
#[inline]
pub(crate) fn rfind(haystack: &[u8], byteset: &[u8]) -> Option<usize> {
match byteset.len() {
0 => return None,
0 => None,
1 => memrchr(byteset[0], haystack),
2 => memrchr2(byteset[0], byteset[1], haystack),
3 => memrchr3(byteset[0], byteset[1], byteset[2], haystack),
Expand All @@ -45,7 +45,7 @@ pub(crate) fn find_not(haystack: &[u8], byteset: &[u8]) -> Option<usize> {
return None;
}
match byteset.len() {
0 => return Some(0),
0 => Some(0),
1 => scalar::inv_memchr(byteset[0], haystack),
2 => scalar::forward_search_bytes(haystack, |b| {
b != byteset[0] && b != byteset[1]
Expand All @@ -65,7 +65,7 @@ pub(crate) fn rfind_not(haystack: &[u8], byteset: &[u8]) -> Option<usize> {
return None;
}
match byteset.len() {
0 => return Some(haystack.len() - 1),
0 => Some(haystack.len() - 1),
1 => scalar::inv_memrchr(byteset[0], haystack),
2 => scalar::reverse_search_bytes(haystack, |b| {
b != byteset[0] && b != byteset[1]
Expand Down
2 changes: 1 addition & 1 deletion src/byteset/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub fn inv_memrchr(n1: u8, haystack: &[u8]) -> Option<usize> {
debug_assert_eq!(0, (ptr as usize) % USIZE_BYTES);

let a = *(ptr.sub(2 * USIZE_BYTES) as *const usize);
let b = *(ptr.sub(1 * USIZE_BYTES) as *const usize);
let b = *(ptr.sub(USIZE_BYTES) as *const usize);
let eqa = (a ^ vn1) != 0;
let eqb = (b ^ vn1) != 0;
if eqa || eqb {
Expand Down
16 changes: 8 additions & 8 deletions src/ext_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ use crate::{
/// string literals. This can be quite convenient!
#[allow(non_snake_case)]
#[inline]
pub fn B<'a, B: ?Sized + AsRef<[u8]>>(bytes: &'a B) -> &'a [u8] {
pub fn B<B: ?Sized + AsRef<[u8]>>(bytes: &B) -> &[u8] {
bytes.as_ref()
}

Expand Down Expand Up @@ -3045,7 +3045,7 @@ pub trait ByteSlice: private::Sealed {
#[inline]
fn last_byte(&self) -> Option<u8> {
let bytes = self.as_bytes();
bytes.get(bytes.len().saturating_sub(1)).map(|&b| b)
bytes.last().copied()
}

/// Returns the index of the first non-ASCII byte in this byte string (if
Expand Down Expand Up @@ -3331,7 +3331,7 @@ impl<'a> Iterator for Bytes<'a> {

#[inline]
fn next(&mut self) -> Option<u8> {
self.it.next().map(|&b| b)
self.it.next().copied()
}

#[inline]
Expand All @@ -3343,7 +3343,7 @@ impl<'a> Iterator for Bytes<'a> {
impl<'a> DoubleEndedIterator for Bytes<'a> {
#[inline]
fn next_back(&mut self) -> Option<u8> {
self.it.next_back().map(|&b| b)
self.it.next_back().copied()
}
}

Expand Down Expand Up @@ -3374,7 +3374,7 @@ pub struct Fields<'a> {
#[cfg(feature = "unicode")]
impl<'a> Fields<'a> {
fn new(bytes: &'a [u8]) -> Fields<'a> {
Fields { it: bytes.fields_with(|ch| ch.is_whitespace()) }
Fields { it: bytes.fields_with(char::is_whitespace) }
}
}

Expand Down Expand Up @@ -3428,7 +3428,7 @@ impl<'a, F: FnMut(char) -> bool> Iterator for FieldsWith<'a, F> {
}
}
}
while let Some((_, e, ch)) = self.chars.next() {
for (_, e, ch) in self.chars.by_ref() {
if (self.f)(ch) {
break;
}
Expand Down Expand Up @@ -3744,7 +3744,7 @@ impl<'a> Iterator for LinesWithTerminator<'a> {
Some(line)
}
Some(end) => {
let line = &self.bytes[..end + 1];
let line = &self.bytes[..=end];
self.bytes = &self.bytes[end + 1..];
Comment on lines -3747 to 3748
Copy link
Contributor Author

@lopopolo lopopolo May 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably replace this suggestion and the code with a call to self.bytes.split_at(end). This would eliminate one bounds check and one panicking branch.

Some(line)
}
Expand All @@ -3764,7 +3764,7 @@ impl<'a> DoubleEndedIterator for LinesWithTerminator<'a> {
}
Some(end) => {
let line = &self.bytes[end + 1..];
self.bytes = &self.bytes[..end + 1];
self.bytes = &self.bytes[..=end];
Comment on lines 3766 to +3767
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Some(line)
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/ext_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ pub trait ByteVec: private::Sealed {
fn imp(os_str: OsString) -> Result<Vec<u8>, OsString> {
use std::os::unix::ffi::OsStringExt;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code could be made to support wasi targets as well, which has a similar OsStringExt which is identical to the unix one.


Ok(Vec::from(os_str.into_vec()))
Ok(os_str.into_vec())
}

#[cfg(not(unix))]
Expand Down Expand Up @@ -223,18 +223,18 @@ pub trait ByteVec: private::Sealed {
/// ```
#[inline]
#[cfg(feature = "std")]
fn from_os_str_lossy<'a>(os_str: &'a OsStr) -> Cow<'a, [u8]> {
fn from_os_str_lossy(os_str: &OsStr) -> Cow<'_, [u8]> {
#[cfg(unix)]
#[inline]
fn imp<'a>(os_str: &'a OsStr) -> Cow<'a, [u8]> {
fn imp(os_str: &OsStr) -> Cow<'_, [u8]> {
use std::os::unix::ffi::OsStrExt;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto, wasi support could be added.


Cow::Borrowed(os_str.as_bytes())
}

#[cfg(not(unix))]
#[inline]
fn imp<'a>(os_str: &'a OsStr) -> Cow<'a, [u8]> {
fn imp(os_str: &OsStr) -> Cow<'_, [u8]> {
match os_str.to_string_lossy() {
Cow::Borrowed(x) => Cow::Borrowed(x.as_bytes()),
Cow::Owned(x) => Cow::Owned(Vec::from(x)),
Expand Down Expand Up @@ -292,7 +292,7 @@ pub trait ByteVec: private::Sealed {
/// ```
#[inline]
#[cfg(feature = "std")]
fn from_path_lossy<'a>(path: &'a Path) -> Cow<'a, [u8]> {
fn from_path_lossy(path: &Path) -> Cow<'_, [u8]> {
Vec::from_os_str_lossy(path.as_os_str())
}

Expand Down Expand Up @@ -961,7 +961,7 @@ pub trait ByteVec: private::Sealed {
R: ops::RangeBounds<usize>,
B: AsRef<[u8]>,
{
self.as_vec_mut().splice(range, replace_with.as_ref().iter().cloned());
self.as_vec_mut().splice(range, replace_with.as_ref().iter().copied());
}

/// Creates a draining iterator that removes the specified range in this
Expand Down
25 changes: 21 additions & 4 deletions src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ macro_rules! impl_partial_ord {
#[cfg(feature = "alloc")]
mod bstring {
use core::{
cmp::Ordering, convert::TryFrom, fmt, iter::FromIterator, ops,
cmp::Ordering, convert::TryFrom, fmt, hash, iter::FromIterator, ops,
};

use alloc::{
Expand Down Expand Up @@ -342,7 +342,7 @@ mod bstring {
impl PartialEq for BString {
#[inline]
fn eq(&self, other: &BString) -> bool {
&self[..] == &other[..]
self[..] == other[..]
}
}

Expand All @@ -355,6 +355,13 @@ mod bstring {
impl_partial_eq!(BString, BStr);
impl_partial_eq!(BString, &'a BStr);

impl hash::Hash for BString {
#[inline]
fn hash<H: hash::Hasher>(&self, state: &mut H) {
self.as_bytes().hash(state);
}
}

impl PartialOrd for BString {
#[inline]
fn partial_cmp(&self, other: &BString) -> Option<Ordering> {
Expand Down Expand Up @@ -384,7 +391,7 @@ mod bstr {
borrow::{Borrow, BorrowMut},
cmp::Ordering,
convert::TryFrom,
fmt, ops,
fmt, hash, ops,
};

#[cfg(feature = "alloc")]
Expand Down Expand Up @@ -479,7 +486,10 @@ mod bstr {
| '\x7f' => {
write!(f, "\\x{:02x}", ch as u32)?;
}
'\n' | '\r' | '\t' | _ => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with this change, I'd probably want to add a comment on why newline, carriage return, and tab are called out specially here to get their own match arm.

'\n' | '\r' | '\t' => {
write!(f, "{}", ch.escape_debug())?;
}
_ => {
write!(f, "{}", ch.escape_debug())?;
}
}
Expand Down Expand Up @@ -814,6 +824,13 @@ mod bstr {
#[cfg(feature = "alloc")]
impl_partial_eq_cow!(&'a BStr, Cow<'a, [u8]>);

impl hash::Hash for BStr {
#[inline]
fn hash<H: hash::Hasher>(&self, state: &mut H) {
self.as_bytes().hash(state);
}
}

impl PartialOrd for BStr {
#[inline]
fn partial_cmp(&self, other: &BStr) -> Option<Ordering> {
Expand Down
8 changes: 4 additions & 4 deletions src/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ pub trait BufReadExt: io::BufRead {
F: FnMut(&[u8]) -> io::Result<bool>,
{
self.for_byte_line_with_terminator(|line| {
for_each_line(&trim_line_slice(&line))
for_each_line(trim_line_slice(line))
})
}

Expand Down Expand Up @@ -193,7 +193,7 @@ pub trait BufReadExt: io::BufRead {
F: FnMut(&[u8]) -> io::Result<bool>,
{
self.for_byte_record_with_terminator(terminator, |chunk| {
for_each_record(&trim_record_slice(&chunk, terminator))
for_each_record(trim_record_slice(chunk, terminator))
})
}

Expand Down Expand Up @@ -306,7 +306,7 @@ pub trait BufReadExt: io::BufRead {
let (record, rest) = buf.split_at(index + 1);
buf = rest;
consumed += record.len();
match for_each_record(&record) {
match for_each_record(record) {
Ok(false) => break 'outer,
Err(err) => {
res = Err(err);
Expand All @@ -319,7 +319,7 @@ pub trait BufReadExt: io::BufRead {
// Copy the final record fragment to our local buffer. This
// saves read_until() from re-scanning a buffer we know
// contains no remaining terminators.
bytes.extend_from_slice(&buf);
bytes.extend_from_slice(buf);
consumed += buf.len();
}

Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::single_char_add_str)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are several places in the crate where the replacement character is used in a single character string. I prefer suppressing this because it means you don't pay the cost for a UTF-8 encode at runtime.


/*!
A byte string library.

Expand Down
2 changes: 2 additions & 0 deletions src/unicode/fsm/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::all)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suppressed all lints because these modules appear to be generated code. clippy didn't like the use of the 'static lifetime in const definitions.


pub mod grapheme_break_fwd;
pub mod grapheme_break_rev;
pub mod regional_indicator_rev;
Expand Down
7 changes: 3 additions & 4 deletions src/unicode/grapheme.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ pub fn decode_grapheme(bs: &[u8]) -> (&str, usize) {
let grapheme = unsafe { bs[..end].to_str_unchecked() };
(grapheme, grapheme.len())
} else {
const INVALID: &'static str = "\u{FFFD}";
const INVALID: &str = "\u{FFFD}";
// No match on non-empty bytes implies we found invalid UTF-8.
let (_, size) = utf8::decode_lossy(bs);
(INVALID, size)
Expand All @@ -232,7 +232,7 @@ fn decode_last_grapheme(bs: &[u8]) -> (&str, usize) {
let grapheme = unsafe { bs[start..].to_str_unchecked() };
(grapheme, grapheme.len())
} else {
const INVALID: &'static str = "\u{FFFD}";
const INVALID: &str = "\u{FFFD}";
// No match on non-empty bytes implies we found invalid UTF-8.
let (_, size) = utf8::decode_last_lossy(bs);
(INVALID, size)
Expand Down Expand Up @@ -365,8 +365,7 @@ mod tests {
/// Return all of the UCD for grapheme breaks.
#[cfg(not(miri))]
fn ucdtests() -> Vec<GraphemeClusterBreakTest> {
const TESTDATA: &'static str =
include_str!("data/GraphemeBreakTest.txt");
const TESTDATA: &str = include_str!("data/GraphemeBreakTest.txt");

let mut tests = vec![];
for mut line in TESTDATA.lines() {
Expand Down
5 changes: 2 additions & 3 deletions src/unicode/sentence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ fn decode_sentence(bs: &[u8]) -> (&str, usize) {
let sentence = unsafe { bs[..end].to_str_unchecked() };
(sentence, sentence.len())
} else {
const INVALID: &'static str = "\u{FFFD}";
const INVALID: &str = "\u{FFFD}";
// No match on non-empty bytes implies we found invalid UTF-8.
let (_, size) = utf8::decode_lossy(bs);
(INVALID, size)
Expand Down Expand Up @@ -209,8 +209,7 @@ mod tests {
/// Return all of the UCD for sentence breaks.
#[cfg(not(miri))]
fn ucdtests() -> Vec<SentenceBreakTest> {
const TESTDATA: &'static str =
include_str!("data/SentenceBreakTest.txt");
const TESTDATA: &str = include_str!("data/SentenceBreakTest.txt");

let mut tests = vec![];
for mut line in TESTDATA.lines() {
Expand Down
Loading