clippy_lints/src/returns.rs

   1 // Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
   2 // file at the top-level directory of this distribution.
   3 //
   4 // Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
   5 // http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
   6 // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
   7 // option. This file may not be copied, modified, or distributed
   8 // except according to those terms.
   9
  10 use crate::rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintContext, LintPass};
  11 use crate::rustc::{declare_tool_lint, lint_array};
  12 use crate::rustc_errors::Applicability;
  13 use crate::syntax::ast;
  14 use crate::syntax::source_map::Span;
  15 use crate::syntax::visit::FnKind;
  16 use crate::syntax_pos::BytePos;
  17 use crate::utils::{in_macro, match_path_ast, snippet_opt, span_lint_and_then, span_note_and_lint};
  18 use if_chain::if_chain;
  19
  20 /// **What it does:** Checks for return statements at the end of a block.
  21 ///
  22 /// **Why is this bad?** Removing the `return` and semicolon will make the code
  23 /// more rusty.
  24 ///
  25 /// **Known problems:** If the computation returning the value borrows a local
  26 /// variable, removing the `return` may run afoul of the borrow checker.
  27 ///
  28 /// **Example:**
  29 /// ```rust
  30 /// fn foo(x: usize) {
  31 ///     return x;
  32 /// }
  33 /// ```
  34 /// simplify to
  35 /// ```rust
  36 /// fn foo(x: usize) {
  37 ///     x
  38 /// }
  39 /// ```
  40 declare_clippy_lint! {
  41     pub NEEDLESS_RETURN,
  42     style,
  43     "using a return statement like `return expr;` where an expression would suffice"
  44 }
  45
  46 /// **What it does:** Checks for `let`-bindings, which are subsequently
  47 /// returned.
  48 ///
  49 /// **Why is this bad?** It is just extraneous code. Remove it to make your code
  50 /// more rusty.
  51 ///
  52 /// **Known problems:** None.
  53 ///
  54 /// **Example:**
  55 /// ```rust
  56 /// fn foo() -> String {
  57 ///     let x = String::new();
  58 ///     x
  59 /// }
  60 /// ```
  61 /// instead, use
  62 /// ```
  63 /// fn foo() -> String {
  64 ///     String::new()
  65 /// }
  66 /// ```
  67 declare_clippy_lint! {
  68 pub LET_AND_RETURN,
  69 style,
  70 "creating a let-binding and then immediately returning it like `let x = expr; x` at \
  71  the end of a block"
  72 }
  73
  74 /// **What it does:** Checks for unit (`()`) expressions that can be removed.
  75 ///
  76 /// **Why is this bad?** Such expressions add no value, but can make the code
  77 /// less readable. Depending on formatting they can make a `break` or `return`
  78 /// statement look like a function call.
  79 ///
  80 /// **Known problems:** The lint currently misses unit return types in types,
  81 /// e.g. the `F` in `fn generic_unit<F: Fn() -> ()>(f: F) { .. }`.
  82 ///
  83 /// **Example:**
  84 /// ```rust
  85 /// fn return_unit() -> () {
  86 ///     ()
  87 /// }
  88 /// ```
  89 declare_clippy_lint! {
  90     pub UNUSED_UNIT,
  91     style,
  92     "needless unit expression"
  93 }
  94
  95 #[derive(Copy, Clone)]
  96 pub struct ReturnPass;
  97
  98 impl ReturnPass {
  99     // Check the final stmt or expr in a block for unnecessary return.
 100     fn check_block_return(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) {
 101         if let Some(stmt) = block.stmts.last() {
 102             match stmt.node {
 103                 ast::StmtKind::Expr(ref expr) | ast::StmtKind::Semi(ref expr) => {
 104                     self.check_final_expr(cx, expr, Some(stmt.span));
 105                 },
 106                 _ => (),
 107             }
 108         }
 109     }
 110
 111     // Check a the final expression in a block if it's a return.
 112     fn check_final_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr, span: Option<Span>) {
 113         match expr.node {
 114             // simple return is always "bad"
 115             ast::ExprKind::Ret(Some(ref inner)) => {
 116                 // allow `#[cfg(a)] return a; #[cfg(b)] return b;`
 117                 if !expr.attrs.iter().any(attr_is_cfg) {
 118                     self.emit_return_lint(cx, span.expect("`else return` is not possible"), inner.span);
 119                 }
 120             },
 121             // a whole block? check it!
 122             ast::ExprKind::Block(ref block, _) => {
 123                 self.check_block_return(cx, block);
 124             },
 125             // an if/if let expr, check both exprs
 126             // note, if without else is going to be a type checking error anyways
 127             // (except for unit type functions) so we don't match it
 128             ast::ExprKind::If(_, ref ifblock, Some(ref elsexpr)) => {
 129                 self.check_block_return(cx, ifblock);
 130                 self.check_final_expr(cx, elsexpr, None);
 131             },
 132             // a match expr, check all arms
 133             ast::ExprKind::Match(_, ref arms) => {
 134                 for arm in arms {
 135                     self.check_final_expr(cx, &arm.body, Some(arm.body.span));
 136                 }
 137             },
 138             _ => (),
 139         }
 140     }
 141
 142     fn emit_return_lint(&mut self, cx: &EarlyContext<'_>, ret_span: Span, inner_span: Span) {
 143         if in_external_macro(cx.sess(), inner_span) || in_macro(inner_span) {
 144             return;
 145         }
 146         span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| {
 147             if let Some(snippet) = snippet_opt(cx, inner_span) {
 148                 db.span_suggestion_with_applicability(
 149                     ret_span,
 150                     "remove `return` as shown",
 151                     snippet,
 152                     Applicability::MachineApplicable,
 153                 );
 154             }
 155         });
 156     }
 157
 158     // Check for "let x = EXPR; x"
 159     fn check_let_return(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) {
 160         let mut it = block.stmts.iter();
 161
 162         // we need both a let-binding stmt and an expr
 163         if_chain! {
 164             if let Some(retexpr) = it.next_back();
 165             if let ast::StmtKind::Expr(ref retexpr) = retexpr.node;
 166             if let Some(stmt) = it.next_back();
 167             if let ast::StmtKind::Local(ref local) = stmt.node;
 168             // don't lint in the presence of type inference
 169             if local.ty.is_none();
 170             if !local.attrs.iter().any(attr_is_cfg);
 171             if let Some(ref initexpr) = local.init;
 172             if let ast::PatKind::Ident(_, ident, _) = local.pat.node;
 173             if let ast::ExprKind::Path(_, ref path) = retexpr.node;
 174             if match_path_ast(path, &[&ident.as_str()]);
 175             if !in_external_macro(cx.sess(), initexpr.span);
 176             then {
 177                     span_note_and_lint(cx,
 178                                        LET_AND_RETURN,
 179                                        retexpr.span,
 180                                        "returning the result of a let binding from a block. \
 181                                        Consider returning the expression directly.",
 182                                        initexpr.span,
 183                                        "this expression can be directly returned");
 184             }
 185         }
 186     }
 187 }
 188
 189 impl LintPass for ReturnPass {
 190     fn get_lints(&self) -> LintArray {
 191         lint_array!(NEEDLESS_RETURN, LET_AND_RETURN, UNUSED_UNIT)
 192     }
 193 }
 194
 195 impl EarlyLintPass for ReturnPass {
 196     fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, decl: &ast::FnDecl, span: Span, _: ast::NodeId) {
 197         match kind {
 198             FnKind::ItemFn(.., block) | FnKind::Method(.., block) => self.check_block_return(cx, block),
 199             FnKind::Closure(body) => self.check_final_expr(cx, body, Some(body.span)),
 200         }
 201         if_chain! {
 202             if let ast::FunctionRetTy::Ty(ref ty) = decl.output;
 203             if let ast::TyKind::Tup(ref vals) = ty.node;
 204             if vals.is_empty() && !in_macro(ty.span) && get_def(span) == get_def(ty.span);
 205             then {
 206                 let (rspan, appl) = if let Ok(fn_source) =
 207                         cx.sess().source_map()
 208                                  .span_to_snippet(span.with_hi(ty.span.hi())) {
 209                     if let Some(rpos) = fn_source.rfind("->") {
 210                         #[allow(clippy::cast_possible_truncation)]
 211                         (ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)),
 212                             Applicability::MachineApplicable)
 213                     } else {
 214                         (ty.span, Applicability::MaybeIncorrect)
 215                     }
 216                 } else {
 217                     (ty.span, Applicability::MaybeIncorrect)
 218                 };
 219                 span_lint_and_then(cx, UNUSED_UNIT, rspan, "unneeded unit return type", |db| {
 220                     db.span_suggestion_with_applicability(
 221                         rspan,
 222                         "remove the `-> ()`",
 223                         String::new(),
 224                         appl,
 225                     );
 226                 });
 227             }
 228         }
 229     }
 230
 231     fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) {
 232         self.check_let_return(cx, block);
 233         if_chain! {
 234             if let Some(ref stmt) = block.stmts.last();
 235             if let ast::StmtKind::Expr(ref expr) = stmt.node;
 236             if is_unit_expr(expr) && !in_macro(expr.span);
 237             then {
 238                 let sp = expr.span;
 239                 span_lint_and_then(cx, UNUSED_UNIT, sp, "unneeded unit expression", |db| {
 240                     db.span_suggestion_with_applicability(
 241                         sp,
 242                         "remove the final `()`",
 243                         String::new(),
 244                         Applicability::MachineApplicable,
 245                     );
 246                 });
 247             }
 248         }
 249     }
 250
 251     fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
 252         match e.node {
 253             ast::ExprKind::Ret(Some(ref expr)) | ast::ExprKind::Break(_, Some(ref expr)) => {
 254                 if is_unit_expr(expr) && !in_macro(expr.span) {
 255                     span_lint_and_then(cx, UNUSED_UNIT, expr.span, "unneeded `()`", |db| {
 256                         db.span_suggestion_with_applicability(
 257                             expr.span,
 258                             "remove the `()`",
 259                             String::new(),
 260                             Applicability::MachineApplicable,
 261                         );
 262                     });
 263                 }
 264             },
 265             _ => (),
 266         }
 267     }
 268 }
 269
 270 fn attr_is_cfg(attr: &ast::Attribute) -> bool {
 271     attr.meta_item_list().is_some() && attr.name() == "cfg"
 272 }
 273
 274 // get the def site
 275 fn get_def(span: Span) -> Option<Span> {
 276     span.ctxt().outer().expn_info().and_then(|info| info.def_site)
 277 }
 278
 279 // is this expr a `()` unit?
 280 fn is_unit_expr(expr: &ast::Expr) -> bool {
 281     if let ast::ExprKind::Tup(ref vals) = expr.node {
 282         vals.is_empty()
 283     } else {
 284         false
 285     }
 286 }