needless_return_with_question_mark ignore let-else

Author: dswij

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 if_chain::if_chain;
 use rustc_errors::Applicability;
@@ -171,6 +171,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)

clippy_utils/src/lib.rs

@@ -1489,6 +1489,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

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(())
 }

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(())
 }

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