Renamed to implicit_return.

daxpedda · daxpedda · commit aed2b986e6d5 · 2018-12-05T14:39:09.000+01:00
Covered all other kinds besides `ExprKind::Lit`.
Added check for replacing `break` with `return`.
diff --git 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
@@ -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
@@ -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
diff --git a/tests/ui/implicit_return.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/implicit_return.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
