From c94c5e904832a0da581169ab1f552cab5fb0cf29 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Tue, 3 Sep 2019 09:41:37 +0900 Subject: [PATCH 1/2] Fix `map_entry` false positive --- clippy_lints/src/entry.rs | 3 ++- tests/ui/entry.rs | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs index e6b09f89b353..16009d8ab86d 100644 --- a/clippy_lints/src/entry.rs +++ b/clippy_lints/src/entry.rs @@ -1,5 +1,5 @@ use crate::utils::SpanlessEq; -use crate::utils::{get_item_name, higher, match_type, paths, snippet, span_lint_and_then, walk_ptrs_ty}; +use crate::utils::{get_item_name, higher, match_type, paths, snippet, snippet_opt, span_lint_and_then, walk_ptrs_ty}; use if_chain::if_chain; use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc::hir::*; @@ -140,6 +140,7 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> { if path.ident.name == sym!(insert); if get_item_name(self.cx, self.map) == get_item_name(self.cx, ¶ms[0]); if SpanlessEq::new(self.cx).eq_expr(self.key, ¶ms[1]); + if snippet_opt(self.cx, self.map.span) == snippet_opt(self.cx, params[0].span); then { span_lint_and_then(self.cx, MAP_ENTRY, self.span, &format!("usage of `contains_key` followed by `insert` on a `{}`", self.ty), |db| { diff --git a/tests/ui/entry.rs b/tests/ui/entry.rs index 00d496e36f6c..eebf2518a65c 100644 --- a/tests/ui/entry.rs +++ b/tests/ui/entry.rs @@ -68,4 +68,11 @@ fn insert_other_if_absent(m: &mut HashMap, k: K, o: K, v: } } +// should not trigger, because the one uses different HashMap from another one +fn insert_other(m: &mut HashMap, n: &mut HashMap, k: K, v: V) { + if !m.contains_key(&k) { + n.insert(k, v); + } +} + fn main() {} From 5c760f055544a694af24226aa1272e0ccbb4f5cf Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Thu, 5 Sep 2019 00:24:45 +0900 Subject: [PATCH 2/2] Improve tests --- tests/ui/entry.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/ui/entry.rs b/tests/ui/entry.rs index eebf2518a65c..0c84cd325c4d 100644 --- a/tests/ui/entry.rs +++ b/tests/ui/entry.rs @@ -69,7 +69,14 @@ fn insert_other_if_absent(m: &mut HashMap, k: K, o: K, v: } // should not trigger, because the one uses different HashMap from another one -fn insert_other(m: &mut HashMap, n: &mut HashMap, k: K, v: V) { +fn insert_from_different_map(m: HashMap, n: &mut HashMap, 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(m: &mut HashMap, n: &mut HashMap, k: K, v: V) { if !m.contains_key(&k) { n.insert(k, v); }