Skip to content

Commit dc13016

Browse files
ebrotoebroto
committed
Make let_and_return a late lint pass
1 parent 67ec96c commit dc13016

File tree

4 files changed

+91
-83
lines changed

4 files changed

+91
-83
lines changed

‎clippy_lints/src/let_and_return.rs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
use if_chain::if_chain;
2+
use rustc_errors::Applicability;
3+
use rustc_hir::{Block, ExprKind, PatKind, StmtKind};
4+
use rustc_lint::{LateContext, LateLintPass, LintContext};
5+
use rustc_middle::lint::in_external_macro;
6+
use rustc_session::{declare_lint_pass, declare_tool_lint};
7+
8+
use crate::utils::{in_macro, match_qpath, snippet_opt, span_lint_and_then};
9+
10+
declare_clippy_lint! {
11+
/// **What it does:** Checks for `let`-bindings, which are subsequently
12+
/// returned.
13+
///
14+
/// **Why is this bad?** It is just extraneous code. Remove it to make your code
15+
/// more rusty.
16+
///
17+
/// **Known problems:** None.
18+
///
19+
/// **Example:**
20+
/// ```rust
21+
/// fn foo() -> String {
22+
/// let x = String::new();
23+
/// x
24+
/// }
25+
/// ```
26+
/// instead, use
27+
/// ```
28+
/// fn foo() -> String {
29+
/// String::new()
30+
/// }
31+
/// ```
32+
pub LET_AND_RETURN,
33+
style,
34+
"creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block"
35+
}
36+
37+
declare_lint_pass!(LetReturn => [LET_AND_RETURN]);
38+
39+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetReturn {
40+
fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx Block<'_>) {
41+
// we need both a let-binding stmt and an expr
42+
if_chain! {
43+
if let Some(retexpr) = block.expr;
44+
if let Some(stmt) = block.stmts.iter().last();
45+
if let StmtKind::Local(local) = &stmt.kind;
46+
if local.ty.is_none();
47+
if local.attrs.is_empty();
48+
if let Some(initexpr) = &local.init;
49+
if let PatKind::Binding(.., ident, _) = local.pat.kind;
50+
if let ExprKind::Path(qpath) = &retexpr.kind;
51+
if match_qpath(qpath, &[&*ident.name.as_str()]);
52+
if !in_external_macro(cx.sess(), initexpr.span);
53+
if !in_external_macro(cx.sess(), retexpr.span);
54+
if !in_external_macro(cx.sess(), local.span);
55+
if !in_macro(local.span);
56+
then {
57+
span_lint_and_then(
58+
cx,
59+
LET_AND_RETURN,
60+
retexpr.span,
61+
"returning the result of a `let` binding from a block",
62+
|err| {
63+
err.span_label(local.span, "unnecessary `let` binding");
64+
65+
if let Some(snippet) = snippet_opt(cx, initexpr.span) {
66+
err.multipart_suggestion(
67+
"return the expression directly",
68+
vec![
69+
(local.span, String::new()),
70+
(retexpr.span, snippet),
71+
],
72+
Applicability::MachineApplicable,
73+
);
74+
} else {
75+
err.span_help(initexpr.span, "this expression can be directly returned");
76+
}
77+
},
78+
);
79+
}
80+
}
81+
}
82+
}

‎clippy_lints/src/lib.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ mod large_const_arrays;
239239
mod large_enum_variant;
240240
mod large_stack_arrays;
241241
mod len_zero;
242+
mod let_and_return;
242243
mod let_if_seq;
243244
mod let_underscore;
244245
mod lifetimes;
@@ -596,6 +597,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
596597
&large_stack_arrays::LARGE_STACK_ARRAYS,
597598
&len_zero::LEN_WITHOUT_IS_EMPTY,
598599
&len_zero::LEN_ZERO,
600+
&let_and_return::LET_AND_RETURN,
599601
&let_if_seq::USELESS_LET_IF_SEQ,
600602
&let_underscore::LET_UNDERSCORE_LOCK,
601603
&let_underscore::LET_UNDERSCORE_MUST_USE,
@@ -772,7 +774,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
772774
&regex::INVALID_REGEX,
773775
&regex::REGEX_MACRO,
774776
&regex::TRIVIAL_REGEX,
775-
&returns::LET_AND_RETURN,
776777
&returns::NEEDLESS_RETURN,
777778
&returns::UNUSED_UNIT,
778779
&serde_api::SERDE_API_MISUSE,
@@ -1022,6 +1023,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10221023
store.register_early_pass(|| box formatting::Formatting);
10231024
store.register_early_pass(|| box misc_early::MiscEarlyLints);
10241025
store.register_early_pass(|| box returns::Return);
1026+
store.register_late_pass(|| box let_and_return::LetReturn);
10251027
store.register_early_pass(|| box collapsible_if::CollapsibleIf);
10261028
store.register_early_pass(|| box items_after_statements::ItemsAfterStatements);
10271029
store.register_early_pass(|| box precedence::Precedence);
@@ -1265,6 +1267,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12651267
LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
12661268
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
12671269
LintId::of(&len_zero::LEN_ZERO),
1270+
LintId::of(&let_and_return::LET_AND_RETURN),
12681271
LintId::of(&let_underscore::LET_UNDERSCORE_LOCK),
12691272
LintId::of(&lifetimes::EXTRA_UNUSED_LIFETIMES),
12701273
LintId::of(&lifetimes::NEEDLESS_LIFETIMES),
@@ -1390,7 +1393,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13901393
LintId::of(&regex::INVALID_REGEX),
13911394
LintId::of(&regex::REGEX_MACRO),
13921395
LintId::of(&regex::TRIVIAL_REGEX),
1393-
LintId::of(&returns::LET_AND_RETURN),
13941396
LintId::of(&returns::NEEDLESS_RETURN),
13951397
LintId::of(&returns::UNUSED_UNIT),
13961398
LintId::of(&serde_api::SERDE_API_MISUSE),
@@ -1474,6 +1476,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
14741476
LintId::of(&inherent_to_string::INHERENT_TO_STRING),
14751477
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
14761478
LintId::of(&len_zero::LEN_ZERO),
1479+
LintId::of(&let_and_return::LET_AND_RETURN),
14771480
LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING),
14781481
LintId::of(&loops::EMPTY_LOOP),
14791482
LintId::of(&loops::FOR_KV_MAP),
@@ -1526,7 +1529,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15261529
LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),
15271530
LintId::of(&regex::REGEX_MACRO),
15281531
LintId::of(&regex::TRIVIAL_REGEX),
1529-
LintId::of(&returns::LET_AND_RETURN),
15301532
LintId::of(&returns::NEEDLESS_RETURN),
15311533
LintId::of(&returns::UNUSED_UNIT),
15321534
LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),

‎clippy_lints/src/returns.rs

Lines changed: 3 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
88
use rustc_span::source_map::Span;
99
use rustc_span::BytePos;
1010

11-
use crate::utils::{in_macro, match_path_ast, snippet_opt, span_lint_and_sugg, span_lint_and_then};
11+
use crate::utils::{snippet_opt, span_lint_and_sugg, span_lint_and_then};
1212

1313
declare_clippy_lint! {
1414
/// **What it does:** Checks for return statements at the end of a block.
@@ -36,33 +36,6 @@ declare_clippy_lint! {
3636
"using a return statement like `return expr;` where an expression would suffice"
3737
}
3838

39-
declare_clippy_lint! {
40-
/// **What it does:** Checks for `let`-bindings, which are subsequently
41-
/// returned.
42-
///
43-
/// **Why is this bad?** It is just extraneous code. Remove it to make your code
44-
/// more rusty.
45-
///
46-
/// **Known problems:** None.
47-
///
48-
/// **Example:**
49-
/// ```rust
50-
/// fn foo() -> String {
51-
/// let x = String::new();
52-
/// x
53-
/// }
54-
/// ```
55-
/// instead, use
56-
/// ```
57-
/// fn foo() -> String {
58-
/// String::new()
59-
/// }
60-
/// ```
61-
pub LET_AND_RETURN,
62-
style,
63-
"creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block"
64-
}
65-
6639
declare_clippy_lint! {
6740
/// **What it does:** Checks for unit (`()`) expressions that can be removed.
6841
///
@@ -90,7 +63,7 @@ enum RetReplacement {
9063
Block,
9164
}
9265

93-
declare_lint_pass!(Return => [NEEDLESS_RETURN, LET_AND_RETURN, UNUSED_UNIT]);
66+
declare_lint_pass!(Return => [NEEDLESS_RETURN, UNUSED_UNIT]);
9467

9568
impl Return {
9669
// Check the final stmt or expr in a block for unnecessary return.
@@ -105,7 +78,7 @@ impl Return {
10578
}
10679
}
10780

108-
// Check a the final expression in a block if it's a return.
81+
// Check the final expression in a block if it's a return.
10982
fn check_final_expr(
11083
&mut self,
11184
cx: &EarlyContext<'_>,
@@ -186,54 +159,6 @@ impl Return {
186159
},
187160
}
188161
}
189-
190-
// Check for "let x = EXPR; x"
191-
fn check_let_return(cx: &EarlyContext<'_>, block: &ast::Block) {
192-
let mut it = block.stmts.iter();
193-
194-
// we need both a let-binding stmt and an expr
195-
if_chain! {
196-
if let Some(retexpr) = it.next_back();
197-
if let ast::StmtKind::Expr(ref retexpr) = retexpr.kind;
198-
if let Some(stmt) = it.next_back();
199-
if let ast::StmtKind::Local(ref local) = stmt.kind;
200-
// don't lint in the presence of type inference
201-
if local.ty.is_none();
202-
if local.attrs.is_empty();
203-
if let Some(ref initexpr) = local.init;
204-
if let ast::PatKind::Ident(_, ident, _) = local.pat.kind;
205-
if let ast::ExprKind::Path(_, ref path) = retexpr.kind;
206-
if match_path_ast(path, &[&*ident.name.as_str()]);
207-
if !in_external_macro(cx.sess(), initexpr.span);
208-
if !in_external_macro(cx.sess(), retexpr.span);
209-
if !in_external_macro(cx.sess(), local.span);
210-
if !in_macro(local.span);
211-
then {
212-
span_lint_and_then(
213-
cx,
214-
LET_AND_RETURN,
215-
retexpr.span,
216-
"returning the result of a `let` binding from a block",
217-
|err| {
218-
err.span_label(local.span, "unnecessary `let` binding");
219-
220-
if let Some(snippet) = snippet_opt(cx, initexpr.span) {
221-
err.multipart_suggestion(
222-
"return the expression directly",
223-
vec![
224-
(local.span, String::new()),
225-
(retexpr.span, snippet),
226-
],
227-
Applicability::MachineApplicable,
228-
);
229-
} else {
230-
err.span_help(initexpr.span, "this expression can be directly returned");
231-
}
232-
},
233-
);
234-
}
235-
}
236-
}
237162
}
238163

239164
impl EarlyLintPass for Return {
@@ -254,7 +179,6 @@ impl EarlyLintPass for Return {
254179
}
255180

256181
fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) {
257-
Self::check_let_return(cx, block);
258182
if_chain! {
259183
if let Some(ref stmt) = block.stmts.last();
260184
if let ast::StmtKind::Expr(ref expr) = stmt.kind;

‎src/lintlist/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1023,7 +1023,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
10231023
group: "style",
10241024
desc: "creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block",
10251025
deprecation: None,
1026-
module: "returns",
1026+
module: "let_and_return",
10271027
},
10281028
Lint {
10291029
name: "let_underscore_lock",

0 commit comments

Comments
 (0)