new lint for unneeded return stmts

src/lib.rs

@@ -53,6 +53,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_lint_pass(box collapsible_if::CollapsibleIf as LintPassObject);
     reg.register_lint_pass(box unicode::Unicode as LintPassObject);
     reg.register_lint_pass(box strings::StringAdd as LintPassObject);
+    reg.register_lint_pass(box misc::NeedlessReturn as LintPassObject);
 
     reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST,
                                            misc::SINGLE_MATCH, misc::STR_TO_STRING,
@@ -73,5 +74,6 @@ pub fn plugin_registrar(reg: &mut Registry) {
                                            collapsible_if::COLLAPSIBLE_IF,
                                            unicode::ZERO_WIDTH_SPACE,
                                            strings::STRING_ADD_ASSIGN,
+                                           misc::NEEDLESS_RETURN,
                                            ]);
 }

src/misc.rs

@@ -269,3 +269,71 @@ fn is_str_arg(cx: &Context, args: &[P<Expr>]) -> bool {
     args.len() == 1 && if let ty::TyStr =
         walk_ty(cx.tcx.expr_ty(&*args[0])).sty { true } else { false }
 }
+
+declare_lint!(pub NEEDLESS_RETURN, Warn,
+              "Warn on using a return statement where an expression would be enough");
+
+#[derive(Copy,Clone)]
+pub struct NeedlessReturn;
+
+impl NeedlessReturn {
+    // Check the final stmt or expr in a block for unnecessary return.
+    fn check_block_return(&mut self, cx: &Context, block: &Block) {
+        if let Some(ref expr) = block.expr {
+            self.check_final_expr(cx, expr);
+        } else if let Some(stmt) = block.stmts.last() {
+            if let StmtSemi(ref expr, _) = stmt.node {
+                if let ExprRet(Some(ref inner)) = expr.node {
+                    self.emit_lint(cx, (expr.span, inner.span));
+                }
+            }
+        }
+    }
+
+    // Check a the final expression in a block if it's a return.
+    fn check_final_expr(&mut self, cx: &Context, expr: &Expr) {
+        match expr.node {
+            // simple return is always "bad"
+            ExprRet(Some(ref inner)) => {
+                self.emit_lint(cx, (expr.span, inner.span));
+            }
+            // a whole block? check it!
+            ExprBlock(ref block) => {
+                self.check_block_return(cx, block);
+            }
+            // an if/if let expr, check both exprs
+            // note, if without else is going to be a type checking error anyways
+            // (except for unit type functions) so we don't match it
+            ExprIf(_, ref ifblock, Some(ref elsexpr)) |
+            ExprIfLet(_, _, ref ifblock, Some(ref elsexpr)) => {
+                self.check_block_return(cx, ifblock);
+                self.check_final_expr(cx, elsexpr);
+            }
+            // a match expr, check all arms
+            ExprMatch(_, ref arms, _) => {
+                for arm in arms {
+                    self.check_final_expr(cx, &*arm.body);
+                }
+            }
+            _ => { }
+        }
+    }
+
+    fn emit_lint(&mut self, cx: &Context, spans: (Span, Span)) {
+        span_lint(cx, NEEDLESS_RETURN, spans.0, &format!(
+            "unneeded return statement. Consider using {} \
+             without trailing semicolon",
+            snippet(cx, spans.1, "..")))
+    }
+}
+
+impl LintPass for NeedlessReturn {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(NEEDLESS_RETURN)
+    }
+
+    fn check_fn(&mut self, cx: &Context, _: FnKind, _: &FnDecl,
+                block: &Block, _: Span, _: ast::NodeId) {
+        self.check_block_return(cx, block);
+    }
+}

tests/compile-fail/needless_return.rs

@@ -0,0 +1,49 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(needless_return)]
+
+fn test_end_of_fn() -> bool {
+    if true {
+        // no error!
+        return true;
+    }
+    return true;           //~ERROR
+}
+
+fn test_no_semicolon() -> bool {
+    return true            //~ERROR
+}
+
+fn test_if_block() -> bool {
+    if true {
+        return true;       //~ERROR
+    } else {
+        return false;      //~ERROR
+    }
+}
+
+fn test_match(x: bool) -> bool {
+    match x {
+        true => {
+            return false;  //~ERROR
+        }
+        false => {
+            return true    //~ERROR
+        }
+    }
+}
+
+fn test_closure() {
+    let _ = || {
+        return true;       //~ERROR
+    };
+}
+
+fn main() {
+    let _ = test_end_of_fn();
+    let _ = test_no_semicolon();
+    let _ = test_if_block();
+    let _ = test_match(true);
+    test_closure();
+}