X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Freturns.rs;h=c3afa156017d21dce3803aa2c0f61d6db568e434;hb=e5a5b0a0774625eebbe7b29c67b49dc6431544d1;hp=5458fa51c7c07e0611d89997f38bf1429f6ccb38;hpb=1cdac4a9c798f8e22a4db220020713175eb9159b;p=rust.git diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 5458fa51c7c..c3afa156017 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -1,13 +1,14 @@ use if_chain::if_chain; +use rustc::declare_lint_pass; use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintContext, LintPass}; -use rustc::{declare_tool_lint, lint_array}; use rustc_errors::Applicability; +use rustc_session::declare_tool_lint; use syntax::ast; use syntax::source_map::Span; use syntax::visit::FnKind; use syntax_pos::BytePos; -use crate::utils::{in_macro, match_path_ast, snippet_opt, span_lint_and_then, span_note_and_lint}; +use crate::utils::{match_path_ast, snippet_opt, span_lint_and_then}; declare_clippy_lint! { /// **What it does:** Checks for return statements at the end of a block. @@ -83,16 +84,21 @@ "needless unit expression" } -#[derive(Copy, Clone)] -pub struct ReturnPass; +#[derive(PartialEq, Eq, Copy, Clone)] +enum RetReplacement { + Empty, + Block, +} + +declare_lint_pass!(Return => [NEEDLESS_RETURN, LET_AND_RETURN, UNUSED_UNIT]); -impl ReturnPass { +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.node { + match stmt.kind { ast::StmtKind::Expr(ref expr) | ast::StmtKind::Semi(ref expr) => { - self.check_final_expr(cx, expr, Some(stmt.span)); + self.check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty); }, _ => (), } @@ -100,13 +106,24 @@ fn check_block_return(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) { } // Check a the final expression in a block if it's a return. - fn check_final_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr, span: Option) { - match expr.node { + fn check_final_expr( + &mut self, + cx: &EarlyContext<'_>, + expr: &ast::Expr, + span: Option, + replacement: RetReplacement, + ) { + match expr.kind { // simple return is always "bad" - ast::ExprKind::Ret(Some(ref inner)) => { + 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.span); + 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! @@ -118,85 +135,114 @@ fn check_final_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr, span: Op // (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); + 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)); + self.check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Block); } }, _ => (), } } - fn emit_return_lint(&mut self, cx: &EarlyContext<'_>, ret_span: Span, inner_span: Span) { - if in_external_macro(cx.sess(), inner_span) || in_macro(inner_span) { - return; + fn emit_return_lint(cx: &EarlyContext<'_>, ret_span: Span, inner_span: Option, 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", |db| { + if let Some(snippet) = snippet_opt(cx, inner_span) { + db.span_suggestion(ret_span, "remove `return`", snippet, Applicability::MachineApplicable); + } + }) + }, + None => match replacement { + RetReplacement::Empty => { + span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| { + db.span_suggestion( + ret_span, + "remove `return`", + String::new(), + Applicability::MachineApplicable, + ); + }); + }, + RetReplacement::Block => { + span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| { + db.span_suggestion( + ret_span, + "replace `return` with an empty block", + "{}".to_string(), + Applicability::MachineApplicable, + ); + }); + }, + }, } - span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| { - if let Some(snippet) = snippet_opt(cx, inner_span) { - db.span_suggestion( - ret_span, - "remove `return` as shown", - snippet, - Applicability::MachineApplicable, - ); - } - }); } // Check for "let x = EXPR; x" - fn check_let_return(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) { + fn check_let_return(cx: &EarlyContext<'_>, block: &ast::Block) { let mut it = block.stmts.iter(); // we need both a let-binding stmt and an expr if_chain! { if let Some(retexpr) = it.next_back(); - if let ast::StmtKind::Expr(ref retexpr) = retexpr.node; + if let ast::StmtKind::Expr(ref retexpr) = retexpr.kind; if let Some(stmt) = it.next_back(); - if let ast::StmtKind::Local(ref local) = stmt.node; + if let ast::StmtKind::Local(ref local) = stmt.kind; // don't lint in the presence of type inference if local.ty.is_none(); - if !local.attrs.iter().any(attr_is_cfg); + if local.attrs.is_empty(); if let Some(ref initexpr) = local.init; - if let ast::PatKind::Ident(_, ident, _) = local.pat.node; - if let ast::ExprKind::Path(_, ref path) = retexpr.node; - if match_path_ast(path, &[&ident.as_str()]); + if let ast::PatKind::Ident(_, ident, _) = local.pat.kind; + if let ast::ExprKind::Path(_, ref path) = retexpr.kind; + if match_path_ast(path, &[&*ident.name.as_str()]); if !in_external_macro(cx.sess(), initexpr.span); + if !in_external_macro(cx.sess(), retexpr.span); + if !in_external_macro(cx.sess(), local.span); then { - span_note_and_lint(cx, - LET_AND_RETURN, - retexpr.span, - "returning the result of a let binding from a block. \ - Consider returning the expression directly.", - initexpr.span, - "this expression can be directly returned"); + span_lint_and_then( + cx, + LET_AND_RETURN, + retexpr.span, + "returning the result of a let binding from a block", + |err| { + err.span_label(local.span, "unnecessary let binding"); + + if let Some(snippet) = snippet_opt(cx, initexpr.span) { + err.multipart_suggestion( + "return the expression directly", + vec![ + (local.span, String::new()), + (retexpr.span, snippet), + ], + Applicability::MachineApplicable, + ); + } else { + err.span_help(initexpr.span, "this expression can be directly returned"); + } + }, + ); } } } } -impl LintPass for ReturnPass { - fn get_lints(&self) -> LintArray { - lint_array!(NEEDLESS_RETURN, LET_AND_RETURN, UNUSED_UNIT) - } - - fn name(&self) -> &'static str { - "Return" - } -} - -impl EarlyLintPass for ReturnPass { +impl EarlyLintPass for Return { fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, decl: &ast::FnDecl, span: Span, _: ast::NodeId) { match kind { FnKind::ItemFn(.., block) | FnKind::Method(.., block) => self.check_block_return(cx, block), - FnKind::Closure(body) => self.check_final_expr(cx, body, Some(body.span)), + FnKind::Closure(body) => self.check_final_expr(cx, body, Some(body.span), RetReplacement::Empty), } if_chain! { if let ast::FunctionRetTy::Ty(ref ty) = decl.output; - if let ast::TyKind::Tup(ref vals) = ty.node; - if vals.is_empty() && !in_macro(ty.span) && get_def(span) == get_def(ty.span); + if let ast::TyKind::Tup(ref vals) = ty.kind; + if vals.is_empty() && !ty.span.from_expansion() && get_def(span) == get_def(ty.span); then { let (rspan, appl) = if let Ok(fn_source) = cx.sess().source_map() @@ -224,11 +270,11 @@ fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, decl: &ast::FnDe } fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) { - self.check_let_return(cx, block); + Self::check_let_return(cx, block); if_chain! { if let Some(ref stmt) = block.stmts.last(); - if let ast::StmtKind::Expr(ref expr) = stmt.node; - if is_unit_expr(expr) && !in_macro(expr.span); + if let ast::StmtKind::Expr(ref expr) = stmt.kind; + if is_unit_expr(expr) && !stmt.span.from_expansion(); then { let sp = expr.span; span_lint_and_then(cx, UNUSED_UNIT, sp, "unneeded unit expression", |db| { @@ -244,9 +290,9 @@ fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) { } fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { - match e.node { + match e.kind { ast::ExprKind::Ret(Some(ref expr)) | ast::ExprKind::Break(_, Some(ref expr)) => { - if is_unit_expr(expr) && !in_macro(expr.span) { + if is_unit_expr(expr) && !expr.span.from_expansion() { span_lint_and_then(cx, UNUSED_UNIT, expr.span, "unneeded `()`", |db| { db.span_suggestion( expr.span, @@ -263,17 +309,22 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { } fn attr_is_cfg(attr: &ast::Attribute) -> bool { - attr.meta_item_list().is_some() && attr.name() == "cfg" + attr.meta_item_list().is_some() && attr.check_name(sym!(cfg)) } // get the def site +#[must_use] fn get_def(span: Span) -> Option { - span.ctxt().outer().expn_info().and_then(|info| info.def_site) + if span.from_expansion() { + Some(span.ctxt().outer_expn_data().def_site) + } else { + None + } } // is this expr a `()` unit? fn is_unit_expr(expr: &ast::Expr) -> bool { - if let ast::ExprKind::Tup(ref vals) = expr.node { + if let ast::ExprKind::Tup(ref vals) = expr.kind { vals.is_empty() } else { false