Catch function calls in argument lists, add tests that tuples don't get linted

4 files changed, 52 insertions, 37 deletions

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index ed8eaba75d1..463ef48f62c 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -2744,12 +2744,8 @@ fn lint_lazy_eval<'a, 'tcx>(
         paths.iter().any(|candidate| match_qpath(path, candidate))
     }
 
-    if let hir::ExprKind::Closure(_, _, eid, _, _) = args[1].kind {
-        let body = cx.tcx.hir().body(eid);
-        let ex = &body.value;
-        let params = &body.params;
-
-        let simplify = match ex.kind {
+    fn can_simplify(expr: &hir::Expr<'_>, params: &[hir::Param<'_>], variant_calls: bool) -> bool {
+        match expr.kind {
             // Closures returning literals can be unconditionally simplified
             hir::ExprKind::Lit(_) => true,
 
@@ -2767,15 +2763,16 @@ fn lint_lazy_eval<'a, 'tcx>(
             hir::ExprKind::Field(ref object, _) => !expr_uses_argument(object, params),
 
             // Paths can be simplified if the root is not the argument, this also covers None
-            hir::ExprKind::Path(_) => !expr_uses_argument(ex, params),
+            hir::ExprKind::Path(_) => !expr_uses_argument(expr, params),
 
             // Calls to Some, Ok, Err can be considered literals if they don't derive an argument
             hir::ExprKind::Call(ref func, ref args) => if_chain! {
-                if allow_variant_calls; // Disable lint when rules conflict with bind_instead_of_map
+                if variant_calls; // Disable lint when rules conflict with bind_instead_of_map
                 if let hir::ExprKind::Path(ref path) = func.kind;
                 if match_any_qpath(path, &[&["Some"], &["Ok"], &["Err"]]);
                 then {
-                    !args.iter().any(|arg| expr_uses_argument(arg, params))
+                    // Recursively check all arguments
+                    args.iter().all(|arg| can_simplify(arg, params, variant_calls))
                 } else {
                     false
                 }
@@ -2784,9 +2781,15 @@ fn lint_lazy_eval<'a, 'tcx>(
             // For anything more complex than the above, a closure is probably the right solution,
             // or the case is handled by an other lint
             _ => false,
-        };
+        }
+    }
+
+    if let hir::ExprKind::Closure(_, _, eid, _, _) = args[1].kind {
+        let body = cx.tcx.hir().body(eid);
+        let ex = &body.value;
+        let params = &body.params;
 
-        if simplify {
+        if can_simplify(ex, params, allow_variant_calls) {
             let msg = if is_option {
                 "unnecessary closure used to substitute value for `Option::None`"
             } else {
diff --git a/tests/ui/unnecessary_lazy_eval.fixed b/tests/ui/unnecessary_lazy_eval.fixed
index c806cf8dce4..7f9d90a8569 100644
--- a/tests/ui/unnecessary_lazy_eval.fixed
+++ b/tests/ui/unnecessary_lazy_eval.fixed
@@ -25,9 +25,12 @@ fn main() {
     let ext_arr: [usize; 1] = [2];
     let ext_str = SomeStruct { some_field: 10 };
 
-    // Should lint - Option
     let mut opt = Some(42);
     let ext_opt = Some(42);
+    let nested_opt = Some(Some(42));
+    let nested_tuple_opt = Some(Some((42, 43)));
+
+    // Should lint - Option
     let _ = opt.unwrap_or(2);
     let _ = opt.unwrap_or(astronomers_pi);
     let _ = opt.unwrap_or(ext_str.some_field);
@@ -56,6 +59,9 @@ fn main() {
 
     // Should not lint - Option
     let _ = opt.unwrap_or_else(|| ext_str.return_some_field());
+    let _ = nested_opt.unwrap_or_else(|| Some(some_call()));
+    let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2)));
+    let _ = nested_tuple_opt.unwrap_or_else(|| Some((some_call(), some_call())));
     let _ = opt.or_else(some_call);
     let _ = opt.or_else(|| some_call());
     let _: Result<usize, usize> = opt.ok_or_else(|| some_call());
diff --git a/tests/ui/unnecessary_lazy_eval.rs b/tests/ui/unnecessary_lazy_eval.rs
index dfc6d3ba573..ca8238d6dcf 100644
--- a/tests/ui/unnecessary_lazy_eval.rs
+++ b/tests/ui/unnecessary_lazy_eval.rs
@@ -25,9 +25,12 @@ fn main() {
     let ext_arr: [usize; 1] = [2];
     let ext_str = SomeStruct { some_field: 10 };
 
-    // Should lint - Option
     let mut opt = Some(42);
     let ext_opt = Some(42);
+    let nested_opt = Some(Some(42));
+    let nested_tuple_opt = Some(Some((42, 43)));
+
+    // Should lint - Option
     let _ = opt.unwrap_or_else(|| 2);
     let _ = opt.unwrap_or_else(|| astronomers_pi);
     let _ = opt.unwrap_or_else(|| ext_str.some_field);
@@ -56,6 +59,9 @@ fn main() {
 
     // Should not lint - Option
     let _ = opt.unwrap_or_else(|| ext_str.return_some_field());
+    let _ = nested_opt.unwrap_or_else(|| Some(some_call()));
+    let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2)));
+    let _ = nested_tuple_opt.unwrap_or_else(|| Some((some_call(), some_call())));
     let _ = opt.or_else(some_call);
     let _ = opt.or_else(|| some_call());
     let _: Result<usize, usize> = opt.ok_or_else(|| some_call());
diff --git a/tests/ui/unnecessary_lazy_eval.stderr b/tests/ui/unnecessary_lazy_eval.stderr
index 15591817540..b8ec654e5c7 100644
--- a/tests/ui/unnecessary_lazy_eval.stderr
+++ b/tests/ui/unnecessary_lazy_eval.stderr
@@ -1,5 +1,5 @@
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:31:13
+  --> $DIR/unnecessary_lazy_eval.rs:34:13
    |
 LL |     let _ = opt.unwrap_or_else(|| 2);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `opt.unwrap_or(2)`
@@ -7,139 +7,139 @@ LL |     let _ = opt.unwrap_or_else(|| 2);
    = note: `-D clippy::unnecessary-lazy-evaluation` implied by `-D warnings`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:32:13
+  --> $DIR/unnecessary_lazy_eval.rs:35:13
    |
 LL |     let _ = opt.unwrap_or_else(|| astronomers_pi);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `opt.unwrap_or(astronomers_pi)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:33:13
+  --> $DIR/unnecessary_lazy_eval.rs:36:13
    |
 LL |     let _ = opt.unwrap_or_else(|| ext_str.some_field);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `opt.unwrap_or(ext_str.some_field)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:34:13
+  --> $DIR/unnecessary_lazy_eval.rs:37:13
    |
 LL |     let _ = opt.unwrap_or_else(|| ext_arr[0]);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `opt.unwrap_or(ext_arr[0])`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:35:13
+  --> $DIR/unnecessary_lazy_eval.rs:38:13
    |
 LL |     let _ = opt.and_then(|_| ext_opt);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `and` instead: `opt.and(ext_opt)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:36:13
+  --> $DIR/unnecessary_lazy_eval.rs:39:13
    |
 LL |     let _ = opt.or_else(|| ext_opt);
    |             ^^^^^^^^^^^^^^^^^^^^^^^ help: Use `or` instead: `opt.or(ext_opt)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:37:13
+  --> $DIR/unnecessary_lazy_eval.rs:40:13
    |
 LL |     let _ = opt.or_else(|| None);
    |             ^^^^^^^^^^^^^^^^^^^^ help: Use `or` instead: `opt.or(None)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:38:13
+  --> $DIR/unnecessary_lazy_eval.rs:41:13
    |
 LL |     let _ = opt.get_or_insert_with(|| 2);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `get_or_insert` instead: `opt.get_or_insert(2)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:39:13
+  --> $DIR/unnecessary_lazy_eval.rs:42:13
    |
 LL |     let _ = opt.ok_or_else(|| 2);
    |             ^^^^^^^^^^^^^^^^^^^^ help: Use `ok_or` instead: `opt.ok_or(2)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:40:13
+  --> $DIR/unnecessary_lazy_eval.rs:43:13
    |
 LL |     let _ = opt.ok_or_else(|| ext_arr[0]);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `ok_or` instead: `opt.ok_or(ext_arr[0])`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:43:13
+  --> $DIR/unnecessary_lazy_eval.rs:46:13
    |
 LL |     let _ = Some(10).unwrap_or_else(|| 2);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `Some(10).unwrap_or(2)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:44:13
+  --> $DIR/unnecessary_lazy_eval.rs:47:13
    |
 LL |     let _ = Some(10).and_then(|_| ext_opt);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `and` instead: `Some(10).and(ext_opt)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:45:28
+  --> $DIR/unnecessary_lazy_eval.rs:48:28
    |
 LL |     let _: Option<usize> = None.or_else(|| ext_opt);
    |                            ^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `or` instead: `None.or(ext_opt)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:46:13
+  --> $DIR/unnecessary_lazy_eval.rs:49:13
    |
 LL |     let _ = None.get_or_insert_with(|| 2);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `get_or_insert` instead: `None.get_or_insert(2)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:47:35
+  --> $DIR/unnecessary_lazy_eval.rs:50:35
    |
 LL |     let _: Result<usize, usize> = None.ok_or_else(|| 2);
    |                                   ^^^^^^^^^^^^^^^^^^^^^ help: Use `ok_or` instead: `None.ok_or(2)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:48:28
+  --> $DIR/unnecessary_lazy_eval.rs:51:28
    |
 LL |     let _: Option<usize> = None.or_else(|| None);
    |                            ^^^^^^^^^^^^^^^^^^^^^ help: Use `or` instead: `None.or(None)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:51:13
+  --> $DIR/unnecessary_lazy_eval.rs:54:13
    |
 LL |     let _ = deep.0.unwrap_or_else(|| 2);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `deep.0.unwrap_or(2)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:52:13
+  --> $DIR/unnecessary_lazy_eval.rs:55:13
    |
 LL |     let _ = deep.0.and_then(|_| ext_opt);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `and` instead: `deep.0.and(ext_opt)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:53:13
+  --> $DIR/unnecessary_lazy_eval.rs:56:13
    |
 LL |     let _ = deep.0.or_else(|| None);
    |             ^^^^^^^^^^^^^^^^^^^^^^^ help: Use `or` instead: `deep.0.or(None)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:54:13
+  --> $DIR/unnecessary_lazy_eval.rs:57:13
    |
 LL |     let _ = deep.0.get_or_insert_with(|| 2);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `get_or_insert` instead: `deep.0.get_or_insert(2)`
 
 error: unnecessary closure used to substitute value for `Option::None`
-  --> $DIR/unnecessary_lazy_eval.rs:55:13
+  --> $DIR/unnecessary_lazy_eval.rs:58:13
    |
 LL |     let _ = deep.0.ok_or_else(|| 2);
    |             ^^^^^^^^^^^^^^^^^^^^^^^ help: Use `ok_or` instead: `deep.0.ok_or(2)`
 
 error: unnecessary closure used to substitute value for `Result::Err`
-  --> $DIR/unnecessary_lazy_eval.rs:78:13
+  --> $DIR/unnecessary_lazy_eval.rs:84:13
    |
 LL |     let _ = res2.unwrap_or_else(|_| 2);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `res2.unwrap_or(2)`
 
 error: unnecessary closure used to substitute value for `Result::Err`
-  --> $DIR/unnecessary_lazy_eval.rs:79:13
+  --> $DIR/unnecessary_lazy_eval.rs:85:13
    |
 LL |     let _ = res2.unwrap_or_else(|_| astronomers_pi);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `res2.unwrap_or(astronomers_pi)`
 
 error: unnecessary closure used to substitute value for `Result::Err`
-  --> $DIR/unnecessary_lazy_eval.rs:80:13
+  --> $DIR/unnecessary_lazy_eval.rs:86:13
    |
 LL |     let _ = res2.unwrap_or_else(|_| ext_str.some_field);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `res2.unwrap_or(ext_str.some_field)`