modify code

[rust.git] / clippy_lints / src / returns.rs

diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 251d527c265221348e81548b6d4e7062bda4a8cf..8068fa22d9ccf8eea0fb4801d0d9b3f92193184a 100644 (file)
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -1,29 +1,29 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::snippet_opt;
-use clippy_utils::{fn_def_id, in_macro, path_to_local_id};
+use clippy_utils::{fn_def_id, path_to_local_id};
 use if_chain::if_chain;
 use rustc_ast::ast::Attribute;
 use rustc_errors::Applicability;
-use rustc_hir::intravisit::{walk_expr, FnKind, NestedVisitorMap, Visitor};
+use rustc_hir::intravisit::{walk_expr, FnKind, Visitor};
 use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, PatKind, StmtKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
-use rustc_middle::hir::map::Map;
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::subst::GenericArgKind;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::hygiene::DesugaringKind;
 use rustc_span::source_map::Span;
 use rustc_span::sym;
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for `let`-bindings, which are subsequently
+    /// ### What it does
+    /// Checks for `let`-bindings, which are subsequently
     /// returned.
     ///
-    /// **Why is this bad?** It is just extraneous code. Remove it to make your code
+    /// ### Why is this bad?
+    /// It is just extraneous code. Remove it to make your code
     /// more rusty.
     ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
+    /// ### Example
     /// ```rust
     /// fn foo() -> String {
     ///     let x = String::new();
@@ -36,20 +36,21 @@
     ///     String::new()
     /// }
     /// ```
+    #[clippy::version = "pre 1.29.0"]
     pub LET_AND_RETURN,
     style,
     "creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block"
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for return statements at the end of a block.
+    /// ### 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
+    /// ### Why is this bad?
+    /// Removing the `return` and semicolon will make the code
     /// more rusty.
     ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
+    /// ### Example
     /// ```rust
     /// fn foo(x: usize) -> usize {
     ///     return x;
@@ -61,6 +62,7 @@
     ///     x
     /// }
     /// ```
+    #[clippy::version = "pre 1.29.0"]
     pub NEEDLESS_RETURN,
     style,
     "using a return statement like `return expr;` where an expression would suffice"
@@ -70,6 +72,7 @@
 enum RetReplacement {
     Empty,
     Block,
+    Unit,
 }
 
 declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN]);
@@ -89,8 +92,7 @@ fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) {
             if !last_statement_borrows(cx, initexpr);
             if !in_external_macro(cx.sess(), initexpr.span);
             if !in_external_macro(cx.sess(), retexpr.span);
-            if !in_external_macro(cx.sess(), local.span);
-            if !in_macro(local.span);
+            if !local.span.from_expansion();
             then {
                 span_lint_and_then(
                     cx,
@@ -199,28 +201,19 @@ fn check_final_expr<'tcx>(
                 check_block_return(cx, ifblock);
             }
             if let Some(else_clause) = else_clause_opt {
-                check_final_expr(cx, else_clause, None, RetReplacement::Empty);
+                if expr.span.desugaring_kind() != Some(DesugaringKind::LetElse) {
+                    check_final_expr(cx, else_clause, None, RetReplacement::Empty);
+                }
             }
         },
         // 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(_, arms, source) => match source {
-            MatchSource::Normal => {
-                for arm in arms.iter() {
-                    check_final_expr(cx, arm.body, Some(arm.body.span), RetReplacement::Block);
-                }
-            },
-            MatchSource::IfLetDesugar {
-                contains_else_clause: true,
-            } => {
-                if let ExprKind::Block(ifblock, _) = arms[0].body.kind {
-                    check_block_return(cx, ifblock);
-                }
-                check_final_expr(cx, arms[1].body, None, RetReplacement::Empty);
-            },
-            _ => (),
+        ExprKind::Match(_, arms, MatchSource::Normal) => {
+            for arm in arms.iter() {
+                check_final_expr(cx, arm.body, Some(arm.body.span), RetReplacement::Unit);
+            }
         },
         ExprKind::DropTemps(expr) => check_final_expr(cx, expr, None, RetReplacement::Empty),
         _ => (),
@@ -266,6 +259,17 @@ fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option<Spa
                     Applicability::MachineApplicable,
                 );
             },
+            RetReplacement::Unit => {
+                span_lint_and_sugg(
+                    cx,
+                    NEEDLESS_RETURN,
+                    ret_span,
+                    "unneeded `return` statement",
+                    "replace `return` with a unit value",
+                    "()".to_string(),
+                    Applicability::MachineApplicable,
+                );
+            },
         },
     }
 }
@@ -282,8 +286,6 @@ struct BorrowVisitor<'a, 'tcx> {
 }
 
 impl<'tcx> Visitor<'tcx> for BorrowVisitor<'_, 'tcx> {
-    type Map = Map<'tcx>;
-
     fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
         if self.borrows {
             return;
@@ -302,8 +304,4 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
 
         walk_expr(self, expr);
     }
-
-    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::None
-    }
 }