Skip to content

Commit 40ce9f8

Browse files
committed
Auto merge of #6549 - ThibsG:FixClosureNeedlessReturn, r=phansch
Fix FP with empty return for `needless_return` lint This fixes a false positive in `needless_return` lint, when triggered in a closure using `return` statement without value. Fixes: #6501 changelog: none
2 parents 3577cf7 + 46aa654 commit 40ce9f8

File tree

4 files changed

+55
-2
lines changed

4 files changed

+55
-2
lines changed

clippy_lints/src/returns.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,16 @@ impl<'tcx> LateLintPass<'tcx> for Return {
131131
_: HirId,
132132
) {
133133
match kind {
134-
FnKind::Closure(_) => check_final_expr(cx, &body.value, Some(body.value.span), RetReplacement::Empty),
134+
FnKind::Closure(_) => {
135+
// when returning without value in closure, replace this `return`
136+
// with an empty block to prevent invalid suggestion (see #6501)
137+
let replacement = if let ExprKind::Ret(None) = &body.value.kind {
138+
RetReplacement::Block
139+
} else {
140+
RetReplacement::Empty
141+
};
142+
check_final_expr(cx, &body.value, Some(body.value.span), replacement)
143+
},
135144
FnKind::ItemFn(..) | FnKind::Method(..) => {
136145
if let ExprKind::Block(ref block, _) = body.value.kind {
137146
check_block_return(cx, block);

tests/ui/needless_return.fixed

+13
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,19 @@ fn test_return_in_macro() {
101101
needed_return!(0);
102102
}
103103

104+
mod issue6501 {
105+
fn foo(bar: Result<(), ()>) {
106+
bar.unwrap_or_else(|_| {})
107+
}
108+
109+
fn test_closure() {
110+
let _ = || {
111+
112+
};
113+
let _ = || {};
114+
}
115+
}
116+
104117
fn main() {
105118
let _ = test_end_of_fn();
106119
let _ = test_no_semicolon();

tests/ui/needless_return.rs

+13
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,19 @@ fn test_return_in_macro() {
101101
needed_return!(0);
102102
}
103103

104+
mod issue6501 {
105+
fn foo(bar: Result<(), ()>) {
106+
bar.unwrap_or_else(|_| return)
107+
}
108+
109+
fn test_closure() {
110+
let _ = || {
111+
return;
112+
};
113+
let _ = || return;
114+
}
115+
}
116+
104117
fn main() {
105118
let _ = test_end_of_fn();
106119
let _ = test_no_semicolon();

tests/ui/needless_return.stderr

+19-1
Original file line numberDiff line numberDiff line change
@@ -84,5 +84,23 @@ error: unneeded `return` statement
8484
LL | return String::new();
8585
| ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()`
8686

87-
error: aborting due to 14 previous errors
87+
error: unneeded `return` statement
88+
--> $DIR/needless_return.rs:106:32
89+
|
90+
LL | bar.unwrap_or_else(|_| return)
91+
| ^^^^^^ help: replace `return` with an empty block: `{}`
92+
93+
error: unneeded `return` statement
94+
--> $DIR/needless_return.rs:111:13
95+
|
96+
LL | return;
97+
| ^^^^^^^ help: remove `return`
98+
99+
error: unneeded `return` statement
100+
--> $DIR/needless_return.rs:113:20
101+
|
102+
LL | let _ = || return;
103+
| ^^^^^^ help: replace `return` with an empty block: `{}`
104+
105+
error: aborting due to 17 previous errors
88106

0 commit comments

Comments
 (0)