From 25028986864fbb3231c54feb5e769c8778e037bf Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sun, 5 Jun 2022 11:39:36 -0400 Subject: [PATCH] Move `ByteCount` into `Methods` lint pass --- clippy_lints/src/bytecount.rs | 103 ---------------------- clippy_lints/src/lib.register_lints.rs | 2 +- clippy_lints/src/lib.register_pedantic.rs | 2 +- clippy_lints/src/lib.rs | 2 - clippy_lints/src/methods/bytecount.rs | 70 +++++++++++++++ clippy_lints/src/methods/mod.rs | 42 ++++++++- 6 files changed, 111 insertions(+), 110 deletions(-) delete mode 100644 clippy_lints/src/bytecount.rs create mode 100644 clippy_lints/src/methods/bytecount.rs diff --git a/clippy_lints/src/bytecount.rs b/clippy_lints/src/bytecount.rs deleted file mode 100644 index 326ce34082a..00000000000 --- a/clippy_lints/src/bytecount.rs +++ /dev/null @@ -1,103 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet_with_applicability; -use clippy_utils::ty::match_type; -use clippy_utils::visitors::is_local_used; -use clippy_utils::{path_to_local_id, paths, peel_blocks, peel_ref_operators, strip_pat_refs}; -use if_chain::if_chain; -use rustc_errors::Applicability; -use rustc_hir::{BinOpKind, Closure, Expr, ExprKind, PatKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{self, UintTy}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; - -declare_clippy_lint! { - /// ### What it does - /// Checks for naive byte counts - /// - /// ### Why is this bad? - /// The [`bytecount`](https://crates.io/crates/bytecount) - /// crate has methods to count your bytes faster, especially for large slices. - /// - /// ### Known problems - /// If you have predominantly small slices, the - /// `bytecount::count(..)` method may actually be slower. However, if you can - /// ensure that less than 2³²-1 matches arise, the `naive_count_32(..)` can be - /// faster in those cases. - /// - /// ### Example - /// ```rust - /// # let vec = vec![1_u8]; - /// let count = vec.iter().filter(|x| **x == 0u8).count(); - /// ``` - /// - /// Use instead: - /// ```rust,ignore - /// # let vec = vec![1_u8]; - /// let count = bytecount::count(&vec, 0u8); - /// ``` - #[clippy::version = "pre 1.29.0"] - pub NAIVE_BYTECOUNT, - pedantic, - "use of naive `.filter(|&x| x == y).count()` to count byte values" -} - -declare_lint_pass!(ByteCount => [NAIVE_BYTECOUNT]); - -impl<'tcx> LateLintPass<'tcx> for ByteCount { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { - if_chain! { - if let ExprKind::MethodCall(count, [count_recv], _) = expr.kind; - if count.ident.name == sym::count; - if let ExprKind::MethodCall(filter, [filter_recv, filter_arg], _) = count_recv.kind; - if filter.ident.name == sym!(filter); - if let ExprKind::Closure(&Closure { body, .. }) = filter_arg.kind; - let body = cx.tcx.hir().body(body); - if let [param] = body.params; - if let PatKind::Binding(_, arg_id, _, _) = strip_pat_refs(param.pat).kind; - if let ExprKind::Binary(ref op, l, r) = body.value.kind; - if op.node == BinOpKind::Eq; - if match_type(cx, - cx.typeck_results().expr_ty(filter_recv).peel_refs(), - &paths::SLICE_ITER); - let operand_is_arg = |expr| { - let expr = peel_ref_operators(cx, peel_blocks(expr)); - path_to_local_id(expr, arg_id) - }; - let needle = if operand_is_arg(l) { - r - } else if operand_is_arg(r) { - l - } else { - return; - }; - if ty::Uint(UintTy::U8) == *cx.typeck_results().expr_ty(needle).peel_refs().kind(); - if !is_local_used(cx, needle, arg_id); - then { - let haystack = if let ExprKind::MethodCall(path, args, _) = - filter_recv.kind { - let p = path.ident.name; - if (p == sym::iter || p == sym!(iter_mut)) && args.len() == 1 { - &args[0] - } else { - filter_recv - } - } else { - filter_recv - }; - let mut applicability = Applicability::MaybeIncorrect; - span_lint_and_sugg( - cx, - NAIVE_BYTECOUNT, - expr.span, - "you appear to be counting bytes the naive way", - "consider using the bytecount crate", - format!("bytecount::count({}, {})", - snippet_with_applicability(cx, haystack.span, "..", &mut applicability), - snippet_with_applicability(cx, needle.span, "..", &mut applicability)), - applicability, - ); - } - }; - } -} diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 7883711e5a3..c3c24fb1668 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -59,7 +59,6 @@ store.register_lints(&[ booleans::NONMINIMAL_BOOL, booleans::OVERLY_COMPLEX_BOOL_EXPR, borrow_deref_ref::BORROW_DEREF_REF, - bytecount::NAIVE_BYTECOUNT, bytes_count_to_len::BYTES_COUNT_TO_LEN, cargo::CARGO_COMMON_METADATA, cargo::MULTIPLE_CRATE_VERSIONS, @@ -330,6 +329,7 @@ store.register_lints(&[ methods::MAP_FLATTEN, methods::MAP_IDENTITY, methods::MAP_UNWRAP_OR, + methods::NAIVE_BYTECOUNT, methods::NEEDLESS_OPTION_AS_DEREF, methods::NEEDLESS_OPTION_TAKE, methods::NEEDLESS_SPLITN, diff --git a/clippy_lints/src/lib.register_pedantic.rs b/clippy_lints/src/lib.register_pedantic.rs index ae9f046f023..241d70d2628 100644 --- a/clippy_lints/src/lib.register_pedantic.rs +++ b/clippy_lints/src/lib.register_pedantic.rs @@ -4,7 +4,6 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![ LintId::of(attrs::INLINE_ALWAYS), - LintId::of(bytecount::NAIVE_BYTECOUNT), LintId::of(case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS), LintId::of(casts::BORROW_AS_PTR), LintId::of(casts::CAST_LOSSLESS), @@ -64,6 +63,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![ LintId::of(methods::IMPLICIT_CLONE), LintId::of(methods::INEFFICIENT_TO_STRING), LintId::of(methods::MAP_UNWRAP_OR), + LintId::of(methods::NAIVE_BYTECOUNT), LintId::of(methods::UNNECESSARY_JOIN), LintId::of(misc::USED_UNDERSCORE_BINDING), LintId::of(mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 63e0fcf6b0f..617f1622325 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -180,7 +180,6 @@ mod blocks_in_if_conditions; mod bool_assert_comparison; mod booleans; mod borrow_deref_ref; -mod bytecount; mod bytes_count_to_len; mod cargo; mod case_sensitive_file_extension_comparisons; @@ -718,7 +717,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ); store.register_late_pass(move || Box::new(pass_by_ref_or_value)); store.register_late_pass(|| Box::new(ref_option_ref::RefOptionRef)); - store.register_late_pass(|| Box::new(bytecount::ByteCount)); store.register_late_pass(|| Box::new(infinite_iter::InfiniteIter)); store.register_late_pass(|| Box::new(inline_fn_without_body::InlineFnWithoutBody)); store.register_late_pass(|| Box::new(useless_conversion::UselessConversion::default())); diff --git a/clippy_lints/src/methods/bytecount.rs b/clippy_lints/src/methods/bytecount.rs new file mode 100644 index 00000000000..6a7c63d76f7 --- /dev/null +++ b/clippy_lints/src/methods/bytecount.rs @@ -0,0 +1,70 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::ty::match_type; +use clippy_utils::visitors::is_local_used; +use clippy_utils::{path_to_local_id, paths, peel_blocks, peel_ref_operators, strip_pat_refs}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{BinOpKind, Closure, Expr, ExprKind, PatKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, UintTy}; +use rustc_span::sym; + +use super::NAIVE_BYTECOUNT; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + filter_recv: &'tcx Expr<'_>, + filter_arg: &'tcx Expr<'_>, +) { + if_chain! { + if let ExprKind::Closure(&Closure { body, .. }) = filter_arg.kind; + let body = cx.tcx.hir().body(body); + if let [param] = body.params; + if let PatKind::Binding(_, arg_id, _, _) = strip_pat_refs(param.pat).kind; + if let ExprKind::Binary(ref op, l, r) = body.value.kind; + if op.node == BinOpKind::Eq; + if match_type(cx, + cx.typeck_results().expr_ty(filter_recv).peel_refs(), + &paths::SLICE_ITER); + let operand_is_arg = |expr| { + let expr = peel_ref_operators(cx, peel_blocks(expr)); + path_to_local_id(expr, arg_id) + }; + let needle = if operand_is_arg(l) { + r + } else if operand_is_arg(r) { + l + } else { + return; + }; + if ty::Uint(UintTy::U8) == *cx.typeck_results().expr_ty(needle).peel_refs().kind(); + if !is_local_used(cx, needle, arg_id); + then { + let haystack = if let ExprKind::MethodCall(path, args, _) = + filter_recv.kind { + let p = path.ident.name; + if (p == sym::iter || p == sym!(iter_mut)) && args.len() == 1 { + &args[0] + } else { + filter_recv + } + } else { + filter_recv + }; + let mut applicability = Applicability::MaybeIncorrect; + span_lint_and_sugg( + cx, + NAIVE_BYTECOUNT, + expr.span, + "you appear to be counting bytes the naive way", + "consider using the bytecount crate", + format!("bytecount::count({}, {})", + snippet_with_applicability(cx, haystack.span, "..", &mut applicability), + snippet_with_applicability(cx, needle.span, "..", &mut applicability)), + applicability, + ); + } + }; +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index b68a2651e1b..f0d9dce5518 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1,4 +1,5 @@ mod bind_instead_of_map; +mod bytecount; mod bytes_nth; mod chars_cmp; mod chars_cmp_with_unwrap; @@ -82,7 +83,9 @@ use bind_instead_of_map::BindInsteadOfMap; use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; use clippy_utils::ty::{contains_adt_constructor, contains_ty, implements_trait, is_copy, is_type_diagnostic_item}; -use clippy_utils::{contains_return, get_trait_def_id, iter_input_pats, meets_msrv, msrvs, paths, return_ty}; +use clippy_utils::{ + contains_return, get_trait_def_id, is_trait_method, iter_input_pats, meets_msrv, msrvs, paths, return_ty, +}; use if_chain::if_chain; use rustc_hir as hir; use rustc_hir::def::Res; @@ -2368,6 +2371,37 @@ declare_clippy_lint! { "Iterator for empty array" } +declare_clippy_lint! { + /// ### What it does + /// Checks for naive byte counts + /// + /// ### Why is this bad? + /// The [`bytecount`](https://crates.io/crates/bytecount) + /// crate has methods to count your bytes faster, especially for large slices. + /// + /// ### Known problems + /// If you have predominantly small slices, the + /// `bytecount::count(..)` method may actually be slower. However, if you can + /// ensure that less than 2³²-1 matches arise, the `naive_count_32(..)` can be + /// faster in those cases. + /// + /// ### Example + /// ```rust + /// # let vec = vec![1_u8]; + /// let count = vec.iter().filter(|x| **x == 0u8).count(); + /// ``` + /// + /// Use instead: + /// ```rust,ignore + /// # let vec = vec![1_u8]; + /// let count = bytecount::count(&vec, 0u8); + /// ``` + #[clippy::version = "pre 1.29.0"] + pub NAIVE_BYTECOUNT, + pedantic, + "use of naive `.filter(|&x| x == y).count()` to count byte values" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Option, @@ -2471,7 +2505,8 @@ impl_lint_pass!(Methods => [ NO_EFFECT_REPLACE, OBFUSCATED_IF_ELSE, ITER_ON_SINGLE_ITEMS, - ITER_ON_EMPTY_COLLECTIONS + ITER_ON_EMPTY_COLLECTIONS, + NAIVE_BYTECOUNT, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -2726,12 +2761,13 @@ impl Methods { }, _ => {}, }, - ("count", []) => match method_call(recv) { + ("count", []) if is_trait_method(cx, expr, sym::Iterator) => match method_call(recv) { Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false), Some((name2 @ ("into_iter" | "iter" | "iter_mut"), [recv2], _)) => { iter_count::check(cx, expr, recv2, name2); }, Some(("map", [_, arg], _)) => suspicious_map::check(cx, expr, recv, arg), + Some(("filter", [recv2, arg], _)) => bytecount::check(cx, expr, recv2, arg), _ => {}, }, ("drain", [arg]) => {