Move ByteCount into Methods lint pass

This commit is contained in:
Jason Newcomb 2022-06-05 11:39:36 -04:00
parent 21f595433e
commit 2502898686
6 changed files with 111 additions and 110 deletions

View File

@ -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 `<slice>.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,
);
}
};
}
}

View File

@ -59,7 +59,6 @@ store.register_lints(&[
booleans::NONMINIMAL_BOOL, booleans::NONMINIMAL_BOOL,
booleans::OVERLY_COMPLEX_BOOL_EXPR, booleans::OVERLY_COMPLEX_BOOL_EXPR,
borrow_deref_ref::BORROW_DEREF_REF, borrow_deref_ref::BORROW_DEREF_REF,
bytecount::NAIVE_BYTECOUNT,
bytes_count_to_len::BYTES_COUNT_TO_LEN, bytes_count_to_len::BYTES_COUNT_TO_LEN,
cargo::CARGO_COMMON_METADATA, cargo::CARGO_COMMON_METADATA,
cargo::MULTIPLE_CRATE_VERSIONS, cargo::MULTIPLE_CRATE_VERSIONS,
@ -330,6 +329,7 @@ store.register_lints(&[
methods::MAP_FLATTEN, methods::MAP_FLATTEN,
methods::MAP_IDENTITY, methods::MAP_IDENTITY,
methods::MAP_UNWRAP_OR, methods::MAP_UNWRAP_OR,
methods::NAIVE_BYTECOUNT,
methods::NEEDLESS_OPTION_AS_DEREF, methods::NEEDLESS_OPTION_AS_DEREF,
methods::NEEDLESS_OPTION_TAKE, methods::NEEDLESS_OPTION_TAKE,
methods::NEEDLESS_SPLITN, methods::NEEDLESS_SPLITN,

View File

@ -4,7 +4,6 @@
store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
LintId::of(attrs::INLINE_ALWAYS), LintId::of(attrs::INLINE_ALWAYS),
LintId::of(bytecount::NAIVE_BYTECOUNT),
LintId::of(case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS), LintId::of(case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS),
LintId::of(casts::BORROW_AS_PTR), LintId::of(casts::BORROW_AS_PTR),
LintId::of(casts::CAST_LOSSLESS), 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::IMPLICIT_CLONE),
LintId::of(methods::INEFFICIENT_TO_STRING), LintId::of(methods::INEFFICIENT_TO_STRING),
LintId::of(methods::MAP_UNWRAP_OR), LintId::of(methods::MAP_UNWRAP_OR),
LintId::of(methods::NAIVE_BYTECOUNT),
LintId::of(methods::UNNECESSARY_JOIN), LintId::of(methods::UNNECESSARY_JOIN),
LintId::of(misc::USED_UNDERSCORE_BINDING), LintId::of(misc::USED_UNDERSCORE_BINDING),
LintId::of(mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER), LintId::of(mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER),

View File

@ -180,7 +180,6 @@ mod blocks_in_if_conditions;
mod bool_assert_comparison; mod bool_assert_comparison;
mod booleans; mod booleans;
mod borrow_deref_ref; mod borrow_deref_ref;
mod bytecount;
mod bytes_count_to_len; mod bytes_count_to_len;
mod cargo; mod cargo;
mod case_sensitive_file_extension_comparisons; 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(move || Box::new(pass_by_ref_or_value));
store.register_late_pass(|| Box::new(ref_option_ref::RefOptionRef)); 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(infinite_iter::InfiniteIter));
store.register_late_pass(|| Box::new(inline_fn_without_body::InlineFnWithoutBody)); store.register_late_pass(|| Box::new(inline_fn_without_body::InlineFnWithoutBody));
store.register_late_pass(|| Box::new(useless_conversion::UselessConversion::default())); store.register_late_pass(|| Box::new(useless_conversion::UselessConversion::default()));

View File

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

View File

@ -1,4 +1,5 @@
mod bind_instead_of_map; mod bind_instead_of_map;
mod bytecount;
mod bytes_nth; mod bytes_nth;
mod chars_cmp; mod chars_cmp;
mod chars_cmp_with_unwrap; mod chars_cmp_with_unwrap;
@ -82,7 +83,9 @@ use bind_instead_of_map::BindInsteadOfMap;
use clippy_utils::consts::{constant, Constant}; use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; 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::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 if_chain::if_chain;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::def::Res; use rustc_hir::def::Res;
@ -2368,6 +2371,37 @@ declare_clippy_lint! {
"Iterator for empty array" "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 `<slice>.filter(|&x| x == y).count()` to count byte values"
}
pub struct Methods { pub struct Methods {
avoid_breaking_exported_api: bool, avoid_breaking_exported_api: bool,
msrv: Option<RustcVersion>, msrv: Option<RustcVersion>,
@ -2471,7 +2505,8 @@ impl_lint_pass!(Methods => [
NO_EFFECT_REPLACE, NO_EFFECT_REPLACE,
OBFUSCATED_IF_ELSE, OBFUSCATED_IF_ELSE,
ITER_ON_SINGLE_ITEMS, 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. /// 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(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false),
Some((name2 @ ("into_iter" | "iter" | "iter_mut"), [recv2], _)) => { Some((name2 @ ("into_iter" | "iter" | "iter_mut"), [recv2], _)) => {
iter_count::check(cx, expr, recv2, name2); iter_count::check(cx, expr, recv2, name2);
}, },
Some(("map", [_, arg], _)) => suspicious_map::check(cx, expr, recv, arg), Some(("map", [_, arg], _)) => suspicious_map::check(cx, expr, recv, arg),
Some(("filter", [recv2, arg], _)) => bytecount::check(cx, expr, recv2, arg),
_ => {}, _ => {},
}, },
("drain", [arg]) => { ("drain", [arg]) => {