Add better error messages for HashMapLint

This commit is contained in:
mcarton 2016-01-03 17:19:49 +01:00
parent d0bb71e6a2
commit 9945bd82a8
2 changed files with 49 additions and 12 deletions

View file

@ -14,7 +14,14 @@ use utils::HASHMAP_PATH;
/// if !m.contains_key(k) { m.insert(k.clone(), v); } /// 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! { declare_lint! {
pub HASHMAP_ENTRY, pub HASHMAP_ENTRY,
Warn, Warn,
@ -49,13 +56,15 @@ impl LateLintPass for HashMapLint {
let obj_ty = walk_ptrs_ty(cx.tcx.expr_ty(map)); let obj_ty = walk_ptrs_ty(cx.tcx.expr_ty(map));
if match_type(cx, obj_ty, &HASHMAP_PATH) { 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 { 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 { for stmt in &then.stmts {
if let StmtSemi(ref stmt, _) = stmt.node { 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! { if_let_chain! {
[ [
let ExprMethodCall(ref name, _, ref params) = expr.node, let ExprMethodCall(ref name, _, ref params) = expr.node,
@ -73,6 +82,15 @@ fn check_for_insert(cx: &LateContext, span: Span, map: &Expr, key: &Expr, expr:
get_item_name(cx, map) == get_item_name(cx, &*params[0]), get_item_name(cx, map) == get_item_name(cx, &*params[0]),
is_exp_equal(cx, key, &params[1]) is_exp_equal(cx, key, &params[1])
], { ], {
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, span_help_and_lint(cx, HASHMAP_ENTRY, span,
"usage of `contains_key` followed by `insert` on `HashMap`", "usage of `contains_key` followed by `insert` on `HashMap`",
&format!("Consider using `{}.entry({})`", &format!("Consider using `{}.entry({})`",
@ -81,3 +99,4 @@ fn check_for_insert(cx: &LateContext, span: Span, map: &Expr, key: &Expr, expr:
} }
} }
} }
}

View file

@ -7,12 +7,30 @@
use std::collections::HashMap; use std::collections::HashMap;
use std::hash::Hash; use std::hash::Hash;
fn insert_if_absent<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) { fn foo() {}
if !m.contains_key(&k) { m.insert(k, v); } //~ERROR: usage of `contains_key` followed by `insert` on `HashMap`
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) { 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) { fn insert_other_if_absent<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, o: K, v: V) {