question_mark: Suggest Some(opt?) for if-else

This commit is contained in:
Shotaro Yamada 2018-12-12 17:46:52 +09:00
parent eb54c1a9a0
commit eba44e1c67
3 changed files with 64 additions and 8 deletions

View file

@ -17,7 +17,7 @@ use if_chain::if_chain;
use crate::rustc_errors::Applicability; use crate::rustc_errors::Applicability;
use crate::utils::paths::*; use crate::utils::paths::*;
use crate::utils::{match_def_path, match_type, span_lint_and_then}; use crate::utils::{match_def_path, match_type, span_lint_and_then, SpanlessEq};
/// **What it does:** Checks for expressions that could be replaced by the question mark operator /// **What it does:** Checks for expressions that could be replaced by the question mark operator
/// ///
@ -64,14 +64,40 @@ impl Pass {
/// If it matches, it will suggest to use the question mark operator instead /// If it matches, it will suggest to use the question mark operator instead
fn check_is_none_and_early_return_none(cx: &LateContext<'_, '_>, expr: &Expr) { fn check_is_none_and_early_return_none(cx: &LateContext<'_, '_>, expr: &Expr) {
if_chain! { if_chain! {
if let ExprKind::If(ref if_expr, ref body, _) = expr.node; if let ExprKind::If(if_expr, body, else_) = &expr.node;
if let ExprKind::MethodCall(ref segment, _, ref args) = if_expr.node; if let ExprKind::MethodCall(segment, _, args) = &if_expr.node;
if segment.ident.name == "is_none"; if segment.ident.name == "is_none";
if Self::expression_returns_none(cx, body); if Self::expression_returns_none(cx, body);
if let Some(subject) = args.get(0); if let Some(subject) = args.get(0);
if Self::is_option(cx, subject); if Self::is_option(cx, subject);
then { then {
if let Some(else_) = else_ {
if_chain! {
if let ExprKind::Block(block, None) = &else_.node;
if block.stmts.len() == 0;
if let Some(block_expr) = &block.expr;
if SpanlessEq::new(cx).ignore_fn().eq_expr(subject, block_expr);
then {
span_lint_and_then(
cx,
QUESTION_MARK,
expr.span,
"this block may be rewritten with the `?` operator",
|db| {
db.span_suggestion_with_applicability(
expr.span,
"replace_it_with",
format!("Some({}?)", Sugg::hir(cx, subject, "..")),
Applicability::MaybeIncorrect, // snippet
);
}
)
}
}
return;
}
span_lint_and_then( span_lint_and_then(
cx, cx,
QUESTION_MARK, QUESTION_MARK,

View file

@ -42,11 +42,22 @@ pub struct SomeStruct {
} }
impl SomeStruct { impl SomeStruct {
#[rustfmt::skip]
pub fn func(&self) -> Option<u32> { pub fn func(&self) -> Option<u32> {
if (self.opt).is_none() { if (self.opt).is_none() {
return None; return None;
} }
if self.opt.is_none() {
return None
}
let _ = if self.opt.is_none() {
return None;
} else {
self.opt
};
self.opt self.opt
} }
} }

View file

@ -9,12 +9,31 @@ error: this block may be rewritten with the `?` operator
= note: `-D clippy::question-mark` implied by `-D warnings` = note: `-D clippy::question-mark` implied by `-D warnings`
error: this block may be rewritten with the `?` operator error: this block may be rewritten with the `?` operator
--> $DIR/question_mark.rs:46:9 --> $DIR/question_mark.rs:47:9
| |
46 | / if (self.opt).is_none() { 47 | / if (self.opt).is_none() {
47 | | return None; 48 | | return None;
48 | | } 49 | | }
| |_________^ help: replace_it_with: `(self.opt)?;` | |_________^ help: replace_it_with: `(self.opt)?;`
error: aborting due to 2 previous errors error: this block may be rewritten with the `?` operator
--> $DIR/question_mark.rs:51:9
|
51 | / if self.opt.is_none() {
52 | | return None
53 | | }
| |_________^ help: replace_it_with: `self.opt?;`
error: this block may be rewritten with the `?` operator
--> $DIR/question_mark.rs:55:17
|
55 | let _ = if self.opt.is_none() {
| _________________^
56 | | return None;
57 | | } else {
58 | | self.opt
59 | | };
| |_________^ help: replace_it_with: `Some(self.opt?)`
error: aborting due to 4 previous errors