add nop if-let expression check.

re-design test cases as some of them are not worth the effort to check.

4 files changed, 224 insertions, 171 deletions

diff --git a/clippy_lints/src/matches/nop_match.rs b/clippy_lints/src/matches/nop_match.rs
index a0fff490cfa..74e7ca2d5fe 100644
--- a/clippy_lints/src/matches/nop_match.rs
+++ b/clippy_lints/src/matches/nop_match.rs
@@ -1,25 +1,13 @@
-#![allow(unused_variables)]
 use super::NOP_MATCH;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
-use clippy_utils::{eq_expr_value, get_parent_expr};
+use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::{eq_expr_value, get_parent_expr, higher, is_else_clause, is_lang_ctor, peel_blocks_with_stmt};
 use rustc_errors::Applicability;
-use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, Pat, PatKind, PathSegment, QPath};
+use rustc_hir::LangItem::OptionNone;
+use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, Pat, PatKind, Path, PathSegment, QPath};
 use rustc_lint::LateContext;
-
-pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) {
-    if false {
-        span_lint_and_sugg(
-            cx,
-            NOP_MATCH,
-            ex.span,
-            "this if-let expression is unnecessary",
-            "replace it with",
-            "".to_string(),
-            Applicability::MachineApplicable,
-        );
-    }
-}
+use rustc_span::sym;
 
 pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
     // This is for avoiding collision with `match_single_binding`.
@@ -52,6 +40,70 @@ pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>])
     }
 }
 
+/// Check for nop `if let` expression that assembled as unnecessary match
+///
+/// ```rust,ignore
+/// if let Some(a) = option {
+///     Some(a)
+/// } else {
+///     None
+/// }
+/// ```
+/// OR
+/// ```rust,ignore
+/// if let SomeEnum::A = some_enum {
+///     SomeEnum::A
+/// } else if let SomeEnum::B = some_enum {
+///     SomeEnum::B
+/// } else {
+///     some_enum
+/// }
+/// ```
+pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) {
+    if_chain! {
+        if let Some(ref if_let) = higher::IfLet::hir(cx, ex);
+        if !is_else_clause(cx.tcx, ex);
+        if check_if_let(cx, if_let);
+        then {
+            let mut applicability = Applicability::MachineApplicable;
+            span_lint_and_sugg(
+                cx,
+                NOP_MATCH,
+                ex.span,
+                "this if-let expression is unnecessary",
+                "replace it with",
+                snippet_with_applicability(cx, if_let.let_expr.span, "..", &mut applicability).to_string(),
+                applicability,
+            );
+        }
+    }
+}
+
+fn check_if_let(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
+    if let Some(else_block) = if_let.if_else {
+        if !pat_same_as_expr(if_let.let_pat, peel_blocks_with_stmt(if_let.if_then)) {
+            return false;
+        }
+
+        let else_expr = peel_blocks_with_stmt(else_block);
+        // Recurrsively check for each `else if let` phrase,
+        if let Some(ref nested_if_let) = higher::IfLet::hir(cx, else_expr) {
+            return check_if_let(cx, nested_if_let);
+        }
+        let ret = strip_return(else_expr);
+        let let_expr_ty = cx.typeck_results().expr_ty(if_let.let_expr);
+        if is_type_diagnostic_item(cx, let_expr_ty, sym::Option) {
+            if let ExprKind::Path(ref qpath) = ret.kind {
+                return is_lang_ctor(cx, qpath, OptionNone) || eq_expr_value(cx, if_let.let_expr, ret);
+            }
+        } else {
+            return eq_expr_value(cx, if_let.let_expr, ret);
+        }
+        return true;
+    }
+    false
+}
+
 fn strip_return<'hir>(expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> {
     if let ExprKind::Ret(Some(ret)) = expr.kind {
         ret
@@ -68,7 +120,7 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
             ExprKind::Call(call_expr, [first_param, ..]),
         ) => {
             if let ExprKind::Path(QPath::Resolved(_, call_path)) = call_expr.kind {
-                if is_identical_segments(path.segments, call_path.segments)
+                if has_identical_segments(path.segments, call_path.segments)
                     && has_same_non_ref_symbol(first_pat, first_param)
                 {
                     return true;
@@ -76,7 +128,7 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
             }
         },
         (PatKind::Path(QPath::Resolved(_, p_path)), ExprKind::Path(QPath::Resolved(_, e_path))) => {
-            return is_identical_segments(p_path.segments, e_path.segments);
+            return has_identical_segments(p_path.segments, e_path.segments);
         },
         (PatKind::Lit(pat_lit_expr), ExprKind::Lit(expr_spanned)) => {
             if let ExprKind::Lit(pat_spanned) = &pat_lit_expr.kind {
@@ -89,7 +141,7 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
     false
 }
 
-fn is_identical_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegment<'_>]) -> bool {
+fn has_identical_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegment<'_>]) -> bool {
     if left_segs.len() != right_segs.len() {
         return false;
     }
@@ -105,8 +157,7 @@ fn has_same_non_ref_symbol(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
     if_chain! {
         if let PatKind::Binding(annot, _, pat_ident, _) = pat.kind;
         if !matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut);
-        if let ExprKind::Path(QPath::Resolved(_, path)) = expr.kind;
-        if let Some(first_seg) = path.segments.first();
+        if let ExprKind::Path(QPath::Resolved(_, Path {segments: [first_seg, ..], .. })) = expr.kind;
         then {
             return pat_ident.name == first_seg.ident.name;
         }
diff --git a/tests/ui/nop_match.fixed b/tests/ui/nop_match.fixed
index eff03a464ce..c8c05b6f4a4 100644
--- a/tests/ui/nop_match.fixed
+++ b/tests/ui/nop_match.fixed
@@ -1,88 +1,67 @@
 // run-rustfix
 #![warn(clippy::nop_match)]
 #![allow(clippy::manual_map)]
-#![allow(clippy::question_mark)]
 #![allow(dead_code)]
 
-fn func_ret_err<T>(err: T) -> Result<(), T> {
-    Err(err)
-}
-
-enum SampleEnum {
+enum Choice {
     A,
     B,
     C,
+    D,
 }
 
-fn useless_prim_type_match(x: i32) -> i32 {
-    x
-}
-
-fn useless_custom_type_match(se: SampleEnum) -> SampleEnum {
-    se
-}
-
-// Don't trigger
-fn mingled_custom_type(se: SampleEnum) -> SampleEnum {
-    match se {
-        SampleEnum::A => SampleEnum::B,
-        SampleEnum::B => SampleEnum::C,
-        SampleEnum::C => SampleEnum::A,
-    }
-}
-
-fn option_match() -> Option<i32> {
-    Some(1)
+fn useless_match(x: i32) {
+    let _: i32 = x;
 }
 
-fn result_match() -> Result<i32, i32> {
-    Ok(1)
+fn custom_type_match(se: Choice) {
+    let _: Choice = se;
+    // Don't trigger
+    let _: Choice = match se {
+        Choice::A => Choice::A,
+        Choice::B => Choice::B,
+        _ => Choice::C,
+    };
+    // Mingled, don't trigger
+    let _: Choice = match se {
+        Choice::A => Choice::B,
+        Choice::B => Choice::C,
+        Choice::C => Choice::D,
+        Choice::D => Choice::A,
+    };
 }
 
-fn result_match_func_call() {
-    let _ = func_ret_err(0_i32);
+fn option_match(x: Option<i32>) {
+    let _: Option<i32> = x;
+    // Don't trigger, this is the case for manual_map_option
+    let _: Option<i32> = match x {
+        Some(a) => Some(-a),
+        None => None,
+    };
 }
 
-fn option_check() -> Option<i32> {
-    if let Some(a) = Some(1) { Some(a) } else { None }
-}
-
-fn option_check_no_else() -> Option<i32> {
-    if let Some(a) = Some(1) {
-        return Some(a);
-    }
-    None
-}
-
-fn result_check_no_else() -> Result<(), i32> {
-    if let Err(e) = func_ret_err(0_i32) {
-        return Err(e);
-    }
-    Ok(())
+fn func_ret_err<T>(err: T) -> Result<i32, T> {
+    Err(err)
 }
 
-fn result_check_a() -> Result<(), i32> {
-    if let Err(e) = func_ret_err(0_i32) {
-        Err(e)
-    } else {
-        Ok(())
-    }
+fn result_match() {
+    let _: Result<i32, i32> = Ok(1);
+    let _: Result<i32, i32> = func_ret_err(0_i32);
 }
 
-// Don't trigger
-fn result_check_b() -> Result<(), i32> {
-    if let Err(e) = Ok(1) { Err(e) } else { Ok(()) }
+fn if_let_option() -> Option<i32> {
+    Some(1)
 }
 
-fn result_check_c() -> Result<(), i32> {
-    let example = Ok(());
-    if let Err(e) = example { Err(e) } else { example }
+fn if_let_result(x: Result<(), i32>) {
+    let _: Result<(), i32> = x;
+    let _: Result<(), i32> = x;
+    // Input type mismatch, don't trigger
+    let _: Result<(), i32> = if let Err(e) = Ok(1) { Err(e) } else { x };
 }
 
-// Don't trigger
-fn result_check_d() -> Result<(), i32> {
-    let example = Ok(1);
-    if let Err(e) = example { Err(e) } else { Ok(()) }
+fn custom_enum_a(x: Choice) -> Choice {
+    x
 }
 
 fn main() {}
diff --git a/tests/ui/nop_match.rs b/tests/ui/nop_match.rs
index 4c4b47ce9c5..978811e28d1 100644
--- a/tests/ui/nop_match.rs
+++ b/tests/ui/nop_match.rs
@@ -1,106 +1,94 @@
 // run-rustfix
 #![warn(clippy::nop_match)]
 #![allow(clippy::manual_map)]
-#![allow(clippy::question_mark)]
 #![allow(dead_code)]
 
-fn func_ret_err<T>(err: T) -> Result<(), T> {
-    Err(err)
-}
-
-enum SampleEnum {
+enum Choice {
     A,
     B,
     C,
+    D,
 }
 
-fn useless_prim_type_match(x: i32) -> i32 {
-    match x {
+fn useless_match(x: i32) {
+    let _: i32 = match x {
         0 => 0,
         1 => 1,
         2 => 2,
         _ => x,
-    }
-}
-
-fn useless_custom_type_match(se: SampleEnum) -> SampleEnum {
-    match se {
-        SampleEnum::A => SampleEnum::A,
-        SampleEnum::B => SampleEnum::B,
-        SampleEnum::C => SampleEnum::C,
-    }
+    };
 }
 
-// Don't trigger
-fn mingled_custom_type(se: SampleEnum) -> SampleEnum {
-    match se {
-        SampleEnum::A => SampleEnum::B,
-        SampleEnum::B => SampleEnum::C,
-        SampleEnum::C => SampleEnum::A,
-    }
+fn custom_type_match(se: Choice) {
+    let _: Choice = match se {
+        Choice::A => Choice::A,
+        Choice::B => Choice::B,
+        Choice::C => Choice::C,
+        Choice::D => Choice::D,
+    };
+    // Don't trigger
+    let _: Choice = match se {
+        Choice::A => Choice::A,
+        Choice::B => Choice::B,
+        _ => Choice::C,
+    };
+    // Mingled, don't trigger
+    let _: Choice = match se {
+        Choice::A => Choice::B,
+        Choice::B => Choice::C,
+        Choice::C => Choice::D,
+        Choice::D => Choice::A,
+    };
 }
 
-fn option_match() -> Option<i32> {
-    match Some(1) {
+fn option_match(x: Option<i32>) {
+    let _: Option<i32> = match x {
         Some(a) => Some(a),
         None => None,
-    }
+    };
+    // Don't trigger, this is the case for manual_map_option
+    let _: Option<i32> = match x {
+        Some(a) => Some(-a),
+        None => None,
+    };
 }
 
-fn result_match() -> Result<i32, i32> {
-    match Ok(1) {
-        Ok(a) => Ok(a),
-        Err(err) => Err(err),
-    }
+fn func_ret_err<T>(err: T) -> Result<i32, T> {
+    Err(err)
 }
 
-fn result_match_func_call() {
-    let _ = match func_ret_err(0_i32) {
+fn result_match() {
+    let _: Result<i32, i32> = match Ok(1) {
         Ok(a) => Ok(a),
         Err(err) => Err(err),
     };
+    let _: Result<i32, i32> = match func_ret_err(0_i32) {
+        Err(err) => Err(err),
+        Ok(a) => Ok(a),
+    };
 }
 
-fn option_check() -> Option<i32> {
+fn if_let_option() -> Option<i32> {
     if let Some(a) = Some(1) { Some(a) } else { None }
 }
 
-fn option_check_no_else() -> Option<i32> {
-    if let Some(a) = Some(1) {
-        return Some(a);
-    }
-    None
-}
-
-fn result_check_no_else() -> Result<(), i32> {
-    if let Err(e) = func_ret_err(0_i32) {
-        return Err(e);
-    }
-    Ok(())
+fn if_let_result(x: Result<(), i32>) {
+    let _: Result<(), i32> = if let Err(e) = x { Err(e) } else { x };
+    let _: Result<(), i32> = if let Ok(val) = x { Ok(val) } else { x };
+    // Input type mismatch, don't trigger
+    let _: Result<(), i32> = if let Err(e) = Ok(1) { Err(e) } else { x };
 }
 
-fn result_check_a() -> Result<(), i32> {
-    if let Err(e) = func_ret_err(0_i32) {
-        Err(e)
+fn custom_enum_a(x: Choice) -> Choice {
+    if let Choice::A = x {
+        Choice::A
+    } else if let Choice::B = x {
+        Choice::B
+    } else if let Choice::C = x {
+        Choice::C
     } else {
-        Ok(())
+        x
     }
 }
 
-// Don't trigger
-fn result_check_b() -> Result<(), i32> {
-    if let Err(e) = Ok(1) { Err(e) } else { Ok(()) }
-}
-
-fn result_check_c() -> Result<(), i32> {
-    let example = Ok(());
-    if let Err(e) = example { Err(e) } else { example }
-}
-
-// Don't trigger
-fn result_check_d() -> Result<(), i32> {
-    let example = Ok(1);
-    if let Err(e) = example { Err(e) } else { Ok(()) }
-}
-
 fn main() {}
diff --git a/tests/ui/nop_match.stderr b/tests/ui/nop_match.stderr
index bb171295f0f..23a1d382a30 100644
--- a/tests/ui/nop_match.stderr
+++ b/tests/ui/nop_match.stderr
@@ -1,53 +1,88 @@
 error: this match expression is unnecessary
-  --> $DIR/nop_match.rs:18:5
+  --> $DIR/nop_match.rs:14:18
    |
-LL | /     match x {
+LL |       let _: i32 = match x {
+   |  __________________^
 LL | |         0 => 0,
 LL | |         1 => 1,
 LL | |         2 => 2,
 LL | |         _ => x,
-LL | |     }
+LL | |     };
    | |_____^ help: replace it with: `x`
    |
    = note: `-D clippy::nop-match` implied by `-D warnings`
 
 error: this match expression is unnecessary
-  --> $DIR/nop_match.rs:27:5
+  --> $DIR/nop_match.rs:23:21
    |
-LL | /     match se {
-LL | |         SampleEnum::A => SampleEnum::A,
-LL | |         SampleEnum::B => SampleEnum::B,
-LL | |         SampleEnum::C => SampleEnum::C,
-LL | |     }
+LL |       let _: Choice = match se {
+   |  _____________________^
+LL | |         Choice::A => Choice::A,
+LL | |         Choice::B => Choice::B,
+LL | |         Choice::C => Choice::C,
+LL | |         Choice::D => Choice::D,
+LL | |     };
    | |_____^ help: replace it with: `se`
 
 error: this match expression is unnecessary
-  --> $DIR/nop_match.rs:44:5
+  --> $DIR/nop_match.rs:45:26
    |
-LL | /     match Some(1) {
+LL |       let _: Option<i32> = match x {
+   |  __________________________^
 LL | |         Some(a) => Some(a),
 LL | |         None => None,
-LL | |     }
-   | |_____^ help: replace it with: `Some(1)`
+LL | |     };
+   | |_____^ help: replace it with: `x`
 
 error: this match expression is unnecessary
-  --> $DIR/nop_match.rs:51:5
+  --> $DIR/nop_match.rs:61:31
    |
-LL | /     match Ok(1) {
+LL |       let _: Result<i32, i32> = match Ok(1) {
+   |  _______________________________^
 LL | |         Ok(a) => Ok(a),
 LL | |         Err(err) => Err(err),
-LL | |     }
+LL | |     };
    | |_____^ help: replace it with: `Ok(1)`
 
 error: this match expression is unnecessary
-  --> $DIR/nop_match.rs:58:13
+  --> $DIR/nop_match.rs:65:31
    |
-LL |       let _ = match func_ret_err(0_i32) {
-   |  _____________^
-LL | |         Ok(a) => Ok(a),
+LL |       let _: Result<i32, i32> = match func_ret_err(0_i32) {
+   |  _______________________________^
 LL | |         Err(err) => Err(err),
+LL | |         Ok(a) => Ok(a),
 LL | |     };
    | |_____^ help: replace it with: `func_ret_err(0_i32)`
 
-error: aborting due to 5 previous errors
+error: this if-let expression is unnecessary
+  --> $DIR/nop_match.rs:72:5
+   |
+LL |     if let Some(a) = Some(1) { Some(a) } else { None }
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `Some(1)`
+
+error: this if-let expression is unnecessary
+  --> $DIR/nop_match.rs:76:30
+   |
+LL |     let _: Result<(), i32> = if let Err(e) = x { Err(e) } else { x };
+   |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x`
+
+error: this if-let expression is unnecessary
+  --> $DIR/nop_match.rs:77:30
+   |
+LL |     let _: Result<(), i32> = if let Ok(val) = x { Ok(val) } else { x };
+   |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x`
+
+error: this if-let expression is unnecessary
+  --> $DIR/nop_match.rs:83:5
+   |
+LL | /     if let Choice::A = x {
+LL | |         Choice::A
+LL | |     } else if let Choice::B = x {
+LL | |         Choice::B
+...  |
+LL | |         x
+LL | |     }
+   | |_____^ help: replace it with: `x`
+
+error: aborting due to 9 previous errors