Skip to content

Commit cd61be7

Browse files
authored
Fix sliced_string_as_bytes FP with a RangeFull (#15873)
changelog: [`sliced_string_as_bytes`]: don't fire on `str[..].as_bytes()` So I ran into this in some codebase I was working on, where the lint fired on this line: ```rust let string: &str; string[..].as_bytes() ``` So I was trying to understand the rationale behind this lint, and it says: > It involves doing an unnecessary UTF-8 alignment check which is less efficient, and can cause a panic. This is obviously not true in the case where a `RangeFull` slice is being taken, since there is no UTF-8 boundary check, and no panic can be caused. So I created an exemption for `RangeFull`s. Two other notes: 1. I'm not sure the word "alignment" in the lint's description (quoted above) is really correct, should probably say "char boundary" instead? 2. I might be missing something, but isn't there a lint for doing superfluous slice indexing, and then calling a slice method? e.g. `str[..].len()` or `slice[..].len()`, where `str` and `slice` are `&str` and `&[T]`, respectively. If we had one, I would expect *it* to fire for the example code I quoted above.
2 parents 62589a2 + 386451c commit cd61be7

File tree

3 files changed

+20
-2
lines changed

3 files changed

+20
-2
lines changed

clippy_lints/src/methods/sliced_string_as_bytes.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::higher;
23
use clippy_utils::res::MaybeDef;
34
use clippy_utils::source::snippet_with_applicability;
45
use rustc_errors::Applicability;
5-
use rustc_hir::{Expr, ExprKind, LangItem, is_range_literal};
6+
use rustc_hir::{Expr, ExprKind, LangItem};
67
use rustc_lint::LateContext;
78

89
use super::SLICED_STRING_AS_BYTES;
910

11+
/// Checks if `index` is any type of range except `RangeFull` (i.e. `..`)
12+
fn is_bounded_range_literal(cx: &LateContext<'_>, index: &Expr<'_>) -> bool {
13+
higher::Range::hir(cx, index).is_some_and(|range| Option::or(range.start, range.end).is_some())
14+
}
15+
1016
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>) {
1117
if let ExprKind::Index(indexed, index, _) = recv.kind
12-
&& is_range_literal(index)
18+
&& is_bounded_range_literal(cx, index)
1319
&& let ty = cx.typeck_results().expr_ty(indexed).peel_refs()
1420
&& (ty.is_str() || ty.is_lang_item(cx, LangItem::String))
1521
{

tests/ui/sliced_string_as_bytes.fixed

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ fn main() {
3232
let bytes = &"consectetur adipiscing".as_bytes()[..=5];
3333
//~^ sliced_string_as_bytes
3434

35+
// this lint is a perf lint meant to catch utf-8 alignment checks.
36+
// while the slicing here *is* redundant, it's more like a needless borrow, and shouldn't affect
37+
// perf
38+
let bytes = s[..].as_bytes();
39+
let bytes = string[..].as_bytes();
40+
3541
let f = Foo;
3642
let bytes = f[0..4].as_bytes();
3743
}

tests/ui/sliced_string_as_bytes.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ fn main() {
3232
let bytes = "consectetur adipiscing"[..=5].as_bytes();
3333
//~^ sliced_string_as_bytes
3434

35+
// this lint is a perf lint meant to catch utf-8 alignment checks.
36+
// while the slicing here *is* redundant, it's more like a needless borrow, and shouldn't affect
37+
// perf
38+
let bytes = s[..].as_bytes();
39+
let bytes = string[..].as_bytes();
40+
3541
let f = Foo;
3642
let bytes = f[0..4].as_bytes();
3743
}

0 commit comments

Comments
 (0)