Renamed to implicit_return.

Covered all other kinds besides `ExprKind::Lit`.
Added check for replacing `break` with `return`.
This commit is contained in:
daxpedda 2018-12-05 14:39:09 +01:00
parent 19db2f1a32
commit aed2b986e6
6 changed files with 154 additions and 121 deletions

View file

@ -706,6 +706,7 @@ All notable changes to this project will be documented in this file.
[`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
[`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
[`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher
[`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return
[`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping
[`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing
[`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask
@ -756,7 +757,6 @@ All notable changes to this project will be documented in this file.
[`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op
[`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items
[`missing_inline_in_public_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_inline_in_public_items
[`missing_returns`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_returns
[`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
[`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
[`module_inception`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_inception

View file

@ -0,0 +1,131 @@
// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use crate::rustc::hir::{intravisit::FnKind, Body, ExprKind, FnDecl};
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use crate::rustc::{declare_tool_lint, lint_array};
use crate::rustc_errors::Applicability;
use crate::syntax::{ast::NodeId, source_map::Span};
use crate::utils::{snippet_opt, span_lint_and_then};
/// **What it does:** Checks for missing return statements at the end of a block.
///
/// **Why is this bad?** Actually omitting the return keyword is idiomatic Rust code. Programmers
/// coming from other languages might prefer the expressiveness of `return`.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// fn foo(x: usize) {
/// x
/// }
/// ```
/// add return
/// ```rust
/// fn foo(x: usize) {
/// return x;
/// }
/// ```
declare_clippy_lint! {
pub IMPLICIT_RETURN,
restriction,
"use a return statement like `return expr` instead of an expression"
}
pub struct Pass;
impl Pass {
fn expr_match(cx: &LateContext<'_, '_>, expr: &rustc::hir::Expr) {
match &expr.node {
ExprKind::Block(block, ..) => {
if let Some(expr) = &block.expr {
Self::expr_match(cx, expr);
}
// only needed in the case of `break` with `;` at the end
else if let Some(stmt) = block.stmts.last() {
if let rustc::hir::StmtKind::Semi(expr, ..) = &stmt.node {
Self::expr_match(cx, expr);
}
}
},
// use `return` instead of `break`
ExprKind::Break(.., break_expr) => {
if let Some(break_expr) = break_expr {
span_lint_and_then(cx, IMPLICIT_RETURN, expr.span, "missing return statement", |db| {
if let Some(snippet) = snippet_opt(cx, break_expr.span) {
db.span_suggestion_with_applicability(
expr.span,
"change `break` to `return` as shown",
format!("return {}", snippet),
Applicability::MachineApplicable,
);
}
});
}
},
ExprKind::If(.., if_expr, else_expr) => {
Self::expr_match(cx, if_expr);
if let Some(else_expr) = else_expr {
Self::expr_match(cx, else_expr);
}
},
ExprKind::Match(_, arms, ..) => {
for arm in arms {
Self::expr_match(cx, &arm.body);
}
},
// loops could be using `break` instead of `return`
ExprKind::Loop(block, ..) => {
if let Some(expr) = &block.expr {
Self::expr_match(cx, expr);
}
},
// skip if it already has a return statement
ExprKind::Ret(..) => (),
// everything else is missing `return`
_ => span_lint_and_then(cx, IMPLICIT_RETURN, expr.span, "missing return statement", |db| {
if let Some(snippet) = snippet_opt(cx, expr.span) {
db.span_suggestion_with_applicability(
expr.span,
"add `return` as shown",
format!("return {}", snippet),
Applicability::MachineApplicable,
);
}
}),
}
}
}
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(IMPLICIT_RETURN)
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_fn(
&mut self,
cx: &LateContext<'a, 'tcx>,
_: FnKind<'tcx>,
_: &'tcx FnDecl,
body: &'tcx Body,
_: Span,
_: NodeId,
) {
let def_id = cx.tcx.hir.body_owner_def_id(body.id());
let mir = cx.tcx.optimized_mir(def_id);
if !mir.return_ty().is_unit() {
Self::expr_match(cx, &body.value);
}
}
}

View file

@ -126,6 +126,7 @@ pub mod functions;
pub mod identity_conversion;
pub mod identity_op;
pub mod if_not_else;
pub mod implicit_return;
pub mod indexing_slicing;
pub mod infallible_destructuring_match;
pub mod infinite_iter;
@ -152,7 +153,6 @@ pub mod misc;
pub mod misc_early;
pub mod missing_doc;
pub mod missing_inline;
pub mod missing_returns;
pub mod multiple_crate_versions;
pub mod mut_mut;
pub mod mut_reference;
@ -372,7 +372,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
reg.register_late_lint_pass(box unicode::Unicode);
reg.register_late_lint_pass(box strings::StringAdd);
reg.register_early_lint_pass(box returns::ReturnPass);
reg.register_late_lint_pass(box missing_returns::Pass);
reg.register_late_lint_pass(box implicit_return::Pass);
reg.register_late_lint_pass(box methods::Pass);
reg.register_late_lint_pass(box map_clone::Pass);
reg.register_late_lint_pass(box shadow::Pass);
@ -487,6 +487,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
arithmetic::FLOAT_ARITHMETIC,
arithmetic::INTEGER_ARITHMETIC,
else_if_without_else::ELSE_IF_WITHOUT_ELSE,
implicit_return::IMPLICIT_RETURN,
indexing_slicing::INDEXING_SLICING,
inherent_impl::MULTIPLE_INHERENT_IMPL,
literal_representation::DECIMAL_LITERAL_REPRESENTATION,
@ -498,7 +499,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
misc::FLOAT_CMP_CONST,
missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS,
missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS,
missing_returns::MISSING_RETURNS,
panic_unimplemented::UNIMPLEMENTED,
shadow::SHADOW_REUSE,
shadow::SHADOW_SAME,

View file

@ -1,106 +0,0 @@
// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use crate::rustc::hir::{intravisit::FnKind, Body, ExprKind, FnDecl};
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use crate::rustc::{declare_tool_lint, lint_array};
use crate::rustc_errors::Applicability;
use crate::syntax::{ast::NodeId, source_map::Span};
use crate::utils::{snippet_opt, span_lint_and_then};
/// **What it does:** Checks for missing return statements at the end of a block.
///
/// **Why is this bad?** Actually omitting the return keyword is idiomatic Rust code. Programmers
/// coming from other languages might prefer the expressiveness of `return`.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// fn foo(x: usize) {
/// x
/// }
/// ```
/// add return
/// ```rust
/// fn foo(x: usize) {
/// return x;
/// }
/// ```
declare_clippy_lint! {
pub MISSING_RETURNS,
restriction,
"use a return statement like `return expr` instead of an expression"
}
pub struct Pass;
impl Pass {
fn show_suggestion(cx: &LateContext<'_, '_>, span: syntax_pos::Span) {
span_lint_and_then(cx, MISSING_RETURNS, span, "missing return statement", |db| {
if let Some(snippet) = snippet_opt(cx, span) {
db.span_suggestion_with_applicability(
span,
"add `return` as shown",
format!("return {}", snippet),
Applicability::MachineApplicable,
);
}
});
}
fn expr_match(cx: &LateContext<'_, '_>, kind: &ExprKind) {
match kind {
ExprKind::Block(ref block, ..) => {
if let Some(ref expr) = block.expr {
Self::expr_match(cx, &expr.node);
}
},
ExprKind::If(.., if_expr, else_expr) => {
Self::expr_match(cx, &if_expr.node);
if let Some(else_expr) = else_expr {
Self::expr_match(cx, &else_expr.node);
}
},
ExprKind::Match(_, arms, ..) => {
for arm in arms {
Self::expr_match(cx, &arm.body.node);
}
},
ExprKind::Lit(lit) => Self::show_suggestion(cx, lit.span),
_ => (),
}
}
}
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(MISSING_RETURNS)
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_fn(
&mut self,
cx: &LateContext<'a, 'tcx>,
_: FnKind<'tcx>,
_: &'tcx FnDecl,
body: &'tcx Body,
_: Span,
_: NodeId,
) {
let def_id = cx.tcx.hir.body_owner_def_id(body.id());
let mir = cx.tcx.optimized_mir(def_id);
if !mir.return_ty().is_unit() {
Self::expr_match(cx, &body.value.node);
}
}
}

View file

@ -11,7 +11,7 @@
#![warn(clippy::missing_returns)]
#![warn(clippy::implicit_return)]
fn test_end_of_fn() -> bool {
if true {
@ -40,6 +40,13 @@ fn test_match(x: bool) -> bool {
}
}
#[allow(clippy::never_loop)]
fn test_loop() -> bool {
loop {
break true;
}
}
fn test_closure() {
let _ = || {
true
@ -51,5 +58,6 @@ fn main() {
let _ = test_end_of_fn();
let _ = test_if_block();
let _ = test_match(true);
let _ = test_loop();
test_closure();
}

View file

@ -1,45 +1,45 @@
error: missing return statement
--> $DIR/missing_returns.rs:21:5
--> $DIR/implicit_return.rs:21:5
|
21 | true
| ^^^^ help: add `return` as shown: `return true`
|
= note: `-D clippy::missing-returns` implied by `-D warnings`
= note: `-D clippy::implicit-return` implied by `-D warnings`
error: missing return statement
--> $DIR/missing_returns.rs:27:9
--> $DIR/implicit_return.rs:27:9
|
27 | true
| ^^^^ help: add `return` as shown: `return true`
error: missing return statement
--> $DIR/missing_returns.rs:29:9
--> $DIR/implicit_return.rs:29:9
|
29 | false
| ^^^^^ help: add `return` as shown: `return false`
error: missing return statement
--> $DIR/missing_returns.rs:36:17
--> $DIR/implicit_return.rs:36:17
|
36 | true => false,
| ^^^^^ help: add `return` as shown: `return false`
error: missing return statement
--> $DIR/missing_returns.rs:38:13
--> $DIR/implicit_return.rs:38:13
|
38 | true
| ^^^^ help: add `return` as shown: `return true`
error: missing return statement
--> $DIR/missing_returns.rs:45:9
--> $DIR/implicit_return.rs:52:9
|
45 | true
52 | true
| ^^^^ help: add `return` as shown: `return true`
error: missing return statement
--> $DIR/missing_returns.rs:47:16
--> $DIR/implicit_return.rs:54:16
|
47 | let _ = || true;
54 | let _ = || true;
| ^^^^ help: add `return` as shown: `return true`
error: aborting due to 7 previous errors