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

…`then_some().unwrap_or_else()`
rust-lang · Feb 21, 2025 · 6c6ffd2 · 6c6ffd2
1 parent 649cef0
commit 6c6ffd2
Show file tree

Hide file tree

Showing 5 changed files with 111 additions and 20 deletions.
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
@@ -5398,7 +5398,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");
                         },
                         _ => {},
                     }
@@ -5417,6 +5417,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");
                         },

diff --git a/clippy_lints/src/methods/obfuscated_if_else.rs b/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,27 @@ 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,
+        };
+
+        // FIXME: Add `unwrap_or_else` symbol
+        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)
+            },
+            "unwrap_or_else" if let ExprKind::Path(_) = unwrap_arg.kind => {
+                snippet_with_applicability(cx, unwrap_arg.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

diff --git a/tests/ui/obfuscated_if_else.fixed b/tests/ui/obfuscated_if_else.fixed
@@ -1,5 +1,10 @@
 #![warn(clippy::obfuscated_if_else)]
-#![allow(clippy::unnecessary_lazy_evaluations, clippy::unit_arg, clippy::unused_unit)]
+#![allow(
+    clippy::unnecessary_lazy_evaluations,
+    clippy::unit_arg,
+    clippy::unused_unit,
+    clippy::unwrap_or_default
+)]
 
 fn main() {
     if true { "a" } else { "b" };
@@ -24,6 +29,23 @@ 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
+
+    if true { 1 } else { Default::default() };
+    //~^ obfuscated_if_else
+
+    let partial = true.then_some(1);
+    partial.unwrap_or_else(|| n * 2); // not lint
 }
 
 fn issue11141() {

diff --git a/tests/ui/obfuscated_if_else.rs b/tests/ui/obfuscated_if_else.rs
@@ -1,5 +1,10 @@
 #![warn(clippy::obfuscated_if_else)]
-#![allow(clippy::unnecessary_lazy_evaluations, clippy::unit_arg, clippy::unused_unit)]
+#![allow(
+    clippy::unnecessary_lazy_evaluations,
+    clippy::unit_arg,
+    clippy::unused_unit,
+    clippy::unwrap_or_default
+)]
 
 fn main() {
     true.then_some("a").unwrap_or("b");
@@ -24,6 +29,23 @@ 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
+
+    true.then_some(1).unwrap_or_else(Default::default);
+    //~^ obfuscated_if_else
+
+    let partial = true.then_some(1);
+    partial.unwrap_or_else(|| n * 2); // not lint
 }
 
 fn issue11141() {

diff --git a/tests/ui/obfuscated_if_else.stderr b/tests/ui/obfuscated_if_else.stderr
@@ -1,5 +1,5 @@
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:5:5
+  --> tests/ui/obfuscated_if_else.rs:10:5
    |
 LL |     true.then_some("a").unwrap_or("b");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }`
@@ -8,82 +8,112 @@ LL |     true.then_some("a").unwrap_or("b");
    = help: to override `-D warnings` add `#[allow(clippy::obfuscated_if_else)]`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:8:5
+  --> tests/ui/obfuscated_if_else.rs:13:5
    |
 LL |     true.then(|| "a").unwrap_or("b");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:12:5
+  --> tests/ui/obfuscated_if_else.rs:17:5
    |
 LL |     (a == 1).then_some("a").unwrap_or("b");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:15:5
+  --> tests/ui/obfuscated_if_else.rs:20:5
    |
 LL |     (a == 1).then(|| "a").unwrap_or("b");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:22:5
+  --> tests/ui/obfuscated_if_else.rs:27:5
    |
 LL |     true.then_some(a += 1).unwrap_or(());
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { a += 1 } else { () }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:25:5
+  --> tests/ui/obfuscated_if_else.rs:30:5
    |
 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:34: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:36: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:38: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:41: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:44:5
+   |
+LL |     true.then_some(1).unwrap_or_else(Default::default);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 1 } else { Default::default() }`
+
+error: this method chain can be written more clearly with `if .. else ..`
+  --> tests/ui/obfuscated_if_else.rs:53: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:57: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:57: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:57: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:63: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:67: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:71: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:75: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 19 previous errors