unnecessary_fold to its own module

This commit is contained in:
Takayuki Maeda 2021-03-06 19:05:51 +09:00
parent 78e572c627
commit bbed852f6f
2 changed files with 107 additions and 94 deletions

View file

@ -40,6 +40,7 @@ mod string_extend_chars;
mod suspicious_map; mod suspicious_map;
mod uninit_assumed_init; mod uninit_assumed_init;
mod unnecessary_filter_map; mod unnecessary_filter_map;
mod unnecessary_fold;
mod unnecessary_lazy_eval; mod unnecessary_lazy_eval;
mod unwrap_used; mod unwrap_used;
mod useless_asref; mod useless_asref;
@ -53,7 +54,7 @@ use if_chain::if_chain;
use rustc_ast::ast; use rustc_ast::ast;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::{PatKind, TraitItem, TraitItemKind}; use rustc_hir::{TraitItem, TraitItemKind};
use rustc_lint::{LateContext, LateLintPass, Lint, LintContext}; use rustc_lint::{LateContext, LateLintPass, Lint, LintContext};
use rustc_middle::lint::in_external_macro; use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, TraitRef, Ty, TyS}; use rustc_middle::ty::{self, TraitRef, Ty, TyS};
@ -67,10 +68,9 @@ use crate::utils::eager_or_lazy::is_lazyness_candidate;
use crate::utils::usage::mutated_variables; use crate::utils::usage::mutated_variables;
use crate::utils::{ use crate::utils::{
contains_return, contains_ty, get_trait_def_id, implements_trait, in_macro, is_copy, is_expn_of, contains_return, contains_ty, get_trait_def_id, implements_trait, in_macro, is_copy, is_expn_of,
is_type_diagnostic_item, iter_input_pats, last_path_segment, match_def_path, match_qpath, match_trait_method, is_type_diagnostic_item, iter_input_pats, last_path_segment, match_def_path, match_qpath, match_type, meets_msrv,
match_type, meets_msrv, method_calls, method_chain_args, path_to_local_id, paths, remove_blocks, return_ty, method_calls, method_chain_args, paths, return_ty, single_segment_path, snippet, snippet_with_applicability,
single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, SpanlessEq,
span_lint_and_help, span_lint_and_sugg, strip_pat_refs, SpanlessEq,
}; };
declare_clippy_lint! { declare_clippy_lint! {
@ -1736,7 +1736,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
["collect", "cloned"] => iter_cloned_collect::check(cx, expr, arg_lists[1]), ["collect", "cloned"] => iter_cloned_collect::check(cx, expr, arg_lists[1]),
["as_ref"] => useless_asref::check(cx, expr, "as_ref", arg_lists[0]), ["as_ref"] => useless_asref::check(cx, expr, "as_ref", arg_lists[0]),
["as_mut"] => useless_asref::check(cx, expr, "as_mut", arg_lists[0]), ["as_mut"] => useless_asref::check(cx, expr, "as_mut", arg_lists[0]),
["fold", ..] => lint_unnecessary_fold(cx, expr, arg_lists[0], method_spans[0]), ["fold", ..] => unnecessary_fold::check(cx, expr, arg_lists[0], method_spans[0]),
["filter_map", ..] => { ["filter_map", ..] => {
unnecessary_filter_map::check(cx, expr, arg_lists[0]); unnecessary_filter_map::check(cx, expr, arg_lists[0]);
filter_map_identity::check(cx, expr, arg_lists[0], method_spans[0]); filter_map_identity::check(cx, expr, arg_lists[0], method_spans[0]);
@ -2324,94 +2324,6 @@ fn lint_expect_fun_call(
); );
} }
fn lint_unnecessary_fold(cx: &LateContext<'_>, expr: &hir::Expr<'_>, fold_args: &[hir::Expr<'_>], fold_span: Span) {
fn check_fold_with_op(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
fold_args: &[hir::Expr<'_>],
fold_span: Span,
op: hir::BinOpKind,
replacement_method_name: &str,
replacement_has_args: bool,
) {
if_chain! {
// Extract the body of the closure passed to fold
if let hir::ExprKind::Closure(_, _, body_id, _, _) = fold_args[2].kind;
let closure_body = cx.tcx.hir().body(body_id);
let closure_expr = remove_blocks(&closure_body.value);
// Check if the closure body is of the form `acc <op> some_expr(x)`
if let hir::ExprKind::Binary(ref bin_op, ref left_expr, ref right_expr) = closure_expr.kind;
if bin_op.node == op;
// Extract the names of the two arguments to the closure
if let [param_a, param_b] = closure_body.params;
if let PatKind::Binding(_, first_arg_id, ..) = strip_pat_refs(&param_a.pat).kind;
if let PatKind::Binding(_, second_arg_id, second_arg_ident, _) = strip_pat_refs(&param_b.pat).kind;
if path_to_local_id(left_expr, first_arg_id);
if replacement_has_args || path_to_local_id(right_expr, second_arg_id);
then {
let mut applicability = Applicability::MachineApplicable;
let sugg = if replacement_has_args {
format!(
"{replacement}(|{s}| {r})",
replacement = replacement_method_name,
s = second_arg_ident,
r = snippet_with_applicability(cx, right_expr.span, "EXPR", &mut applicability),
)
} else {
format!(
"{replacement}()",
replacement = replacement_method_name,
)
};
span_lint_and_sugg(
cx,
UNNECESSARY_FOLD,
fold_span.with_hi(expr.span.hi()),
// TODO #2371 don't suggest e.g., .any(|x| f(x)) if we can suggest .any(f)
"this `.fold` can be written more succinctly using another method",
"try",
sugg,
applicability,
);
}
}
}
// Check that this is a call to Iterator::fold rather than just some function called fold
if !match_trait_method(cx, expr, &paths::ITERATOR) {
return;
}
assert!(
fold_args.len() == 3,
"Expected fold_args to have three entries - the receiver, the initial value and the closure"
);
// Check if the first argument to .fold is a suitable literal
if let hir::ExprKind::Lit(ref lit) = fold_args[1].kind {
match lit.node {
ast::LitKind::Bool(false) => {
check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::Or, "any", true)
},
ast::LitKind::Bool(true) => {
check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::And, "all", true)
},
ast::LitKind::Int(0, _) => {
check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::Add, "sum", false)
},
ast::LitKind::Int(1, _) => {
check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::Mul, "product", false)
},
_ => (),
}
}
}
fn derefs_to_slice<'tcx>( fn derefs_to_slice<'tcx>(
cx: &LateContext<'tcx>, cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'tcx>, expr: &'tcx hir::Expr<'tcx>,

View file

@ -0,0 +1,101 @@
use crate::utils::{
match_trait_method, path_to_local_id, paths, remove_blocks, snippet_with_applicability, span_lint_and_sugg,
strip_pat_refs,
};
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::PatKind;
use rustc_lint::LateContext;
use rustc_span::source_map::Span;
use super::UNNECESSARY_FOLD;
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, fold_args: &[hir::Expr<'_>], fold_span: Span) {
fn check_fold_with_op(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
fold_args: &[hir::Expr<'_>],
fold_span: Span,
op: hir::BinOpKind,
replacement_method_name: &str,
replacement_has_args: bool,
) {
if_chain! {
// Extract the body of the closure passed to fold
if let hir::ExprKind::Closure(_, _, body_id, _, _) = fold_args[2].kind;
let closure_body = cx.tcx.hir().body(body_id);
let closure_expr = remove_blocks(&closure_body.value);
// Check if the closure body is of the form `acc <op> some_expr(x)`
if let hir::ExprKind::Binary(ref bin_op, ref left_expr, ref right_expr) = closure_expr.kind;
if bin_op.node == op;
// Extract the names of the two arguments to the closure
if let [param_a, param_b] = closure_body.params;
if let PatKind::Binding(_, first_arg_id, ..) = strip_pat_refs(&param_a.pat).kind;
if let PatKind::Binding(_, second_arg_id, second_arg_ident, _) = strip_pat_refs(&param_b.pat).kind;
if path_to_local_id(left_expr, first_arg_id);
if replacement_has_args || path_to_local_id(right_expr, second_arg_id);
then {
let mut applicability = Applicability::MachineApplicable;
let sugg = if replacement_has_args {
format!(
"{replacement}(|{s}| {r})",
replacement = replacement_method_name,
s = second_arg_ident,
r = snippet_with_applicability(cx, right_expr.span, "EXPR", &mut applicability),
)
} else {
format!(
"{replacement}()",
replacement = replacement_method_name,
)
};
span_lint_and_sugg(
cx,
UNNECESSARY_FOLD,
fold_span.with_hi(expr.span.hi()),
// TODO #2371 don't suggest e.g., .any(|x| f(x)) if we can suggest .any(f)
"this `.fold` can be written more succinctly using another method",
"try",
sugg,
applicability,
);
}
}
}
// Check that this is a call to Iterator::fold rather than just some function called fold
if !match_trait_method(cx, expr, &paths::ITERATOR) {
return;
}
assert!(
fold_args.len() == 3,
"Expected fold_args to have three entries - the receiver, the initial value and the closure"
);
// Check if the first argument to .fold is a suitable literal
if let hir::ExprKind::Lit(ref lit) = fold_args[1].kind {
match lit.node {
ast::LitKind::Bool(false) => {
check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::Or, "any", true)
},
ast::LitKind::Bool(true) => {
check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::And, "all", true)
},
ast::LitKind::Int(0, _) => {
check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::Add, "sum", false)
},
ast::LitKind::Int(1, _) => {
check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::Mul, "product", false)
},
_ => (),
}
}
}