From 99480fa976adb7dacb6540a7cef0e2aabdd9f934 Mon Sep 17 00:00:00 2001 From: A-Walrus Date: Mon, 19 Jun 2023 14:42:23 +0300 Subject: [PATCH 1/3] Rework new `slice_as_bytes` lint --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/mod.rs | 25 ++++++++++++++ clippy_lints/src/methods/slice_as_bytes.rs | 39 ++++++++++++++++++++++ tests/ui/slice_as_bytes.fixed | 35 +++++++++++++++++++ tests/ui/slice_as_bytes.rs | 35 +++++++++++++++++++ tests/ui/slice_as_bytes.stderr | 22 ++++++++++++ 7 files changed, 158 insertions(+) create mode 100644 clippy_lints/src/methods/slice_as_bytes.rs create mode 100644 tests/ui/slice_as_bytes.fixed create mode 100644 tests/ui/slice_as_bytes.rs create mode 100644 tests/ui/slice_as_bytes.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 14d822083d8b..e61cd7d1ff3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5205,6 +5205,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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 9d9ee6ba3079..27703c124ea8 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -398,6 +398,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, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 24dbe8c1d751..bb57807a91ce 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -81,6 +81,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; @@ -3316,6 +3317,26 @@ declare_clippy_lint! { "checks for usage of `Iterator::fold` with a type that implements `Try`" } +declare_clippy_lint! { + /// 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, @@ -3448,6 +3469,7 @@ impl_lint_pass!(Methods => [ UNNECESSARY_LITERAL_UNWRAP, DRAIN_COLLECT, MANUAL_TRY_FOLD, + SLICE_AS_BYTES, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -3656,6 +3678,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); }, diff --git a/clippy_lints/src/methods/slice_as_bytes.rs b/clippy_lints/src/methods/slice_as_bytes.rs new file mode 100644 index 000000000000..ca1e31bc4743 --- /dev/null +++ b/clippy_lints/src/methods/slice_as_bytes.rs @@ -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, + ); + } + } + } +} diff --git a/tests/ui/slice_as_bytes.fixed b/tests/ui/slice_as_bytes.fixed new file mode 100644 index 000000000000..22e6ba580e20 --- /dev/null +++ b/tests/ui/slice_as_bytes.fixed @@ -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> for Foo { + type Output = Bar; + + fn index(&self, _: Range) -> &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(); +} diff --git a/tests/ui/slice_as_bytes.rs b/tests/ui/slice_as_bytes.rs new file mode 100644 index 000000000000..2966a01e5ae0 --- /dev/null +++ b/tests/ui/slice_as_bytes.rs @@ -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> for Foo { + type Output = Bar; + + fn index(&self, _: Range) -> &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(); +} diff --git a/tests/ui/slice_as_bytes.stderr b/tests/ui/slice_as_bytes.stderr new file mode 100644 index 000000000000..d0068315d534 --- /dev/null +++ b/tests/ui/slice_as_bytes.stderr @@ -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 + From ea24a25cf24eb560835db6c0520b3f2ff07a2486 Mon Sep 17 00:00:00 2001 From: A-Walrus Date: Mon, 19 Jun 2023 14:54:04 +0300 Subject: [PATCH 2/3] Allow `slice_as_bytes` on bytes_nth --- tests/ui/bytes_nth.fixed | 1 + tests/ui/bytes_nth.rs | 1 + tests/ui/bytes_nth.stderr | 6 +++--- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/ui/bytes_nth.fixed b/tests/ui/bytes_nth.fixed index d3e0f676b29f..19ee7f6f4c89 100644 --- a/tests/ui/bytes_nth.fixed +++ b/tests/ui/bytes_nth.fixed @@ -1,6 +1,7 @@ //@run-rustfix #![allow(clippy::unnecessary_operation)] +#![allow(clippy::slice_as_bytes)] #![warn(clippy::bytes_nth)] fn main() { diff --git a/tests/ui/bytes_nth.rs b/tests/ui/bytes_nth.rs index b7d813fe296a..72df502d94fa 100644 --- a/tests/ui/bytes_nth.rs +++ b/tests/ui/bytes_nth.rs @@ -1,6 +1,7 @@ //@run-rustfix #![allow(clippy::unnecessary_operation)] +#![allow(clippy::slice_as_bytes)] #![warn(clippy::bytes_nth)] fn main() { diff --git a/tests/ui/bytes_nth.stderr b/tests/ui/bytes_nth.stderr index e8b15027829e..68c0e77216a8 100644 --- a/tests/ui/bytes_nth.stderr +++ b/tests/ui/bytes_nth.stderr @@ -1,5 +1,5 @@ error: called `.bytes().nth()` on a `String` - --> $DIR/bytes_nth.rs:8:13 + --> $DIR/bytes_nth.rs:9:13 | LL | let _ = s.bytes().nth(3); | ^^^^^^^^^^^^^^^^ help: try: `s.as_bytes().get(3).copied()` @@ -7,13 +7,13 @@ LL | let _ = s.bytes().nth(3); = note: `-D clippy::bytes-nth` implied by `-D warnings` error: called `.bytes().nth().unwrap()` on a `String` - --> $DIR/bytes_nth.rs:9:14 + --> $DIR/bytes_nth.rs:10:14 | LL | let _ = &s.bytes().nth(3).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `s.as_bytes()[3]` error: called `.bytes().nth()` on a `str` - --> $DIR/bytes_nth.rs:10:13 + --> $DIR/bytes_nth.rs:11:13 | LL | let _ = s[..].bytes().nth(3); | ^^^^^^^^^^^^^^^^^^^^ help: try: `s[..].as_bytes().get(3).copied()` From 70f100faed1604052f51ca79eff1b53b179ea1af Mon Sep 17 00:00:00 2001 From: A-Walrus Date: Tue, 20 Jun 2023 16:56:40 +0300 Subject: [PATCH 3/3] Use `is_range_literal` to simplifiy code --- clippy_lints/src/methods/slice_as_bytes.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/methods/slice_as_bytes.rs b/clippy_lints/src/methods/slice_as_bytes.rs index ca1e31bc4743..6567b6ba1a20 100644 --- a/clippy_lints/src/methods/slice_as_bytes.rs +++ b/clippy_lints/src/methods/slice_as_bytes.rs @@ -1,19 +1,13 @@ 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_hir::{is_range_literal, Expr, ExprKind, LangItem}; 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 - { + if is_range_literal(index) { 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);