Skip to content

Extend if_then_some_else_none to also suggest bool::then_some #9289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 48 additions & 42 deletions clippy_lints/src/if_then_some_else_none.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
use clippy_utils::source::snippet_with_macro_callsite;
use clippy_utils::{contains_return, higher, is_else_clause, is_lang_ctor, meets_msrv, msrvs, peel_blocks};
use if_chain::if_chain;
use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_hir::{Expr, ExprKind, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
Expand All @@ -11,10 +11,12 @@ use rustc_session::{declare_tool_lint, impl_lint_pass};

declare_clippy_lint! {
/// ### What it does
/// Checks for if-else that could be written to `bool::then`.
/// Checks for if-else that could be written using either `bool::then` or `bool::then_some`.
///
/// ### Why is this bad?
/// Looks a little redundant. Using `bool::then` helps it have less lines of code.
/// Looks a little redundant. Using `bool::then` is more concise and incurs no loss of clarity.
/// For simple calculations and known values, use `bool::then_some`, which is eagerly evaluated
/// in comparison to `bool::then`.
///
/// ### Example
/// ```rust
Expand All @@ -39,7 +41,7 @@ declare_clippy_lint! {
#[clippy::version = "1.53.0"]
pub IF_THEN_SOME_ELSE_NONE,
restriction,
"Finds if-else that could be written using `bool::then`"
"Finds if-else that could be written using either `bool::then` or `bool::then_some`"
}

pub struct IfThenSomeElseNone {
Expand All @@ -56,7 +58,7 @@ impl IfThenSomeElseNone {
impl_lint_pass!(IfThenSomeElseNone => [IF_THEN_SOME_ELSE_NONE]);

impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'tcx Expr<'_>) {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if !meets_msrv(self.msrv, msrvs::BOOL_THEN) {
return;
}
Expand All @@ -70,43 +72,47 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone {
return;
}

if_chain! {
if let Some(higher::If { cond, then, r#else: Some(els) }) = higher::If::hir(expr);
if let ExprKind::Block(then_block, _) = then.kind;
if let Some(then_expr) = then_block.expr;
if let ExprKind::Call(then_call, [then_arg]) = then_expr.kind;
if let ExprKind::Path(ref then_call_qpath) = then_call.kind;
if is_lang_ctor(cx, then_call_qpath, OptionSome);
if let ExprKind::Path(ref qpath) = peel_blocks(els).kind;
if is_lang_ctor(cx, qpath, OptionNone);
if !stmts_contains_early_return(then_block.stmts);
then {
let cond_snip = 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 = 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(|| {})`",
cond_snip,
closure_body,
);
span_lint_and_help(
cx,
IF_THEN_SOME_ELSE_NONE,
expr.span,
"this could be simplified with `bool::then`",
None,
&help,
);
}
if let Some(higher::If { cond, then, r#else: Some(els) }) = higher::If::hir(expr)
&& 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 ExprKind::Path(ref then_call_qpath) = then_call.kind
&& is_lang_ctor(cx, then_call_qpath, OptionSome)
&& let ExprKind::Path(ref qpath) = peel_blocks(els).kind
&& is_lang_ctor(cx, qpath, OptionNone)
&& !stmts_contains_early_return(then_block.stmts)
{
let cond_snip = 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 = snippet_with_macro_callsite(cx, then_arg.span, "");
let mut method_body = if then_block.stmts.is_empty() {
arg_snip.into_owned()
} else {
format!("{{ /* snippet */ {} }}", arg_snip)
};
let method_name = if switch_to_eager_eval(cx, expr) && meets_msrv(self.msrv, msrvs::BOOL_THEN_SOME) {
"then_some"
} else {
method_body.insert_str(0, "|| ");
"then"
};

let help = format!(
"consider using `bool::{}` like: `{}.{}({})`",
method_name, cond_snip, method_name, method_body,
);
span_lint_and_help(
cx,
IF_THEN_SOME_ELSE_NONE,
expr.span,
&format!("this could be simplified with `bool::{}`", method_name),
None,
&help,
);
}
}

Expand Down
8 changes: 4 additions & 4 deletions tests/ui/if_then_some_else_none.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,21 @@ LL | | };
|
= help: consider using `bool::then` like: `matches!(true, true).then(|| { /* snippet */ matches!(true, false) })`

error: this could be simplified with `bool::then`
error: this could be simplified with `bool::then_some`
--> $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)`
= help: consider using `bool::then_some` like: `(o < 32).then_some(o)`

error: this could be simplified with `bool::then`
error: this could be simplified with `bool::then_some`
--> $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)`
= help: consider using `bool::then_some` like: `(!x).then_some(0)`

error: this could be simplified with `bool::then`
--> $DIR/if_then_some_else_none.rs:82:13
Expand Down