Get the block content from the proper context (#15014)

The first statement of the block might have been in a different context
from the expression. Walk up to the right context to get bounds
properly.

Also, switch to `snippet_with_applicability()` since we know that we are
in the right context already.

changelog: [`if_then_some_else_none`]: emit well-formed suggestion, and
do not lint inside macros

Fixes rust-lang/rust-clippy#15005

4 files changed, 133 insertions, 16 deletions

diff --git a/clippy_lints/src/if_then_some_else_none.rs b/clippy_lints/src/if_then_some_else_none.rs
index 7158f9419c1..b50d91f1014 100644
--- a/clippy_lints/src/if_then_some_else_none.rs
+++ b/clippy_lints/src/if_then_some_else_none.rs
@@ -2,16 +2,16 @@ use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::eager_or_lazy::switch_to_eager_eval;
 use clippy_utils::msrvs::{self, Msrv};
-use clippy_utils::source::snippet_with_context;
+use clippy_utils::source::{snippet_with_applicability, snippet_with_context, walk_span_to_context};
 use clippy_utils::sugg::Sugg;
 use clippy_utils::{
     contains_return, expr_adjustment_requires_coercion, higher, is_else_clause, is_in_const_context, is_res_lang_ctor,
-    path_res, peel_blocks,
+    path_res, peel_blocks, sym,
 };
 use rustc_errors::Applicability;
 use rustc_hir::LangItem::{OptionNone, OptionSome};
 use rustc_hir::{Expr, ExprKind};
-use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
 
 declare_clippy_lint! {
@@ -71,21 +71,21 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone {
             && let ExprKind::Block(then_block, _) = then.kind
             && let Some(then_expr) = then_block.expr
             && let ExprKind::Call(then_call, [then_arg]) = then_expr.kind
-            && let ctxt = expr.span.ctxt()
-            && then_expr.span.ctxt() == ctxt
+            && !expr.span.from_expansion()
+            && !then_expr.span.from_expansion()
             && is_res_lang_ctor(cx, path_res(cx, then_call), OptionSome)
             && is_res_lang_ctor(cx, path_res(cx, peel_blocks(els)), OptionNone)
             && !is_else_clause(cx.tcx, expr)
             && !is_in_const_context(cx)
-            && !expr.span.in_external_macro(cx.sess().source_map())
             && self.msrv.meets(cx, msrvs::BOOL_THEN)
             && !contains_return(then_block.stmts)
         {
             let method_name = if switch_to_eager_eval(cx, expr) && self.msrv.meets(cx, msrvs::BOOL_THEN_SOME) {
-                "then_some"
+                sym::then_some
             } else {
-                "then"
+                sym::then
             };
+            let ctxt = expr.span.ctxt();
 
             span_lint_and_then(
                 cx,
@@ -98,16 +98,18 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone {
                     }
 
                     let mut app = Applicability::MachineApplicable;
-                    let cond_snip = Sugg::hir_with_context(cx, cond, expr.span.ctxt(), "[condition]", &mut app)
+                    let cond_snip = Sugg::hir_with_context(cx, cond, ctxt, "[condition]", &mut app)
                         .maybe_paren()
                         .to_string();
                     let arg_snip = snippet_with_context(cx, then_arg.span, ctxt, "[body]", &mut app).0;
-                    let method_body = if let Some(first_stmt) = then_block.stmts.first() {
-                        let (block_snippet, _) =
-                            snippet_with_context(cx, first_stmt.span.until(then_arg.span), ctxt, "..", &mut app);
-                        let closure = if method_name == "then" { "|| " } else { "" };
-                        format!("{closure} {{ {block_snippet}; {arg_snip} }}")
-                    } else if method_name == "then" {
+                    let method_body = if let Some(first_stmt) = then_block.stmts.first()
+                        && let Some(first_stmt_span) = walk_span_to_context(first_stmt.span, ctxt)
+                    {
+                        let block_snippet =
+                            snippet_with_applicability(cx, first_stmt_span.until(then_expr.span), "..", &mut app);
+                        let closure = if method_name == sym::then { "|| " } else { "" };
+                        format!("{closure} {{ {} {arg_snip} }}", block_snippet.trim_end())
+                    } else if method_name == sym::then {
                         (std::borrow::Cow::Borrowed("|| ") + arg_snip).into_owned()
                     } else {
                         arg_snip.into_owned()
diff --git a/tests/ui/if_then_some_else_none.fixed b/tests/ui/if_then_some_else_none.fixed
index d14a805b666..0fd130609ae 100644
--- a/tests/ui/if_then_some_else_none.fixed
+++ b/tests/ui/if_then_some_else_none.fixed
@@ -165,3 +165,44 @@ mod issue15257 {
         do_something((i % 2 == 0).then_some(closure_fn));
     }
 }
+
+fn issue15005() {
+    struct Counter {
+        count: u32,
+    }
+
+    impl Counter {
+        fn new() -> Counter {
+            Counter { count: 0 }
+        }
+    }
+
+    impl Iterator for Counter {
+        type Item = u32;
+
+        fn next(&mut self) -> Option<Self::Item> {
+            //~v if_then_some_else_none
+            (self.count < 5).then(||  { self.count += 1; self.count })
+        }
+    }
+}
+
+fn statements_from_macro() {
+    macro_rules! mac {
+        () => {
+            println!("foo");
+            println!("bar");
+        };
+    }
+    //~v if_then_some_else_none
+    let _ = true.then(||  { mac!(); 42 });
+}
+
+fn dont_lint_inside_macros() {
+    macro_rules! mac {
+        ($cond:expr, $res:expr) => {
+            if $cond { Some($res) } else { None }
+        };
+    }
+    let _: Option<u32> = mac!(true, 42);
+}
diff --git a/tests/ui/if_then_some_else_none.rs b/tests/ui/if_then_some_else_none.rs
index bb0072f3157..640828aa9bf 100644
--- a/tests/ui/if_then_some_else_none.rs
+++ b/tests/ui/if_then_some_else_none.rs
@@ -211,3 +211,54 @@ mod issue15257 {
         });
     }
 }
+
+fn issue15005() {
+    struct Counter {
+        count: u32,
+    }
+
+    impl Counter {
+        fn new() -> Counter {
+            Counter { count: 0 }
+        }
+    }
+
+    impl Iterator for Counter {
+        type Item = u32;
+
+        fn next(&mut self) -> Option<Self::Item> {
+            //~v if_then_some_else_none
+            if self.count < 5 {
+                self.count += 1;
+                Some(self.count)
+            } else {
+                None
+            }
+        }
+    }
+}
+
+fn statements_from_macro() {
+    macro_rules! mac {
+        () => {
+            println!("foo");
+            println!("bar");
+        };
+    }
+    //~v if_then_some_else_none
+    let _ = if true {
+        mac!();
+        Some(42)
+    } else {
+        None
+    };
+}
+
+fn dont_lint_inside_macros() {
+    macro_rules! mac {
+        ($cond:expr, $res:expr) => {
+            if $cond { Some($res) } else { None }
+        };
+    }
+    let _: Option<u32> = mac!(true, 42);
+}
diff --git a/tests/ui/if_then_some_else_none.stderr b/tests/ui/if_then_some_else_none.stderr
index c2e624a0a73..58651a05594 100644
--- a/tests/ui/if_then_some_else_none.stderr
+++ b/tests/ui/if_then_some_else_none.stderr
@@ -116,5 +116,28 @@ LL | |             None
 LL | |         });
    | |_________^ help: try: `(i % 2 == 0).then_some(closure_fn)`
 
-error: aborting due to 11 previous errors
+error: this could be simplified with `bool::then`
+  --> tests/ui/if_then_some_else_none.rs:231:13
+   |
+LL | /             if self.count < 5 {
+LL | |                 self.count += 1;
+LL | |                 Some(self.count)
+LL | |             } else {
+LL | |                 None
+LL | |             }
+   | |_____________^ help: try: `(self.count < 5).then(||  { self.count += 1; self.count })`
+
+error: this could be simplified with `bool::then`
+  --> tests/ui/if_then_some_else_none.rs:249:13
+   |
+LL |       let _ = if true {
+   |  _____________^
+LL | |         mac!();
+LL | |         Some(42)
+LL | |     } else {
+LL | |         None
+LL | |     };
+   | |_____^ help: try: `true.then(||  { mac!(); 42 })`
+
+error: aborting due to 13 previous errors