Minor cleanup of `map_entry` and a few additional tests.

10 files changed, 238 insertions, 220 deletions

diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs
index 0decb22a491..8db5050a5ac 100644
--- a/clippy_lints/src/entry.rs
+++ b/clippy_lints/src/entry.rs
@@ -77,40 +77,33 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
         let map_str = snippet_with_context(cx, contains_expr.map.span, contains_expr.call_ctxt, "..", &mut app).0;
         let key_str = snippet_with_context(cx, contains_expr.key.span, contains_expr.call_ctxt, "..", &mut app).0;
         let sugg = if let Some(else_expr) = else_expr {
-            // if .. { .. } else { .. }
             let else_search = match find_insert_calls(cx, &contains_expr, else_expr) {
-                Some(search) if !(then_search.edits.is_empty() && search.edits.is_empty()) => search,
-                _ => return,
+                Some(search) => search,
+                None => return,
             };
 
-            if then_search.edits.is_empty() || else_search.edits.is_empty() {
-                // if .. { insert } else { .. } or if .. { .. } else { then } of
-                let (then_str, else_str, entry_kind) = if else_search.edits.is_empty() {
-                    if contains_expr.negated {
-                        (
-                            then_search.snippet_vacant(cx, then_expr.span, &mut app),
-                            snippet_with_applicability(cx, else_expr.span, "{ .. }", &mut app),
-                            "Vacant(e)",
-                        )
-                    } else {
-                        (
-                            then_search.snippet_occupied(cx, then_expr.span, &mut app),
-                            snippet_with_applicability(cx, else_expr.span, "{ .. }", &mut app),
-                            "Occupied(mut e)",
-                        )
-                    }
-                } else if contains_expr.negated {
-                    (
+            if then_search.edits.is_empty() && else_search.edits.is_empty() {
+                // No insertions
+                return;
+            } else if then_search.edits.is_empty() || else_search.edits.is_empty() {
+                // if .. { insert } else { .. } or if .. { .. } else { insert }
+                let ((then_str, entry_kind), else_str) = match (else_search.edits.is_empty(), contains_expr.negated) {
+                    (true, true) => (
+                        then_search.snippet_vacant(cx, then_expr.span, &mut app),
+                        snippet_with_applicability(cx, else_expr.span, "{ .. }", &mut app),
+                    ),
+                    (true, false) => (
+                        then_search.snippet_occupied(cx, then_expr.span, &mut app),
+                        snippet_with_applicability(cx, else_expr.span, "{ .. }", &mut app),
+                    ),
+                    (false, true) => (
                         else_search.snippet_occupied(cx, else_expr.span, &mut app),
                         snippet_with_applicability(cx, then_expr.span, "{ .. }", &mut app),
-                        "Occupied(mut e)",
-                    )
-                } else {
-                    (
+                    ),
+                    (false, false) => (
                         else_search.snippet_vacant(cx, else_expr.span, &mut app),
                         snippet_with_applicability(cx, then_expr.span, "{ .. }", &mut app),
-                        "Vacant(e)",
-                    )
+                    ),
                 };
                 format!(
                     "if let {}::{} = {}.entry({}) {} else {}",
@@ -123,19 +116,15 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
                 )
             } else {
                 // if .. { insert } else { insert }
-                let (then_str, else_str, then_entry, else_entry) = if contains_expr.negated {
+                let ((then_str, then_entry), (else_str, else_entry)) = if contains_expr.negated {
                     (
                         then_search.snippet_vacant(cx, then_expr.span, &mut app),
                         else_search.snippet_occupied(cx, else_expr.span, &mut app),
-                        "Vacant(e)",
-                        "Occupied(mut e)",
                     )
                 } else {
                     (
                         then_search.snippet_occupied(cx, then_expr.span, &mut app),
                         else_search.snippet_vacant(cx, else_expr.span, &mut app),
-                        "Occupied(mut e)",
-                        "Vacant(e)",
                     )
                 };
                 let indent_str = snippet_indent(cx, expr.span);
@@ -153,19 +142,18 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
                     indent = indent_str,
                 )
             }
-        } else if then_search.edits.is_empty() {
-            // no insertions
-            return;
         } else {
+            if then_search.edits.is_empty() {
+                // no insertions
+                return;
+            }
+
             // if .. { insert }
             if !then_search.allow_insert_closure {
                 let (body_str, entry_kind) = if contains_expr.negated {
-                    (then_search.snippet_vacant(cx, then_expr.span, &mut app), "Vacant(e)")
+                    then_search.snippet_vacant(cx, then_expr.span, &mut app)
                 } else {
-                    (
-                        then_search.snippet_occupied(cx, then_expr.span, &mut app),
-                        "Occupied(mut e)",
-                    )
+                    then_search.snippet_occupied(cx, then_expr.span, &mut app)
                 };
                 format!(
                     "if let {}::{} = {}.entry({}) {}",
@@ -184,7 +172,8 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
                         format!("{}.entry({}).or_insert({});", map_str, key_str, value_str)
                     }
                 } else {
-                    // Todo: if let Some(v) = map.get_mut(k)
+                    // TODO: suggest using `if let Some(v) = map.get_mut(k) { .. }` here.
+                    // This would need to be a different lint.
                     return;
                 }
             } else {
@@ -192,7 +181,8 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
                 if contains_expr.negated {
                     format!("{}.entry({}).or_insert_with(|| {});", map_str, key_str, block_str)
                 } else {
-                    // Todo: if let Some(v) = map.get_mut(k)
+                    // TODO: suggest using `if let Some(v) = map.get_mut(k) { .. }` here.
+                    // This would need to be a different lint.
                     return;
                 }
             }
@@ -222,7 +212,7 @@ impl MapType {
             Self::BTree => "BTreeMap",
         }
     }
-    fn entry_path(self) -> &'staic str {
+    fn entry_path(self) -> &'static str {
         match self {
             Self::Hash => "std::collections::hash_map::Entry",
             Self::BTree => "std::collections::btree_map::Entry",
@@ -312,15 +302,16 @@ struct Insertion<'tcx> {
     value: &'tcx Expr<'tcx>,
 }
 
-// This visitor needs to do a multiple things:
-// * Find all usages of the map. Only insertions into the map which share the same key are
-//   permitted. All others will prevent the lint.
-// * Determine if the final statement executed is an insertion. This is needed to use `insert_with`.
-// * Determine if there's any sub-expression that can't be placed in a closure.
-// * Determine if there's only a single insert statement. This is needed to give better suggestions.
-
+/// This visitor needs to do a multiple things:
+/// * Find all usages of the map. An insertion can only be made before any other usages of the map.
+/// * Determine if there's an insertion using the same key. There's no need for the entry api
+///   otherwise.
+/// * Determine if the final statement executed is an insertion. This is needed to use
+///   `or_insert_with`.
+/// * Determine if there's any sub-expression that can't be placed in a closure.
+/// * Determine if there's only a single insert statement. `or_insert` can be used in this case.
 #[allow(clippy::struct_excessive_bools)]
-struct InsertSearcher<'cx, 'i, 'tcx> {
+struct InsertSearcher<'cx, 'tcx> {
     cx: &'cx LateContext<'tcx>,
     /// The map expression used in the contains call.
     map: &'tcx Expr<'tcx>,
@@ -334,13 +325,16 @@ struct InsertSearcher<'cx, 'i, '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,
-    // A single insert expression has a slightly different suggestion.
+    // 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,
-    edits: &'i mut Vec<Edit<'tcx>>,
+    /// The locations where changes need to be made for the suggestion.
+    edits: Vec<Edit<'tcx>>,
+    /// A stack of loops the visitor is currently in.
     loops: Vec<HirId>,
 }
-impl<'tcx> InsertSearcher<'_, '_, 'tcx> {
+impl<'tcx> InsertSearcher<'_, 'tcx> {
     /// Visit the expression as a branch in control flow. Multiple insert calls can be used, but
     /// only if they are on separate code paths. This will return whether the map was used in the
     /// given expression.
@@ -363,7 +357,7 @@ impl<'tcx> InsertSearcher<'_, '_, 'tcx> {
         self.in_tail_pos = in_tail_pos;
     }
 }
-impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, '_, 'tcx> {
+impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> {
     type Map = ErasedMap<'tcx>;
     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
         NestedVisitorMap::None
@@ -483,6 +477,7 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, '_, 'tcx> {
                 },
                 ExprKind::Loop(block, ..) => {
                     self.loops.push(expr.hir_id);
+                    self.is_single_insert = false;
                     self.allow_insert_closure &= !self.in_tail_pos;
                     // Don't allow insertions inside of a loop.
                     let edit_len = self.edits.len();
@@ -519,7 +514,13 @@ impl InsertSearchResults<'tcx> {
         self.is_single_insert.then(|| self.edits[0].as_insertion().unwrap())
     }
 
-    fn snippet_occupied(&self, cx: &LateContext<'_>, mut span: Span, app: &mut Applicability) -> String {
+    fn snippet(
+        &self,
+        cx: &LateContext<'_>,
+        mut span: Span,
+        app: &mut Applicability,
+        write_wrapped: impl Fn(&mut String, Insertion<'_>, SyntaxContext, &mut Applicability),
+    ) -> String {
         let ctxt = span.ctxt();
         let mut res = String::new();
         for insertion in self.edits.iter().filter_map(|e| e.as_insertion()) {
@@ -530,13 +531,13 @@ impl InsertSearchResults<'tcx> {
                 app,
             ));
             if is_expr_used_or_unified(cx.tcx, insertion.call) {
-                res.push_str("Some(e.insert(");
-                res.push_str(&snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0);
-                res.push_str("))");
+                write_wrapped(&mut res, insertion, ctxt, app);
             } else {
-                res.push_str("e.insert(");
-                res.push_str(&snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0);
-                res.push(')');
+                let _ = write!(
+                    res,
+                    "e.insert({})",
+                    snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0
+                );
             }
             span = span.trim_start(insertion.call.span).unwrap_or(DUMMY_SP);
         }
@@ -544,42 +545,41 @@ impl InsertSearchResults<'tcx> {
         res
     }
 
-    fn snippet_vacant(&self, cx: &LateContext<'_>, mut span: Span, app: &mut Applicability) -> String {
-        let ctxt = span.ctxt();
-        let mut res = String::new();
-        for insertion in self.edits.iter().filter_map(|e| e.as_insertion()) {
-            res.push_str(&snippet_with_applicability(
-                cx,
-                span.until(insertion.call.span),
-                "..",
-                app,
-            ));
-            if is_expr_used_or_unified(cx.tcx, insertion.call) {
-                if is_expr_final_block_expr(cx.tcx, insertion.call) {
-                    let _ = write!(
+    fn snippet_occupied(&self, cx: &LateContext<'_>, span: Span, app: &mut Applicability) -> (String, &'static str) {
+        (
+            self.snippet(cx, span, app, |res, insertion, ctxt, app| {
+                // Insertion into a map would return `Some(&mut value)`, but the entry returns `&mut value`
+                let _ = write!(
+                    res,
+                    "Some(e.insert({}))",
+                    snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0
+                );
+            }),
+            "Occupied(mut e)",
+        )
+    }
+
+    fn snippet_vacant(&self, cx: &LateContext<'_>, span: Span, app: &mut Applicability) -> (String, &'static str) {
+        (
+            self.snippet(cx, span, app, |res, insertion, ctxt, app| {
+                // Insertion into a map would return `None`, but the entry returns a mutable reference.
+                let _ = if is_expr_final_block_expr(cx.tcx, insertion.call) {
+                    write!(
                         res,
                         "e.insert({});\n{}None",
                         snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0,
                         snippet_indent(cx, insertion.call.span).as_deref().unwrap_or(""),
-                    );
+                    )
                 } else {
-                    let _ = write!(
+                    write!(
                         res,
                         "{{ e.insert({}); None }}",
                         snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0,
-                    );
-                }
-            } else {
-                let _ = write!(
-                    res,
-                    "e.insert({})",
-                    snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0,
-                );
-            }
-            span = span.trim_start(insertion.call.span).unwrap_or(DUMMY_SP);
-        }
-        res.push_str(&snippet_with_applicability(cx, span, "..", app));
-        res
+                    )
+                };
+            }),
+            "Vacant(e)",
+        )
     }
 
     fn snippet_closure(&self, cx: &LateContext<'_>, mut span: Span, app: &mut Applicability) -> String {
@@ -588,6 +588,7 @@ impl InsertSearchResults<'tcx> {
         for edit in &self.edits {
             match *edit {
                 Edit::Insertion(insertion) => {
+                    // Cut out the value from `map.insert(key, value)`
                     res.push_str(&snippet_with_applicability(
                         cx,
                         span.until(insertion.call.span),
@@ -598,6 +599,7 @@ impl InsertSearchResults<'tcx> {
                     span = span.trim_start(insertion.call.span).unwrap_or(DUMMY_SP);
                 },
                 Edit::RemoveSemi(semi_span) => {
+                    // Cut out the semicolon. This allows the value to be returned from the closure.
                     res.push_str(&snippet_with_applicability(cx, span.until(semi_span), "..", app));
                     span = span.trim_start(semi_span).unwrap_or(DUMMY_SP);
                 },
@@ -607,18 +609,18 @@ impl InsertSearchResults<'tcx> {
         res
     }
 }
+
 fn find_insert_calls(
     cx: &LateContext<'tcx>,
     contains_expr: &ContainsExpr<'tcx>,
     expr: &'tcx Expr<'_>,
 ) -> Option<InsertSearchResults<'tcx>> {
-    let mut edits = Vec::new();
     let mut s = InsertSearcher {
         cx,
         map: contains_expr.map,
         key: contains_expr.key,
         ctxt: expr.span.ctxt(),
-        edits: &mut edits,
+        edits: Vec::new(),
         is_map_used: false,
         allow_insert_closure: true,
         can_use_entry: true,
@@ -629,6 +631,7 @@ fn find_insert_calls(
     s.visit_expr(expr);
     let allow_insert_closure = s.allow_insert_closure;
     let is_single_insert = s.is_single_insert;
+    let edits = s.edits;
     s.can_use_entry.then(|| InsertSearchResults {
         edits,
         allow_insert_closure,
diff --git a/clippy_lints/src/manual_map.rs b/clippy_lints/src/manual_map.rs
index 3f8b976fcac..d9b75149b9f 100644
--- a/clippy_lints/src/manual_map.rs
+++ b/clippy_lints/src/manual_map.rs
@@ -2,13 +2,13 @@ use crate::{map_unit_fn::OPTION_MAP_UNIT_FN, matches::MATCH_AS_REF};
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
 use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable};
-use clippy_utils::{can_move_expr_to_closure, in_constant, is_allowed, is_else_clause, is_lang_ctor, match_var, peel_hir_expr_refs};
+use clippy_utils::{
+    can_move_expr_to_closure, in_constant, is_allowed, is_else_clause, is_lang_ctor, match_var, peel_hir_expr_refs,
+};
 use rustc_ast::util::parser::PREC_POSTFIX;
 use rustc_errors::Applicability;
 use rustc_hir::LangItem::{OptionNone, OptionSome};
-use rustc_hir::{
-    Arm, BindingAnnotation, Block, Expr, ExprKind, MatchSource, Mutability, Pat, PatKind, QPath,
-};
+use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, MatchSource, Mutability, Pat, PatKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 5d6c1337be6..90a6fd225ab 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -64,8 +64,8 @@ use rustc_hir::intravisit::{self, walk_expr, ErasedMap, NestedVisitorMap, Visito
 use rustc_hir::LangItem::{ResultErr, ResultOk};
 use rustc_hir::{
     def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, GenericArgs, HirId, Impl,
-    ImplItem, ImplItemKind, Item, ItemKind, LangItem, MatchSource, Node, Param, Pat, PatKind, Path, PathSegment, QPath,
-    Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind,
+    ImplItem, ImplItemKind, Item, ItemKind, LangItem, Local, MatchSource, Node, Param, Pat, PatKind, Path, PathSegment,
+    QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind,
 };
 use rustc_lint::{LateContext, Level, Lint, LintContext};
 use rustc_middle::hir::exports::Export;
@@ -1312,48 +1312,7 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
     did.map_or(false, |did| must_use_attr(&cx.tcx.get_attrs(did)).is_some())
 }
 
-pub fn get_expr_use_node(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<Node<'tcx>> {
-    let map = tcx.hir();
-    let mut child_id = expr.hir_id;
-    let mut iter = map.parent_iter(child_id);
-    loop {
-        match iter.next() {
-            None => break None,
-            Some((id, Node::Block(_))) => child_id = id,
-            Some((id, Node::Arm(arm))) if arm.body.hir_id == child_id => child_id = id,
-            Some((_, Node::Expr(expr))) => match expr.kind {
-                ExprKind::Break(
-                    Destination {
-                        target_id: Ok(dest), ..
-                    },
-                    _,
-                ) => {
-                    iter = map.parent_iter(dest);
-                    child_id = dest;
-                },
-                ExprKind::DropTemps(_) | ExprKind::Block(..) => child_id = expr.hir_id,
-                ExprKind::If(control_expr, ..) | ExprKind::Match(control_expr, ..)
-                    if control_expr.hir_id != child_id =>
-                {
-                    child_id = expr.hir_id
-                },
-                _ => break Some(Node::Expr(expr)),
-            },
-            Some((_, node)) => break Some(node),
-        }
-    }
-}
-
-pub fn is_expr_used(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
-    !matches!(
-        get_expr_use_node(tcx, expr),
-        Some(Node::Stmt(Stmt {
-            kind: StmtKind::Expr(_) | StmtKind::Semi(_),
-            ..
-        }))
-    )
-}
-
+/// Gets the node where an expression is either used, or it's type is unified with another branch.
 pub fn get_expr_use_or_unification_node(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<Node<'tcx>> {
     let map = tcx.hir();
     let mut child_id = expr.hir_id;
@@ -1374,16 +1333,26 @@ pub fn get_expr_use_or_unification_node(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> O
     }
 }
 
+/// Checks if the result of an expression is used, or it's type is unified with another branch.
 pub fn is_expr_used_or_unified(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
     !matches!(
         get_expr_use_or_unification_node(tcx, expr),
         None | Some(Node::Stmt(Stmt {
-            kind: StmtKind::Expr(_) | StmtKind::Semi(_),
+            kind: StmtKind::Expr(_)
+                | StmtKind::Semi(_)
+                | StmtKind::Local(Local {
+                    pat: Pat {
+                        kind: PatKind::Wild,
+                        ..
+                    },
+                    ..
+                }),
             ..
         }))
     )
 }
 
+/// Checks if the expression is the final expression returned from a block.
 pub fn is_expr_final_block_expr(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
     matches!(get_parent_node(tcx, expr.hir_id), Some(Node::Block(..)))
 }
diff --git a/tests/ui/entry.fixed b/tests/ui/entry.fixed
index 524f2132797..cfad3090ba3 100644
--- a/tests/ui/entry.fixed
+++ b/tests/ui/entry.fixed
@@ -2,6 +2,7 @@
 
 #![allow(unused, clippy::needless_pass_by_value, clippy::collapsible_if)]
 #![warn(clippy::map_entry)]
+#![feature(asm)]
 
 use std::collections::{BTreeMap, HashMap};
 use std::hash::Hash;
@@ -10,11 +11,19 @@ macro_rules! m {
     ($e:expr) => {{ $e }};
 }
 
+macro_rules! insert {
+    ($map:expr, $key:expr, $val:expr) => {
+        $map.insert($key, $val)
+    };
+}
+
 fn foo() {}
 
-fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2: V) {
+fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, m2: &mut HashMap<K, V>, k: K, k2: K, v: V, v2: V) {
+    // or_insert(v)
     m.entry(k).or_insert(v);
 
+    // semicolon on insert, use or_insert_with(..)
     m.entry(k).or_insert_with(|| {
         if true {
             v
@@ -23,6 +32,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
         }
     });
 
+    // semicolon on if, use or_insert_with(..)
     m.entry(k).or_insert_with(|| {
         if true {
             v
@@ -31,6 +41,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
         }
     });
 
+    // early return, use if let
     if let std::collections::hash_map::Entry::Vacant(e) = m.entry(k) {
         if true {
             e.insert(v);
@@ -40,11 +51,13 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
         }
     }
 
+    // use or_insert_with(..)
     m.entry(k).or_insert_with(|| {
         foo();
         v
     });
 
+    // semicolon on insert and match, use or_insert_with(..)
     m.entry(k).or_insert_with(|| {
         match 0 {
             1 if true => {
@@ -56,18 +69,17 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
         }
     });
 
+    // one branch doesn't insert, use if let
     if let std::collections::hash_map::Entry::Vacant(e) = m.entry(k) {
         match 0 {
-            0 => {},
-            1 => {
-                e.insert(v);
-            },
+            0 => foo(),
             _ => {
                 e.insert(v2);
             },
         };
     }
 
+    // use or_insert_with
     m.entry(k).or_insert_with(|| {
         foo();
         match 0 {
@@ -94,10 +106,46 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
         }
     });
 
+    // ok, insert in loop
+    if !m.contains_key(&k) {
+        for _ in 0..2 {
+            m.insert(k, v);
+        }
+    }
+
+    // macro_expansion test, use or_insert(..)
     m.entry(m!(k)).or_insert_with(|| m!(v));
+
+    // ok, map used before insertion
+    if !m.contains_key(&k) {
+        let _ = m.len();
+        m.insert(k, v);
+    }
+
+    // ok, inline asm
+    if !m.contains_key(&k) {
+        unsafe { asm!("nop") }
+        m.insert(k, v);
+    }
+
+    // ok, different keys.
+    if !m.contains_key(&k) {
+        m.insert(k2, v);
+    }
+
+    // ok, different maps
+    if !m.contains_key(&k) {
+        m2.insert(k, v);
+    }
+
+    // ok, insert in macro
+    if !m.contains_key(&k) {
+        insert!(m, k, v);
+    }
 }
 
 fn btree_map<K: Eq + Ord + Copy, V: Copy>(m: &mut BTreeMap<K, V>, k: K, v: V, v2: V) {
+    // insert then do something, use if let
     if let std::collections::btree_map::Entry::Vacant(e) = m.entry(k) {
         e.insert(v);
         foo();
diff --git a/tests/ui/entry.rs b/tests/ui/entry.rs
index ff4890eeeb6..fa9280b58de 100644
--- a/tests/ui/entry.rs
+++ b/tests/ui/entry.rs
@@ -2,6 +2,7 @@
 
 #![allow(unused, clippy::needless_pass_by_value, clippy::collapsible_if)]
 #![warn(clippy::map_entry)]
+#![feature(asm)]
 
 use std::collections::{BTreeMap, HashMap};
 use std::hash::Hash;
@@ -10,13 +11,21 @@ macro_rules! m {
     ($e:expr) => {{ $e }};
 }
 
+macro_rules! insert {
+    ($map:expr, $key:expr, $val:expr) => {
+        $map.insert($key, $val)
+    };
+}
+
 fn foo() {}
 
-fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2: V) {
+fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, m2: &mut HashMap<K, V>, k: K, k2: K, v: V, v2: V) {
+    // or_insert(v)
     if !m.contains_key(&k) {
         m.insert(k, v);
     }
 
+    // semicolon on insert, use or_insert_with(..)
     if !m.contains_key(&k) {
         if true {
             m.insert(k, v);
@@ -25,6 +34,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
         }
     }
 
+    // semicolon on if, use or_insert_with(..)
     if !m.contains_key(&k) {
         if true {
             m.insert(k, v)
@@ -33,6 +43,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
         };
     }
 
+    // early return, use if let
     if !m.contains_key(&k) {
         if true {
             m.insert(k, v);
@@ -42,11 +53,13 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
         }
     }
 
+    // use or_insert_with(..)
     if !m.contains_key(&k) {
         foo();
         m.insert(k, v);
     }
 
+    // semicolon on insert and match, use or_insert_with(..)
     if !m.contains_key(&k) {
         match 0 {
             1 if true => {
@@ -58,18 +71,17 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
         };
     }
 
+    // one branch doesn't insert, use if let
     if !m.contains_key(&k) {
         match 0 {
-            0 => {},
-            1 => {
-                m.insert(k, v);
-            },
+            0 => foo(),
             _ => {
                 m.insert(k, v2);
             },
         };
     }
 
+    // use or_insert_with
     if !m.contains_key(&k) {
         foo();
         match 0 {
@@ -96,12 +108,48 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
         }
     }
 
+    // ok, insert in loop
+    if !m.contains_key(&k) {
+        for _ in 0..2 {
+            m.insert(k, v);
+        }
+    }
+
+    // macro_expansion test, use or_insert(..)
     if !m.contains_key(&m!(k)) {
         m.insert(m!(k), m!(v));
     }
+
+    // ok, map used before insertion
+    if !m.contains_key(&k) {
+        let _ = m.len();
+        m.insert(k, v);
+    }
+
+    // ok, inline asm
+    if !m.contains_key(&k) {
+        unsafe { asm!("nop") }
+        m.insert(k, v);
+    }
+
+    // ok, different keys.
+    if !m.contains_key(&k) {
+        m.insert(k2, v);
+    }
+
+    // ok, different maps
+    if !m.contains_key(&k) {
+        m2.insert(k, v);
+    }
+
+    // ok, insert in macro
+    if !m.contains_key(&k) {
+        insert!(m, k, v);
+    }
 }
 
 fn btree_map<K: Eq + Ord + Copy, V: Copy>(m: &mut BTreeMap<K, V>, k: K, v: V, v2: V) {
+    // insert then do something, use if let
     if !m.contains_key(&k) {
         m.insert(k, v);
         foo();
diff --git a/tests/ui/entry.stderr b/tests/ui/entry.stderr
index 63b5f5a0b2c..2f075a97010 100644
--- a/tests/ui/entry.stderr
+++ b/tests/ui/entry.stderr
@@ -1,5 +1,5 @@
 error: usage of `contains_key` followed by `insert` on a `HashMap`
-  --> $DIR/entry.rs:16:5
+  --> $DIR/entry.rs:24:5
    |
 LL | /     if !m.contains_key(&k) {
 LL | |         m.insert(k, v);
@@ -9,7 +9,7 @@ LL | |     }
    = note: `-D clippy::map-entry` implied by `-D warnings`
 
 error: usage of `contains_key` followed by `insert` on a `HashMap`
-  --> $DIR/entry.rs:20:5
+  --> $DIR/entry.rs:29:5
    |
 LL | /     if !m.contains_key(&k) {
 LL | |         if true {
@@ -31,7 +31,7 @@ LL |         }
  ...
 
 error: usage of `contains_key` followed by `insert` on a `HashMap`
-  --> $DIR/entry.rs:28:5
+  --> $DIR/entry.rs:38:5
    |
 LL | /     if !m.contains_key(&k) {
 LL | |         if true {
@@ -53,7 +53,7 @@ LL |         }
  ...
 
 error: usage of `contains_key` followed by `insert` on a `HashMap`
-  --> $DIR/entry.rs:36:5
+  --> $DIR/entry.rs:47:5
    |
 LL | /     if !m.contains_key(&k) {
 LL | |         if true {
@@ -75,7 +75,7 @@ LL |             return;
  ...
 
 error: usage of `contains_key` followed by `insert` on a `HashMap`
-  --> $DIR/entry.rs:45:5
+  --> $DIR/entry.rs:57:5
    |
 LL | /     if !m.contains_key(&k) {
 LL | |         foo();
@@ -92,7 +92,7 @@ LL |     });
    |
 
 error: usage of `contains_key` followed by `insert` on a `HashMap`
-  --> $DIR/entry.rs:50:5
+  --> $DIR/entry.rs:63:5
    |
 LL | /     if !m.contains_key(&k) {
 LL | |         match 0 {
@@ -114,12 +114,12 @@ LL |             _ => {
  ...
 
 error: usage of `contains_key` followed by `insert` on a `HashMap`
-  --> $DIR/entry.rs:61:5
+  --> $DIR/entry.rs:75:5
    |
 LL | /     if !m.contains_key(&k) {
 LL | |         match 0 {
-LL | |             0 => {},
-LL | |             1 => {
+LL | |             0 => foo(),
+LL | |             _ => {
 ...  |
 LL | |         };
 LL | |     }
@@ -129,14 +129,14 @@ help: try this
    |
 LL |     if let std::collections::hash_map::Entry::Vacant(e) = m.entry(k) {
 LL |         match 0 {
-LL |             0 => {},
-LL |             1 => {
-LL |                 e.insert(v);
+LL |             0 => foo(),
+LL |             _ => {
+LL |                 e.insert(v2);
 LL |             },
  ...
 
 error: usage of `contains_key` followed by `insert` on a `HashMap`
-  --> $DIR/entry.rs:73:5
+  --> $DIR/entry.rs:85:5
    |
 LL | /     if !m.contains_key(&k) {
 LL | |         foo();
@@ -158,7 +158,7 @@ LL |             },
  ...
 
 error: usage of `contains_key` followed by `insert` on a `HashMap`
-  --> $DIR/entry.rs:99:5
+  --> $DIR/entry.rs:119:5
    |
 LL | /     if !m.contains_key(&m!(k)) {
 LL | |         m.insert(m!(k), m!(v));
@@ -166,7 +166,7 @@ LL | |     }
    | |_____^ help: try this: `m.entry(m!(k)).or_insert_with(|| m!(v));`
 
 error: usage of `contains_key` followed by `insert` on a `BTreeMap`
-  --> $DIR/entry.rs:105:5
+  --> $DIR/entry.rs:153:5
    |
 LL | /     if !m.contains_key(&k) {
 LL | |         m.insert(k, v);
diff --git a/tests/ui/entry_unfixable.rs b/tests/ui/entry_unfixable.rs
deleted file mode 100644
index beb2d5c97b1..00000000000
--- a/tests/ui/entry_unfixable.rs
+++ /dev/null
@@ -1,50 +0,0 @@
-#![allow(unused, clippy::needless_pass_by_value)]
-#![warn(clippy::map_entry)]
-
-use std::collections::{BTreeMap, HashMap};
-use std::hash::Hash;
-
-macro_rules! m {
-    ($map:expr, $key:expr, $value:expr) => {
-        $map.insert($key, $value)
-    };
-    ($e:expr) => {{ $e }};
-}
-
-fn foo() {}
-
-// should not trigger
-fn insert_other_if_absent<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, o: K, v: V) {
-    if !m.contains_key(&k) {
-        m.insert(o, v);
-    }
-}
-
-// should not trigger, because the one uses different HashMap from another one
-fn insert_from_different_map<K: Eq + Hash, V>(m: HashMap<K, V>, n: &mut HashMap<K, V>, k: K, v: V) {
-    if !m.contains_key(&k) {
-        n.insert(k, v);
-    }
-}
-
-// should not trigger, because the one uses different HashMap from another one
-fn insert_from_different_map2<K: Eq + Hash, V>(m: &mut HashMap<K, V>, n: &mut HashMap<K, V>, k: K, v: V) {
-    if !m.contains_key(&k) {
-        n.insert(k, v);
-    }
-}
-
-fn insert_in_macro<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
-    if !m.contains_key(&k) {
-        m!(m, k, v);
-    }
-}
-
-fn use_map_then_insert<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
-    if !m.contains_key(&k) {
-        let _ = m.len();
-        m.insert(k, v);
-    }
-}
-
-fn main() {}
diff --git a/tests/ui/string_lit_as_bytes.fixed b/tests/ui/string_lit_as_bytes.fixed
index dd22bfa5c53..df2256e4f97 100644
--- a/tests/ui/string_lit_as_bytes.fixed
+++ b/tests/ui/string_lit_as_bytes.fixed
@@ -22,7 +22,7 @@ fn str_lit_as_bytes() {
 
     let current_version = env!("CARGO_PKG_VERSION").as_bytes();
 
-    let includestr = include_bytes!("entry_unfixable.rs");
+    let includestr = include_bytes!("string_lit_as_bytes.rs");
 
     let _ = b"string with newline\t\n";
 }
diff --git a/tests/ui/string_lit_as_bytes.rs b/tests/ui/string_lit_as_bytes.rs
index d2a710ed6b8..c6bf8f732ed 100644
--- a/tests/ui/string_lit_as_bytes.rs
+++ b/tests/ui/string_lit_as_bytes.rs
@@ -22,7 +22,7 @@ fn str_lit_as_bytes() {
 
     let current_version = env!("CARGO_PKG_VERSION").as_bytes();
 
-    let includestr = include_str!("entry_unfixable.rs").as_bytes();
+    let includestr = include_str!("string_lit_as_bytes.rs").as_bytes();
 
     let _ = "string with newline\t\n".as_bytes();
 }
diff --git a/tests/ui/string_lit_as_bytes.stderr b/tests/ui/string_lit_as_bytes.stderr
index e0ddb070b50..f47d6161c6c 100644
--- a/tests/ui/string_lit_as_bytes.stderr
+++ b/tests/ui/string_lit_as_bytes.stderr
@@ -27,8 +27,8 @@ LL |     let bs = "lit to owned".to_owned().into_bytes();
 error: calling `as_bytes()` on `include_str!(..)`
   --> $DIR/string_lit_as_bytes.rs:25:22
    |
-LL |     let includestr = include_str!("entry_unfixable.rs").as_bytes();
-   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `include_bytes!(..)` instead: `include_bytes!("entry_unfixable.rs")`
+LL |     let includestr = include_str!("string_lit_as_bytes.rs").as_bytes();
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `include_bytes!(..)` instead: `include_bytes!("string_lit_as_bytes.rs")`
 
 error: calling `as_bytes()` on a string literal
   --> $DIR/string_lit_as_bytes.rs:27:13