From 2183cfcc13c12f21571879dcc8ef40c4dfc9d8a7 Mon Sep 17 00:00:00 2001
From: daxpedda <daxpedda@users.noreply.github.com>
Date: Sun, 20 Jan 2019 13:45:22 +0100
Subject: [PATCH] Fix `implicit_return` false positives.

---
 clippy_lints/src/implicit_return.rs | 28 ++++++++++++++++++++++------
 tests/ui/implicit_return.rs         | 19 +++++++++++++++++++
 tests/ui/implicit_return.stderr     | 10 +++++-----
 3 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/clippy_lints/src/implicit_return.rs b/clippy_lints/src/implicit_return.rs
index cd5db359628..073c37eefc5 100644
--- a/clippy_lints/src/implicit_return.rs
+++ b/clippy_lints/src/implicit_return.rs
@@ -1,5 +1,5 @@
-use crate::utils::{in_macro, snippet_opt, span_lint_and_then};
-use rustc::hir::{intravisit::FnKind, Body, ExprKind, FnDecl};
+use crate::utils::{in_macro, is_expn_of, snippet_opt, span_lint_and_then};
+use rustc::hir::{intravisit::FnKind, Body, ExprKind, FnDecl, MatchSource};
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::{declare_tool_lint, lint_array};
 use rustc_errors::Applicability;
@@ -81,15 +81,31 @@ fn expr_match(cx: &LateContext<'_, '_>, expr: &rustc::hir::Expr) {
                     Self::expr_match(cx, else_expr);
                 }
             },
-            ExprKind::Match(_, arms, ..) => {
-                for arm in arms {
-                    Self::expr_match(cx, &arm.body);
+            ExprKind::Match(.., arms, source) => {
+                let check_all_arms = match source {
+                    MatchSource::IfLetDesugar {
+                        contains_else_clause: has_else,
+                    } => *has_else,
+                    _ => true,
+                };
+
+                if check_all_arms {
+                    for arm in arms {
+                        Self::expr_match(cx, &arm.body);
+                    }
+                } else {
+                    Self::expr_match(cx, &arms.first().expect("if let doesn't have a single arm").body);
                 }
             },
             // skip if it already has a return statement
             ExprKind::Ret(..) => (),
             // everything else is missing `return`
-            _ => Self::lint(cx, expr.span, expr.span, "add `return` as shown"),
+            _ => {
+                // make sure it's not just an unreachable expression
+                if is_expn_of(expr.span, "unreachable").is_none() {
+                    Self::lint(cx, expr.span, expr.span, "add `return` as shown")
+                }
+            },
         }
     }
 }
diff --git a/tests/ui/implicit_return.rs b/tests/ui/implicit_return.rs
index 0fe4a283abf..d1c63ca1697 100644
--- a/tests/ui/implicit_return.rs
+++ b/tests/ui/implicit_return.rs
@@ -26,6 +26,14 @@ fn test_match(x: bool) -> bool {
     }
 }
 
+#[allow(clippy::match_bool, clippy::needless_return)]
+fn test_match_with_unreachable(x: bool) -> bool {
+    match x {
+        true => return false,
+        false => unreachable!(),
+    }
+}
+
 #[allow(clippy::never_loop)]
 fn test_loop() -> bool {
     loop {
@@ -53,6 +61,15 @@ fn test_loop_with_nests() -> bool {
     }
 }
 
+#[allow(clippy::redundant_pattern_matching)]
+fn test_loop_with_if_let() -> bool {
+    loop {
+        if let Some(x) = Some(true) {
+            return x;
+        }
+    }
+}
+
 fn test_closure() {
     #[rustfmt::skip]
     let _ = || { true };
@@ -63,8 +80,10 @@ fn main() {
     let _ = test_end_of_fn();
     let _ = test_if_block();
     let _ = test_match(true);
+    let _ = test_match_with_unreachable(true);
     let _ = test_loop();
     let _ = test_loop_with_block();
     let _ = test_loop_with_nests();
+    let _ = test_loop_with_if_let();
     test_closure();
 }
diff --git a/tests/ui/implicit_return.stderr b/tests/ui/implicit_return.stderr
index c07fced1259..98b588f1a74 100644
--- a/tests/ui/implicit_return.stderr
+++ b/tests/ui/implicit_return.stderr
@@ -31,31 +31,31 @@ LL |         false => { true },
    |                    ^^^^ help: add `return` as shown: `return true`
 
 error: missing return statement
-  --> $DIR/implicit_return.rs:32:9
+  --> $DIR/implicit_return.rs:40:9
    |
 LL |         break true;
    |         ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`
 
 error: missing return statement
-  --> $DIR/implicit_return.rs:40:13
+  --> $DIR/implicit_return.rs:48:13
    |
 LL |             break true;
    |             ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`
 
 error: missing return statement
-  --> $DIR/implicit_return.rs:49:13
+  --> $DIR/implicit_return.rs:57:13
    |
 LL |             break true;
    |             ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`
 
 error: missing return statement
-  --> $DIR/implicit_return.rs:58:18
+  --> $DIR/implicit_return.rs:75:18
    |
 LL |     let _ = || { true };
    |                  ^^^^ help: add `return` as shown: `return true`
 
 error: missing return statement
-  --> $DIR/implicit_return.rs:59:16
+  --> $DIR/implicit_return.rs:76:16
    |
 LL |     let _ = || true;
    |                ^^^^ help: add `return` as shown: `return true`
-- 
2.44.0