Make let_and_return a late lint pass

This commit is contained in:
Eduardo Broto 2020-06-03 01:13:57 +02:00
parent 67ec96c8f9
commit dc13016ac2
4 changed files with 91 additions and 83 deletions

View file

@ -0,0 +1,82 @@
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Block, ExprKind, PatKind, StmtKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use crate::utils::{in_macro, match_qpath, snippet_opt, span_lint_and_then};
declare_clippy_lint! {
/// **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
/// more rusty.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// fn foo() -> String {
/// let x = String::new();
/// x
/// }
/// ```
/// instead, use
/// ```
/// fn foo() -> String {
/// String::new()
/// }
/// ```
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_lint_pass!(LetReturn => [LET_AND_RETURN]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetReturn {
fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx Block<'_>) {
// we need both a let-binding stmt and an expr
if_chain! {
if let Some(retexpr) = block.expr;
if let Some(stmt) = block.stmts.iter().last();
if let StmtKind::Local(local) = &stmt.kind;
if local.ty.is_none();
if local.attrs.is_empty();
if let Some(initexpr) = &local.init;
if let PatKind::Binding(.., ident, _) = local.pat.kind;
if let ExprKind::Path(qpath) = &retexpr.kind;
if match_qpath(qpath, &[&*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);
if !in_macro(local.span);
then {
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");
}
},
);
}
}
}
}

View file

@ -239,6 +239,7 @@ mod large_const_arrays;
mod large_enum_variant;
mod large_stack_arrays;
mod len_zero;
mod let_and_return;
mod let_if_seq;
mod let_underscore;
mod lifetimes;
@ -596,6 +597,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&large_stack_arrays::LARGE_STACK_ARRAYS,
&len_zero::LEN_WITHOUT_IS_EMPTY,
&len_zero::LEN_ZERO,
&let_and_return::LET_AND_RETURN,
&let_if_seq::USELESS_LET_IF_SEQ,
&let_underscore::LET_UNDERSCORE_LOCK,
&let_underscore::LET_UNDERSCORE_MUST_USE,
@ -772,7 +774,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&regex::INVALID_REGEX,
&regex::REGEX_MACRO,
&regex::TRIVIAL_REGEX,
&returns::LET_AND_RETURN,
&returns::NEEDLESS_RETURN,
&returns::UNUSED_UNIT,
&serde_api::SERDE_API_MISUSE,
@ -1022,6 +1023,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| box formatting::Formatting);
store.register_early_pass(|| box misc_early::MiscEarlyLints);
store.register_early_pass(|| box returns::Return);
store.register_late_pass(|| box let_and_return::LetReturn);
store.register_early_pass(|| box collapsible_if::CollapsibleIf);
store.register_early_pass(|| box items_after_statements::ItemsAfterStatements);
store.register_early_pass(|| box precedence::Precedence);
@ -1265,6 +1267,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
LintId::of(&len_zero::LEN_ZERO),
LintId::of(&let_and_return::LET_AND_RETURN),
LintId::of(&let_underscore::LET_UNDERSCORE_LOCK),
LintId::of(&lifetimes::EXTRA_UNUSED_LIFETIMES),
LintId::of(&lifetimes::NEEDLESS_LIFETIMES),
@ -1390,7 +1393,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&regex::INVALID_REGEX),
LintId::of(&regex::REGEX_MACRO),
LintId::of(&regex::TRIVIAL_REGEX),
LintId::of(&returns::LET_AND_RETURN),
LintId::of(&returns::NEEDLESS_RETURN),
LintId::of(&returns::UNUSED_UNIT),
LintId::of(&serde_api::SERDE_API_MISUSE),
@ -1474,6 +1476,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&inherent_to_string::INHERENT_TO_STRING),
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
LintId::of(&len_zero::LEN_ZERO),
LintId::of(&let_and_return::LET_AND_RETURN),
LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING),
LintId::of(&loops::EMPTY_LOOP),
LintId::of(&loops::FOR_KV_MAP),
@ -1526,7 +1529,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),
LintId::of(&regex::REGEX_MACRO),
LintId::of(&regex::TRIVIAL_REGEX),
LintId::of(&returns::LET_AND_RETURN),
LintId::of(&returns::NEEDLESS_RETURN),
LintId::of(&returns::UNUSED_UNIT),
LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),

View file

@ -8,7 +8,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use rustc_span::BytePos;
use crate::utils::{in_macro, match_path_ast, snippet_opt, span_lint_and_sugg, span_lint_and_then};
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.
@ -36,33 +36,6 @@ declare_clippy_lint! {
"using a return statement like `return expr;` where an expression would suffice"
}
declare_clippy_lint! {
/// **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
/// more rusty.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// fn foo() -> String {
/// let x = String::new();
/// x
/// }
/// ```
/// instead, use
/// ```
/// fn foo() -> String {
/// String::new()
/// }
/// ```
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 unit (`()`) expressions that can be removed.
///
@ -90,7 +63,7 @@ enum RetReplacement {
Block,
}
declare_lint_pass!(Return => [NEEDLESS_RETURN, LET_AND_RETURN, UNUSED_UNIT]);
declare_lint_pass!(Return => [NEEDLESS_RETURN, UNUSED_UNIT]);
impl Return {
// Check the final stmt or expr in a block for unnecessary return.
@ -105,7 +78,7 @@ impl Return {
}
}
// Check a the final expression in a block if it's a return.
// Check the final expression in a block if it's a return.
fn check_final_expr(
&mut self,
cx: &EarlyContext<'_>,
@ -186,54 +159,6 @@ impl Return {
},
}
}
// Check for "let x = EXPR; x"
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.kind;
if let Some(stmt) = it.next_back();
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.is_empty();
if let Some(ref initexpr) = local.init;
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);
if !in_macro(local.span);
then {
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 EarlyLintPass for Return {
@ -254,7 +179,6 @@ impl EarlyLintPass for Return {
}
fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::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.kind;

View file

@ -1023,7 +1023,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
group: "style",
desc: "creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block",
deprecation: None,
module: "returns",
module: "let_and_return",
},
Lint {
name: "let_underscore_lock",