trigger obfuscated_if_else for .then(..).unwrap_or(..) (rust-lang#14021)

part of rust-lang/rust-clippy#9100

The `obfuscated_if_else` lint currently only triggers for the pattern
`.then_some(..).unwrap_or(..)`, but there're other cases where this lint
should be triggered, one of which is `.then(..).unwrap_or(..)`.

changelog: [`obfuscated_if_else`]: trigger lint for the
`.then(..).unwrap_or(..)` pattern as well

Author: llogiq

clippy_lints/src/methods/mod.rs

@@ -2418,14 +2418,14 @@ declare_clippy_lint! {
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for usage of `.then_some(..).unwrap_or(..)`
+    /// Checks for unnecessary method chains that can be simplified into `if .. else ..`.
     ///
     /// ### Why is this bad?
     /// This can be written more clearly with `if .. else ..`
     ///
     /// ### Limitations
     /// This lint currently only looks for usages of
-    /// `.then_some(..).unwrap_or(..)`, but will be expanded
+    /// `.then_some(..).unwrap_or(..)` and `.then(..).unwrap_or(..)`, but will be expanded
     /// to account for similar patterns.
     ///
     /// ### Example
@@ -5276,8 +5276,8 @@ impl Methods {
                         Some(("map", m_recv, [m_arg], span, _)) => {
                             option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span, &self.msrv);
                         },
-                        Some(("then_some", t_recv, [t_arg], _, _)) => {
-                            obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg);
+                        Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => {
+                            obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method);
                         },
                         _ => {},
                     }

clippy_lints/src/methods/obfuscated_if_else.rs

@@ -1,8 +1,10 @@
 use super::OBFUSCATED_IF_ELSE;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::sugg::Sugg;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
+use rustc_hir::ExprKind;
 use rustc_lint::LateContext;
 
 pub(super) fn check<'tcx>(
@@ -11,28 +13,33 @@ pub(super) fn check<'tcx>(
     then_recv: &'tcx hir::Expr<'_>,
     then_arg: &'tcx hir::Expr<'_>,
     unwrap_arg: &'tcx hir::Expr<'_>,
+    then_method_name: &str,
 ) {
-    // something.then_some(blah).unwrap_or(blah)
-    // ^^^^^^^^^-then_recv ^^^^-then_arg   ^^^^- unwrap_arg
-    // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- expr
-
     let recv_ty = cx.typeck_results().expr_ty(then_recv);
 
     if recv_ty.is_bool() {
         let mut applicability = Applicability::MachineApplicable;
+        let if_then = match then_method_name {
+            "then" if let ExprKind::Closure(closure) = then_arg.kind => {
+                let body = cx.tcx.hir().body(closure.body);
+                snippet_with_applicability(cx, body.value.span, "..", &mut applicability)
+            },
+            "then_some" => snippet_with_applicability(cx, then_arg.span, "..", &mut applicability),
+            _ => String::new().into(),
+        };
+
         let sugg = format!(
             "if {} {{ {} }} else {{ {} }}",
-            snippet_with_applicability(cx, then_recv.span, "..", &mut applicability),
-            snippet_with_applicability(cx, then_arg.span, "..", &mut applicability),
+            Sugg::hir_with_applicability(cx, then_recv, "..", &mut applicability),
+            if_then,
             snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability)
         );
 
         span_lint_and_sugg(
             cx,
             OBFUSCATED_IF_ELSE,
             expr.span,
-            "use of `.then_some(..).unwrap_or(..)` can be written \
-            more clearly with `if .. else ..`",
+            "this method chain can be written more clearly with `if .. else ..`",
             "try",
             sugg,
             applicability,

tests/ui/obfuscated_if_else.fixed

@@ -1,5 +1,14 @@
 #![warn(clippy::obfuscated_if_else)]
+#![allow(clippy::unnecessary_lazy_evaluations)]
 
 fn main() {
     if true { "a" } else { "b" };
+    if true { "a" } else { "b" };
+
+    let a = 1;
+    if a == 1 { "a" } else { "b" };
+    if a == 1 { "a" } else { "b" };
+
+    let partial = (a == 1).then_some("a");
+    partial.unwrap_or("b"); // not lint
 }

tests/ui/obfuscated_if_else.rs

@@ -1,5 +1,14 @@
 #![warn(clippy::obfuscated_if_else)]
+#![allow(clippy::unnecessary_lazy_evaluations)]
 
 fn main() {
     true.then_some("a").unwrap_or("b");
+    true.then(|| "a").unwrap_or("b");
+
+    let a = 1;
+    (a == 1).then_some("a").unwrap_or("b");
+    (a == 1).then(|| "a").unwrap_or("b");
+
+    let partial = (a == 1).then_some("a");
+    partial.unwrap_or("b"); // not lint
 }

tests/ui/obfuscated_if_else.stderr

@@ -1,11 +1,29 @@
-error: use of `.then_some(..).unwrap_or(..)` can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:4:5
+error: this method chain can be written more clearly with `if .. else ..`
+  --> tests/ui/obfuscated_if_else.rs:5:5
    |
 LL |     true.then_some("a").unwrap_or("b");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }`
    |
    = note: `-D clippy::obfuscated-if-else` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::obfuscated_if_else)]`
 
-error: aborting due to 1 previous error
+error: this method chain can be written more clearly with `if .. else ..`
+  --> tests/ui/obfuscated_if_else.rs:6: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:9: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:10:5
+   |
+LL |     (a == 1).then(|| "a").unwrap_or("b");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }`
+
+error: aborting due to 4 previous errors