From 70610c00186cd9fd33f357fe2243822a3008049a Mon Sep 17 00:00:00 2001
From: y21 <30553356+y21@users.noreply.github.com>
Date: Wed, 28 Jun 2023 12:41:18 +0200
Subject: [PATCH] lint in nested bodies if `try` is in outer body

---
 clippy_lints/src/question_mark.rs | 22 ++++++++++++++++++----
 tests/ui/question_mark.fixed      | 10 ++++++++++
 tests/ui/question_mark.rs         | 14 ++++++++++++++
 tests/ui/question_mark.stderr     | 12 +++++++++++-
 4 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs
index 036ad3e0d15..e3d940ad2a4 100644
--- a/clippy_lints/src/question_mark.rs
+++ b/clippy_lints/src/question_mark.rs
@@ -49,7 +49,7 @@ pub struct QuestionMark {
     /// very quickly, without having to walk up the parent chain, by simply checking
     /// if it is greater than zero.
     /// As for why we need this in the first place: <https://github.com/rust-lang/rust-clippy/issues/8628>
-    try_block_depth: u32,
+    try_block_depth_stack: Vec<u32>,
 }
 impl_lint_pass!(QuestionMark => [QUESTION_MARK]);
 
@@ -150,7 +150,7 @@ fn expr_return_none_or_err(
 
 impl QuestionMark {
     fn inside_try_block(&self) -> bool {
-        self.try_block_depth > 0
+        self.try_block_depth_stack.last() > Some(&0)
     }
 
     /// Checks if the given expression on the given context matches the following structure:
@@ -268,13 +268,27 @@ impl<'tcx> LateLintPass<'tcx> for QuestionMark {
 
     fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx rustc_hir::Block<'tcx>) {
         if is_try_block(cx, block) {
-            self.try_block_depth += 1;
+            *self
+                .try_block_depth_stack
+                .last_mut()
+                .expect("blocks are always part of bodies and must have a depth") += 1;
         }
     }
 
+    fn check_body(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::Body<'tcx>) {
+        self.try_block_depth_stack.push(0);
+    }
+
+    fn check_body_post(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::Body<'tcx>) {
+        self.try_block_depth_stack.pop();
+    }
+
     fn check_block_post(&mut self, cx: &LateContext<'tcx>, block: &'tcx rustc_hir::Block<'tcx>) {
         if is_try_block(cx, block) {
-            self.try_block_depth -= 1;
+            *self
+                .try_block_depth_stack
+                .last_mut()
+                .expect("blocks are always part of bodies and must have a depth") -= 1;
         }
     }
 }
diff --git a/tests/ui/question_mark.fixed b/tests/ui/question_mark.fixed
index f257378842b..2d8920ccc42 100644
--- a/tests/ui/question_mark.fixed
+++ b/tests/ui/question_mark.fixed
@@ -239,6 +239,16 @@ fn issue8628(a: Option<u32>) -> Option<u32> {
     b.or(Some(128))
 }
 
+fn issue6828_nested_body() -> Option<u32> {
+    try {
+        fn f2(a: Option<i32>) -> Option<i32> {
+            a?;
+            Some(32)
+        }
+        123
+    }
+}
+
 // should not lint, `?` operator not available in const context
 const fn issue9175(option: Option<()>) -> Option<()> {
     if option.is_none() {
diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs
index 29d79b078ac..69451c17ee7 100644
--- a/tests/ui/question_mark.rs
+++ b/tests/ui/question_mark.rs
@@ -275,6 +275,20 @@ fn issue8628(a: Option<u32>) -> Option<u32> {
     b.or(Some(128))
 }
 
+fn issue6828_nested_body() -> Option<u32> {
+    try {
+        fn f2(a: Option<i32>) -> Option<i32> {
+            if a.is_none() {
+                return None;
+                // do lint here, the outer `try` is not relevant here
+                // https://github.com/rust-lang/rust-clippy/pull/11001#issuecomment-1610636867
+            }
+            Some(32)
+        }
+        123
+    }
+}
+
 // should not lint, `?` operator not available in const context
 const fn issue9175(option: Option<()>) -> Option<()> {
     if option.is_none() {
diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr
index 86d89942cc0..2cfd7586308 100644
--- a/tests/ui/question_mark.stderr
+++ b/tests/ui/question_mark.stderr
@@ -130,5 +130,15 @@ LL | |         return Err(err);
 LL | |     }
    | |_____^ help: replace it with: `func_returning_result()?;`
 
-error: aborting due to 15 previous errors
+error: this block may be rewritten with the `?` operator
+  --> $DIR/question_mark.rs:281:13
+   |
+LL | /             if a.is_none() {
+LL | |                 return None;
+LL | |                 // do lint here, the outer `try` is not relevant here
+LL | |                 // https://github.com/rust-lang/rust-clippy/pull/11001#issuecomment-1610636867
+LL | |             }
+   | |_____________^ help: replace it with: `a?;`
+
+error: aborting due to 16 previous errors