Skip to content

Commit 7ba1891

Browse files
committed
Added some clippy restriction lints.
Changed most indexing to `get_unchecked` because of lint `clippy::indexing_slicing`. This should also improve performance, as the compiler doesn't need to emmit panic code. `clippy::option_if_let_else` is now allowed throughout the whole crate, as it doesn't add much.
1 parent d0b34ad commit 7ba1891

File tree

4 files changed

+89
-34
lines changed

4 files changed

+89
-34
lines changed

src/ascii_char.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,10 @@ impl AsciiChar {
353353
h, i, j, k, l, m, n, o,
354354
p, q, r, s, t, u, v, w,
355355
x, y, z, CurlyBraceOpen, VerticalBar, CurlyBraceClose, Tilde, DEL,
356-
];
356+
];
357+
358+
// We want to slice here and detect `const_err` from rustc if the slice is invalid
359+
#[allow(clippy::indexing_slicing)]
357360
ALL[ch as usize]
358361
}
359362

@@ -688,6 +691,7 @@ impl AsciiChar {
688691
/// ```
689692
#[inline]
690693
#[must_use]
694+
#[allow(clippy::indexing_slicing)] // We're sure it'll either access one or the other, as `bool` is either `0` or `1`
691695
pub const fn to_ascii_uppercase(&self) -> Self {
692696
[*self, AsciiChar::new((*self as u8 & 0b101_1111) as char)][self.is_lowercase() as usize]
693697
}
@@ -705,6 +709,7 @@ impl AsciiChar {
705709
/// ```
706710
#[inline]
707711
#[must_use]
712+
#[allow(clippy::indexing_slicing)] // We're sure it'll either access one or the other, as `bool` is either `0` or `1`
708713
pub const fn to_ascii_lowercase(&self) -> Self {
709714
[*self, AsciiChar::new(self.to_not_upper() as char)][self.is_uppercase() as usize]
710715
}

src/ascii_str.rs

Lines changed: 71 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,13 @@ impl AsciiStr {
216216
/// ```
217217
#[must_use]
218218
pub fn trim_start(&self) -> &Self {
219-
&self[self
219+
let whitespace_len = self
220220
.chars()
221-
.take_while(|ascii| ascii.is_whitespace())
222-
.count()..]
221+
.position(|ascii| !ascii.is_whitespace())
222+
.unwrap_or_else(|| self.len());
223+
224+
// SAFETY: `whitespace_len` is `0..=len`, which is at most `len`, which is a valid empty slice.
225+
unsafe { self.as_slice().get_unchecked(whitespace_len..).into() }
223226
}
224227

225228
/// Returns an ASCII string slice with trailing whitespace removed.
@@ -232,12 +235,19 @@ impl AsciiStr {
232235
/// ```
233236
#[must_use]
234237
pub fn trim_end(&self) -> &Self {
235-
let trimmed = self
238+
// Number of whitespace characters counting from the end
239+
let whitespace_len = self
236240
.chars()
237241
.rev()
238-
.take_while(|ch| ch.is_whitespace())
239-
.count();
240-
&self[..self.len() - trimmed]
242+
.position(|ascii| !ascii.is_whitespace())
243+
.unwrap_or_else(|| self.len());
244+
245+
// SAFETY: `whitespace_len` is `0..=len`, which is at most `len`, which is a valid empty slice, and at least `0`, which is the whole slice.
246+
unsafe {
247+
self.as_slice()
248+
.get_unchecked(..self.len() - whitespace_len)
249+
.into()
250+
}
241251
}
242252

243253
/// Compares two strings case-insensitively.
@@ -470,6 +480,7 @@ impl fmt::Debug for AsciiStr {
470480

471481
macro_rules! impl_index {
472482
($idx:ty) => {
483+
#[allow(clippy::indexing_slicing)] // In `Index`, if it's out of bounds, panic is the default
473484
impl Index<$idx> for AsciiStr {
474485
type Output = AsciiStr;
475486

@@ -479,6 +490,7 @@ macro_rules! impl_index {
479490
}
480491
}
481492

493+
#[allow(clippy::indexing_slicing)] // In `IndexMut`, if it's out of bounds, panic is the default
482494
impl IndexMut<$idx> for AsciiStr {
483495
#[inline]
484496
fn index_mut(&mut self, index: $idx) -> &mut AsciiStr {
@@ -495,6 +507,7 @@ impl_index! { RangeFull }
495507
impl_index! { RangeInclusive<usize> }
496508
impl_index! { RangeToInclusive<usize> }
497509

510+
#[allow(clippy::indexing_slicing)] // In `Index`, if it's out of bounds, panic is the default
498511
impl Index<usize> for AsciiStr {
499512
type Output = AsciiChar;
500513

@@ -504,6 +517,7 @@ impl Index<usize> for AsciiStr {
504517
}
505518
}
506519

520+
#[allow(clippy::indexing_slicing)] // In `IndexMut`, if it's out of bounds, panic is the default
507521
impl IndexMut<usize> for AsciiStr {
508522
#[inline]
509523
fn index_mut(&mut self, index: usize) -> &mut AsciiChar {
@@ -636,13 +650,14 @@ struct Split<'a> {
636650
impl<'a> Iterator for Split<'a> {
637651
type Item = &'a AsciiStr;
638652

639-
#[allow(clippy::option_if_let_else)] // There are side effects with `else` so we don't use iterator adaptors
640653
fn next(&mut self) -> Option<&'a AsciiStr> {
641654
if !self.ended {
642655
let start: &AsciiStr = self.chars.as_str();
643656
let split_on = self.on;
644-
if let Some(at) = self.chars.position(|ch| ch == split_on) {
645-
Some(&start[..at])
657+
658+
if let Some(at) = self.chars.position(|ascii| ascii == split_on) {
659+
// SAFETY: `at` is guaranteed to be in bounds, as `position` returns `Ok(0..len)`.
660+
Some(unsafe { start.as_slice().get_unchecked(..at).into() })
646661
} else {
647662
self.ended = true;
648663
Some(start)
@@ -653,13 +668,14 @@ impl<'a> Iterator for Split<'a> {
653668
}
654669
}
655670
impl<'a> DoubleEndedIterator for Split<'a> {
656-
#[allow(clippy::option_if_let_else)] // There are side effects with `else` so we don't use iterator adaptors
657671
fn next_back(&mut self) -> Option<&'a AsciiStr> {
658672
if !self.ended {
659673
let start: &AsciiStr = self.chars.as_str();
660674
let split_on = self.on;
661-
if let Some(at) = self.chars.rposition(|ch| ch == split_on) {
662-
Some(&start[at + 1..])
675+
676+
if let Some(at) = self.chars.rposition(|ascii| ascii == split_on) {
677+
// SAFETY: `at` is guaranteed to be in bounds, as `rposition` returns `Ok(0..len)`, and slices `1..`, `2..`, etc... until `len..` inclusive, are valid.
678+
Some(unsafe { start.as_slice().get_unchecked(at + 1..).into() })
663679
} else {
664680
self.ended = true;
665681
Some(start)
@@ -684,40 +700,65 @@ impl<'a> Iterator for Lines<'a> {
684700
.chars()
685701
.position(|chr| chr == AsciiChar::LineFeed)
686702
{
687-
let line = if idx > 0 && self.string[idx - 1] == AsciiChar::CarriageReturn {
688-
&self.string[..idx - 1]
703+
// SAFETY: `idx` is guaranteed to be `1..len`, as we get it from `position` as `0..len` and make sure it's not `0`.
704+
let line = if idx > 0
705+
&& *unsafe { self.string.as_slice().get_unchecked(idx - 1) }
706+
== AsciiChar::CarriageReturn
707+
{
708+
// SAFETY: As per above, `idx` is guaranteed to be `1..len`
709+
unsafe { self.string.as_slice().get_unchecked(..idx - 1).into() }
689710
} else {
690-
&self.string[..idx]
711+
// SAFETY: As per above, `idx` is guaranteed to be `0..len`
712+
unsafe { self.string.as_slice().get_unchecked(..idx).into() }
691713
};
692-
self.string = &self.string[idx + 1..];
714+
// SAFETY: As per above, `idx` is guaranteed to be `0..len`, so at the extreme, slicing `len..` is a valid empty slice.
715+
self.string = unsafe { self.string.as_slice().get_unchecked(idx + 1..).into() };
693716
Some(line)
694717
} else if self.string.is_empty() {
695718
None
696719
} else {
697720
let line = self.string;
698-
self.string = &self.string[..0];
721+
// SAFETY: Slicing `..0` is always valid and yields an empty slice
722+
self.string = unsafe { self.string.as_slice().get_unchecked(..0).into() };
699723
Some(line)
700724
}
701725
}
702726
}
727+
703728
impl<'a> DoubleEndedIterator for Lines<'a> {
704729
fn next_back(&mut self) -> Option<&'a AsciiStr> {
705730
if self.string.is_empty() {
706731
return None;
707732
}
708-
let mut i = self.string.len();
709-
if self.string[i - 1] == AsciiChar::LineFeed {
710-
i -= 1;
711-
if i > 0 && self.string[i - 1] == AsciiChar::CarriageReturn {
712-
i -= 1;
713-
}
714-
}
715-
self.string = &self.string[..i];
716-
while i > 0 && self.string[i - 1] != AsciiChar::LineFeed {
717-
i -= 1;
733+
734+
// If we end with `LF` / `CR/LF`, remove them
735+
if let [slice @ .., AsciiChar::CarriageReturn, AsciiChar::LineFeed]
736+
| [slice @ .., AsciiChar::LineFeed] = self.string.as_slice()
737+
{
738+
self.string = slice.into();
718739
}
719-
let line = &self.string[i..];
720-
self.string = &self.string[..i];
740+
741+
// SAFETY: This will never be `0`, as we remove any `LF` from the end, it is `1..len`
742+
let lf_rev_pos = self
743+
.string
744+
.chars()
745+
.rev()
746+
.position(|ascii| ascii == AsciiChar::LineFeed)
747+
.unwrap_or_else(|| self.string.len());
748+
749+
// SAFETY: As per above, `self.len() - lf_rev_pos` will be in range `0..len - 1`, so both indexes are correct.
750+
let line = unsafe {
751+
self.string
752+
.as_slice()
753+
.get_unchecked(self.string.len() - lf_rev_pos..)
754+
.into()
755+
};
756+
self.string = unsafe {
757+
self.string
758+
.as_slice()
759+
.get_unchecked(..self.string.len() - lf_rev_pos)
760+
.into()
761+
};
721762
Some(line)
722763
}
723764
}
@@ -975,7 +1016,6 @@ impl AsMutAsciiStr for [AsciiChar] {
9751016
impl AsAsciiStr for [u8] {
9761017
type Inner = u8;
9771018

978-
#[allow(clippy::option_if_let_else)] // `if` needs to use past results, so it's more expressive like this
9791019
fn slice_ascii<R>(&self, range: R) -> Result<&AsciiStr, AsAsciiStrError>
9801020
where
9811021
R: SliceIndex<[u8], Output = [u8]>,
@@ -1006,7 +1046,6 @@ impl AsAsciiStr for [u8] {
10061046
}
10071047
}
10081048
impl AsMutAsciiStr for [u8] {
1009-
#[allow(clippy::option_if_let_else)] // `if` needs to use past results, so it's more expressive like this
10101049
fn slice_ascii_mut<R>(&mut self, range: R) -> Result<&mut AsciiStr, AsAsciiStrError>
10111050
where
10121051
R: SliceIndex<[u8], Output = [u8]>,

src/ascii_string.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,7 @@ impl<'a> AddAssign<&'a AsciiStr> for AsciiString {
620620
}
621621
}
622622

623+
#[allow(clippy::indexing_slicing)] // In `Index`, if it's out of bounds, panic is the default
623624
impl<T> Index<T> for AsciiString
624625
where
625626
AsciiStr: Index<T>,
@@ -632,6 +633,7 @@ where
632633
}
633634
}
634635

636+
#[allow(clippy::indexing_slicing)] // In `IndexMut`, if it's out of bounds, panic is the default
635637
impl<T> IndexMut<T> for AsciiString
636638
where
637639
AsciiStr: IndexMut<T>,

src/lib.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,12 @@
3030
3131
#![cfg_attr(not(feature = "std"), no_std)]
3232
// Clippy lints
33-
#![warn(clippy::pedantic)]
33+
#![warn(
34+
clippy::pedantic,
35+
clippy::decimal_literal_representation,
36+
clippy::get_unwrap,
37+
clippy::indexing_slicing
38+
)]
3439
// Naming conventions sometimes go against this lint
3540
#![allow(clippy::module_name_repetitions)]
3641
// We need to get literal non-asciis for tests
@@ -40,6 +45,10 @@
4045
// Shadowing is common and doesn't affect understanding
4146
// TODO: Consider removing `shadow_unrelated`, as it can show some actual logic errors
4247
#![allow(clippy::shadow_unrelated, clippy::shadow_reuse, clippy::shadow_same)]
48+
// A `if let` / `else` sometimes looks better than using iterator adaptors
49+
#![allow(clippy::option_if_let_else)]
50+
// In tests, we're fine with indexing, since a panic is a failure.
51+
#![cfg_attr(test, allow(clippy::indexing_slicing))]
4352
// for compatibility with methods on char and u8
4453
#![allow(clippy::trivially_copy_pass_by_ref)]
4554
// In preparation for feature `unsafe_block_in_unsafe_fn` (https://github.com/rust-lang/rust/issues/71668)

0 commit comments

Comments
 (0)