suggest passing function instead of calling it in [`option_if_let_else`]

4 files changed, 93 insertions, 29 deletions

diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs
index a7a7f4fd8fa..156b95d644a 100644
--- a/clippy_lints/src/option_if_let_else.rs
+++ b/clippy_lints/src/option_if_let_else.rs
@@ -165,6 +165,13 @@ fn try_get_option_occurrence<'tcx>(
             }
 
             let mut app = Applicability::Unspecified;
+
+            let (none_body, is_argless_call) = if let Some(call_expr) = try_get_argless_call_expr(none_body) {
+                (call_expr, true)
+            } else {
+                (none_body, false)
+            };
+
             return Some(OptionOccurrence {
                 option: format_option_in_sugg(
                     Sugg::hir_with_context(cx, cond_expr, ctxt, "..", &mut app),
@@ -178,7 +185,7 @@ fn try_get_option_occurrence<'tcx>(
                 ),
                 none_expr: format!(
                     "{}{}",
-                    if method_sugg == "map_or" { "" } else if is_result { "|_| " } else { "|| "},
+                    if method_sugg == "map_or" || is_argless_call { "" } else if is_result { "|_| " } else { "|| "},
                     Sugg::hir_with_context(cx, none_body, ctxt, "..", &mut app),
                 ),
             });
@@ -188,6 +195,16 @@ fn try_get_option_occurrence<'tcx>(
     None
 }
 
+/// Gets the call expr iff it does not have any args and was not from macro expansion.
+fn try_get_argless_call_expr<'tcx>(expr: &Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+    if !expr.span.from_expansion() &&
+        let ExprKind::Call(call_expr, []) = expr.kind
+    {
+        return Some(call_expr);
+    }
+    None
+}
+
 fn try_get_inner_pat_and_is_result<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<(&'tcx Pat<'tcx>, bool)> {
     if let PatKind::TupleStruct(ref qpath, [inner_pat], ..) = pat.kind {
         let res = cx.qpath_res(qpath, pat.hir_id);
diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed
index c3415a7df9e..f0113ca696e 100644
--- a/tests/ui/option_if_let_else.fixed
+++ b/tests/ui/option_if_let_else.fixed
@@ -1,7 +1,6 @@
 #![warn(clippy::option_if_let_else)]
 #![allow(
     unused_tuple_struct_fields,
-    clippy::redundant_closure,
     clippy::ref_option_ref,
     clippy::equatable_if_let,
     clippy::let_unit_value,
@@ -52,7 +51,7 @@ fn impure_else(arg: Option<i32>) {
         println!("return 1");
         1
     };
-    let _ = arg.map_or_else(|| side_effect(), |x| x);
+    let _ = arg.map_or_else(side_effect, |x| x);
 }
 
 fn test_map_or_else(arg: Option<u32>) {
@@ -224,3 +223,17 @@ mod issue10729 {
     fn do_something(_value: &str) {}
     fn do_something2(_value: &mut str) {}
 }
+
+fn issue11429() {
+    use std::collections::HashMap;
+
+    macro_rules! new_map {
+        () => {{ HashMap::new() }};
+    }
+
+    let opt: Option<HashMap<u8, u8>> = None;
+
+    let mut _hashmap = opt.as_ref().map_or_else(HashMap::new, |hm| hm.clone());
+
+    let mut _hm = opt.as_ref().map_or_else(|| new_map!(), |hm| hm.clone());
+}
diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs
index 86537f62057..18b7af44392 100644
--- a/tests/ui/option_if_let_else.rs
+++ b/tests/ui/option_if_let_else.rs
@@ -1,7 +1,6 @@
 #![warn(clippy::option_if_let_else)]
 #![allow(
     unused_tuple_struct_fields,
-    clippy::redundant_closure,
     clippy::ref_option_ref,
     clippy::equatable_if_let,
     clippy::let_unit_value,
@@ -271,3 +270,21 @@ mod issue10729 {
     fn do_something(_value: &str) {}
     fn do_something2(_value: &mut str) {}
 }
+
+fn issue11429() {
+    use std::collections::HashMap;
+
+    macro_rules! new_map {
+        () => {{ HashMap::new() }};
+    }
+
+    let opt: Option<HashMap<u8, u8>> = None;
+
+    let mut _hashmap = if let Some(hm) = &opt {
+        hm.clone()
+    } else {
+        HashMap::new()
+    };
+
+    let mut _hm = if let Some(hm) = &opt { hm.clone() } else { new_map!() };
+}
diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr
index aa2da217400..fe436a349c5 100644
--- a/tests/ui/option_if_let_else.stderr
+++ b/tests/ui/option_if_let_else.stderr
@@ -1,5 +1,5 @@
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:12:5
+  --> $DIR/option_if_let_else.rs:11:5
    |
 LL | /     if let Some(x) = string {
 LL | |         (true, x)
@@ -11,19 +11,19 @@ LL | |     }
    = note: `-D clippy::option-if-let-else` implied by `-D warnings`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:30:13
+  --> $DIR/option_if_let_else.rs:29:13
    |
 LL |     let _ = if let Some(s) = *string { s.len() } else { 0 };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `string.map_or(0, |s| s.len())`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:31:13
+  --> $DIR/option_if_let_else.rs:30:13
    |
 LL |     let _ = if let Some(s) = &num { s } else { &0 };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:32:13
+  --> $DIR/option_if_let_else.rs:31:13
    |
 LL |       let _ = if let Some(s) = &mut num {
    |  _____________^
@@ -43,13 +43,13 @@ LL ~     });
    |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:38:13
+  --> $DIR/option_if_let_else.rs:37:13
    |
 LL |     let _ = if let Some(ref s) = num { s } else { &0 };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:39:13
+  --> $DIR/option_if_let_else.rs:38:13
    |
 LL |       let _ = if let Some(mut s) = num {
    |  _____________^
@@ -69,7 +69,7 @@ LL ~     });
    |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:45:13
+  --> $DIR/option_if_let_else.rs:44:13
    |
 LL |       let _ = if let Some(ref mut s) = num {
    |  _____________^
@@ -89,7 +89,7 @@ LL ~     });
    |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:54:5
+  --> $DIR/option_if_let_else.rs:53:5
    |
 LL | /     if let Some(x) = arg {
 LL | |         let y = x * x;
@@ -108,7 +108,7 @@ LL +     })
    |
 
 error: use Option::map_or_else instead of an if let/else
-  --> $DIR/option_if_let_else.rs:67:13
+  --> $DIR/option_if_let_else.rs:66:13
    |
 LL |       let _ = if let Some(x) = arg {
    |  _____________^
@@ -117,10 +117,10 @@ LL | |     } else {
 LL | |         // map_or_else must be suggested
 LL | |         side_effect()
 LL | |     };
-   | |_____^ help: try: `arg.map_or_else(|| side_effect(), |x| x)`
+   | |_____^ help: try: `arg.map_or_else(side_effect, |x| x)`
 
 error: use Option::map_or_else instead of an if let/else
-  --> $DIR/option_if_let_else.rs:76:13
+  --> $DIR/option_if_let_else.rs:75:13
    |
 LL |       let _ = if let Some(x) = arg {
    |  _____________^
@@ -143,7 +143,7 @@ LL ~     }, |x| x * x * x * x);
    |
 
 error: use Option::map_or_else instead of an if let/else
-  --> $DIR/option_if_let_else.rs:109:13
+  --> $DIR/option_if_let_else.rs:108:13
    |
 LL | /             if let Some(idx) = s.find('.') {
 LL | |                 vec![s[..idx].to_string(), s[idx..].to_string()]
@@ -153,7 +153,7 @@ LL | |             }
    | |_____________^ help: try: `s.find('.').map_or_else(|| vec![s.to_string()], |idx| vec![s[..idx].to_string(), s[idx..].to_string()])`
 
 error: use Option::map_or_else instead of an if let/else
-  --> $DIR/option_if_let_else.rs:120:5
+  --> $DIR/option_if_let_else.rs:119:5
    |
 LL | /     if let Ok(binding) = variable {
 LL | |         println!("Ok {binding}");
@@ -172,13 +172,13 @@ LL +     })
    |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:142:13
+  --> $DIR/option_if_let_else.rs:141:13
    |
 LL |     let _ = if let Some(x) = optional { x + 2 } else { 5 };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:152:13
+  --> $DIR/option_if_let_else.rs:151:13
    |
 LL |       let _ = if let Some(x) = Some(0) {
    |  _____________^
@@ -200,13 +200,13 @@ LL ~         });
    |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:180:13
+  --> $DIR/option_if_let_else.rs:179:13
    |
 LL |     let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or(s.len(), |x| s.len() + x)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:184:13
+  --> $DIR/option_if_let_else.rs:183:13
    |
 LL |       let _ = if let Some(x) = Some(0) {
    |  _____________^
@@ -226,7 +226,7 @@ LL ~     });
    |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:223:13
+  --> $DIR/option_if_let_else.rs:222:13
    |
 LL |       let _ = match s {
    |  _____________^
@@ -236,7 +236,7 @@ LL | |     };
    | |_____^ help: try: `s.map_or(1, |string| string.len())`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:227:13
+  --> $DIR/option_if_let_else.rs:226:13
    |
 LL |       let _ = match Some(10) {
    |  _____________^
@@ -246,7 +246,7 @@ LL | |     };
    | |_____^ help: try: `Some(10).map_or(5, |a| a + 1)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:233:13
+  --> $DIR/option_if_let_else.rs:232:13
    |
 LL |       let _ = match res {
    |  _____________^
@@ -256,7 +256,7 @@ LL | |     };
    | |_____^ help: try: `res.map_or(1, |a| a + 1)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:237:13
+  --> $DIR/option_if_let_else.rs:236:13
    |
 LL |       let _ = match res {
    |  _____________^
@@ -266,13 +266,13 @@ LL | |     };
    | |_____^ help: try: `res.map_or(1, |a| a + 1)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:241:13
+  --> $DIR/option_if_let_else.rs:240:13
    |
 LL |     let _ = if let Ok(a) = res { a + 1 } else { 5 };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or(5, |a| a + 1)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:258:9
+  --> $DIR/option_if_let_else.rs:257:9
    |
 LL | /         match initial {
 LL | |             Some(value) => do_something(value),
@@ -281,7 +281,7 @@ LL | |         }
    | |_________^ help: try: `initial.as_ref().map_or({}, |value| do_something(value))`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:265:9
+  --> $DIR/option_if_let_else.rs:264:9
    |
 LL | /         match initial {
 LL | |             Some(value) => do_something2(value),
@@ -289,5 +289,22 @@ LL | |             None => {},
 LL | |         }
    | |_________^ help: try: `initial.as_mut().map_or({}, |value| do_something2(value))`
 
-error: aborting due to 23 previous errors
+error: use Option::map_or_else instead of an if let/else
+  --> $DIR/option_if_let_else.rs:283:24
+   |
+LL |       let mut _hashmap = if let Some(hm) = &opt {
+   |  ________________________^
+LL | |         hm.clone()
+LL | |     } else {
+LL | |         HashMap::new()
+LL | |     };
+   | |_____^ help: try: `opt.as_ref().map_or_else(HashMap::new, |hm| hm.clone())`
+
+error: use Option::map_or_else instead of an if let/else
+  --> $DIR/option_if_let_else.rs:289:19
+   |
+LL |     let mut _hm = if let Some(hm) = &opt { hm.clone() } else { new_map!() };
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.as_ref().map_or_else(|| new_map!(), |hm| hm.clone())`
+
+error: aborting due to 25 previous errors