diff options
| author | daxpedda <daxpedda@users.noreply.github.com> | 2018-12-05 14:39:09 +0100 |
|---|---|---|
| committer | daxpedda <daxpedda@users.noreply.github.com> | 2018-12-05 14:39:09 +0100 |
| commit | aed2b986e6d596f6ed154625030b8c83a67066fe (patch) | |
| tree | de0cf3499dae2dabf3f36f0883cdd9897ddc96e3 | |
| parent | 19db2f1a325d5030c05d6b3a64cab165c7e15d09 (diff) | |
| download | rust-aed2b986e6d596f6ed154625030b8c83a67066fe.tar.gz rust-aed2b986e6d596f6ed154625030b8c83a67066fe.zip | |
Renamed to `implicit_return`.
Covered all other kinds besides `ExprKind::Lit`. Added check for replacing `break` with `return`.
| -rw-r--r-- | CHANGELOG.md | 2 | ||||
| -rw-r--r-- | clippy_lints/src/implicit_return.rs | 131 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 6 | ||||
| -rw-r--r-- | clippy_lints/src/missing_returns.rs | 106 | ||||
| -rw-r--r-- | tests/ui/implicit_return.rs (renamed from tests/ui/missing_returns.rs) | 10 | ||||
| -rw-r--r-- | tests/ui/implicit_return.stderr (renamed from tests/ui/missing_returns.stderr) | 20 |
6 files changed, 154 insertions, 121 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index ed5057daf29..e691ec9412f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/implicit_return.rs b/clippy_lints/src/implicit_return.rs new file mode 100644 index 00000000000..de783aabefe --- /dev/null +++ b/clippy_lints/src/implicit_return.rs @@ -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); + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index cb0535a03ac..ee41c632077 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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, diff --git a/clippy_lints/src/missing_returns.rs b/clippy_lints/src/missing_returns.rs deleted file mode 100644 index 71dc71d563b..00000000000 --- a/clippy_lints/src/missing_returns.rs +++ /dev/null @@ -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); - } - } -} diff --git a/tests/ui/missing_returns.rs b/tests/ui/implicit_return.rs index 96935eb6b53..73cf2908833 100644 --- a/tests/ui/missing_returns.rs +++ b/tests/ui/implicit_return.rs @@ -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(); } diff --git a/tests/ui/missing_returns.stderr b/tests/ui/implicit_return.stderr index 874bec9e109..bba8d942e27 100644 --- a/tests/ui/missing_returns.stderr +++ b/tests/ui/implicit_return.stderr @@ -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 |
