From 4368771548782d0c28748353e65636782ff0b25e Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 04:38:17 -0700 Subject: [PATCH] map_entry test: Fix semicolon, add run-rustfix --- clippy_lints/src/entry.rs | 5 +-- tests/ui/entry_fixable.fixed | 15 ++++++++ tests/ui/entry_fixable.rs | 8 ++--- tests/ui/entry_fixable.stderr | 4 +-- tests/ui/entry_unfixable.rs | 7 ++++ tests/ui/entry_unfixable.stderr | 62 +++------------------------------ 6 files changed, 34 insertions(+), 67 deletions(-) create mode 100644 tests/ui/entry_fixable.fixed diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs index a590a8179c2..c0eba516dee 100644 --- a/clippy_lints/src/entry.rs +++ b/clippy_lints/src/entry.rs @@ -65,6 +65,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for HashMapPass { } else { true } + // XXXManishearth we can also check for if/else blocks containing `None`. }; let mut visitor = InsertVisitor { @@ -147,7 +148,7 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> { &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({})", + let help = format!("{}.entry({}).or_insert({});", 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)); @@ -164,7 +165,7 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> { snippet(self.cx, self.map.span, "map"), snippet(self.cx, params[1].span, "..")); - db.span_help( + db.span_label( self.span, &help, ); diff --git a/tests/ui/entry_fixable.fixed b/tests/ui/entry_fixable.fixed new file mode 100644 index 00000000000..dcdaae7e724 --- /dev/null +++ b/tests/ui/entry_fixable.fixed @@ -0,0 +1,15 @@ +// run-rustfix + +#![allow(unused, clippy::needless_pass_by_value)] +#![warn(clippy::map_entry)] + +use std::collections::{BTreeMap, HashMap}; +use std::hash::Hash; + +fn foo() {} + +fn insert_if_absent0(m: &mut HashMap, k: K, v: V) { + m.entry(k).or_insert(v); +} + +fn main() {} diff --git a/tests/ui/entry_fixable.rs b/tests/ui/entry_fixable.rs index 82267cf34a7..55d5b21568d 100644 --- a/tests/ui/entry_fixable.rs +++ b/tests/ui/entry_fixable.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![allow(unused, clippy::needless_pass_by_value)] #![warn(clippy::map_entry)] @@ -12,10 +14,4 @@ fn insert_if_absent0(m: &mut HashMap, k: K, v: V) { } } -fn insert_other_if_absent(m: &mut HashMap, k: K, o: K, v: V) { - if !m.contains_key(&k) { - m.insert(o, v); - } -} - fn main() {} diff --git a/tests/ui/entry_fixable.stderr b/tests/ui/entry_fixable.stderr index 59a20e5c00b..87403200ced 100644 --- a/tests/ui/entry_fixable.stderr +++ b/tests/ui/entry_fixable.stderr @@ -1,10 +1,10 @@ error: usage of `contains_key` followed by `insert` on a `HashMap` - --> $DIR/entry_fixable.rs:10:5 + --> $DIR/entry_fixable.rs:12:5 | LL | / if !m.contains_key(&k) { LL | | m.insert(k, v); LL | | } - | |_____^ help: consider using: `m.entry(k).or_insert(v)` + | |_____^ help: consider using: `m.entry(k).or_insert(v);` | = note: `-D clippy::map-entry` implied by `-D warnings` diff --git a/tests/ui/entry_unfixable.rs b/tests/ui/entry_unfixable.rs index 8859a3a2f9b..f530fc023cf 100644 --- a/tests/ui/entry_unfixable.rs +++ b/tests/ui/entry_unfixable.rs @@ -49,6 +49,13 @@ fn insert_in_btreemap(m: &mut BTreeMap, k: K, v: V) { }; } +// should not trigger +fn insert_other_if_absent(m: &mut HashMap, 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(m: HashMap, n: &mut HashMap, k: K, v: V) { if !m.contains_key(&k) { diff --git a/tests/ui/entry_unfixable.stderr b/tests/ui/entry_unfixable.stderr index ab655fcfead..e58c8d22dc4 100644 --- a/tests/ui/entry_unfixable.stderr +++ b/tests/ui/entry_unfixable.stderr @@ -6,18 +6,9 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ + | |_____^ consider using `m.entry(k)` | = note: `-D clippy::map-entry` implied by `-D warnings` -help: consider using `m.entry(k)` - --> $DIR/entry_unfixable.rs:10: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_unfixable.rs:18:5 @@ -27,17 +18,7 @@ LL | | None LL | | } else { LL | | m.insert(k, v) LL | | }; - | |_____^ - | -help: consider using `m.entry(k)` - --> $DIR/entry_unfixable.rs:18:5 - | -LL | / if m.contains_key(&k) { -LL | | None -LL | | } else { -LL | | m.insert(k, v) -LL | | }; - | |_____^ + | |_____^ consider using `m.entry(k)` error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry_unfixable.rs:26:5 @@ -48,18 +29,7 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ - | -help: consider using `m.entry(k)` - --> $DIR/entry_unfixable.rs:26:5 - | -LL | / if !m.contains_key(&k) { -LL | | foo(); -LL | | m.insert(k, v) -LL | | } else { -LL | | None -LL | | }; - | |_____^ + | |_____^ consider using `m.entry(k)` error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry_unfixable.rs:35:5 @@ -70,18 +40,7 @@ LL | | } else { LL | | foo(); LL | | m.insert(k, v) LL | | }; - | |_____^ - | -help: consider using `m.entry(k)` - --> $DIR/entry_unfixable.rs:35:5 - | -LL | / if m.contains_key(&k) { -LL | | None -LL | | } else { -LL | | foo(); -LL | | m.insert(k, v) -LL | | }; - | |_____^ + | |_____^ consider using `m.entry(k)` error: usage of `contains_key` followed by `insert` on a `BTreeMap` --> $DIR/entry_unfixable.rs:44:5 @@ -92,18 +51,7 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ - | -help: consider using `m.entry(k)` - --> $DIR/entry_unfixable.rs:44:5 - | -LL | / if !m.contains_key(&k) { -LL | | foo(); -LL | | m.insert(k, v) -LL | | } else { -LL | | None -LL | | }; - | |_____^ + | |_____^ consider using `m.entry(k)` error: aborting due to 5 previous errors