From 4e72ca31b5ff35bbe6ff02431bc214138417ed30 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sat, 16 Mar 2024 14:15:45 +0100 Subject: [PATCH] [`map_entry`]: call the visitor on the local's `else` block --- clippy_lints/src/entry.rs | 5 ++++- tests/ui/entry.fixed | 10 ++++++++++ tests/ui/entry.rs | 10 ++++++++++ tests/ui/entry.stderr | 23 ++++++++++++++++++++++- 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 @@ fn visit_stmt(&mut self, stmt: &'tcx Stmt<'_>) { 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) -> 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) -> 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