From 2132e5c58c5b9493fdfa28dc70fd6c7e3fe47bfa Mon Sep 17 00:00:00 2001 From: Phil Ellison Date: Mon, 22 Jan 2018 05:34:42 +0000 Subject: [PATCH] Fix unnecessary_fold bug --- clippy_lints/src/methods.rs | 11 +++++++++-- tests/ui/methods.rs | 3 +++ tests/ui/methods.stderr | 4 ++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index d9b61a9e5..e9b82ca0a 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -1141,6 +1141,13 @@ fn lint_unnecessary_fold(cx: &LateContext, expr: &hir::Expr, fold_args: &[hir::E assert!(fold_args.len() == 3, "Expected fold_args to have three entries - the receiver, the initial value and the closure"); + fn is_exactly_closure_param(expr: &hir::Expr, closure_param: ast::Name) -> bool { + if let hir::ExprPath(hir::QPath::Resolved(None, ref path)) = expr.node { + return path.segments.len() == 1 && &path.segments[0].name == &closure_param; + } + false + } + fn check_fold_with_op( cx: &LateContext, fold_args: &[hir::Expr], @@ -1162,8 +1169,8 @@ fn lint_unnecessary_fold(cx: &LateContext, expr: &hir::Expr, fold_args: &[hir::E if let Some(first_arg_ident) = get_arg_name(&closure_body.arguments[0].pat); if let Some(second_arg_ident) = get_arg_name(&closure_body.arguments[1].pat); - if let hir::ExprPath(hir::QPath::Resolved(None, ref path)) = left_expr.node; - if path.segments.len() == 1 && &path.segments[0].name == &first_arg_ident; + if is_exactly_closure_param(&*left_expr, first_arg_ident); + if replacement_has_args || is_exactly_closure_param(&*right_expr, second_arg_ident); then { // Span containing `.fold(...)` diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs index f7a4b39c7..4afab4c8b 100644 --- a/tests/ui/methods.rs +++ b/tests/ui/methods.rs @@ -419,6 +419,9 @@ fn unnecessary_fold_should_ignore() { let _ = (0..3).fold(true, |acc, x| x > 2 && acc); let _ = (0..3).fold(0, |acc, x| x + acc); let _ = (0..3).fold(1, |acc, x| x * acc); + + let _ = [(0..2), (0..3)].iter().fold(0, |a, b| a + b.len()); + let _ = [(0..2), (0..3)].iter().fold(1, |a, b| a * b.len()); } #[allow(similar_names)] diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr index 5d3015f5e..ef52d85c3 100644 --- a/tests/ui/methods.stderr +++ b/tests/ui/methods.stderr @@ -526,9 +526,9 @@ error: this `.fold` can be written more succinctly using another method | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.any(|x| x > 2)` error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message - --> $DIR/methods.rs:427:13 + --> $DIR/methods.rs:430:13 | -427 | let _ = opt.unwrap(); +430 | let _ = opt.unwrap(); | ^^^^^^^^^^^^ | = note: `-D option-unwrap-used` implied by `-D warnings`