From c1eb5828fafae326919c6999bfda075c87dcc296 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Thu, 14 Jul 2016 18:32:09 +0200
Subject: [PATCH] Fix suggestion spans for `NEEDLESS_RETURN`

---
 clippy_lints/src/returns.rs           | 20 ++++++++++----------
 tests/compile-fail/needless_return.rs | 11 +++++------
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index fda151cd6d7..deea50f0303 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -39,7 +39,7 @@ fn check_block_return(&mut self, cx: &EarlyContext, block: &Block) {
         if let Some(stmt) = block.stmts.last() {
             match stmt.node {
                 StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => {
-                    self.check_final_expr(cx, expr);
+                    self.check_final_expr(cx, expr, Some(stmt.span));
                 }
                 _ => (),
             }
@@ -47,11 +47,11 @@ fn check_block_return(&mut self, cx: &EarlyContext, block: &Block) {
     }
 
     // Check a the final expression in a block if it's a return.
-    fn check_final_expr(&mut self, cx: &EarlyContext, expr: &Expr) {
+    fn check_final_expr(&mut self, cx: &EarlyContext, expr: &Expr, span: Option<Span>) {
         match expr.node {
             // simple return is always "bad"
             ExprKind::Ret(Some(ref inner)) => {
-                self.emit_return_lint(cx, (expr.span, inner.span));
+                self.emit_return_lint(cx, span.expect("`else return` is not possible"), inner.span);
             }
             // a whole block? check it!
             ExprKind::Block(ref block) => {
@@ -62,25 +62,25 @@ fn check_final_expr(&mut self, cx: &EarlyContext, expr: &Expr) {
             // (except for unit type functions) so we don't match it
             ExprKind::If(_, ref ifblock, Some(ref elsexpr)) => {
                 self.check_block_return(cx, ifblock);
-                self.check_final_expr(cx, elsexpr);
+                self.check_final_expr(cx, elsexpr, None);
             }
             // a match expr, check all arms
             ExprKind::Match(_, ref arms) => {
                 for arm in arms {
-                    self.check_final_expr(cx, &arm.body);
+                    self.check_final_expr(cx, &arm.body, Some(arm.body.span));
                 }
             }
             _ => (),
         }
     }
 
-    fn emit_return_lint(&mut self, cx: &EarlyContext, spans: (Span, Span)) {
-        if in_external_macro(cx, spans.1) {
+    fn emit_return_lint(&mut self, cx: &EarlyContext, ret_span: Span, inner_span: Span) {
+        if in_external_macro(cx, inner_span) {
             return;
         }
-        span_lint_and_then(cx, NEEDLESS_RETURN, spans.0, "unneeded return statement", |db| {
-            if let Some(snippet) = snippet_opt(cx, spans.1) {
-                db.span_suggestion(spans.0, "remove `return` as shown:", snippet);
+        span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| {
+            if let Some(snippet) = snippet_opt(cx, inner_span) {
+                db.span_suggestion(ret_span, "remove `return` as shown:", snippet);
             }
         });
     }
diff --git a/tests/compile-fail/needless_return.rs b/tests/compile-fail/needless_return.rs
index 80fed2818ef..5a391a358b1 100644
--- a/tests/compile-fail/needless_return.rs
+++ b/tests/compile-fail/needless_return.rs
@@ -37,12 +37,11 @@ fn test_if_block() -> bool {
 
 fn test_match(x: bool) -> bool {
     match x {
-        true => {
-            return false;
-            //~^ ERROR unneeded return statement
-            //~| HELP remove `return` as shown
-            //~| SUGGESTION false
-        }
+        true => return false,
+        //~^ ERROR unneeded return statement
+        //~| HELP remove `return` as shown
+        //~| SUGGESTION false
+
         false => {
             return true;
             //~^ ERROR unneeded return statement
-- 
2.44.0