]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/implicit_return.rs
Auto merge of #4809 - iankronquist:patch-1, r=flip1995
[rust.git] / clippy_lints / src / implicit_return.rs
index 559b0942d2b57a2947976c40b3aea31e834d25a8..b0d80aa0d539d30875d1a18d2e2617d81aa6e2b6 100644 (file)
@@ -1,9 +1,15 @@
-use crate::utils::{in_macro, is_expn_of, snippet_opt, span_lint_and_then};
-use rustc::hir::{intravisit::FnKind, Body, ExprKind, FnDecl, HirId, MatchSource};
-use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
-use rustc::{declare_lint_pass, declare_tool_lint};
+use crate::utils::{
+    match_def_path,
+    paths::{BEGIN_PANIC, BEGIN_PANIC_FMT},
+    snippet_opt, span_lint_and_then,
+};
+use if_chain::if_chain;
 use rustc_errors::Applicability;
-use syntax::source_map::Span;
+use rustc_hir::intravisit::FnKind;
+use rustc_hir::{Body, Expr, ExprKind, FnDecl, HirId, MatchSource, StmtKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::source_map::Span;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for missing return statements at the end of a block.
     ///
     /// **Example:**
     /// ```rust
-    /// fn foo(x: usize) {
+    /// fn foo(x: usize) -> usize {
     ///     x
     /// }
     /// ```
     /// add return
     /// ```rust
-    /// fn foo(x: usize) {
+    /// fn foo(x: usize) -> usize {
     ///     return x;
     /// }
     /// ```
 
 declare_lint_pass!(ImplicitReturn => [IMPLICIT_RETURN]);
 
-impl ImplicitReturn {
-    fn lint(cx: &LateContext<'_, '_>, outer_span: syntax_pos::Span, inner_span: syntax_pos::Span, msg: &str) {
-        span_lint_and_then(cx, IMPLICIT_RETURN, outer_span, "missing return statement", |db| {
-            if let Some(snippet) = snippet_opt(cx, inner_span) {
-                db.span_suggestion(
-                    outer_span,
-                    msg,
-                    format!("return {}", snippet),
-                    Applicability::MachineApplicable,
-                );
-            }
-        });
-    }
+static LINT_BREAK: &str = "change `break` to `return` as shown";
+static LINT_RETURN: &str = "add `return` as shown";
 
-    fn expr_match(cx: &LateContext<'_, '_>, expr: &rustc::hir::Expr) {
-        match &expr.node {
-            // loops could be using `break` instead of `return`
-            ExprKind::Block(block, ..) | ExprKind::Loop(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 {
-                        // make sure it's a break, otherwise we want to skip
-                        if let ExprKind::Break(.., break_expr) = &expr.node {
-                            if let Some(break_expr) = break_expr {
-                                Self::lint(cx, expr.span, break_expr.span, "change `break` to `return` as shown");
-                            }
-                        }
-                    }
-                }
-            },
-            // use `return` instead of `break`
-            ExprKind::Break(.., break_expr) => {
-                if let Some(break_expr) = break_expr {
-                    Self::lint(cx, expr.span, break_expr.span, "change `break` to `return` as shown");
-                }
-            },
-            ExprKind::If(.., if_expr, else_expr) => {
-                Self::expr_match(cx, if_expr);
+fn lint(cx: &LateContext<'_, '_>, outer_span: Span, inner_span: Span, msg: &str) {
+    let outer_span = outer_span.source_callsite();
+    let inner_span = inner_span.source_callsite();
 
-                if let Some(else_expr) = else_expr {
-                    Self::expr_match(cx, else_expr);
-                }
-            },
-            ExprKind::Match(.., arms, source) => {
-                let check_all_arms = match source {
-                    MatchSource::IfLetDesugar {
-                        contains_else_clause: has_else,
-                    } => *has_else,
-                    _ => true,
-                };
+    span_lint_and_then(cx, IMPLICIT_RETURN, outer_span, "missing `return` statement", |db| {
+        if let Some(snippet) = snippet_opt(cx, inner_span) {
+            db.span_suggestion(
+                outer_span,
+                msg,
+                format!("return {}", snippet),
+                Applicability::MachineApplicable,
+            );
+        }
+    });
+}
 
-                if check_all_arms {
-                    for arm in arms {
-                        Self::expr_match(cx, &arm.body);
+fn expr_match(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
+    match expr.kind {
+        // loops could be using `break` instead of `return`
+        ExprKind::Block(block, ..) | ExprKind::Loop(block, ..) => {
+            if let Some(expr) = &block.expr {
+                expr_match(cx, expr);
+            }
+            // only needed in the case of `break` with `;` at the end
+            else if let Some(stmt) = block.stmts.last() {
+                if_chain! {
+                    if let StmtKind::Semi(expr, ..) = &stmt.kind;
+                    // make sure it's a break, otherwise we want to skip
+                    if let ExprKind::Break(.., break_expr) = &expr.kind;
+                    if let Some(break_expr) = break_expr;
+                    then {
+                            lint(cx, expr.span, break_expr.span, LINT_BREAK);
                     }
-                } else {
-                    Self::expr_match(cx, &arms.first().expect("if let doesn't have a single arm").body);
                 }
-            },
-            // skip if it already has a return statement
-            ExprKind::Ret(..) => (),
-            // everything else is missing `return`
-            _ => {
-                // make sure it's not just an unreachable expression
-                if is_expn_of(expr.span, "unreachable").is_none() {
-                    Self::lint(cx, expr.span, expr.span, "add `return` as shown")
+            }
+        },
+        // use `return` instead of `break`
+        ExprKind::Break(.., break_expr) => {
+            if let Some(break_expr) = break_expr {
+                lint(cx, expr.span, break_expr.span, LINT_BREAK);
+            }
+        },
+        ExprKind::Match(.., arms, source) => {
+            let check_all_arms = match source {
+                MatchSource::IfLetDesugar {
+                    contains_else_clause: has_else,
+                } => has_else,
+                _ => true,
+            };
+
+            if check_all_arms {
+                for arm in arms {
+                    expr_match(cx, &arm.body);
+                }
+            } else {
+                expr_match(cx, &arms.first().expect("`if let` doesn't have a single arm").body);
+            }
+        },
+        // skip if it already has a return statement
+        ExprKind::Ret(..) => (),
+        // make sure it's not a call that panics
+        ExprKind::Call(expr, ..) => {
+            if_chain! {
+                if let ExprKind::Path(qpath) = &expr.kind;
+                if let Some(path_def_id) = cx.tables.qpath_res(qpath, expr.hir_id).opt_def_id();
+                if match_def_path(cx, path_def_id, &BEGIN_PANIC) ||
+                    match_def_path(cx, path_def_id, &BEGIN_PANIC_FMT);
+                then { }
+                else {
+                    lint(cx, expr.span, expr.span, LINT_RETURN)
                 }
-            },
-        }
+            }
+        },
+        // everything else is missing `return`
+        _ => lint(cx, expr.span, expr.span, LINT_RETURN),
     }
 }
 
@@ -115,8 +127,8 @@ fn check_fn(
         &mut self,
         cx: &LateContext<'a, 'tcx>,
         _: FnKind<'tcx>,
-        _: &'tcx FnDecl,
-        body: &'tcx Body,
+        _: &'tcx FnDecl<'_>,
+        body: &'tcx Body<'_>,
         span: Span,
         _: HirId,
     ) {
@@ -125,8 +137,8 @@ fn check_fn(
 
         // checking return type through MIR, HIR is not able to determine inferred closure return types
         // make sure it's not a macro
-        if !mir.return_ty().is_unit() && !in_macro(span) {
-            Self::expr_match(cx, &body.value);
+        if !mir.return_ty().is_unit() && !span.from_expansion() {
+            expr_match(cx, &body.value);
         }
     }
 }