diff --git a/clippy_lints/src/if_then_some_else_none.rs b/clippy_lints/src/if_then_some_else_none.rs index 9e94280fc074..4433ddb6210d 100644 --- a/clippy_lints/src/if_then_some_else_none.rs +++ b/clippy_lints/src/if_then_some_else_none.rs @@ -2,15 +2,15 @@ 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, higher, is_else_clause, is_in_const_context, is_res_lang_ctor, path_res, peel_blocks, + contains_return, higher, is_else_clause, is_in_const_context, is_res_lang_ctor, 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! { @@ -70,21 +70,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, @@ -93,16 +93,18 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone { format!("this could be simplified with `bool::{method_name}`"), |diag| { 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 f774608712d1..47ac30b254fc 100644 --- a/tests/ui/if_then_some_else_none.fixed +++ b/tests/ui/if_then_some_else_none.fixed @@ -122,3 +122,44 @@ const fn issue12103(x: u32) -> Option { // Should not issue an error in `const` context if x > 42 { Some(150) } else { None } } + +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 { + //~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 = mac!(true, 42); +} diff --git a/tests/ui/if_then_some_else_none.rs b/tests/ui/if_then_some_else_none.rs index 8b8ff0a6ea00..e98edd999f6a 100644 --- a/tests/ui/if_then_some_else_none.rs +++ b/tests/ui/if_then_some_else_none.rs @@ -143,3 +143,54 @@ const fn issue12103(x: u32) -> Option { // Should not issue an error in `const` context if x > 42 { Some(150) } else { None } } + +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 { + //~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 = mac!(true, 42); +} diff --git a/tests/ui/if_then_some_else_none.stderr b/tests/ui/if_then_some_else_none.stderr index 71285574ef24..18b3b5b2defa 100644 --- a/tests/ui/if_then_some_else_none.stderr +++ b/tests/ui/if_then_some_else_none.stderr @@ -58,5 +58,28 @@ error: this could be simplified with `bool::then` LL | if s == "1" { Some(true) } else { None } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(s == "1").then(|| true)` -error: aborting due to 6 previous errors +error: this could be simplified with `bool::then` + --> tests/ui/if_then_some_else_none.rs:163: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:181: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 8 previous errors