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