Auto merge of #12498 - y21:issue12489, r=blyxyas

[`map_entry`]: call the visitor on the local's `else` block

Fixes #12489

The lint already has all the logic it needs for figuring out if it can or can't suggest a closure if it sees control flow expressions like `break` or `continue`, but it was ignoring the local's else block, which meant that it didn't see the `return None;` in a `let..else`.

changelog: [`map_entry`]: suggest `if let` instead of a closure when `return` expressions exist in the else block of a `let..else`

4 files changed, 46 insertions, 2 deletions

diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs
index de6073c2723..219fe588d3c 100644
--- a/clippy_lints/src/entry.rs
+++ b/clippy_lints/src/entry.rs
@@ -358,7 +358,7 @@ struct InsertSearcher<'cx, 'tcx> {
     can_use_entry: bool,
     /// Whether this expression is the final expression in this code path. This may be a statement.
     in_tail_pos: bool,
-    // Is this expression a single insert. A slightly better suggestion can be made in this case.
+    /// Is this expression a single insert. A slightly better suggestion can be made in this case.
     is_single_insert: bool,
     /// If the visitor has seen the map being used.
     is_map_used: bool,
@@ -431,6 +431,9 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> {
                     self.is_single_insert = false;
                     self.visit_expr(e);
                 }
+                if let Some(els) = &l.els {
+                    self.visit_block(els);
+                }
             },
             StmtKind::Item(_) => {
                 self.allow_insert_closure &= !self.in_tail_pos;
diff --git a/tests/ui/entry.fixed b/tests/ui/entry.fixed
index 71ec13f4610..abdfae2a3e1 100644
--- a/tests/ui/entry.fixed
+++ b/tests/ui/entry.fixed
@@ -176,4 +176,14 @@ pub fn issue_11935() {
     }
 }
 
+fn issue12489(map: &mut HashMap<u64, u64>) -> Option<()> {
+    if let std::collections::hash_map::Entry::Vacant(e) = map.entry(1) {
+        let Some(1) = Some(2) else {
+            return None;
+        };
+        e.insert(42);
+    }
+    Some(())
+}
+
 fn main() {}
diff --git a/tests/ui/entry.rs b/tests/ui/entry.rs
index 86092b7c055..7774f99a2a2 100644
--- a/tests/ui/entry.rs
+++ b/tests/ui/entry.rs
@@ -180,4 +180,14 @@ pub fn issue_11935() {
     }
 }
 
+fn issue12489(map: &mut HashMap<u64, u64>) -> Option<()> {
+    if !map.contains_key(&1) {
+        let Some(1) = Some(2) else {
+            return None;
+        };
+        map.insert(1, 42);
+    }
+    Some(())
+}
+
 fn main() {}
diff --git a/tests/ui/entry.stderr b/tests/ui/entry.stderr
index ef4c36bcf54..fb467676606 100644
--- a/tests/ui/entry.stderr
+++ b/tests/ui/entry.stderr
@@ -214,5 +214,26 @@ LL +         v
 LL +     });
    |
 
-error: aborting due to 10 previous errors
+error: usage of `contains_key` followed by `insert` on a `HashMap`
+  --> tests/ui/entry.rs:184:5
+   |
+LL | /     if !map.contains_key(&1) {
+LL | |         let Some(1) = Some(2) else {
+LL | |             return None;
+LL | |         };
+LL | |         map.insert(1, 42);
+LL | |     }
+   | |_____^
+   |
+help: try
+   |
+LL ~     if let std::collections::hash_map::Entry::Vacant(e) = map.entry(1) {
+LL +         let Some(1) = Some(2) else {
+LL +             return None;
+LL +         };
+LL +         e.insert(42);
+LL +     }
+   |
+
+error: aborting due to 11 previous errors