extend obfuscated_if_else to support then().unwrap_or_else() and then_some().unwrap_or_else()

Author: lapla-cogito

clippy_lints/src/methods/mod.rs

@@ -5367,7 +5367,7 @@ impl Methods {
                             option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span, &self.msrv);
                         },
                         Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => {
-                            obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method);
+                            obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method, "unwrap_or");
                         },
                         _ => {},
                     }
@@ -5386,6 +5386,9 @@ impl Methods {
                     match method_call(recv) {
                         Some(("map", recv, [map_arg], _, _))
                             if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, &self.msrv) => {},
+                        Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => {
+                            obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method, "unwrap_or_else");
+                        },
                         _ => {
                             unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or");
                         },

clippy_lints/src/methods/obfuscated_if_else.rs

@@ -16,6 +16,7 @@ pub(super) fn check<'tcx>(
     then_arg: &'tcx hir::Expr<'_>,
     unwrap_arg: &'tcx hir::Expr<'_>,
     then_method_name: &str,
+    unwrap_method_name: &str,
 ) {
     let recv_ty = cx.typeck_results().expr_ty(then_recv);
 
@@ -32,14 +33,23 @@ pub(super) fn check<'tcx>(
                 snippet_with_applicability(cx, body.value.span, "..", &mut applicability)
             },
             "then_some" => snippet_with_applicability(cx, then_arg.span, "..", &mut applicability),
-            _ => String::new().into(),
+            _ => return,
+        };
+
+        let els = match unwrap_method_name {
+            "unwrap_or" => snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability),
+            "unwrap_or_else" if let ExprKind::Closure(closure) = unwrap_arg.kind => {
+                let body = cx.tcx.hir().body(closure.body);
+                snippet_with_applicability(cx, body.value.span, "..", &mut applicability)
+            },
+            _ => return,
         };
 
         let sugg = format!(
             "if {} {{ {} }} else {{ {} }}",
             Sugg::hir_with_applicability(cx, then_recv, "..", &mut applicability),
             if_then,
-            snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability)
+            els
         );
 
         // To be parsed as an expression, the `if { … } else { … }` as the left operand of a binary operator

tests/ui/obfuscated_if_else.fixed

@@ -24,6 +24,20 @@ fn main() {
 
     if true { () } else { a += 2 };
     //~^ obfuscated_if_else
+
+    let mut n = 1;
+    if true { n = 1 } else { n = 2 };
+    //~^ obfuscated_if_else
+    if true { 1 } else { n * 2 };
+    //~^ obfuscated_if_else
+    if true { n += 1 } else { () };
+    //~^ obfuscated_if_else
+
+    let _ = if true { 1 } else { n * 2 };
+    //~^ obfuscated_if_else
+
+    let partial = true.then_some(1);
+    partial.unwrap_or_else(|| n * 2); // not lint
 }
 
 fn issue11141() {

tests/ui/obfuscated_if_else.rs

@@ -24,6 +24,20 @@ fn main() {
 
     true.then_some(()).unwrap_or(a += 2);
     //~^ obfuscated_if_else
+
+    let mut n = 1;
+    true.then(|| n = 1).unwrap_or_else(|| n = 2);
+    //~^ obfuscated_if_else
+    true.then_some(1).unwrap_or_else(|| n * 2);
+    //~^ obfuscated_if_else
+    true.then_some(n += 1).unwrap_or_else(|| ());
+    //~^ obfuscated_if_else
+
+    let _ = true.then_some(1).unwrap_or_else(|| n * 2);
+    //~^ obfuscated_if_else
+
+    let partial = true.then_some(1);
+    partial.unwrap_or_else(|| n * 2); // not lint
 }
 
 fn issue11141() {

tests/ui/obfuscated_if_else.stderr

@@ -38,52 +38,76 @@ LL |     true.then_some(()).unwrap_or(a += 2);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { () } else { a += 2 }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:31:13
+  --> tests/ui/obfuscated_if_else.rs:29:5
+   |
+LL |     true.then(|| n = 1).unwrap_or_else(|| n = 2);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { n = 1 } else { n = 2 }`
+
+error: this method chain can be written more clearly with `if .. else ..`
+  --> tests/ui/obfuscated_if_else.rs:31:5
+   |
+LL |     true.then_some(1).unwrap_or_else(|| n * 2);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 1 } else { n * 2 }`
+
+error: this method chain can be written more clearly with `if .. else ..`
+  --> tests/ui/obfuscated_if_else.rs:33:5
+   |
+LL |     true.then_some(n += 1).unwrap_or_else(|| ());
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { n += 1 } else { () }`
+
+error: this method chain can be written more clearly with `if .. else ..`
+  --> tests/ui/obfuscated_if_else.rs:36:13
+   |
+LL |     let _ = true.then_some(1).unwrap_or_else(|| n * 2);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 1 } else { n * 2 }`
+
+error: this method chain can be written more clearly with `if .. else ..`
+  --> tests/ui/obfuscated_if_else.rs:45:13
    |
 LL |     let _ = true.then_some(40).unwrap_or(17) | 2;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(if true { 40 } else { 17 })`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:35:13
+  --> tests/ui/obfuscated_if_else.rs:49:13
    |
 LL |     let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(if true { 30 } else { 17 })`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:35:48
+  --> tests/ui/obfuscated_if_else.rs:49:48
    |
 LL |     let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1);
    |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 2 } else { 3 }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:35:81
+  --> tests/ui/obfuscated_if_else.rs:49:81
    |
 LL |     let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1);
    |                                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 10 } else { 1 }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:41:17
+  --> tests/ui/obfuscated_if_else.rs:55:17
    |
 LL |     let _ = 2 | true.then_some(40).unwrap_or(17);
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 40 } else { 17 }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:45:13
+  --> tests/ui/obfuscated_if_else.rs:59:13
    |
 LL |     let _ = true.then_some(42).unwrap_or(17) as u8;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 42 } else { 17 }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:49:14
+  --> tests/ui/obfuscated_if_else.rs:63:14
    |
 LL |     let _ = *true.then_some(&42).unwrap_or(&17);
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { &42 } else { &17 }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:53:14
+  --> tests/ui/obfuscated_if_else.rs:67:14
    |
 LL |     let _ = *true.then_some(&42).unwrap_or(&17) as u8;
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { &42 } else { &17 }`
 
-error: aborting due to 14 previous errors
+error: aborting due to 18 previous errors