diff --git a/src/lib.rs b/src/lib.rs index 54f3bb11d..7d29d9663 100755 --- a/src/lib.rs +++ b/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, ]); } diff --git a/src/misc.rs b/src/misc.rs index ff0594b2b..305a11abe 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -269,3 +269,71 @@ fn is_str_arg(cx: &Context, args: &[P]) -> 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); + } +} diff --git a/tests/compile-fail/needless_return.rs b/tests/compile-fail/needless_return.rs new file mode 100755 index 000000000..34d571279 --- /dev/null +++ b/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(); +}