diff options
| author | Takayuki Maeda <takoyaki0316@gmail.com> | 2021-03-06 19:05:51 +0900 |
|---|---|---|
| committer | Takayuki Maeda <takoyaki0316@gmail.com> | 2021-03-11 19:40:24 +0900 |
| commit | bbed852f6fb82aa85f72fc8b2aeffdb90f409495 (patch) | |
| tree | e1063e69f72b33d4f03f95dec669fd5a5417bebb | |
| parent | 78e572c62782b4bd027c23b9d56a14b0b42ba93a (diff) | |
| download | rust-bbed852f6fb82aa85f72fc8b2aeffdb90f409495.tar.gz rust-bbed852f6fb82aa85f72fc8b2aeffdb90f409495.zip | |
unnecessary_fold to its own module
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 100 | ||||
| -rw-r--r-- | clippy_lints/src/methods/unnecessary_fold.rs | 101 |
2 files changed, 107 insertions, 94 deletions
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index eaf247536b6..702c8e76f49 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -40,6 +40,7 @@ mod string_extend_chars; mod suspicious_map; mod uninit_assumed_init; mod unnecessary_filter_map; +mod unnecessary_fold; mod unnecessary_lazy_eval; mod unwrap_used; mod useless_asref; @@ -53,7 +54,7 @@ use if_chain::if_chain; use rustc_ast::ast; use rustc_errors::Applicability; 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_middle::lint::in_external_macro; 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::{ 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, - match_type, meets_msrv, method_calls, method_chain_args, path_to_local_id, paths, remove_blocks, return_ty, - single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint, - span_lint_and_help, span_lint_and_sugg, strip_pat_refs, SpanlessEq, + is_type_diagnostic_item, iter_input_pats, last_path_segment, match_def_path, match_qpath, match_type, meets_msrv, + method_calls, method_chain_args, paths, return_ty, single_segment_path, snippet, snippet_with_applicability, + snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, SpanlessEq, }; declare_clippy_lint! { @@ -1736,7 +1736,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { ["collect", "cloned"] => iter_cloned_collect::check(cx, expr, arg_lists[1]), ["as_ref"] => useless_asref::check(cx, expr, "as_ref", 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", ..] => { unnecessary_filter_map::check(cx, expr, arg_lists[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(¶m_a.pat).kind; - if let PatKind::Binding(_, second_arg_id, second_arg_ident, _) = strip_pat_refs(¶m_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>( cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, diff --git a/clippy_lints/src/methods/unnecessary_fold.rs b/clippy_lints/src/methods/unnecessary_fold.rs new file mode 100644 index 00000000000..a26443f4ee9 --- /dev/null +++ b/clippy_lints/src/methods/unnecessary_fold.rs @@ -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(¶m_a.pat).kind; + if let PatKind::Binding(_, second_arg_id, second_arg_ident, _) = strip_pat_refs(¶m_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) + }, + _ => (), + } + } +} |
