diff options
| -rw-r--r-- | clippy_lints/src/returns.rs | 3 | ||||
| -rw-r--r-- | clippy_utils/src/lib.rs | 37 | ||||
| -rw-r--r-- | tests/ui/needless_return_with_question_mark.fixed | 22 | ||||
| -rw-r--r-- | tests/ui/needless_return_with_question_mark.rs | 22 | ||||
| -rw-r--r-- | tests/ui/needless_return_with_question_mark.stderr | 2 |
5 files changed, 84 insertions, 2 deletions
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 7468bda02e8..4bcced239c7 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then, span_lin use clippy_utils::source::{snippet_opt, snippet_with_context}; use clippy_utils::sugg::has_enclosing_paren; use clippy_utils::visitors::{for_each_expr_with_closures, Descend}; -use clippy_utils::{fn_def_id, is_from_proc_macro, path_to_local_id, span_find_starting_semi}; +use clippy_utils::{fn_def_id, is_from_proc_macro, is_inside_let_else, path_to_local_id, span_find_starting_semi}; use core::ops::ControlFlow; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; @@ -170,6 +170,7 @@ impl<'tcx> LateLintPass<'tcx> for Return { && let ItemKind::Fn(_, _, body) = item.kind && let block = cx.tcx.hir().body(body).value && let ExprKind::Block(block, _) = block.kind + && !is_inside_let_else(cx.tcx, expr) && let [.., final_stmt] = block.stmts && final_stmt.hir_id != stmt.hir_id && !is_from_proc_macro(cx, expr) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 2445c4e0672..0998e00c754 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1487,6 +1487,43 @@ pub fn is_else_clause(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool { } } +/// Checks if the given expression is a part of `let else` +/// returns `true` for both the `init` and the `else` part +pub fn is_inside_let_else(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool { + let mut child_id = expr.hir_id; + for (parent_id, node) in tcx.hir().parent_iter(child_id) { + if let Node::Local(Local { + init: Some(init), + els: Some(els), + .. + }) = node + && (init.hir_id == child_id || els.hir_id == child_id) + { + return true; + } + + child_id = parent_id; + } + + false +} + +/// Checks if the given expression is the else clause of a `let else` expression +pub fn is_else_clause_in_let_else(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool { + let mut child_id = expr.hir_id; + for (parent_id, node) in tcx.hir().parent_iter(child_id) { + if let Node::Local(Local { els: Some(els), .. }) = node + && els.hir_id == child_id + { + return true; + } + + child_id = parent_id; + } + + false +} + /// Checks whether the given `Expr` is a range equivalent to a `RangeFull`. /// For the lower bound, this means that: /// - either there is none diff --git a/tests/ui/needless_return_with_question_mark.fixed b/tests/ui/needless_return_with_question_mark.fixed index 52d54180921..d5932ebcf12 100644 --- a/tests/ui/needless_return_with_question_mark.fixed +++ b/tests/ui/needless_return_with_question_mark.fixed @@ -4,6 +4,7 @@ clippy::no_effect, clippy::unit_arg, clippy::useless_conversion, + clippy::diverging_sub_expression, unused )] @@ -35,5 +36,26 @@ fn main() -> Result<(), ()> { with_span! { return Err(())?; } + + // Issue #11765 + // Should not lint + let Some(s) = Some("") else { + return Err(())?; + }; + + let Some(s) = Some("") else { + let Some(s) = Some("") else { + return Err(())?; + }; + + return Err(())?; + }; + + let Some(_): Option<()> = ({ + return Err(())?; + }) else { + panic!(); + }; + Err(()) } diff --git a/tests/ui/needless_return_with_question_mark.rs b/tests/ui/needless_return_with_question_mark.rs index d253cae4dc2..2485e25f05c 100644 --- a/tests/ui/needless_return_with_question_mark.rs +++ b/tests/ui/needless_return_with_question_mark.rs @@ -4,6 +4,7 @@ clippy::no_effect, clippy::unit_arg, clippy::useless_conversion, + clippy::diverging_sub_expression, unused )] @@ -35,5 +36,26 @@ fn main() -> Result<(), ()> { with_span! { return Err(())?; } + + // Issue #11765 + // Should not lint + let Some(s) = Some("") else { + return Err(())?; + }; + + let Some(s) = Some("") else { + let Some(s) = Some("") else { + return Err(())?; + }; + + return Err(())?; + }; + + let Some(_): Option<()> = ({ + return Err(())?; + }) else { + panic!(); + }; + Err(()) } diff --git a/tests/ui/needless_return_with_question_mark.stderr b/tests/ui/needless_return_with_question_mark.stderr index 0de0633803b..580970a41aa 100644 --- a/tests/ui/needless_return_with_question_mark.stderr +++ b/tests/ui/needless_return_with_question_mark.stderr @@ -1,5 +1,5 @@ error: unneeded `return` statement with `?` operator - --> $DIR/needless_return_with_question_mark.rs:27:5 + --> $DIR/needless_return_with_question_mark.rs:28:5 | LL | return Err(())?; | ^^^^^^^ help: remove it |
