Add entry_or_insert_with_default lint

Author: cauebs

CHANGELOG.md

@@ -1694,6 +1694,7 @@ Released 2018-09-13
 [`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum
 [`empty_line_after_outer_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_outer_attr
 [`empty_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_loop
+[`entry_or_insert_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#entry_or_insert_with_default
 [`enum_clike_unportable_variant`]: https://rust-lang.github.io/rust-clippy/master/index.html#enum_clike_unportable_variant
 [`enum_glob_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#enum_glob_use
 [`enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names

clippy_lints/src/lib.rs

@@ -679,6 +679,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &methods::CLONE_DOUBLE_REF,
         &methods::CLONE_ON_COPY,
         &methods::CLONE_ON_REF_PTR,
+        &methods::ENTRY_OR_INSERT_WITH_DEFAULT,
         &methods::EXPECT_FUN_CALL,
         &methods::EXPECT_USED,
         &methods::FILETYPE_IS_FILE,
@@ -1409,6 +1410,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::CHARS_NEXT_CMP),
         LintId::of(&methods::CLONE_DOUBLE_REF),
         LintId::of(&methods::CLONE_ON_COPY),
+        LintId::of(&methods::ENTRY_OR_INSERT_WITH_DEFAULT),
         LintId::of(&methods::EXPECT_FUN_CALL),
         LintId::of(&methods::FILTER_NEXT),
         LintId::of(&methods::FLAT_MAP_IDENTITY),
@@ -1609,6 +1611,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&mem_replace::MEM_REPLACE_WITH_DEFAULT),
         LintId::of(&methods::CHARS_LAST_CMP),
         LintId::of(&methods::CHARS_NEXT_CMP),
+        LintId::of(&methods::ENTRY_OR_INSERT_WITH_DEFAULT),
         LintId::of(&methods::INTO_ITER_ON_REF),
         LintId::of(&methods::ITER_CLONED_COLLECT),
         LintId::of(&methods::ITER_NEXT_SLICE),

clippy_lints/src/methods/mod.rs

@@ -563,6 +563,46 @@ declare_clippy_lint! {
     "using `.chars().next()` to check if a string starts with a char"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for calls to `.or_insert_with` on `Entry`
+    /// (for `BTreeMap` or `HashMap`) with `Default::default` or equivalent
+    /// as the argument, and suggests `.or_default`.
+    ///
+    /// **Why is this bad?** There is a specific method for this, and using it can make
+    /// the code cleaner.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// use std::collections::{BTreeMap, HashMap};
+    ///
+    /// fn btree(map: &mut BTreeMap<i32, i32>) -> i32 {
+    ///     map.entry(42).or_insert_with(Default::default)
+    /// }
+    ///
+    /// fn hash(map: &mut HashMap<i32, i32>) -> i32 {
+    ///     map.entry(42).or_insert_with(i32::default)
+    /// }
+    /// ```
+    /// both can be instead written as
+    /// ```rust
+    /// use std::collections::{BTreeMap, HashMap};
+    ///
+    /// fn btree(map: &mut BTreeMap<i32, i32>) -> i32 {
+    ///     map.entry(42).or_default()
+    /// }
+    ///
+    /// fn hash(map: &mut HashMap<i32, i32>) -> i32 {
+    ///     map.entry(42).or_default()
+    /// }
+    /// ```
+    pub ENTRY_OR_INSERT_WITH_DEFAULT,
+    style,
+    "calling `or_insert_with` on an `Entry` with `Default::default`"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for calls to `.or(foo(..))`, `.unwrap_or(foo(..))`,
     /// etc., and suggests to use `or_else`, `unwrap_or_else`, etc., or
@@ -1394,6 +1434,7 @@ declare_lint_pass!(Methods => [
     RESULT_MAP_OR_INTO_OPTION,
     OPTION_MAP_OR_NONE,
     BIND_INSTEAD_OF_MAP,
+    ENTRY_OR_INSERT_WITH_DEFAULT,
     OR_FUN_CALL,
     EXPECT_FUN_CALL,
     CHARS_NEXT_CMP,
@@ -1515,6 +1556,9 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
             ["unwrap_or_else", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "unwrap_or"),
             ["get_or_insert_with", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "get_or_insert"),
             ["ok_or_else", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "ok_or"),
+            ["or_insert_with", ..] => {
+                lint_entry_or_insert_with_default(cx, method_spans[0].with_hi(expr.span.hi()), arg_lists[0])
+            },
             _ => {},
         }
 
@@ -1709,6 +1753,28 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
     }
 }
 
+/// Checks for the `ENTRY_OR_INSERT_WITH_DEFAULT` lint.
+fn lint_entry_or_insert_with_default(cx: &LateContext<'tcx>, span: Span, args: &'tcx [hir::Expr<'_>]) {
+    if_chain! {
+        let ty = cx.typeck_results().expr_ty(&args[0]);
+        if match_type(cx, ty, &paths::BTREEMAP_ENTRY) || match_type(cx, ty, &paths::HASHMAP_ENTRY);
+        if let hir::ExprKind::Path(arg_path) = &args[1].kind;
+        if let hir::def::Res::Def(_, arg_def_id) = cx.qpath_res(&arg_path, args[1].hir_id);
+        if match_def_path(cx, arg_def_id, &paths::DEFAULT_TRAIT_METHOD);
+        then {
+            span_lint_and_sugg(
+                cx,
+                ENTRY_OR_INSERT_WITH_DEFAULT,
+                span,
+                "use of `or_insert_with` with `Default::default`",
+                "replace with",
+                "or_default()".to_owned(),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+}
+
 /// Checks for the `OR_FUN_CALL` lint.
 #[allow(clippy::too_many_lines)]
 fn lint_or_fun_call<'tcx>(

src/lintlist/mod.rs

@@ -494,6 +494,13 @@ vec![
         deprecation: None,
         module: "loops",
     },
+    Lint {
+        name: "entry_or_insert_with_default",
+        group: "style",
+        desc: "calling `or_insert_with` on an `Entry` with `Default::default`",
+        deprecation: None,
+        module: "methods",
+    },
     Lint {
         name: "enum_clike_unportable_variant",
         group: "correctness",

tests/ui/entry_or_insert_with_default.rs

@@ -0,0 +1,43 @@
+#![warn(clippy::entry_or_insert_with_default)]
+
+use std::collections::{BTreeMap, HashMap};
+
+fn btree(map: &mut BTreeMap<i32, i32>) {
+    map.entry(42).or_insert_with(Default::default);
+    map.entry(42).or_insert_with(i32::default);
+    map.entry(42).or_default();
+}
+
+fn hash(map: &mut HashMap<i32, i32>) {
+    map.entry(42).or_insert_with(Default::default);
+    map.entry(42).or_insert_with(i32::default);
+    map.entry(42).or_default();
+}
+
+struct InformalDefault;
+
+impl InformalDefault {
+    fn default() -> Self {
+        InformalDefault
+    }
+}
+
+fn not_default_btree(map: &mut BTreeMap<i32, InformalDefault>) {
+    map.entry(42).or_insert_with(InformalDefault::default);
+}
+
+fn not_default_hash(map: &mut HashMap<i32, InformalDefault>) {
+    map.entry(42).or_insert_with(InformalDefault::default);
+}
+
+#[derive(Default)]
+struct NotAnEntry;
+
+impl NotAnEntry {
+    fn or_insert_with(self, f: impl Fn() -> Self) {}
+}
+
+fn main() {
+    NotAnEntry.or_insert_with(Default::default);
+    NotAnEntry.or_insert_with(NotAnEntry::default);
+}

tests/ui/entry_or_insert_with_default.stderr

@@ -0,0 +1,28 @@
+error: use of `or_insert_with` with `Default::default`
+  --> $DIR/entry_or_insert_with_default.rs:6:19
+   |
+LL |     map.entry(42).or_insert_with(Default::default);
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `or_default()`
+   |
+   = note: `-D clippy::entry-or-insert-with-default` implied by `-D warnings`
+
+error: use of `or_insert_with` with `Default::default`
+  --> $DIR/entry_or_insert_with_default.rs:7:19
+   |
+LL |     map.entry(42).or_insert_with(i32::default);
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `or_default()`
+
+error: use of `or_insert_with` with `Default::default`
+  --> $DIR/entry_or_insert_with_default.rs:12:19
+   |
+LL |     map.entry(42).or_insert_with(Default::default);
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `or_default()`
+
+error: use of `or_insert_with` with `Default::default`
+  --> $DIR/entry_or_insert_with_default.rs:13:19
+   |
+LL |     map.entry(42).or_insert_with(i32::default);
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `or_default()`
+
+error: aborting due to 4 previous errors
+