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

3 files changed, 64 insertions, 8 deletions

diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs
index 61178ccdd56..057b4850e4f 100644
--- a/clippy_lints/src/question_mark.rs
+++ b/clippy_lints/src/question_mark.rs
@@ -17,7 +17,7 @@ use if_chain::if_chain;
 
 use crate::rustc_errors::Applicability;
 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
 ///
@@ -64,14 +64,40 @@ impl Pass {
     /// 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) {
         if_chain! {
-            if let ExprKind::If(ref if_expr, ref body, _) = expr.node;
-            if let ExprKind::MethodCall(ref segment, _, ref args) = if_expr.node;
+            if let ExprKind::If(if_expr, body, else_) = &expr.node;
+            if let ExprKind::MethodCall(segment, _, args) = &if_expr.node;
             if segment.ident.name == "is_none";
             if Self::expression_returns_none(cx, body);
             if let Some(subject) = args.get(0);
             if Self::is_option(cx, subject);
 
             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(
                     cx,
                     QUESTION_MARK,
diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs
index 7f1d06fbd29..b1edec32eee 100644
--- a/tests/ui/question_mark.rs
+++ b/tests/ui/question_mark.rs
@@ -42,11 +42,22 @@ pub struct SomeStruct {
 }
 
 impl SomeStruct {
+    #[rustfmt::skip]
     pub fn func(&self) -> Option<u32> {
         if (self.opt).is_none() {
             return None;
         }
 
+        if self.opt.is_none() {
+            return None
+        }
+
+        let _ = if self.opt.is_none() {
+            return None;
+        } else {
+            self.opt
+        };
+
         self.opt
     }
 }
diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr
index d3daaaa9270..c9d5538f36f 100644
--- a/tests/ui/question_mark.stderr
+++ b/tests/ui/question_mark.stderr
@@ -9,12 +9,31 @@ error: this block may be rewritten with the `?` operator
    = note: `-D clippy::question-mark` implied by `-D warnings`
 
 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 | |             return None;
-48 | |         }
+47 | /         if (self.opt).is_none() {
+48 | |             return None;
+49 | |         }
    | |_________^ 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