Skip to content

Commit e0bcec7

Browse files
committed
Auto merge of #3676 - daxpedda:implicit_return, r=oli-obk
Fix `implicit_return` false positives. Fixes the following false positives: - linting on `if let` without `else` in a `loop` even with a present `return` - linting on `unreachable!()`
2 parents 54978a5 + 2183cfc commit e0bcec7

File tree

3 files changed

+46
-11
lines changed

3 files changed

+46
-11
lines changed

clippy_lints/src/implicit_return.rs

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
use crate::utils::{in_macro, snippet_opt, span_lint_and_then};
2-
use rustc::hir::{intravisit::FnKind, Body, ExprKind, FnDecl};
1+
use crate::utils::{in_macro, is_expn_of, snippet_opt, span_lint_and_then};
2+
use rustc::hir::{intravisit::FnKind, Body, ExprKind, FnDecl, MatchSource};
33
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
44
use rustc::{declare_tool_lint, lint_array};
55
use rustc_errors::Applicability;
@@ -81,15 +81,31 @@ impl Pass {
8181
Self::expr_match(cx, else_expr);
8282
}
8383
},
84-
ExprKind::Match(_, arms, ..) => {
85-
for arm in arms {
86-
Self::expr_match(cx, &arm.body);
84+
ExprKind::Match(.., arms, source) => {
85+
let check_all_arms = match source {
86+
MatchSource::IfLetDesugar {
87+
contains_else_clause: has_else,
88+
} => *has_else,
89+
_ => true,
90+
};
91+
92+
if check_all_arms {
93+
for arm in arms {
94+
Self::expr_match(cx, &arm.body);
95+
}
96+
} else {
97+
Self::expr_match(cx, &arms.first().expect("if let doesn't have a single arm").body);
8798
}
8899
},
89100
// skip if it already has a return statement
90101
ExprKind::Ret(..) => (),
91102
// everything else is missing `return`
92-
_ => Self::lint(cx, expr.span, expr.span, "add `return` as shown"),
103+
_ => {
104+
// make sure it's not just an unreachable expression
105+
if is_expn_of(expr.span, "unreachable").is_none() {
106+
Self::lint(cx, expr.span, expr.span, "add `return` as shown")
107+
}
108+
},
93109
}
94110
}
95111
}

tests/ui/implicit_return.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ fn test_match(x: bool) -> bool {
2626
}
2727
}
2828

29+
#[allow(clippy::match_bool, clippy::needless_return)]
30+
fn test_match_with_unreachable(x: bool) -> bool {
31+
match x {
32+
true => return false,
33+
false => unreachable!(),
34+
}
35+
}
36+
2937
#[allow(clippy::never_loop)]
3038
fn test_loop() -> bool {
3139
loop {
@@ -53,6 +61,15 @@ fn test_loop_with_nests() -> bool {
5361
}
5462
}
5563

64+
#[allow(clippy::redundant_pattern_matching)]
65+
fn test_loop_with_if_let() -> bool {
66+
loop {
67+
if let Some(x) = Some(true) {
68+
return x;
69+
}
70+
}
71+
}
72+
5673
fn test_closure() {
5774
#[rustfmt::skip]
5875
let _ = || { true };
@@ -63,8 +80,10 @@ fn main() {
6380
let _ = test_end_of_fn();
6481
let _ = test_if_block();
6582
let _ = test_match(true);
83+
let _ = test_match_with_unreachable(true);
6684
let _ = test_loop();
6785
let _ = test_loop_with_block();
6886
let _ = test_loop_with_nests();
87+
let _ = test_loop_with_if_let();
6988
test_closure();
7089
}

tests/ui/implicit_return.stderr

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,31 +31,31 @@ LL | false => { true },
3131
| ^^^^ help: add `return` as shown: `return true`
3232

3333
error: missing return statement
34-
--> $DIR/implicit_return.rs:32:9
34+
--> $DIR/implicit_return.rs:40:9
3535
|
3636
LL | break true;
3737
| ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`
3838

3939
error: missing return statement
40-
--> $DIR/implicit_return.rs:40:13
40+
--> $DIR/implicit_return.rs:48:13
4141
|
4242
LL | break true;
4343
| ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`
4444

4545
error: missing return statement
46-
--> $DIR/implicit_return.rs:49:13
46+
--> $DIR/implicit_return.rs:57:13
4747
|
4848
LL | break true;
4949
| ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`
5050

5151
error: missing return statement
52-
--> $DIR/implicit_return.rs:58:18
52+
--> $DIR/implicit_return.rs:75:18
5353
|
5454
LL | let _ = || { true };
5555
| ^^^^ help: add `return` as shown: `return true`
5656

5757
error: missing return statement
58-
--> $DIR/implicit_return.rs:59:16
58+
--> $DIR/implicit_return.rs:76:16
5959
|
6060
LL | let _ = || true;
6161
| ^^^^ help: add `return` as shown: `return true`

0 commit comments

Comments
 (0)