Skip to content

Commit

Permalink
Improve needless_as_bytes to also detect str::bytes() (#13972)
Browse files Browse the repository at this point in the history
I ran Clippy on some projects after upgrading to 1.84, which found a
[needless use of
`as_bytes()`](#13437). It
made me notice that the code was also using `bytes()` needlessly in
other places. This PR improves on the `as_bytes()` lint to also lint
`bytes()`.

----

changelog: none
  • Loading branch information
llogiq authored Jan 9, 2025
2 parents 19e305b + d5264c7 commit 197d58d
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 19 deletions.
8 changes: 4 additions & 4 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4924,8 +4924,8 @@ impl Methods {
},
("is_empty", []) => {
match method_call(recv) {
Some(("as_bytes", prev_recv, [], _, _)) => {
needless_as_bytes::check(cx, "is_empty", recv, prev_recv, expr.span);
Some((prev_method @ ("as_bytes" | "bytes"), prev_recv, [], _, _)) => {
needless_as_bytes::check(cx, prev_method, "is_empty", prev_recv, expr.span);
},
Some(("as_str", recv, [], as_str_span, _)) => {
redundant_as_str::check(cx, expr, recv, as_str_span, span);
Expand Down Expand Up @@ -4962,8 +4962,8 @@ impl Methods {
double_ended_iterator_last::check(cx, expr, recv, call_span);
},
("len", []) => {
if let Some(("as_bytes", prev_recv, [], _, _)) = method_call(recv) {
needless_as_bytes::check(cx, "len", recv, prev_recv, expr.span);
if let Some((prev_method @ ("as_bytes" | "bytes"), prev_recv, [], _, _)) = method_call(recv) {
needless_as_bytes::check(cx, prev_method, "len", prev_recv, expr.span);
}
},
("lock", []) => {
Expand Down
10 changes: 4 additions & 6 deletions clippy_lints/src/methods/needless_as_bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,16 @@ use rustc_span::Span;

use super::NEEDLESS_AS_BYTES;

pub fn check(cx: &LateContext<'_>, method: &str, recv: &Expr<'_>, prev_recv: &Expr<'_>, span: Span) {
if cx.typeck_results().expr_ty_adjusted(recv).peel_refs().is_slice()
&& let ty1 = cx.typeck_results().expr_ty_adjusted(prev_recv).peel_refs()
&& (is_type_lang_item(cx, ty1, LangItem::String) || ty1.is_str())
{
pub fn check(cx: &LateContext<'_>, prev_method: &str, method: &str, prev_recv: &Expr<'_>, span: Span) {
let ty1 = cx.typeck_results().expr_ty_adjusted(prev_recv).peel_refs();
if is_type_lang_item(cx, ty1, LangItem::String) || ty1.is_str() {
let mut app = Applicability::MachineApplicable;
let sugg = Sugg::hir_with_context(cx, prev_recv, span.ctxt(), "..", &mut app);
span_lint_and_sugg(
cx,
NEEDLESS_AS_BYTES,
span,
"needless call to `as_bytes()`",
format!("needless call to `{prev_method}`"),
format!("`{method}()` can be called directly on strings"),
format!("{sugg}.{method}()"),
app,
Expand Down
36 changes: 36 additions & 0 deletions tests/ui/needless_as_bytes.fixed
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
#![warn(clippy::needless_as_bytes)]
#![allow(clippy::const_is_empty)]
#![feature(exact_size_is_empty)]

struct S;

impl S {
fn as_bytes(&self) -> &[u8] {
&[]
}
fn bytes(&self) -> &[u8] {
&[]
}
}

fn main() {
Expand All @@ -15,13 +19,23 @@ fn main() {
println!("len = {}", "some string".len());
//~^ needless_as_bytes
}
if "some string".is_empty() {
//~^ needless_as_bytes
println!("len = {}", "some string".len());
//~^ needless_as_bytes
}

let s = String::from("yet another string");
if s.is_empty() {
//~^ needless_as_bytes
println!("len = {}", s.len());
//~^ needless_as_bytes
}
if s.is_empty() {
//~^ needless_as_bytes
println!("len = {}", s.len());
//~^ needless_as_bytes
}

// Do not lint
let _ = S.as_bytes().is_empty();
Expand All @@ -36,6 +50,18 @@ fn main() {
};
}
m!(1).as_bytes().len();
let _ = S.bytes().is_empty();
let _ = S.bytes().len();
let _ = (&String::new() as &dyn Bytes).bytes().len();
macro_rules! m {
(1) => {
""
};
(2) => {
"".bytes()
};
}
m!(1).bytes().len();
m!(2).len();
}

Expand All @@ -48,3 +74,13 @@ impl AsBytes for String {
&[]
}
}

pub trait Bytes {
fn bytes(&self) -> &[u8];
}

impl Bytes for String {
fn bytes(&self) -> &[u8] {
&[]
}
}
36 changes: 36 additions & 0 deletions tests/ui/needless_as_bytes.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
#![warn(clippy::needless_as_bytes)]
#![allow(clippy::const_is_empty)]
#![feature(exact_size_is_empty)]

struct S;

impl S {
fn as_bytes(&self) -> &[u8] {
&[]
}
fn bytes(&self) -> &[u8] {
&[]
}
}

fn main() {
Expand All @@ -15,13 +19,23 @@ fn main() {
println!("len = {}", "some string".as_bytes().len());
//~^ needless_as_bytes
}
if "some string".bytes().is_empty() {
//~^ needless_as_bytes
println!("len = {}", "some string".bytes().len());
//~^ needless_as_bytes
}

let s = String::from("yet another string");
if s.as_bytes().is_empty() {
//~^ needless_as_bytes
println!("len = {}", s.as_bytes().len());
//~^ needless_as_bytes
}
if s.bytes().is_empty() {
//~^ needless_as_bytes
println!("len = {}", s.bytes().len());
//~^ needless_as_bytes
}

// Do not lint
let _ = S.as_bytes().is_empty();
Expand All @@ -36,6 +50,18 @@ fn main() {
};
}
m!(1).as_bytes().len();
let _ = S.bytes().is_empty();
let _ = S.bytes().len();
let _ = (&String::new() as &dyn Bytes).bytes().len();
macro_rules! m {
(1) => {
""
};
(2) => {
"".bytes()
};
}
m!(1).bytes().len();
m!(2).len();
}

Expand All @@ -48,3 +74,13 @@ impl AsBytes for String {
&[]
}
}

pub trait Bytes {
fn bytes(&self) -> &[u8];
}

impl Bytes for String {
fn bytes(&self) -> &[u8] {
&[]
}
}
42 changes: 33 additions & 9 deletions tests/ui/needless_as_bytes.stderr
Original file line number Diff line number Diff line change
@@ -1,29 +1,53 @@
error: needless call to `as_bytes()`
--> tests/ui/needless_as_bytes.rs:13:8
error: needless call to `as_bytes`
--> tests/ui/needless_as_bytes.rs:17:8
|
LL | if "some string".as_bytes().is_empty() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `is_empty()` can be called directly on strings: `"some string".is_empty()`
|
= note: `-D clippy::needless-as-bytes` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::needless_as_bytes)]`

error: needless call to `as_bytes()`
--> tests/ui/needless_as_bytes.rs:15:30
error: needless call to `as_bytes`
--> tests/ui/needless_as_bytes.rs:19:30
|
LL | println!("len = {}", "some string".as_bytes().len());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `len()` can be called directly on strings: `"some string".len()`

error: needless call to `as_bytes()`
--> tests/ui/needless_as_bytes.rs:20:8
error: needless call to `bytes`
--> tests/ui/needless_as_bytes.rs:22:8
|
LL | if "some string".bytes().is_empty() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `is_empty()` can be called directly on strings: `"some string".is_empty()`

error: needless call to `bytes`
--> tests/ui/needless_as_bytes.rs:24:30
|
LL | println!("len = {}", "some string".bytes().len());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `len()` can be called directly on strings: `"some string".len()`

error: needless call to `as_bytes`
--> tests/ui/needless_as_bytes.rs:29:8
|
LL | if s.as_bytes().is_empty() {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: `is_empty()` can be called directly on strings: `s.is_empty()`

error: needless call to `as_bytes()`
--> tests/ui/needless_as_bytes.rs:22:30
error: needless call to `as_bytes`
--> tests/ui/needless_as_bytes.rs:31:30
|
LL | println!("len = {}", s.as_bytes().len());
| ^^^^^^^^^^^^^^^^^^ help: `len()` can be called directly on strings: `s.len()`

error: aborting due to 4 previous errors
error: needless call to `bytes`
--> tests/ui/needless_as_bytes.rs:34:8
|
LL | if s.bytes().is_empty() {
| ^^^^^^^^^^^^^^^^^^^^ help: `is_empty()` can be called directly on strings: `s.is_empty()`

error: needless call to `bytes`
--> tests/ui/needless_as_bytes.rs:36:30
|
LL | println!("len = {}", s.bytes().len());
| ^^^^^^^^^^^^^^^ help: `len()` can be called directly on strings: `s.len()`

error: aborting due to 8 previous errors

0 comments on commit 197d58d

Please sign in to comment.