Make needless_return a late lint pass

clippy_lints/src/lib.rs

@@ -256,6 +256,7 @@ mod needless_borrow;
 mod needless_borrowed_ref;
 mod needless_continue;
 mod needless_pass_by_value;
+mod needless_return;
 mod needless_update;
 mod neg_cmp_op_on_partial_ord;
 mod neg_multiply;
@@ -726,6 +727,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE,
         &needless_continue::NEEDLESS_CONTINUE,
         &needless_pass_by_value::NEEDLESS_PASS_BY_VALUE,
+        &needless_return::NEEDLESS_RETURN,
         &needless_update::NEEDLESS_UPDATE,
         &neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD,
         &neg_multiply::NEG_MULTIPLY,
@@ -769,7 +771,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &regex::INVALID_REGEX,
         &regex::TRIVIAL_REGEX,
         &repeat_once::REPEAT_ONCE,
-        &returns::NEEDLESS_RETURN,
         &returns::UNUSED_UNIT,
         &serde_api::SERDE_API_MISUSE,
         &shadow::SHADOW_REUSE,
@@ -1027,6 +1028,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box redundant_closure_call::RedundantClosureCall);
     store.register_early_pass(|| box returns::Return);
     store.register_late_pass(|| box let_and_return::LetReturn);
+    store.register_late_pass(|| box needless_return::NeedlessReturn);
     store.register_early_pass(|| box collapsible_if::CollapsibleIf);
     store.register_early_pass(|| box items_after_statements::ItemsAfterStatements);
     store.register_early_pass(|| box precedence::Precedence);
@@ -1381,6 +1383,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&needless_bool::BOOL_COMPARISON),
         LintId::of(&needless_bool::NEEDLESS_BOOL),
         LintId::of(&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
+        LintId::of(&needless_return::NEEDLESS_RETURN),
         LintId::of(&needless_update::NEEDLESS_UPDATE),
         LintId::of(&neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
         LintId::of(&neg_multiply::NEG_MULTIPLY),
@@ -1413,7 +1416,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&regex::INVALID_REGEX),
         LintId::of(&regex::TRIVIAL_REGEX),
         LintId::of(&repeat_once::REPEAT_ONCE),
-        LintId::of(&returns::NEEDLESS_RETURN),
         LintId::of(&returns::UNUSED_UNIT),
         LintId::of(&serde_api::SERDE_API_MISUSE),
         LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
@@ -1543,6 +1545,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&misc_early::MIXED_CASE_HEX_LITERALS),
         LintId::of(&misc_early::REDUNDANT_PATTERN),
         LintId::of(&mut_reference::UNNECESSARY_MUT_PASSED),
+        LintId::of(&needless_return::NEEDLESS_RETURN),
         LintId::of(&neg_multiply::NEG_MULTIPLY),
         LintId::of(&new_without_default::NEW_WITHOUT_DEFAULT),
         LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
@@ -1554,7 +1557,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES),
         LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),
         LintId::of(&regex::TRIVIAL_REGEX),
-        LintId::of(&returns::NEEDLESS_RETURN),
         LintId::of(&returns::UNUSED_UNIT),
         LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
         LintId::of(&strings::STRING_LIT_AS_BYTES),

clippy_lints/src/needless_return.rs

@@ -0,0 +1,166 @@
+use rustc_lint::{LateLintPass, LateContext};
+use rustc_ast::ast::Attribute;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_errors::Applicability;
+use rustc_hir::intravisit::FnKind;
+use rustc_span::source_map::Span;
+use rustc_middle::lint::in_external_macro;
+use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, StmtKind};
+
+use crate::utils::{snippet_opt, span_lint_and_sugg, span_lint_and_then};
+
+declare_clippy_lint! {
+    /// **What it does:** Checks for return statements at the end of a block.
+    ///
+    /// **Why is this bad?** Removing the `return` and semicolon will make the code
+    /// more rusty.
+    ///
+    /// **Known problems:** If the computation returning the value borrows a local
+    /// variable, removing the `return` may run afoul of the borrow checker.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// fn foo(x: usize) -> usize {
+    ///     return x;
+    /// }
+    /// ```
+    /// simplify to
+    /// ```rust
+    /// fn foo(x: usize) -> usize {
+    ///     x
+    /// }
+    /// ```
+    pub NEEDLESS_RETURN,
+    style,
+    "using a return statement like `return expr;` where an expression would suffice"
+}
+
+#[derive(PartialEq, Eq, Copy, Clone)]
+enum RetReplacement {
+    Empty,
+    Block,
+}
+
+declare_lint_pass!(NeedlessReturn => [NEEDLESS_RETURN]);
+
+impl<'tcx> LateLintPass<'tcx> for NeedlessReturn {
+    fn check_fn(&mut self, cx: &LateContext<'tcx>, kind: FnKind<'tcx>, _: &'tcx FnDecl<'tcx>, body: &'tcx Body<'tcx>, _: Span, _: HirId) {
+        match kind {
+            FnKind::Closure(_) => {
+                check_final_expr(cx, &body.value, Some(body.value.span), RetReplacement::Empty)
+            }
+            FnKind::ItemFn(..) | FnKind::Method(..) => {
+                if let ExprKind::Block(ref block, _) = body.value.kind {
+                    check_block_return(cx, block)
+                }
+            }
+        }
+    }
+}
+
+fn attr_is_cfg(attr: &Attribute) -> bool {
+    attr.meta_item_list().is_some() && attr.has_name(sym!(cfg))
+}
+
+fn check_block_return(cx: &LateContext<'_>, block: &Block<'_>) {
+    if let Some(expr) = block.expr {
+        check_final_expr(cx, expr, Some(expr.span), RetReplacement::Empty);
+    } else if let Some(stmt) = block.stmts.iter().last() {
+        match stmt.kind {
+            StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => {
+                check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty);
+            }
+            _ => (),
+        }
+    }
+}
+
+
+fn check_final_expr(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    span: Option<Span>,
+    replacement: RetReplacement,
+) {
+    match expr.kind {
+        // simple return is always "bad"
+        ExprKind::Ret(ref inner) => {
+
+            // allow `#[cfg(a)] return a; #[cfg(b)] return b;`
+            if !expr.attrs.iter().any(attr_is_cfg) {
+                emit_return_lint(
+                    cx,
+                    span.expect("`else return` is not possible"),
+                    inner.as_ref().map(|i| i.span),
+                    replacement,
+                );
+            }
+        }
+        // a whole block? check it!
+        ExprKind::Block(ref block, _) => {
+            check_block_return(cx, block);
+        }
+        // a match expr, check all arms
+        // 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
+
+        ExprKind::Match(_, ref arms, source) => {
+            match source {
+                MatchSource::Normal => {
+                    for arm in arms.iter() {
+                        check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Block);
+                    }
+                }
+                MatchSource::IfDesugar { contains_else_clause: true } | MatchSource::IfLetDesugar { contains_else_clause: true } => {
+                    if let ExprKind::Block(ref ifblock, _) = arms[0].body.kind {
+                        check_block_return(cx, ifblock);
+                    }
+                    check_final_expr(cx, arms[1].body, None, RetReplacement::Empty);
+                }
+                _ => ()
+            }
+        }
+        _ => (),
+    }
+}
+
+fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option<Span>, replacement: RetReplacement) {
+    match inner_span {
+        Some(inner_span) => {
+            if in_external_macro(cx.tcx.sess, inner_span) || inner_span.from_expansion() {
+                return;
+            }
+
+            span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded `return` statement", |diag| {
+                if let Some(snippet) = snippet_opt(cx, inner_span) {
+                    diag.span_suggestion(ret_span, "remove `return`", snippet, Applicability::MachineApplicable);
+                }
+            })
+        }
+        None => match replacement {
+            RetReplacement::Empty => {
+                span_lint_and_sugg(
+                    cx,
+                    NEEDLESS_RETURN,
+                    ret_span,
+                    "unneeded `return` statement",
+                    "remove `return`",
+                    String::new(),
+                    Applicability::MachineApplicable,
+                );
+            }
+            RetReplacement::Block => {
+                span_lint_and_sugg(
+                    cx,
+                    NEEDLESS_RETURN,
+                    ret_span,
+                    "unneeded `return` statement",
+                    "replace `return` with an empty block",
+                    "{}".to_string(),
+                    Applicability::MachineApplicable,
+                );
+            }
+        },
+    }
+}

clippy_lints/src/returns.rs

@@ -3,38 +3,11 @@ use rustc_ast::ast;
 use rustc_ast::visit::FnKind;
 use rustc_errors::Applicability;
 use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
-use rustc_middle::lint::in_external_macro;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
 use rustc_span::BytePos;
 
-use crate::utils::{snippet_opt, span_lint_and_sugg, span_lint_and_then};
+use crate::utils::{span_lint_and_sugg};
-
-declare_clippy_lint! {
-    /// **What it does:** Checks for return statements at the end of a block.
-    ///
-    /// **Why is this bad?** Removing the `return` and semicolon will make the code
-    /// more rusty.
-    ///
-    /// **Known problems:** If the computation returning the value borrows a local
-    /// variable, removing the `return` may run afoul of the borrow checker.
-    ///
-    /// **Example:**
-    /// ```rust
-    /// fn foo(x: usize) -> usize {
-    ///     return x;
-    /// }
-    /// ```
-    /// simplify to
-    /// ```rust
-    /// fn foo(x: usize) -> usize {
-    ///     x
-    /// }
-    /// ```
-    pub NEEDLESS_RETURN,
-    style,
-    "using a return statement like `return expr;` where an expression would suffice"
-}
 
 declare_clippy_lint! {
     /// **What it does:** Checks for unit (`()`) expressions that can be removed.
@@ -57,117 +30,12 @@ declare_clippy_lint! {
     "needless unit expression"
 }
 
-#[derive(PartialEq, Eq, Copy, Clone)]
-enum RetReplacement {
-    Empty,
-    Block,
-}
 
-declare_lint_pass!(Return => [NEEDLESS_RETURN, UNUSED_UNIT]);
+declare_lint_pass!(Return => [UNUSED_UNIT]);
-
-impl Return {
-    // Check the final stmt or expr in a block for unnecessary return.
-    fn check_block_return(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) {
-        if let Some(stmt) = block.stmts.last() {
-            match stmt.kind {
-                ast::StmtKind::Expr(ref expr) | ast::StmtKind::Semi(ref expr) => {
-                    self.check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty);
-                },
-                _ => (),
-            }
-        }
-    }
-
-    // Check the final expression in a block if it's a return.
-    fn check_final_expr(
-        &mut self,
-        cx: &EarlyContext<'_>,
-        expr: &ast::Expr,
-        span: Option<Span>,
-        replacement: RetReplacement,
-    ) {
-        match expr.kind {
-            // simple return is always "bad"
-            ast::ExprKind::Ret(ref inner) => {
-                // allow `#[cfg(a)] return a; #[cfg(b)] return b;`
-                if !expr.attrs.iter().any(attr_is_cfg) {
-                    Self::emit_return_lint(
-                        cx,
-                        span.expect("`else return` is not possible"),
-                        inner.as_ref().map(|i| i.span),
-                        replacement,
-                    );
-                }
-            },
-            // a whole block? check it!
-            ast::ExprKind::Block(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
-            ast::ExprKind::If(_, ref ifblock, Some(ref elsexpr)) => {
-                self.check_block_return(cx, ifblock);
-                self.check_final_expr(cx, elsexpr, None, RetReplacement::Empty);
-            },
-            // a match expr, check all arms
-            ast::ExprKind::Match(_, ref arms) => {
-                for arm in arms {
-                    self.check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Block);
-                }
-            },
-            _ => (),
-        }
-    }
-
-    fn emit_return_lint(cx: &EarlyContext<'_>, ret_span: Span, inner_span: Option<Span>, replacement: RetReplacement) {
-        match inner_span {
-            Some(inner_span) => {
-                if in_external_macro(cx.sess(), inner_span) || inner_span.from_expansion() {
-                    return;
-                }
-
-                span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded `return` statement", |diag| {
-                    if let Some(snippet) = snippet_opt(cx, inner_span) {
-                        diag.span_suggestion(ret_span, "remove `return`", snippet, Applicability::MachineApplicable);
-                    }
-                })
-            },
-            None => match replacement {
-                RetReplacement::Empty => {
-                    span_lint_and_sugg(
-                        cx,
-                        NEEDLESS_RETURN,
-                        ret_span,
-                        "unneeded `return` statement",
-                        "remove `return`",
-                        String::new(),
-                        Applicability::MachineApplicable,
-                    );
-                },
-                RetReplacement::Block => {
-                    span_lint_and_sugg(
-                        cx,
-                        NEEDLESS_RETURN,
-                        ret_span,
-                        "unneeded `return` statement",
-                        "replace `return` with an empty block",
-                        "{}".to_string(),
-                        Applicability::MachineApplicable,
-                    );
-                },
-            },
-        }
-    }
-}
 
 impl EarlyLintPass for Return {
     fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, span: Span, _: ast::NodeId) {
-        match kind {
+
-            FnKind::Fn(.., Some(block)) => self.check_block_return(cx, block),
-            FnKind::Closure(_, body) => self.check_final_expr(cx, body, Some(body.span), RetReplacement::Empty),
-            FnKind::Fn(.., None) => {},
-        }
         if_chain! {
             if let ast::FnRetTy::Ty(ref ty) = kind.decl().output;
             if let ast::TyKind::Tup(ref vals) = ty.kind;
@@ -234,9 +102,6 @@ impl EarlyLintPass for Return {
     }
 }
 
-fn attr_is_cfg(attr: &ast::Attribute) -> bool {
-    attr.meta_item_list().is_some() && attr.has_name(sym!(cfg))
-}
 
 // get the def site
 #[must_use]

src/lintlist/mod.rs

@@ -1534,7 +1534,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         group: "style",
         desc: "using a return statement like `return expr;` where an expression would suffice",
         deprecation: None,
-        module: "returns",
+        module: "needless_return",
     },
     Lint {
         name: "needless_update",