Skip to content

Commit

Permalink
Rework new slice_as_bytes lint
Browse files Browse the repository at this point in the history
  • Loading branch information
A-Walrus committed Jun 19, 2023
1 parent 06b444b commit 5dd77b2
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5171,6 +5171,7 @@ Released 2018-09-13
[`size_of_in_element_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count
[`size_of_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_ref
[`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
[`slice_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#slice_as_bytes
[`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
[`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
[`std_instead_of_alloc`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_alloc
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::SINGLE_CHAR_ADD_STR_INFO,
crate::methods::SINGLE_CHAR_PATTERN_INFO,
crate::methods::SKIP_WHILE_NEXT_INFO,
crate::methods::SLICE_AS_BYTES_INFO,
crate::methods::STABLE_SORT_PRIMITIVE_INFO,
crate::methods::STRING_EXTEND_CHARS_INFO,
crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO,
Expand Down
28 changes: 27 additions & 1 deletion clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ mod single_char_insert_string;
mod single_char_pattern;
mod single_char_push_string;
mod skip_while_next;
mod slice_as_bytes;
mod stable_sort_primitive;
mod str_splitn;
mod string_extend_chars;
Expand Down Expand Up @@ -3284,6 +3285,27 @@ declare_clippy_lint! {
"calling `.drain(..).collect()` to move all elements into a new collection"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for string slices immediantly followed by `as_bytes`.
/// ### Why is this bad?
/// It involves doing an unnecessary UTF-8 alignment check which is less efficient, and can cause a panic.
/// ### Example
/// ```rust
/// let s = "Lorem ipsum";
/// s[1..5].as_bytes();
/// ```
/// Use instead:
/// ```rust
/// let s = "Lorem ipsum";
/// &s.as_bytes()[1..5];
/// ```
#[clippy::version = "1.72.0"]
pub SLICE_AS_BYTES,
perf,
"slicing a string and immediately calling as_bytes is less efficient and can lead to panics"
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
Expand Down Expand Up @@ -3414,7 +3436,8 @@ impl_lint_pass!(Methods => [
CLEAR_WITH_DRAIN,
MANUAL_NEXT_BACK,
UNNECESSARY_LITERAL_UNWRAP,
DRAIN_COLLECT
DRAIN_COLLECT,
SLICE_AS_BYTES,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -3623,6 +3646,9 @@ impl Methods {
("arg", [arg]) => {
suspicious_command_arg_space::check(cx, recv, arg, span);
}
("as_bytes",[]) => {
slice_as_bytes::check(cx, expr, recv);
}
("as_deref" | "as_deref_mut", []) => {
needless_option_as_deref::check(cx, expr, recv, name);
},
Expand Down
39 changes: 39 additions & 0 deletions clippy_lints/src/methods/slice_as_bytes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet_with_applicability, ty::is_type_lang_item};
use rustc_errors::Applicability;
use rustc_hir::{
Expr, ExprKind,
LangItem::{self, Range, RangeFrom, RangeFull, RangeTo, RangeToInclusive},
QPath,
};
use rustc_lint::LateContext;

use super::SLICE_AS_BYTES;

pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>) {
if let ExprKind::Index(indexed, index) = recv.kind {
if let ExprKind::Struct(QPath::LangItem(Range | RangeFrom | RangeTo | RangeToInclusive | RangeFull, ..), ..) =
index.kind
{
let ty = cx.typeck_results().expr_ty(indexed).peel_refs();
let is_str = ty.is_str();
let is_string = is_type_lang_item(cx, ty, LangItem::String);
if is_str || is_string {
let mut applicability = Applicability::MachineApplicable;
let stringish = snippet_with_applicability(cx, indexed.span, "..", &mut applicability);
let range = snippet_with_applicability(cx, index.span, "..", &mut applicability);
let type_name = if is_str { "str" } else { "String" };
span_lint_and_sugg(
cx,
SLICE_AS_BYTES,
expr.span,
&(format!(
"slicing a {type_name} before calling `as_bytes` results in needless UTF-8 alignment checks, and has the possiblity of panicking"
)),
"try",
format!("&{stringish}.as_bytes()[{range}]"),
applicability,
);
}
}
}
}
35 changes: 35 additions & 0 deletions tests/ui/slice_as_bytes.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//@run-rustfix
#![allow(unused)]
#![warn(clippy::slice_as_bytes)]

use std::ops::{Index, Range};

struct Foo;

struct Bar;

impl Bar {
fn as_bytes(&self) -> &[u8] {
&[0, 1, 2, 3]
}
}

impl Index<Range<usize>> for Foo {
type Output = Bar;

fn index(&self, _: Range<usize>) -> &Self::Output {
&Bar
}
}

fn main() {
let s = "Lorem ipsum";
let string: String = "dolor sit amet".to_owned();

let bytes = &s.as_bytes()[1..5];
let bytes = &string.as_bytes()[1..];
let bytes = &"consectetur adipiscing".as_bytes()[..=5];

let f = Foo;
let bytes = f[0..4].as_bytes();
}
35 changes: 35 additions & 0 deletions tests/ui/slice_as_bytes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//@run-rustfix
#![allow(unused)]
#![warn(clippy::slice_as_bytes)]

use std::ops::{Index, Range};

struct Foo;

struct Bar;

impl Bar {
fn as_bytes(&self) -> &[u8] {
&[0, 1, 2, 3]
}
}

impl Index<Range<usize>> for Foo {
type Output = Bar;

fn index(&self, _: Range<usize>) -> &Self::Output {
&Bar
}
}

fn main() {
let s = "Lorem ipsum";
let string: String = "dolor sit amet".to_owned();

let bytes = s[1..5].as_bytes();
let bytes = string[1..].as_bytes();
let bytes = "consectetur adipiscing"[..=5].as_bytes();

let f = Foo;
let bytes = f[0..4].as_bytes();
}
22 changes: 22 additions & 0 deletions tests/ui/slice_as_bytes.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error: slicing a str before calling `as_bytes` results in needless UTF-8 alignment checks, and has the possiblity of panicking
--> $DIR/slice_as_bytes.rs:29:17
|
LL | let bytes = s[1..5].as_bytes();
| ^^^^^^^^^^^^^^^^^^ help: try: `&s.as_bytes()[1..5]`
|
= note: `-D clippy::slice-as-bytes` implied by `-D warnings`

error: slicing a String before calling `as_bytes` results in needless UTF-8 alignment checks, and has the possiblity of panicking
--> $DIR/slice_as_bytes.rs:30:17
|
LL | let bytes = string[1..].as_bytes();
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&string.as_bytes()[1..]`

error: slicing a str before calling `as_bytes` results in needless UTF-8 alignment checks, and has the possiblity of panicking
--> $DIR/slice_as_bytes.rs:31:17
|
LL | let bytes = "consectetur adipiscing"[..=5].as_bytes();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&"consectetur adipiscing".as_bytes()[..=5]`

error: aborting due to 3 previous errors

0 comments on commit 5dd77b2

Please sign in to comment.