Tidy using if_chain and snippet function. Actually check that the initial fold value...

diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs
index 4277b4b15d3f853c707e989c03fe745f4dedb13c..36e6fb4b1a29a11b6ddd242a0ff45181908dd0f5 100644 (file)
--- a/clippy_lints/src/methods.rs
+++ b/clippy_lints/src/methods.rs
@@ -1139,53 +1139,42 @@ fn lint_fold_any(cx: &LateContext, expr: &hir::Expr, fold_args: &[hir::Expr]) {
     assert!(fold_args.len() == 3,
         "Expected fold_args to have three entries - the receiver, the initial value and the closure");
 
-    if let hir::ExprLit(ref lit) = fold_args[1].node {
-        if let ast::LitKind::Bool(ref b) = lit.node {
-            let initial_value = b.to_string();
+    if_chain! {
+        // Check if the initial value for the fold is the literal `false`
+        if let hir::ExprLit(ref lit) = fold_args[1].node;
+        if lit.node == ast::LitKind::Bool(false);
 
-            if let hir::ExprClosure(_, ref decl, body_id, _, _) = fold_args[2].node {
-                let closure_body = cx.tcx.hir.body(body_id);
-                let closure_expr = remove_blocks(&closure_body.value);
+        // Extract the body of the closure passed to fold
+        if let hir::ExprClosure(_, _, body_id, _, _) = fold_args[2].node;
+        let closure_body = cx.tcx.hir.body(body_id);
+        let closure_expr = remove_blocks(&closure_body.value);
 
-                let first_arg = &closure_body.arguments[0];
-                let arg_ident = get_arg_name(&first_arg.pat).unwrap();
+        // Extract the names of the two arguments to the closure
+        if let Some(first_arg_ident) = get_arg_name(&closure_body.arguments[0].pat);
+        if let Some(second_first_arg_ident) = get_arg_name(&closure_body.arguments[1].pat);
 
-                let second_arg = &closure_body.arguments[1];
-                let second_arg_ident = get_arg_name(&second_arg.pat).unwrap();
+        // Check if the closure body is of the form `acc || some_expr(x)`
+        if let hir::ExprBinary(ref bin_op, ref left_expr, ref right_expr) = closure_expr.node;
+        if bin_op.node == hir::BinOp_::BiOr;
+        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 let hir::ExprBinary(ref bin_op, ref left_expr, ref right_expr) = closure_expr.node {
-                    if bin_op.node != hir::BinOp_::BiOr {
-                        return;
-                    }
-                    if let hir::ExprPath(hir::QPath::Resolved(None, ref path)) = left_expr.node {
-                        if path.segments.len() == 1 {
-                            let left_name =  &path.segments[0].name;
-                            let right_source = cx.sess().codemap().span_to_snippet(right_expr.span).unwrap();
-
-                            if left_name == &arg_ident {
-                                span_lint(
-                                    cx,
-                                    FOLD_ANY,
-                                    expr.span,
-                                    // TODO: don't suggest .any(|x| f(x)) if we can suggest .any(f)
-                                    // TODO: these have difference semantics - original code might be deliberately avoiding short-circuiting
-                                    &format!(
-                                        ".fold(false, |{f}, {s}| {f} || {r})) is more succinctly expressed as .any(|{s}| {r})",
-                                        f = arg_ident,
-                                        s = second_arg_ident,
-                                        r = right_source
-                                    ),
-                                );
-                            }
-                        }
-                    }
-                }
-            } else{
-                panic!("DONOTMERGE: can this happen?");
-            }
+        then {
+            let right_source = snippet(cx, right_expr.span, "EXPR");
+
+            span_lint(
+                cx,
+                FOLD_ANY,
+                expr.span,
+                // TODO: don't suggest .any(|x| f(x)) if we can suggest .any(f)
+                &format!(
+                    ".fold(false, |{f}, {s}| {f} || {r})) is more succinctly expressed as .any(|{s}| {r})",
+                    f = first_arg_ident,
+                    s = second_first_arg_ident,
+                    r = right_source
+                ),
+            );
         }
-    } else {
-        return;
     }
 }
 

diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs
index 2dcc085add322bb9702bd0651c6a840375a91769..0dd4ff47fa9097fcafaf75ee68876ebbe5f8c5fa 100644 (file)
--- a/tests/ui/methods.rs
+++ b/tests/ui/methods.rs
@@ -390,6 +390,11 @@ fn fold_any() {
     let _ = (0..3).fold(false, |acc, x| acc || x > 2);
 }
 
+/// Checks implementation of the `FOLD_ANY` lint
+fn fold_any_ignore_initial_value_of_true() {
+    let _ = (0..3).fold(true, |acc, x| acc || x > 2);
+}
+
 #[allow(similar_names)]
 fn main() {
     let opt = Some(0);

diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr
index 768fbd1df54569180fc25a576f220894855db1c9..ef24e4a8e22de892c807657ea97447b1b93c5d5f 100644 (file)
--- a/tests/ui/methods.stderr
+++ b/tests/ui/methods.stderr
@@ -502,9 +502,9 @@ error: .fold(false, |acc, x| acc || x > 2)) is more succinctly expressed as .any
     = note: `-D fold-any` implied by `-D warnings`
 
 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:396:13
+   --> $DIR/methods.rs:401:13
     |
-396 |     let _ = opt.unwrap();
+401 |     let _ = opt.unwrap();
     |             ^^^^^^^^^^^^
     |
     = note: `-D option-unwrap-used` implied by `-D warnings`