Fix unnecessary_unwrap emitted twice in closure (#14763) (#14770)

The problem is that `check_fn` is triggered by both function and
closure, and `visit_expr` can visit expressions in another closures
within a function or closure.

So just skip walking in a inner closure.

changelog: Fix [`unnecessary_unwrap`] emitted twice in closure which
inside in a function or another closure.

Fixes: #14763

Author: samueltardieu

clippy_lints/src/unwrap.rs

@@ -292,6 +292,10 @@ impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> {
         if expr.span.in_external_macro(self.cx.tcx.sess.source_map()) {
             return;
         }
+        // Skip checking inside closures since they are visited through `Unwrap::check_fn()` already.
+        if matches!(expr.kind, ExprKind::Closure(_)) {
+            return;
+        }
         if let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr) {
             walk_expr(self, cond);
             self.visit_branch(expr, cond, then, false);

tests/ui/checked_unwrap/simple_conditionals.rs

@@ -240,6 +240,39 @@ fn issue14725() {
     }
 }
 
+fn issue14763(x: Option<String>, r: Result<(), ()>) {
+    _ = || {
+        if x.is_some() {
+            _ = x.unwrap();
+            //~^ unnecessary_unwrap
+        } else {
+            _ = x.unwrap();
+            //~^ panicking_unwrap
+        }
+    };
+    _ = || {
+        if r.is_ok() {
+            _ = r.as_ref().unwrap();
+            //~^ unnecessary_unwrap
+        } else {
+            _ = r.as_ref().unwrap();
+            //~^ panicking_unwrap
+        }
+    };
+}
+
+const ISSUE14763: fn(Option<String>) = |x| {
+    _ = || {
+        if x.is_some() {
+            _ = x.unwrap();
+            //~^ unnecessary_unwrap
+        } else {
+            _ = x.unwrap();
+            //~^ panicking_unwrap
+        }
+    }
+};
+
 fn check_expect() {
     let x = Some(());
     if x.is_some() {

tests/ui/checked_unwrap/simple_conditionals.stderr

@@ -271,6 +271,57 @@ LL |     if result.is_ok() {
 LL |         result.as_ref().unwrap();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^
 
+error: called `unwrap` on `x` after checking its variant with `is_some`
+  --> tests/ui/checked_unwrap/simple_conditionals.rs:246:17
+   |
+LL |         if x.is_some() {
+   |         -------------- help: try: `if let Some(<item>) = x`
+LL |             _ = x.unwrap();
+   |                 ^^^^^^^^^^
+
+error: this call to `unwrap()` will always panic
+  --> tests/ui/checked_unwrap/simple_conditionals.rs:249:17
+   |
+LL |         if x.is_some() {
+   |            ----------- because of this check
+...
+LL |             _ = x.unwrap();
+   |                 ^^^^^^^^^^
+
+error: called `unwrap` on `r` after checking its variant with `is_ok`
+  --> tests/ui/checked_unwrap/simple_conditionals.rs:255:17
+   |
+LL |         if r.is_ok() {
+   |         ------------ help: try: `if let Ok(<item>) = &r`
+LL |             _ = r.as_ref().unwrap();
+   |                 ^^^^^^^^^^^^^^^^^^^
+
+error: this call to `unwrap()` will always panic
+  --> tests/ui/checked_unwrap/simple_conditionals.rs:258:17
+   |
+LL |         if r.is_ok() {
+   |            --------- because of this check
+...
+LL |             _ = r.as_ref().unwrap();
+   |                 ^^^^^^^^^^^^^^^^^^^
+
+error: called `unwrap` on `x` after checking its variant with `is_some`
+  --> tests/ui/checked_unwrap/simple_conditionals.rs:267:17
+   |
+LL |         if x.is_some() {
+   |         -------------- help: try: `if let Some(<item>) = x`
+LL |             _ = x.unwrap();
+   |                 ^^^^^^^^^^
+
+error: this call to `unwrap()` will always panic
+  --> tests/ui/checked_unwrap/simple_conditionals.rs:270:17
+   |
+LL |         if x.is_some() {
+   |            ----------- because of this check
+...
+LL |             _ = x.unwrap();
+   |                 ^^^^^^^^^^
+
 error: creating a shared reference to mutable static
   --> tests/ui/checked_unwrap/simple_conditionals.rs:183:12
    |
@@ -281,5 +332,5 @@ LL |         if X.is_some() {
    = note: shared references to mutable statics are dangerous; it's undefined behavior if the static is mutated or if a mutable reference is created for it while the shared reference lives
    = note: `#[deny(static_mut_refs)]` on by default
 
-error: aborting due to 30 previous errors
+error: aborting due to 36 previous errors