Lint if let Some in question_mark lint

clippy_lints/src/question_mark.rs

@@ -1,13 +1,15 @@
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::{def, Block, Expr, ExprKind, StmtKind};
+use rustc_hir::{def, BindingAnnotation, Block, Expr, ExprKind, MatchSource, PatKind, StmtKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
 use crate::utils::paths::{OPTION, OPTION_NONE};
 use crate::utils::sugg::Sugg;
-use crate::utils::{higher, match_def_path, match_type, span_lint_and_then, SpanlessEq};
+use crate::utils::{
+    higher, match_def_path, match_qpath, match_type, snippet_with_applicability, span_lint_and_then, SpanlessEq,
+};
 
 declare_clippy_lint! {
     /// **What it does:** Checks for expressions that could be replaced by the question mark operator.
@@ -82,7 +84,7 @@ impl QuestionMark {
                         |db| {
                             db.span_suggestion(
                                 expr.span,
-                                "replace_it_with",
+                                "replace it with",
                                 replacement_str,
                                 Applicability::MaybeIncorrect, // snippet
                             );
@@ -93,6 +95,52 @@ impl QuestionMark {
         }
     }
 
+    fn check_if_let_some_and_early_return_none(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
+        if_chain! {
+            if let ExprKind::Match(subject, arms, source) = &expr.kind;
+            if *source == MatchSource::IfLetDesugar { contains_else_clause: true };
+            if Self::is_option(cx, subject);
+
+            if let PatKind::TupleStruct(path1, fields, None) = &arms[0].pat.kind;
+            if match_qpath(path1, &["Some"]);
+            if let PatKind::Binding(annot, _, bind, _) = &fields[0].kind;
+            let by_ref = matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut);
+
+            if let ExprKind::Block(block, None) = &arms[0].body.kind;
+            if block.stmts.is_empty();
+            if let Some(trailing_expr) = &block.expr;
+            if let ExprKind::Path(path) = &trailing_expr.kind;
+            if match_qpath(path, &[&bind.as_str()]);
+
+            if let PatKind::Wild = arms[1].pat.kind;
+            if Self::expression_returns_none(cx, arms[1].body);
+            then {
+                let mut applicability = Applicability::MachineApplicable;
+                let receiver_str = snippet_with_applicability(cx, subject.span, "..", &mut applicability);
+                let replacement = format!(
+                    "{}{}?",
+                    receiver_str,
+                    if by_ref { ".as_ref()" } else { "" },
+                );
+
+                span_lint_and_then(
+                    cx,
+                    QUESTION_MARK,
+                    expr.span,
+                    "this if-let-else may be rewritten with the `?` operator",
+                    |db| {
+                        db.span_suggestion(
+                            expr.span,
+                            "replace it with",
+                            replacement,
+                            applicability,
+                        );
+                    }
+                )
+            }
+        }
+    }
+
     fn moves_by_default(cx: &LateContext<'_, '_>, expression: &Expr<'_>) -> bool {
         let expr_ty = cx.tables.expr_ty(expression);
 
@@ -158,5 +206,6 @@ impl QuestionMark {
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for QuestionMark {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
         Self::check_is_none_and_early_return_none(cx, expr);
+        Self::check_if_let_some_and_early_return_none(cx, expr);
     }
 }

clippy_lints/src/types.rs

@@ -1698,12 +1698,7 @@ fn detect_absurd_comparison<'a, 'tcx>(
         return None;
     }
 
-    let normalized = normalize_comparison(op, lhs, rhs);
-    let (rel, normalized_lhs, normalized_rhs) = if let Some(val) = normalized {
-        val
-    } else {
-        return None;
-    };
+    let (rel, normalized_lhs, normalized_rhs) = normalize_comparison(op, lhs, rhs)?;
 
     let lx = detect_extreme_expr(cx, normalized_lhs);
     let rx = detect_extreme_expr(cx, normalized_rhs);

tests/ui/question_mark.rs

@@ -58,6 +58,12 @@ impl CopyStruct {
             self.opt
         };
 
+        let _ = if let Some(x) = self.opt {
+            x
+        } else {
+            return None;
+        };
+
         self.opt
     }
 }
@@ -90,6 +96,26 @@ impl MoveStruct {
         }
         Some(Vec::new())
     }
+
+    pub fn if_let_ref_func(self) -> Option<Vec<u32>> {
+        let mut v: &Vec<_> = if let Some(ref v) = self.opt {
+            v
+        } else {
+            return None;
+        };
+
+        Some(v.clone())
+    }
+
+    pub fn if_let_mov_func(self) -> Option<Vec<u32>> {
+        let mut v = if let Some(v) = self.opt {
+            v
+        } else {
+            return None;
+        };
+
+        Some(v)
+    }
 }
 
 fn main() {

tests/ui/question_mark.stderr

@@ -4,7 +4,7 @@ error: this block may be rewritten with the `?` operator
 LL | /     if a.is_none() {
 LL | |         return None;
 LL | |     }
-   | |_____^ help: replace_it_with: `a?;`
+   | |_____^ help: replace it with: `a?;`
    |
    = note: `-D clippy::question-mark` implied by `-D warnings`
 
@@ -14,15 +14,15 @@ error: this block may be rewritten with the `?` operator
 LL | /         if (self.opt).is_none() {
 LL | |             return None;
 LL | |         }
-   | |_________^ help: replace_it_with: `(self.opt)?;`
+   | |_________^ help: replace it with: `(self.opt)?;`
 
 error: this block may be rewritten with the `?` operator
   --> $DIR/question_mark.rs:51:9
    |
 LL | /         if self.opt.is_none() {
 LL | |             return None
 LL | |         }
-   | |_________^ help: replace_it_with: `self.opt?;`
+   | |_________^ help: replace it with: `self.opt?;`
 
 error: this block may be rewritten with the `?` operator
   --> $DIR/question_mark.rs:55:17
@@ -33,31 +33,64 @@ LL | |             return None;
 LL | |         } else {
 LL | |             self.opt
 LL | |         };
-   | |_________^ help: replace_it_with: `Some(self.opt?)`
+   | |_________^ help: replace it with: `Some(self.opt?)`
+
+error: this if-let-else may be rewritten with the `?` operator
+  --> $DIR/question_mark.rs:61:17
+   |
+LL |           let _ = if let Some(x) = self.opt {
+   |  _________________^
+LL | |             x
+LL | |         } else {
+LL | |             return None;
+LL | |         };
+   | |_________^ help: replace it with: `self.opt?`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:72:9
+  --> $DIR/question_mark.rs:78:9
    |
 LL | /         if self.opt.is_none() {
 LL | |             return None;
 LL | |         }
-   | |_________^ help: replace_it_with: `self.opt.as_ref()?;`
+   | |_________^ help: replace it with: `self.opt.as_ref()?;`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:80:9
+  --> $DIR/question_mark.rs:86:9
    |
 LL | /         if self.opt.is_none() {
 LL | |             return None;
 LL | |         }
-   | |_________^ help: replace_it_with: `self.opt.as_ref()?;`
+   | |_________^ help: replace it with: `self.opt.as_ref()?;`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:88:9
+  --> $DIR/question_mark.rs:94:9
    |
 LL | /         if self.opt.is_none() {
 LL | |             return None;
 LL | |         }
-   | |_________^ help: replace_it_with: `self.opt.as_ref()?;`
+   | |_________^ help: replace it with: `self.opt.as_ref()?;`
+
+error: this if-let-else may be rewritten with the `?` operator
+  --> $DIR/question_mark.rs:101:30
+   |
+LL |           let mut v: &Vec<_> = if let Some(ref v) = self.opt {
+   |  ______________________________^
+LL | |             v
+LL | |         } else {
+LL | |             return None;
+LL | |         };
+   | |_________^ help: replace it with: `self.opt.as_ref()?`
+
+error: this if-let-else may be rewritten with the `?` operator
+  --> $DIR/question_mark.rs:111:21
+   |
+LL |           let mut v = if let Some(v) = self.opt {
+   |  _____________________^
+LL | |             v
+LL | |         } else {
+LL | |             return None;
+LL | |         };
+   | |_________^ help: replace it with: `self.opt?`
 
-error: aborting due to 7 previous errors
+error: aborting due to 10 previous errors