Skip to content

Commit 7ad3373

Browse files
committed
Auto merge of rust-lang#11802 - dswij:issue-11765, r=xFrednet
`needless_return_with_question_mark` ignore let-else Fixes rust-lang#11765 This PR makes `needless_return_with_question_mark` to ignore expr inside let-else. changelog: [`needless_return_with_question_mark`] ignore let-else
2 parents 662ea3f + 48f38eb commit 7ad3373

File tree

5 files changed

+84
-2
lines changed

5 files changed

+84
-2
lines changed

clippy_lints/src/returns.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then, span_lin
22
use clippy_utils::source::{snippet_opt, snippet_with_context};
33
use clippy_utils::sugg::has_enclosing_paren;
44
use clippy_utils::visitors::{for_each_expr_with_closures, Descend};
5-
use clippy_utils::{fn_def_id, is_from_proc_macro, path_to_local_id, span_find_starting_semi};
5+
use clippy_utils::{fn_def_id, is_from_proc_macro, is_inside_let_else, path_to_local_id, span_find_starting_semi};
66
use core::ops::ControlFlow;
77
use rustc_errors::Applicability;
88
use rustc_hir::intravisit::FnKind;
@@ -170,6 +170,7 @@ impl<'tcx> LateLintPass<'tcx> for Return {
170170
&& let ItemKind::Fn(_, _, body) = item.kind
171171
&& let block = cx.tcx.hir().body(body).value
172172
&& let ExprKind::Block(block, _) = block.kind
173+
&& !is_inside_let_else(cx.tcx, expr)
173174
&& let [.., final_stmt] = block.stmts
174175
&& final_stmt.hir_id != stmt.hir_id
175176
&& !is_from_proc_macro(cx, expr)

clippy_utils/src/lib.rs

+37
Original file line numberDiff line numberDiff line change
@@ -1487,6 +1487,43 @@ pub fn is_else_clause(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
14871487
}
14881488
}
14891489

1490+
/// Checks if the given expression is a part of `let else`
1491+
/// returns `true` for both the `init` and the `else` part
1492+
pub fn is_inside_let_else(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
1493+
let mut child_id = expr.hir_id;
1494+
for (parent_id, node) in tcx.hir().parent_iter(child_id) {
1495+
if let Node::Local(Local {
1496+
init: Some(init),
1497+
els: Some(els),
1498+
..
1499+
}) = node
1500+
&& (init.hir_id == child_id || els.hir_id == child_id)
1501+
{
1502+
return true;
1503+
}
1504+
1505+
child_id = parent_id;
1506+
}
1507+
1508+
false
1509+
}
1510+
1511+
/// Checks if the given expression is the else clause of a `let else` expression
1512+
pub fn is_else_clause_in_let_else(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
1513+
let mut child_id = expr.hir_id;
1514+
for (parent_id, node) in tcx.hir().parent_iter(child_id) {
1515+
if let Node::Local(Local { els: Some(els), .. }) = node
1516+
&& els.hir_id == child_id
1517+
{
1518+
return true;
1519+
}
1520+
1521+
child_id = parent_id;
1522+
}
1523+
1524+
false
1525+
}
1526+
14901527
/// Checks whether the given `Expr` is a range equivalent to a `RangeFull`.
14911528
/// For the lower bound, this means that:
14921529
/// - either there is none

tests/ui/needless_return_with_question_mark.fixed

+22
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
clippy::no_effect,
55
clippy::unit_arg,
66
clippy::useless_conversion,
7+
clippy::diverging_sub_expression,
78
unused
89
)]
910

@@ -35,5 +36,26 @@ fn main() -> Result<(), ()> {
3536
with_span! {
3637
return Err(())?;
3738
}
39+
40+
// Issue #11765
41+
// Should not lint
42+
let Some(s) = Some("") else {
43+
return Err(())?;
44+
};
45+
46+
let Some(s) = Some("") else {
47+
let Some(s) = Some("") else {
48+
return Err(())?;
49+
};
50+
51+
return Err(())?;
52+
};
53+
54+
let Some(_): Option<()> = ({
55+
return Err(())?;
56+
}) else {
57+
panic!();
58+
};
59+
3860
Err(())
3961
}

tests/ui/needless_return_with_question_mark.rs

+22
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
clippy::no_effect,
55
clippy::unit_arg,
66
clippy::useless_conversion,
7+
clippy::diverging_sub_expression,
78
unused
89
)]
910

@@ -35,5 +36,26 @@ fn main() -> Result<(), ()> {
3536
with_span! {
3637
return Err(())?;
3738
}
39+
40+
// Issue #11765
41+
// Should not lint
42+
let Some(s) = Some("") else {
43+
return Err(())?;
44+
};
45+
46+
let Some(s) = Some("") else {
47+
let Some(s) = Some("") else {
48+
return Err(())?;
49+
};
50+
51+
return Err(())?;
52+
};
53+
54+
let Some(_): Option<()> = ({
55+
return Err(())?;
56+
}) else {
57+
panic!();
58+
};
59+
3860
Err(())
3961
}

tests/ui/needless_return_with_question_mark.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: unneeded `return` statement with `?` operator
2-
--> $DIR/needless_return_with_question_mark.rs:27:5
2+
--> $DIR/needless_return_with_question_mark.rs:28:5
33
|
44
LL | return Err(())?;
55
| ^^^^^^^ help: remove it

0 commit comments

Comments
 (0)