From 9945bd82a8273f9c5506e9c26d51b312188d620f Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Sun, 3 Jan 2016 17:19:49 +0100
Subject: [PATCH] Add better error messages for HashMapLint

---
 src/hashmap.rs                | 37 ++++++++++++++++++++++++++---------
 tests/compile-fail/hashmap.rs | 24 ++++++++++++++++++++---
 2 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/src/hashmap.rs b/src/hashmap.rs
index 14f535b3820..095a00e3777 100644
--- a/src/hashmap.rs
+++ b/src/hashmap.rs
@@ -14,7 +14,14 @@ use utils::HASHMAP_PATH;
 /// if !m.contains_key(k) { m.insert(k.clone(), v); }
 /// ```
 ///
-/// **Example:** `if !m.contains_key(&k) { m.insert(k, v) }`
+/// **Example:**
+/// ```rust
+/// if !m.contains_key(&k) { m.insert(k, v) }
+/// ```
+/// can be rewritten as:
+/// ```rust
+/// m.entry(k).or_insert(v);
+/// ```
 declare_lint! {
     pub HASHMAP_ENTRY,
     Warn,
@@ -49,13 +56,15 @@ impl LateLintPass for HashMapLint {
                 let obj_ty = walk_ptrs_ty(cx.tcx.expr_ty(map));
 
                 if match_type(cx, obj_ty, &HASHMAP_PATH) {
+                    let sole_expr = if then.expr.is_some() { 1 } else { 0 } + then.stmts.len() == 1;
+
                     if let Some(ref then) = then.expr {
-                        check_for_insert(cx, expr.span, map, key, then);
+                        check_for_insert(cx, expr.span, map, key, then, sole_expr);
                     }
 
                     for stmt in &then.stmts {
                         if let StmtSemi(ref stmt, _) = stmt.node {
-                            check_for_insert(cx, expr.span, map, key, stmt);
+                            check_for_insert(cx, expr.span, map, key, stmt, sole_expr);
                         }
                     }
                 }
@@ -64,7 +73,7 @@ impl LateLintPass for HashMapLint {
     }
 }
 
-fn check_for_insert(cx: &LateContext, span: Span, map: &Expr, key: &Expr, expr: &Expr) {
+fn check_for_insert(cx: &LateContext, span: Span, map: &Expr, key: &Expr, expr: &Expr, sole_expr: bool) {
     if_let_chain! {
         [
             let ExprMethodCall(ref name, _, ref params) = expr.node,
@@ -73,11 +82,21 @@ fn check_for_insert(cx: &LateContext, span: Span, map: &Expr, key: &Expr, expr:
             get_item_name(cx, map) == get_item_name(cx, &*params[0]),
             is_exp_equal(cx, key, &params[1])
         ], {
-            span_help_and_lint(cx, HASHMAP_ENTRY, span,
-                               "usage of `contains_key` followed by `insert` on `HashMap`",
-                               &format!("Consider using `{}.entry({})`",
-                                        snippet(cx, map.span, ".."),
-                                        snippet(cx, params[1].span, "..")));
+            if sole_expr {
+                span_help_and_lint(cx, HASHMAP_ENTRY, span,
+                                   "usage of `contains_key` followed by `insert` on `HashMap`",
+                                   &format!("Consider using `{}.entry({}).or_insert({})`",
+                                            snippet(cx, map.span, ".."),
+                                            snippet(cx, params[1].span, ".."),
+                                            snippet(cx, params[2].span, ".."))); 
+            }
+            else {
+                span_help_and_lint(cx, HASHMAP_ENTRY, span,
+                                   "usage of `contains_key` followed by `insert` on `HashMap`",
+                                   &format!("Consider using `{}.entry({})`",
+                                            snippet(cx, map.span, ".."),
+                                            snippet(cx, params[1].span, "..")));
+            }
         }
     }
 }
diff --git a/tests/compile-fail/hashmap.rs b/tests/compile-fail/hashmap.rs
index 9aeacac0e14..a53566a794e 100644
--- a/tests/compile-fail/hashmap.rs
+++ b/tests/compile-fail/hashmap.rs
@@ -7,12 +7,30 @@
 use std::collections::HashMap;
 use std::hash::Hash;
 
-fn insert_if_absent<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
-    if !m.contains_key(&k) { m.insert(k, v); } //~ERROR: usage of `contains_key` followed by `insert` on `HashMap`
+fn foo() {}
+
+fn insert_if_absent0<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
+    if !m.contains_key(&k) { m.insert(k, v); }
+    //~^ERROR: usage of `contains_key` followed by `insert` on `HashMap`
+    //~^^HELP: Consider using `m.entry(k).or_insert(v)`
+}
+
+fn insert_if_absent1<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
+    if !m.contains_key(&k) { foo(); m.insert(k, v); }
+    //~^ERROR: usage of `contains_key` followed by `insert` on `HashMap`
+    //~^^HELP: Consider using `m.entry(k)`
 }
 
 fn insert_if_absent2<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
-    if !m.contains_key(&k) { m.insert(k, v) } else { None }; //~ERROR: usage of `contains_key` followed by `insert` on `HashMap`
+    if !m.contains_key(&k) { m.insert(k, v) } else { None };
+    //~^ERROR: usage of `contains_key` followed by `insert` on `HashMap`
+    //~^^HELP: Consider using `m.entry(k).or_insert(v)`
+}
+
+fn insert_if_absent3<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
+    if !m.contains_key(&k) { foo(); m.insert(k, v) } else { None };
+    //~^ERROR: usage of `contains_key` followed by `insert` on `HashMap`
+    //~^^HELP: Consider using `m.entry(k)`
 }
 
 fn insert_other_if_absent<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, o: K, v: V) {