Skip to content

Commit

Permalink
Improve suggestion and make it work for macros
Browse files Browse the repository at this point in the history
  • Loading branch information
magurotuna committed Mar 12, 2021
1 parent 0327c2e commit 11d2af7
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 6 deletions.
18 changes: 14 additions & 4 deletions clippy_lints/src/if_then_some_else_none.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/if_then_some_else_none.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!");
Expand Down
34 changes: 32 additions & 2 deletions tests/ui/if_then_some_else_none.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
| _____________^
Expand All @@ -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

0 comments on commit 11d2af7

Please sign in to comment.