From 11d2af7e9694e8e63000e33f77d4926e936e3822 Mon Sep 17 00:00:00 2001
From: Yusuke Tanaka <yusuktan@maguro.dev>
Date: Fri, 12 Mar 2021 20:46:46 +0900
Subject: [PATCH] Improve suggestion and make it work for macros

---
 clippy_lints/src/if_then_some_else_none.rs | 18 +++++++++---
 tests/ui/if_then_some_else_none.rs         | 16 ++++++++++
 tests/ui/if_then_some_else_none.stderr     | 34 ++++++++++++++++++++--
 3 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/clippy_lints/src/if_then_some_else_none.rs b/clippy_lints/src/if_then_some_else_none.rs
index 569a7f06f..a527f51b1 100644
--- a/clippy_lints/src/if_then_some_else_none.rs
+++ b/clippy_lints/src/if_then_some_else_none.rs
@@ -82,12 +82,22 @@ impl LateLintPass<'_> for IfThenSomeElseNone {
             if let ExprKind::Path(ref els_call_qpath) = els_expr.kind;
             if utils::match_qpath(els_call_qpath, &utils::paths::OPTION_NONE);
             then {
-                let cond_snip = utils::snippet(cx, cond.span, "[condition]");
-                let arg_snip = utils::snippet(cx, then_arg.span, "");
+                let cond_snip = utils::snippet_with_macro_callsite(cx, cond.span, "[condition]");
+                let cond_snip = if matches!(cond.kind, ExprKind::Unary(_, _) | ExprKind::Binary(_, _, _)) {
+                    format!("({})", cond_snip)
+                } else {
+                    cond_snip.into_owned()
+                };
+                let arg_snip = utils::snippet_with_macro_callsite(cx, then_arg.span, "");
+                let closure_body = if then_block.stmts.is_empty() {
+                    arg_snip.into_owned()
+                } else {
+                    format!("{{ /* snippet */ {} }}", arg_snip)
+                };
                 let help = format!(
-                    "consider using `bool::then` like: `{}.then(|| {{ /* snippet */ {} }})`",
+                    "consider using `bool::then` like: `{}.then(|| {})`",
                     cond_snip,
-                    arg_snip,
+                    closure_body,
                 );
                 utils::span_lint_and_help(
                     cx,
diff --git a/tests/ui/if_then_some_else_none.rs b/tests/ui/if_then_some_else_none.rs
index 14a5fe762..54789bf93 100644
--- a/tests/ui/if_then_some_else_none.rs
+++ b/tests/ui/if_then_some_else_none.rs
@@ -10,6 +10,22 @@ fn main() {
         None
     };
 
+    // Should issue an error when macros are used.
+    let _ = if matches!(true, true) {
+        println!("true!");
+        Some(matches!(true, false))
+    } else {
+        None
+    };
+
+    // Should issue an error. Binary expression `o < 32` should be parenthesized.
+    let x = Some(5);
+    let _ = x.and_then(|o| if o < 32 { Some(o) } else { None });
+
+    // Should issue an error. Unary expression `!x` should be parenthesized.
+    let x = true;
+    let _ = if !x { Some(0) } else { None };
+
     // Should not issue an error since the `else` block has a statement besides `None`.
     let _ = if foo() {
         println!("true!");
diff --git a/tests/ui/if_then_some_else_none.stderr b/tests/ui/if_then_some_else_none.stderr
index 722c52b1c..8cb22d569 100644
--- a/tests/ui/if_then_some_else_none.stderr
+++ b/tests/ui/if_then_some_else_none.stderr
@@ -14,7 +14,37 @@ LL | |     };
    = help: consider using `bool::then` like: `foo().then(|| { /* snippet */ "foo" })`
 
 error: this could be simplified with `bool::then`
-  --> $DIR/if_then_some_else_none.rs:66:13
+  --> $DIR/if_then_some_else_none.rs:14:13
+   |
+LL |       let _ = if matches!(true, true) {
+   |  _____________^
+LL | |         println!("true!");
+LL | |         Some(matches!(true, false))
+LL | |     } else {
+LL | |         None
+LL | |     };
+   | |_____^
+   |
+   = help: consider using `bool::then` like: `matches!(true, true).then(|| { /* snippet */ matches!(true, false) })`
+
+error: this could be simplified with `bool::then`
+  --> $DIR/if_then_some_else_none.rs:23:28
+   |
+LL |     let _ = x.and_then(|o| if o < 32 { Some(o) } else { None });
+   |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using `bool::then` like: `(o < 32).then(|| o)`
+
+error: this could be simplified with `bool::then`
+  --> $DIR/if_then_some_else_none.rs:27:13
+   |
+LL |     let _ = if !x { Some(0) } else { None };
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using `bool::then` like: `(!x).then(|| 0)`
+
+error: this could be simplified with `bool::then`
+  --> $DIR/if_then_some_else_none.rs:82:13
    |
 LL |       let _ = if foo() {
    |  _____________^
@@ -27,5 +57,5 @@ LL | |     };
    |
    = help: consider using `bool::then` like: `foo().then(|| { /* snippet */ 150 })`
 
-error: aborting due to 2 previous errors
+error: aborting due to 5 previous errors