From befd434bdaf5aae33b79f0e9b8816941ad3a6602 Mon Sep 17 00:00:00 2001 From: Chase Ruskin Date: Sun, 30 Jan 2022 19:16:10 -0500 Subject: [PATCH 1/5] adds lint logic and test for bytes_count_to_len --- CHANGELOG.md | 1 + clippy_lints/src/bytes_count_to_len.rs | 61 +++++++++++++++++++++ clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_complexity.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + tests/ui/bytes_count_to_len.rs | 15 +++++ tests/ui/bytes_count_to_len.stderr | 19 +++++++ 8 files changed, 101 insertions(+) create mode 100644 clippy_lints/src/bytes_count_to_len.rs create mode 100644 tests/ui/bytes_count_to_len.rs create mode 100644 tests/ui/bytes_count_to_len.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 9548366daf9a..3149a9b7511e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2894,6 +2894,7 @@ Released 2018-09-13 [`boxed_local`]: https://rust-lang.github.io/rust-clippy/master/index.html#boxed_local [`branches_sharing_code`]: https://rust-lang.github.io/rust-clippy/master/index.html#branches_sharing_code [`builtin_type_shadow`]: https://rust-lang.github.io/rust-clippy/master/index.html#builtin_type_shadow +[`bytes_count_to_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#bytes_count_to_len [`bytes_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#bytes_nth [`cargo_common_metadata`]: https://rust-lang.github.io/rust-clippy/master/index.html#cargo_common_metadata [`case_sensitive_file_extension_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#case_sensitive_file_extension_comparisons diff --git a/clippy_lints/src/bytes_count_to_len.rs b/clippy_lints/src/bytes_count_to_len.rs new file mode 100644 index 000000000000..85cdaa2736fe --- /dev/null +++ b/clippy_lints/src/bytes_count_to_len.rs @@ -0,0 +1,61 @@ +use clippy_utils::diagnostics::span_lint_and_note; +use if_chain::if_chain; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// It checks for `str::bytes().count()` and suggests replacing it with + /// `str::len()`. + /// + /// ### Why is this bad? + /// `str::bytes().count()` is longer and may not be as performant as using + /// `str::len()`. + /// + /// ### Example + /// ```rust + /// "hello".bytes().count(); + /// ``` + /// Use instead: + /// ```rust + /// "hello".len(); + /// ``` + #[clippy::version = "1.60.0"] + pub BYTES_COUNT_TO_LEN, + complexity, + "Using bytest().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.as_str() == "count"; + if let [bytes_expr] = &**count_args; + then { + match &bytes_expr.kind { + hir::ExprKind::MethodCall(bytes_path, _, _) => { + if_chain! { + if bytes_path.ident.name.as_str() == "bytes"; + then { + span_lint_and_note( + cx, + BYTES_COUNT_TO_LEN, + expr.span, + "using long and hard to read `.bytes().count()`", + None, + "`.len()` achieves same functionality" + ); + } + } + } + _ => () + }; + } + } + } +} \ No newline at end of file diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 4721b7f2b472..e567bcf218c9 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -21,6 +21,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON), LintId::of(booleans::LOGIC_BUG), LintId::of(booleans::NONMINIMAL_BOOL), + LintId::of(bytes_count_to_len::BYTES_COUNT_TO_LEN), LintId::of(casts::CAST_REF_TO_MUT), LintId::of(casts::CHAR_LIT_AS_U8), LintId::of(casts::FN_TO_NUMERIC_CAST), diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs index bd5ff613447c..57daf4ad8740 100644 --- a/clippy_lints/src/lib.register_complexity.rs +++ b/clippy_lints/src/lib.register_complexity.rs @@ -5,6 +5,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec![ LintId::of(attrs::DEPRECATED_CFG_ATTR), LintId::of(booleans::NONMINIMAL_BOOL), + LintId::of(bytes_count_to_len::BYTES_COUNT_TO_LEN), LintId::of(casts::CHAR_LIT_AS_U8), LintId::of(casts::UNNECESSARY_CAST), LintId::of(derivable_impls::DERIVABLE_IMPLS), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index e5e1c052c15e..5582f6b0874e 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -61,6 +61,7 @@ store.register_lints(&[ booleans::NONMINIMAL_BOOL, borrow_as_ptr::BORROW_AS_PTR, bytecount::NAIVE_BYTECOUNT, + bytes_count_to_len::BYTES_COUNT_TO_LEN, cargo_common_metadata::CARGO_COMMON_METADATA, case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS, casts::CAST_LOSSLESS, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9d42185dc0ec..15f553ebb2c3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -175,6 +175,7 @@ mod bool_assert_comparison; mod booleans; mod borrow_as_ptr; mod bytecount; +mod bytes_count_to_len; mod cargo_common_metadata; mod case_sensitive_file_extension_comparisons; mod casts; @@ -861,6 +862,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(borrow_as_ptr::BorrowAsPtr::new(msrv))); store.register_late_pass(move || Box::new(manual_bits::ManualBits::new(msrv))); store.register_late_pass(|| Box::new(default_union_representation::DefaultUnionRepresentation)); + store.register_late_pass(|| Box::new(bytes_count_to_len::BytesCountToLen)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/bytes_count_to_len.rs b/tests/ui/bytes_count_to_len.rs new file mode 100644 index 000000000000..c978e6587624 --- /dev/null +++ b/tests/ui/bytes_count_to_len.rs @@ -0,0 +1,15 @@ +#![warn(clippy::bytes_count_to_len)] + +fn main() { + let s1 = String::from("world"); + + //test warning against a string literal + "hello".bytes().count(); + + //test warning against a string variable + s1.bytes().count(); + + //make sure using count() normally doesn't trigger warning + let vector = [0, 1, 2]; + let size = vector.iter().count(); +} diff --git a/tests/ui/bytes_count_to_len.stderr b/tests/ui/bytes_count_to_len.stderr new file mode 100644 index 000000000000..ddd08ee824c1 --- /dev/null +++ b/tests/ui/bytes_count_to_len.stderr @@ -0,0 +1,19 @@ +error: using long and hard to read `.bytes().count()` + --> $DIR/bytes_count_to_len.rs:7:5 + | +LL | "hello".bytes().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = 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 + | +LL | s1.bytes().count(); + | ^^^^^^^^^^^^^^^^^^ + | + = note: `.len()` achieves same functionality + +error: aborting due to 2 previous errors + From 107a3507279ac7a720022723350db0b57f883291 Mon Sep 17 00:00:00 2001 From: Chase Ruskin Date: Sun, 30 Jan 2022 19:24:43 -0500 Subject: [PATCH 2/5] formats code with --- clippy_lints/src/bytes_count_to_len.rs | 8 ++++---- tests/ui/bytes_count_to_len.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/bytes_count_to_len.rs b/clippy_lints/src/bytes_count_to_len.rs index 85cdaa2736fe..86ba961c4b15 100644 --- a/clippy_lints/src/bytes_count_to_len.rs +++ b/clippy_lints/src/bytes_count_to_len.rs @@ -6,13 +6,13 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// ### What it does - /// It checks for `str::bytes().count()` and suggests replacing it with + /// It checks for `str::bytes().count()` and suggests replacing it with /// `str::len()`. - /// + /// /// ### Why is this bad? /// `str::bytes().count()` is longer and may not be as performant as using /// `str::len()`. - /// + /// /// ### Example /// ```rust /// "hello".bytes().count(); @@ -58,4 +58,4 @@ impl<'tcx> LateLintPass<'tcx> for BytesCountToLen { } } } -} \ No newline at end of file +} diff --git a/tests/ui/bytes_count_to_len.rs b/tests/ui/bytes_count_to_len.rs index c978e6587624..b2dd2bfeeed7 100644 --- a/tests/ui/bytes_count_to_len.rs +++ b/tests/ui/bytes_count_to_len.rs @@ -2,7 +2,7 @@ fn main() { let s1 = String::from("world"); - + //test warning against a string literal "hello".bytes().count(); From 681187c387c1b6161be9f07634efb192294585aa Mon Sep 17 00:00:00 2001 From: Chase Ruskin Date: Sun, 30 Jan 2022 19:24:43 -0500 Subject: [PATCH 3/5] formats rust code with 'dev fmt' --- clippy_lints/src/bytes_count_to_len.rs | 8 ++++---- tests/ui/bytes_count_to_len.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/bytes_count_to_len.rs b/clippy_lints/src/bytes_count_to_len.rs index 85cdaa2736fe..86ba961c4b15 100644 --- a/clippy_lints/src/bytes_count_to_len.rs +++ b/clippy_lints/src/bytes_count_to_len.rs @@ -6,13 +6,13 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// ### What it does - /// It checks for `str::bytes().count()` and suggests replacing it with + /// It checks for `str::bytes().count()` and suggests replacing it with /// `str::len()`. - /// + /// /// ### Why is this bad? /// `str::bytes().count()` is longer and may not be as performant as using /// `str::len()`. - /// + /// /// ### Example /// ```rust /// "hello".bytes().count(); @@ -58,4 +58,4 @@ impl<'tcx> LateLintPass<'tcx> for BytesCountToLen { } } } -} \ No newline at end of file +} diff --git a/tests/ui/bytes_count_to_len.rs b/tests/ui/bytes_count_to_len.rs index c978e6587624..b2dd2bfeeed7 100644 --- a/tests/ui/bytes_count_to_len.rs +++ b/tests/ui/bytes_count_to_len.rs @@ -2,7 +2,7 @@ fn main() { let s1 = String::from("world"); - + //test warning against a string literal "hello".bytes().count(); From 8fd5fec64bad9f39e37cdd9fc81c64f2db28c40a Mon Sep 17 00:00:00 2001 From: Chase Ruskin Date: Sun, 30 Jan 2022 19:50:52 -0500 Subject: [PATCH 4/5] fixes single match clippy error to replace with if let --- clippy_lints/src/bytes_count_to_len.rs | 31 ++++++++++---------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/bytes_count_to_len.rs b/clippy_lints/src/bytes_count_to_len.rs index 86ba961c4b15..d77f57f3bced 100644 --- a/clippy_lints/src/bytes_count_to_len.rs +++ b/clippy_lints/src/bytes_count_to_len.rs @@ -36,26 +36,19 @@ impl<'tcx> LateLintPass<'tcx> for BytesCountToLen { if let hir::ExprKind::MethodCall(count_path, count_args, _) = &expr.kind; if count_path.ident.name.as_str() == "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"; then { - match &bytes_expr.kind { - hir::ExprKind::MethodCall(bytes_path, _, _) => { - if_chain! { - if bytes_path.ident.name.as_str() == "bytes"; - then { - span_lint_and_note( - cx, - BYTES_COUNT_TO_LEN, - expr.span, - "using long and hard to read `.bytes().count()`", - None, - "`.len()` achieves same functionality" - ); - } - } - } - _ => () - }; + span_lint_and_note( + cx, + BYTES_COUNT_TO_LEN, + expr.span, + "using long and hard to read `.bytes().count()`", + None, + "`.len()` achieves same functionality" + ); } - } + }; } } From 20ca5701f3a9f3c53e60dadd5721de1181f60d10 Mon Sep 17 00:00:00 2001 From: Chase Ruskin Date: Sun, 30 Jan 2022 20:01:20 -0500 Subject: [PATCH 5/5] swaps ident.name.as_str to ident.name == sym for count fn --- clippy_lints/src/bytes_count_to_len.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/bytes_count_to_len.rs b/clippy_lints/src/bytes_count_to_len.rs index d77f57f3bced..799d3783f74f 100644 --- a/clippy_lints/src/bytes_count_to_len.rs +++ b/clippy_lints/src/bytes_count_to_len.rs @@ -34,7 +34,7 @@ impl<'tcx> LateLintPass<'tcx> for BytesCountToLen { if_chain! { //check for method call called "count" if let hir::ExprKind::MethodCall(count_path, count_args, _) = &expr.kind; - if count_path.ident.name.as_str() == "count"; + 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;