Implement new lint: if_then_some_else_none

This commit is contained in:
Yusuke Tanaka 2021-03-07 02:08:46 +09:00
parent 0153679a4e
commit a672d335a2
No known key found for this signature in database
GPG Key ID: 409D7EEE1E7A716A
5 changed files with 175 additions and 0 deletions

View File

@ -2103,6 +2103,7 @@ Released 2018-09-13
[`if_let_some_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_some_result
[`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else
[`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
[`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none
[`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
[`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone
[`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher

View File

@ -0,0 +1,88 @@
use crate::utils;
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
declare_clippy_lint! {
/// **What it does:** Checks for if-else that could be written to `bool::then`.
///
/// **Why is this bad?** Looks a little redundant. Using `bool::then` helps it have less lines of code.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// # let v = vec![0];
/// let a = if v.is_empty() {
/// println!("true!");
/// Some(42)
/// } else {
/// None
/// };
/// ```
///
/// Could be written:
///
/// ```rust
/// # let v = vec![0];
/// let a = v.is_empty().then(|| {
/// println!("true!");
/// 42
/// });
/// ```
pub IF_THEN_SOME_ELSE_NONE,
restriction,
"Finds if-else that could be written using `bool::then`"
}
declare_lint_pass!(IfThenSomeElseNone => [IF_THEN_SOME_ELSE_NONE]);
impl LateLintPass<'_> for IfThenSomeElseNone {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'tcx Expr<'_>) {
if in_external_macro(cx.sess(), expr.span) {
return;
}
// We only care about the top-most `if` in the chain
if utils::parent_node_is_if_expr(expr, cx) {
return;
}
if_chain! {
if let ExprKind::If(ref cond, ref then, Some(ref els)) = expr.kind;
if let ExprKind::Block(ref then_block, _) = then.kind;
if let Some(ref then_expr) = then_block.expr;
if let ExprKind::Call(ref then_call, [then_arg]) = then_expr.kind;
if let ExprKind::Path(ref then_call_qpath) = then_call.kind;
if utils::match_qpath(then_call_qpath, &utils::paths::OPTION_SOME);
if let ExprKind::Block(ref els_block, _) = els.kind;
if els_block.stmts.is_empty();
if let Some(ref els_expr) = els_block.expr;
if let ExprKind::Path(ref els_call_qpath) = els_expr.kind;
if utils::match_qpath(els_call_qpath, &utils::paths::OPTION_NONE);
then {
let mut applicability = Applicability::MachineApplicable;
let cond_snip = utils::snippet_with_applicability(cx, cond.span, "[condition]", &mut applicability);
let arg_snip = utils::snippet_with_applicability(cx, then_arg.span, "", &mut applicability);
let sugg = format!(
"{}.then(|| {{ /* snippet */ {} }})",
cond_snip,
arg_snip,
);
utils::span_lint_and_sugg(
cx,
IF_THEN_SOME_ELSE_NONE,
expr.span,
"this could be simplified with `bool::then`",
"try this",
sugg,
applicability,
);
}
}
}
}

View File

@ -230,6 +230,7 @@ mod identity_op;
mod if_let_mutex;
mod if_let_some_result;
mod if_not_else;
mod if_then_some_else_none;
mod implicit_return;
mod implicit_saturating_sub;
mod inconsistent_struct_constructor;
@ -667,6 +668,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&if_let_mutex::IF_LET_MUTEX,
&if_let_some_result::IF_LET_SOME_RESULT,
&if_not_else::IF_NOT_ELSE,
&if_then_some_else_none::IF_THEN_SOME_ELSE_NONE,
&implicit_return::IMPLICIT_RETURN,
&implicit_saturating_sub::IMPLICIT_SATURATING_SUB,
&inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR,
@ -1282,6 +1284,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box redundant_slicing::RedundantSlicing);
store.register_late_pass(|| box from_str_radix_10::FromStrRadix10);
store.register_late_pass(|| box manual_map::ManualMap);
store.register_late_pass(|| box if_then_some_else_none::IfThenSomeElseNone);
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@ -1297,6 +1300,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&exhaustive_items::EXHAUSTIVE_STRUCTS),
LintId::of(&exit::EXIT),
LintId::of(&float_literal::LOSSY_FLOAT_LITERAL),
LintId::of(&if_then_some_else_none::IF_THEN_SOME_ELSE_NONE),
LintId::of(&implicit_return::IMPLICIT_RETURN),
LintId::of(&indexing_slicing::INDEXING_SLICING),
LintId::of(&inherent_impl::MULTIPLE_INHERENT_IMPL),

View File

@ -0,0 +1,66 @@
#![warn(clippy::if_then_some_else_none)]
fn main() {
// Should issue an error.
let _ = if foo() {
println!("true!");
Some("foo")
} else {
None
};
// Should not issue an error since the `else` block has a statement besides `None`.
let _ = if foo() {
println!("true!");
Some("foo")
} else {
eprintln!("false...");
None
};
// Should not issue an error since there are more than 2 blocks in the if-else chain.
let _ = if foo() {
println!("foo true!");
Some("foo")
} else if bar() {
println!("bar true!");
Some("bar")
} else {
None
};
let _ = if foo() {
println!("foo true!");
Some("foo")
} else {
bar().then(|| {
println!("bar true!");
"bar"
})
};
// Should not issue an error since the `then` block has `None`, not `Some`.
let _ = if foo() { None } else { Some("foo is false") };
// Should not issue an error since the `else` block doesn't use `None` directly.
let _ = if foo() { Some("foo is true") } else { into_none() };
// Should not issue an error since the `then` block doesn't use `Some` directly.
let _ = if foo() { into_some("foo") } else { None };
}
fn foo() -> bool {
unimplemented!()
}
fn bar() -> bool {
unimplemented!()
}
fn into_some<T>(v: T) -> Option<T> {
Some(v)
}
fn into_none<T>() -> Option<T> {
None
}

View File

@ -0,0 +1,16 @@
error: this could be simplified with `bool::then`
--> $DIR/if_then_some_else_none.rs:5:13
|
LL | let _ = if foo() {
| _____________^
LL | | println!("true!");
LL | | Some("foo")
LL | | } else {
LL | | None
LL | | };
| |_____^ help: try this: `foo().then(|| { /* snippet */ "foo" })`
|
= note: `-D clippy::if-then-some-else-none` implied by `-D warnings`
error: aborting due to previous error