Auto merge of #8711 - kyoto7250:new-lint-bytes-count-to-len, r=giraffate

Take over: New lint bytes count to len

take over #8375
close #8083

This PR adds new lint about  considering replacing `.bytes().count()` with `.len()`.

Thank you in advance.

---

r! `@Manishearth`

changelog: adds new lint [`bytes_count_to_len`] to consider replacing `.bytes().count()` with `.len()`

CHANGELOG.md

@@ -3300,6 +3300,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

clippy_lints/src/bytes_count_to_len.rs

@@ -0,0 +1,70 @@
+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
+    /// 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();
+    /// String::from("hello").bytes().count();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// "hello".len();
+    /// String::from("hello").len();
+    /// ```
+    #[clippy::version = "1.62.0"]
+    pub BYTES_COUNT_TO_LEN,
+    complexity,
+    "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! {
+            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 {
+                let mut applicability = Applicability::MachineApplicable;
+                span_lint_and_sugg(
+                    cx,
+                    BYTES_COUNT_TO_LEN,
+                    expr.span,
+                    "using long and hard to read `.bytes().count()`",
+                    "consider calling `.len()` instead",
+                    format!("{}.len()", snippet_with_applicability(cx, str_expr.span, "..", &mut applicability)),
+                    applicability
+                );
+            }
+        };
+    }
+}

clippy_lints/src/lib.register_all.rs

@@ -24,6 +24,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_ABS_TO_UNSIGNED),
     LintId::of(casts::CAST_ENUM_CONSTRUCTOR),
     LintId::of(casts::CAST_ENUM_TRUNCATION),

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),

clippy_lints/src/lib.register_lints.rs

@@ -65,6 +65,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::CARGO_COMMON_METADATA,
     cargo::MULTIPLE_CRATE_VERSIONS,
     cargo::NEGATIVE_FEATURE_NAMES,

clippy_lints/src/lib.rs

@@ -181,6 +181,7 @@ mod bool_assert_comparison;
 mod booleans;
 mod borrow_as_ptr;
 mod bytecount;
+mod bytes_count_to_len;
 mod cargo;
 mod case_sensitive_file_extension_comparisons;
 mod casts;
@@ -880,6 +881,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings));
     store.register_early_pass(|| Box::new(pub_use::PubUse));
     store.register_late_pass(|| Box::new(format_push_string::FormatPushString));
+    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`
 }
 

clippy_utils/src/paths.rs

@@ -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"];
@@ -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"];

tests/ui/bytes_count_to_len.fixed

@@ -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();
+}

tests/ui/bytes_count_to_len.rs

@@ -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").bytes().count();
+
+    let s1 = String::from("foo");
+    let _ = s1.bytes().count();
+
+    // should fix, because type is &str
+    let _ = "foo".bytes().count();
+
+    let s2 = "foo";
+    let _ = s2.bytes().count();
+
+    // 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();
+}

tests/ui/bytes_count_to_len.stderr

@@ -0,0 +1,28 @@
+error: using long and hard to read `.bytes().count()`
+  --> $DIR/bytes_count_to_len.rs:8:13
+   |
+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`
+
+error: using long and hard to read `.bytes().count()`
+  --> $DIR/bytes_count_to_len.rs:11:13
+   |
+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
+   |
+LL |     let _ = s2.bytes().count();
+   |             ^^^^^^^^^^^^^^^^^^ help: consider calling `.len()` instead: `s2.len()`
+
+error: aborting due to 4 previous errors
+