Skip to content

Commit

Permalink
add checking type
Browse files Browse the repository at this point in the history
adding test patterns

cargo dev bless

fix comment

add ;

delete :

fix suggestion code

and update stderr in tests.

use match_def_path when checking method name
  • Loading branch information
kyoto7250 committed Apr 19, 2022
1 parent df1ec91 commit f19387d
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 29 deletions.
42 changes: 29 additions & 13 deletions clippy_lints/src/bytes_count_to_len.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{match_def_path, paths};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;

declare_clippy_lint! {
/// ### What it does
Expand All @@ -16,37 +22,47 @@ declare_clippy_lint! {
/// ### Example
/// ```rust
/// "hello".bytes().count();
/// String::from("hello").bytes().count();
/// ```
/// Use instead:
/// ```rust
/// "hello".len();
/// String::from("hello").len();
/// ```
#[clippy::version = "1.60.0"]
#[clippy::version = "1.62.0"]
pub BYTES_COUNT_TO_LEN,
complexity,
"Using bytest().count() when len() performs the same functionality"
"Using `bytes().count()` when `len()` performs the same functionality"
}

declare_lint_pass!(BytesCountToLen => [BYTES_COUNT_TO_LEN]);

impl<'tcx> LateLintPass<'tcx> for BytesCountToLen {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
if_chain! {
//check for method call called "count"
if let hir::ExprKind::MethodCall(count_path, count_args, _) = &expr.kind;
if count_path.ident.name == rustc_span::sym::count;
if let [bytes_expr] = &**count_args;
//check for method call called "bytes" that was linked to "count"
if let hir::ExprKind::MethodCall(bytes_path, _, _) = &bytes_expr.kind;
if bytes_path.ident.name.as_str() == "bytes";
if let hir::ExprKind::MethodCall(_, expr_args, _) = &expr.kind;
if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if match_def_path(cx, expr_def_id, &paths::ITER_COUNT);

if let [bytes_expr] = &**expr_args;
if let hir::ExprKind::MethodCall(_, bytes_args, _) = &bytes_expr.kind;
if let Some(bytes_def_id) = cx.typeck_results().type_dependent_def_id(bytes_expr.hir_id);
if match_def_path(cx, bytes_def_id, &paths::STR_BYTES);

if let [str_expr] = &**bytes_args;
let ty = cx.typeck_results().expr_ty(str_expr).peel_refs();

if is_type_diagnostic_item(cx, ty, sym::String) || ty.kind() == &ty::Str;
then {
span_lint_and_note(
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
BYTES_COUNT_TO_LEN,
expr.span,
"using long and hard to read `.bytes().count()`",
None,
"`.len()` achieves same functionality"
"consider calling `.len()` instead",
format!("{}.len()", snippet_with_applicability(cx, str_expr.span, "..", &mut applicability)),
applicability
);
}
};
Expand Down
2 changes: 2 additions & 0 deletions clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ pub const IO_READ: [&str; 3] = ["std", "io", "Read"];
pub const IO_WRITE: [&str; 3] = ["std", "io", "Write"];
pub const IPADDR_V4: [&str; 5] = ["std", "net", "ip", "IpAddr", "V4"];
pub const IPADDR_V6: [&str; 5] = ["std", "net", "ip", "IpAddr", "V6"];
pub const ITER_COUNT: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "count"];
pub const ITER_REPEAT: [&str; 5] = ["core", "iter", "sources", "repeat", "repeat"];
#[allow(clippy::invalid_paths)] // internal lints do not know about all external crates
pub const ITERTOOLS_NEXT_TUPLE: [&str; 3] = ["itertools", "Itertools", "next_tuple"];
Expand Down Expand Up @@ -149,6 +150,7 @@ pub const STD_FS_CREATE_DIR: [&str; 3] = ["std", "fs", "create_dir"];
pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"];
pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"];
pub const STRING_NEW: [&str; 4] = ["alloc", "string", "String", "new"];
pub const STR_BYTES: [&str; 4] = ["core", "str", "<impl str>", "bytes"];
pub const STR_ENDS_WITH: [&str; 4] = ["core", "str", "<impl str>", "ends_with"];
pub const STR_FROM_UTF8: [&str; 4] = ["core", "str", "converts", "from_utf8"];
pub const STR_LEN: [&str; 4] = ["core", "str", "<impl str>", "len"];
Expand Down
34 changes: 34 additions & 0 deletions tests/ui/bytes_count_to_len.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// run-rustfix
#![warn(clippy::bytes_count_to_len)]
use std::fs::File;
use std::io::Read;

fn main() {
// should fix, because type is String
let _ = String::from("foo").len();

let s1 = String::from("foo");
let _ = s1.len();

// should fix, because type is &str
let _ = "foo".len();

let s2 = "foo";
let _ = s2.len();

// make sure using count() normally doesn't trigger warning
let vector = [0, 1, 2];
let _ = vector.iter().count();

// The type is slice, so should not fix
let _ = &[1, 2, 3].bytes().count();

let bytes: &[u8] = &[1, 2, 3];
bytes.bytes().count();

// The type is File, so should not fix
let _ = File::open("foobar").unwrap().bytes().count();

let f = File::open("foobar").unwrap();
let _ = f.bytes().count();
}
33 changes: 26 additions & 7 deletions tests/ui/bytes_count_to_len.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,34 @@
// run-rustfix
#![warn(clippy::bytes_count_to_len)]
use std::fs::File;
use std::io::Read;

fn main() {
let s1 = String::from("world");
// should fix, because type is String
let _ = String::from("foo").bytes().count();

//test warning against a string literal
"hello".bytes().count();
let s1 = String::from("foo");
let _ = s1.bytes().count();

//test warning against a string variable
s1.bytes().count();
// should fix, because type is &str
let _ = "foo".bytes().count();

//make sure using count() normally doesn't trigger warning
let s2 = "foo";
let _ = s2.bytes().count();

// make sure using count() normally doesn't trigger warning
let vector = [0, 1, 2];
let size = vector.iter().count();
let _ = vector.iter().count();

// The type is slice, so should not fix
let _ = &[1, 2, 3].bytes().count();

let bytes: &[u8] = &[1, 2, 3];
bytes.bytes().count();

// The type is File, so should not fix
let _ = File::open("foobar").unwrap().bytes().count();

let f = File::open("foobar").unwrap();
let _ = f.bytes().count();
}
27 changes: 18 additions & 9 deletions tests/ui/bytes_count_to_len.stderr
Original file line number Diff line number Diff line change
@@ -1,19 +1,28 @@
error: using long and hard to read `.bytes().count()`
--> $DIR/bytes_count_to_len.rs:7:5
--> $DIR/bytes_count_to_len.rs:8:13
|
LL | "hello".bytes().count();
| ^^^^^^^^^^^^^^^^^^^^^^^
LL | let _ = String::from("foo").bytes().count();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.len()` instead: `String::from("foo").len()`
|
= note: `-D clippy::bytes-count-to-len` implied by `-D warnings`
= note: `.len()` achieves same functionality

error: using long and hard to read `.bytes().count()`
--> $DIR/bytes_count_to_len.rs:10:5
--> $DIR/bytes_count_to_len.rs:11:13
|
LL | s1.bytes().count();
| ^^^^^^^^^^^^^^^^^^
LL | let _ = s1.bytes().count();
| ^^^^^^^^^^^^^^^^^^ help: consider calling `.len()` instead: `s1.len()`

error: using long and hard to read `.bytes().count()`
--> $DIR/bytes_count_to_len.rs:14:13
|
LL | let _ = "foo".bytes().count();
| ^^^^^^^^^^^^^^^^^^^^^ help: consider calling `.len()` instead: `"foo".len()`

error: using long and hard to read `.bytes().count()`
--> $DIR/bytes_count_to_len.rs:17:13
|
= note: `.len()` achieves same functionality
LL | let _ = s2.bytes().count();
| ^^^^^^^^^^^^^^^^^^ help: consider calling `.len()` instead: `s2.len()`

error: aborting due to 2 previous errors
error: aborting due to 4 previous errors

0 comments on commit f19387d

Please sign in to comment.