Skip to content

Commit f935912

Browse files
authored
Merge pull request #3494 from daxpedda/master
Added `IMPLICIT_RETURN` lint.
2 parents 7cb1b1f + b0f3ed2 commit f935912

File tree

6 files changed

+249
-1
lines changed

6 files changed

+249
-1
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,7 @@ All notable changes to this project will be documented in this file.
706706
[`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
707707
[`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
708708
[`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher
709+
[`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return
709710
[`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping
710711
[`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing
711712
[`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
99

10-
[There are 289 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
10+
[There are 290 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1111

1212
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1313

clippy_lints/src/implicit_return.rs

+135
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
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::hir::{intravisit::FnKind, Body, ExprKind, FnDecl};
11+
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
12+
use crate::rustc::{declare_tool_lint, lint_array};
13+
use crate::rustc_errors::Applicability;
14+
use crate::syntax::{ast::NodeId, source_map::Span};
15+
use crate::utils::{snippet_opt, span_lint_and_then};
16+
17+
/// **What it does:** Checks for missing return statements at the end of a block.
18+
///
19+
/// **Why is this bad?** Actually omitting the return keyword is idiomatic Rust code. Programmers
20+
/// coming from other languages might prefer the expressiveness of `return`. It's possible to miss
21+
/// the last returning statement because the only difference is a missing `;`. Especially in bigger
22+
/// code with multiple return paths having a `return` keyword makes it easier to find the
23+
/// corresponding statements.
24+
///
25+
/// **Known problems:** None.
26+
///
27+
/// **Example:**
28+
/// ```rust
29+
/// fn foo(x: usize) {
30+
/// x
31+
/// }
32+
/// ```
33+
/// add return
34+
/// ```rust
35+
/// fn foo(x: usize) {
36+
/// return x;
37+
/// }
38+
/// ```
39+
declare_clippy_lint! {
40+
pub IMPLICIT_RETURN,
41+
restriction,
42+
"use a return statement like `return expr` instead of an expression"
43+
}
44+
45+
pub struct Pass;
46+
47+
impl Pass {
48+
fn expr_match(cx: &LateContext<'_, '_>, expr: &rustc::hir::Expr) {
49+
match &expr.node {
50+
ExprKind::Block(block, ..) => {
51+
if let Some(expr) = &block.expr {
52+
Self::expr_match(cx, expr);
53+
}
54+
// only needed in the case of `break` with `;` at the end
55+
else if let Some(stmt) = block.stmts.last() {
56+
if let rustc::hir::StmtKind::Semi(expr, ..) = &stmt.node {
57+
Self::expr_match(cx, expr);
58+
}
59+
}
60+
},
61+
// use `return` instead of `break`
62+
ExprKind::Break(.., break_expr) => {
63+
if let Some(break_expr) = break_expr {
64+
span_lint_and_then(cx, IMPLICIT_RETURN, expr.span, "missing return statement", |db| {
65+
if let Some(snippet) = snippet_opt(cx, break_expr.span) {
66+
db.span_suggestion_with_applicability(
67+
expr.span,
68+
"change `break` to `return` as shown",
69+
format!("return {}", snippet),
70+
Applicability::MachineApplicable,
71+
);
72+
}
73+
});
74+
}
75+
},
76+
ExprKind::If(.., if_expr, else_expr) => {
77+
Self::expr_match(cx, if_expr);
78+
79+
if let Some(else_expr) = else_expr {
80+
Self::expr_match(cx, else_expr);
81+
}
82+
},
83+
ExprKind::Match(_, arms, ..) => {
84+
for arm in arms {
85+
Self::expr_match(cx, &arm.body);
86+
}
87+
},
88+
// loops could be using `break` instead of `return`
89+
ExprKind::Loop(block, ..) => {
90+
if let Some(expr) = &block.expr {
91+
Self::expr_match(cx, expr);
92+
}
93+
},
94+
// skip if it already has a return statement
95+
ExprKind::Ret(..) => (),
96+
// everything else is missing `return`
97+
_ => span_lint_and_then(cx, IMPLICIT_RETURN, expr.span, "missing return statement", |db| {
98+
if let Some(snippet) = snippet_opt(cx, expr.span) {
99+
db.span_suggestion_with_applicability(
100+
expr.span,
101+
"add `return` as shown",
102+
format!("return {}", snippet),
103+
Applicability::MachineApplicable,
104+
);
105+
}
106+
}),
107+
}
108+
}
109+
}
110+
111+
impl LintPass for Pass {
112+
fn get_lints(&self) -> LintArray {
113+
lint_array!(IMPLICIT_RETURN)
114+
}
115+
}
116+
117+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
118+
fn check_fn(
119+
&mut self,
120+
cx: &LateContext<'a, 'tcx>,
121+
_: FnKind<'tcx>,
122+
_: &'tcx FnDecl,
123+
body: &'tcx Body,
124+
_: Span,
125+
_: NodeId,
126+
) {
127+
let def_id = cx.tcx.hir.body_owner_def_id(body.id());
128+
let mir = cx.tcx.optimized_mir(def_id);
129+
130+
// checking return type through MIR, HIR is not able to determine inferred closure return types
131+
if !mir.return_ty().is_unit() {
132+
Self::expr_match(cx, &body.value);
133+
}
134+
}
135+
}

clippy_lints/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ pub mod functions;
126126
pub mod identity_conversion;
127127
pub mod identity_op;
128128
pub mod if_not_else;
129+
pub mod implicit_return;
129130
pub mod indexing_slicing;
130131
pub mod infallible_destructuring_match;
131132
pub mod infinite_iter;
@@ -371,6 +372,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
371372
reg.register_late_lint_pass(box unicode::Unicode);
372373
reg.register_late_lint_pass(box strings::StringAdd);
373374
reg.register_early_lint_pass(box returns::ReturnPass);
375+
reg.register_late_lint_pass(box implicit_return::Pass);
374376
reg.register_late_lint_pass(box methods::Pass);
375377
reg.register_late_lint_pass(box map_clone::Pass);
376378
reg.register_late_lint_pass(box shadow::Pass);
@@ -485,6 +487,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
485487
arithmetic::FLOAT_ARITHMETIC,
486488
arithmetic::INTEGER_ARITHMETIC,
487489
else_if_without_else::ELSE_IF_WITHOUT_ELSE,
490+
implicit_return::IMPLICIT_RETURN,
488491
indexing_slicing::INDEXING_SLICING,
489492
inherent_impl::MULTIPLE_INHERENT_IMPL,
490493
literal_representation::DECIMAL_LITERAL_REPRESENTATION,

tests/ui/implicit_return.rs

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
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+
11+
12+
13+
14+
#![warn(clippy::implicit_return)]
15+
16+
fn test_end_of_fn() -> bool {
17+
if true {
18+
// no error!
19+
return true;
20+
}
21+
true
22+
}
23+
24+
#[allow(clippy::needless_bool)]
25+
fn test_if_block() -> bool {
26+
if true {
27+
true
28+
} else {
29+
false
30+
}
31+
}
32+
33+
#[allow(clippy::match_bool)]
34+
fn test_match(x: bool) -> bool {
35+
match x {
36+
true => false,
37+
false => {
38+
true
39+
}
40+
}
41+
}
42+
43+
#[allow(clippy::never_loop)]
44+
fn test_loop() -> bool {
45+
loop {
46+
break true;
47+
}
48+
}
49+
50+
fn test_closure() {
51+
let _ = || {
52+
true
53+
};
54+
let _ = || true;
55+
}
56+
57+
fn main() {
58+
let _ = test_end_of_fn();
59+
let _ = test_if_block();
60+
let _ = test_match(true);
61+
let _ = test_loop();
62+
test_closure();
63+
}

tests/ui/implicit_return.stderr

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
error: missing return statement
2+
--> $DIR/implicit_return.rs:21:5
3+
|
4+
21 | true
5+
| ^^^^ help: add `return` as shown: `return true`
6+
|
7+
= note: `-D clippy::implicit-return` implied by `-D warnings`
8+
9+
error: missing return statement
10+
--> $DIR/implicit_return.rs:27:9
11+
|
12+
27 | true
13+
| ^^^^ help: add `return` as shown: `return true`
14+
15+
error: missing return statement
16+
--> $DIR/implicit_return.rs:29:9
17+
|
18+
29 | false
19+
| ^^^^^ help: add `return` as shown: `return false`
20+
21+
error: missing return statement
22+
--> $DIR/implicit_return.rs:36:17
23+
|
24+
36 | true => false,
25+
| ^^^^^ help: add `return` as shown: `return false`
26+
27+
error: missing return statement
28+
--> $DIR/implicit_return.rs:38:13
29+
|
30+
38 | true
31+
| ^^^^ help: add `return` as shown: `return true`
32+
33+
error: missing return statement
34+
--> $DIR/implicit_return.rs:52:9
35+
|
36+
52 | true
37+
| ^^^^ help: add `return` as shown: `return true`
38+
39+
error: missing return statement
40+
--> $DIR/implicit_return.rs:54:16
41+
|
42+
54 | let _ = || true;
43+
| ^^^^ help: add `return` as shown: `return true`
44+
45+
error: aborting due to 7 previous errors
46+

0 commit comments

Comments
 (0)