From d445bf2e898e231e52388f036aee9156ad33ebb1 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 04:29:39 -0700 Subject: [PATCH] Remove suggestion for complex map_entry cases --- clippy_lints/src/entry.rs | 18 +++++----- tests/ui/entry.stderr | 74 +++++++++++++++++++++++++++++++++++---- 2 files changed, 77 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs index 16009d8ab86..a590a8179c2 100644 --- a/clippy_lints/src/entry.rs +++ b/clippy_lints/src/entry.rs @@ -1,5 +1,6 @@ use crate::utils::SpanlessEq; -use crate::utils::{get_item_name, higher, match_type, paths, snippet, snippet_opt, span_lint_and_then, walk_ptrs_ty}; +use crate::utils::{get_item_name, higher, match_type, paths, snippet, snippet_opt}; +use crate::utils::{snippet_with_applicability, span_lint_and_then, walk_ptrs_ty}; use if_chain::if_chain; use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc::hir::*; @@ -145,10 +146,11 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> { span_lint_and_then(self.cx, MAP_ENTRY, self.span, &format!("usage of `contains_key` followed by `insert` on a `{}`", self.ty), |db| { if self.sole_expr { + let mut app = Applicability::MachineApplicable; let help = format!("{}.entry({}).or_insert({})", - snippet(self.cx, self.map.span, "map"), - snippet(self.cx, params[1].span, ".."), - snippet(self.cx, params[2].span, "..")); + snippet_with_applicability(self.cx, self.map.span, "map", &mut app), + snippet_with_applicability(self.cx, params[1].span, "..", &mut app), + snippet_with_applicability(self.cx, params[2].span, "..", &mut app)); db.span_suggestion( self.span, @@ -158,15 +160,13 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> { ); } else { - let help = format!("{}.entry({})", + let help = format!("consider using `{}.entry({})`", snippet(self.cx, self.map.span, "map"), snippet(self.cx, params[1].span, "..")); - db.span_suggestion( + db.span_help( self.span, - "consider using", - help, - Applicability::MachineApplicable, // snippet + &help, ); } }); diff --git a/tests/ui/entry.stderr b/tests/ui/entry.stderr index efacec1e777..d17456ef852 100644 --- a/tests/ui/entry.stderr +++ b/tests/ui/entry.stderr @@ -15,7 +15,16 @@ LL | / if !m.contains_key(&k) { LL | | foo(); LL | | m.insert(k, v); LL | | } - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:16:5 + | +LL | / if !m.contains_key(&k) { +LL | | foo(); +LL | | m.insert(k, v); +LL | | } + | |_____^ error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry.rs:23:5 @@ -25,7 +34,17 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:23:5 + | +LL | / if !m.contains_key(&k) { +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry.rs:31:5 @@ -35,7 +54,17 @@ LL | | None LL | | } else { LL | | m.insert(k, v) LL | | }; - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:31:5 + | +LL | / if m.contains_key(&k) { +LL | | None +LL | | } else { +LL | | m.insert(k, v) +LL | | }; + | |_____^ error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry.rs:39:5 @@ -46,7 +75,18 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:39:5 + | +LL | / if !m.contains_key(&k) { +LL | | foo(); +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry.rs:48:5 @@ -57,7 +97,18 @@ LL | | } else { LL | | foo(); LL | | m.insert(k, v) LL | | }; - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:48:5 + | +LL | / if m.contains_key(&k) { +LL | | None +LL | | } else { +LL | | foo(); +LL | | m.insert(k, v) +LL | | }; + | |_____^ error: usage of `contains_key` followed by `insert` on a `BTreeMap` --> $DIR/entry.rs:57:5 @@ -68,7 +119,18 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:57:5 + | +LL | / if !m.contains_key(&k) { +LL | | foo(); +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ error: aborting due to 7 previous errors