Auto merge of rust-lang#6820 - mgacek8:issue_6562_enhance_mem_replace…

…_with_default_with_other_ctors, r=phansch mem_replace_with_default: recognize some std library ctors fixes rust-lang#6562 changelog: mem_replace_with_default: recognize some common constructors equivalent to `Default::default()`
flip1995 · Mar 13, 2021 · 92b9677 · 92b9677
2 parents 28759b2 + 41be515
commit 92b9677
Show file tree

Hide file tree

Showing 5 changed files with 166 additions and 8 deletions.
diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs
@@ -4,6 +4,7 @@ use crate::utils::{
 };
 use if_chain::if_chain;
 use rustc_errors::Applicability;
+use rustc_hir::def_id::DefId;
 use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, QPath};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
@@ -12,6 +13,8 @@ use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::Span;
 use rustc_span::symbol::sym;
 
+use clippy_utils::is_diagnostic_assoc_item;
+
 declare_clippy_lint! {
     /// **What it does:** Checks for `mem::replace()` on an `Option` with
     /// `None`.
@@ -194,13 +197,44 @@ fn check_replace_with_uninit(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'
     }
 }
 
+/// Returns true if the `def_id` associated with the `path` is recognized as a "default-equivalent"
+/// constructor from the std library
+fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath<'_>) -> bool {
+    let std_types_symbols = &[
+        sym::string_type,
+        sym::vec_type,
+        sym::vecdeque_type,
+        sym::LinkedList,
+        sym::hashmap_type,
+        sym::BTreeMap,
+        sym::hashset_type,
+        sym::BTreeSet,
+        sym::BinaryHeap,
+    ];
+
+    if std_types_symbols
+        .iter()
+        .any(|symbol| is_diagnostic_assoc_item(cx, def_id, *symbol))
+    {
+        if let QPath::TypeRelative(_, ref method) = path {
+            if method.ident.name == sym::new {
+                return true;
+            }
+        }
+    }
+
+    false
+}
+
 fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) {
     if let ExprKind::Call(ref repl_func, _) = src.kind {
         if_chain! {
             if !in_external_macro(cx.tcx.sess, expr_span);
             if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind;
             if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
-            if match_def_path(cx, repl_def_id, &paths::DEFAULT_TRAIT_METHOD);
+            if is_diagnostic_assoc_item(cx, repl_def_id, sym::Default)
+                || is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath);
+
             then {
                 span_lint_and_then(
                     cx,

diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
@@ -280,7 +280,7 @@ pub fn match_trait_method(cx: &LateContext<'_>, expr: &Expr<'_>, path: &[&str])
     trt_id.map_or(false, |trt_id| match_def_path(cx, trt_id, path))
 }
 
-/// Checks if the method call given in `expr` belongs to a trait or other container with a given
+/// Checks if the method call given in `def_id` belongs to a trait or other container with a given
 /// diagnostic item
 pub fn is_diagnostic_assoc_item(cx: &LateContext<'_>, def_id: DefId, diag_item: Symbol) -> bool {
     cx.tcx

diff --git a/tests/ui/mem_replace.fixed b/tests/ui/mem_replace.fixed
@@ -7,6 +7,7 @@
     clippy::mem_replace_with_default
 )]
 
+use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList, VecDeque};
 use std::mem;
 
 fn replace_option_with_none() {
@@ -19,9 +20,37 @@ fn replace_option_with_none() {
 fn replace_with_default() {
     let mut s = String::from("foo");
     let _ = std::mem::take(&mut s);
+
     let s = &mut String::from("foo");
     let _ = std::mem::take(s);
     let _ = std::mem::take(s);
+
+    let mut v = vec![123];
+    let _ = std::mem::take(&mut v);
+    let _ = std::mem::take(&mut v);
+    let _ = std::mem::take(&mut v);
+    let _ = std::mem::take(&mut v);
+
+    let mut hash_map: HashMap<i32, i32> = HashMap::new();
+    let _ = std::mem::take(&mut hash_map);
+
+    let mut btree_map: BTreeMap<i32, i32> = BTreeMap::new();
+    let _ = std::mem::take(&mut btree_map);
+
+    let mut vd: VecDeque<i32> = VecDeque::new();
+    let _ = std::mem::take(&mut vd);
+
+    let mut hash_set: HashSet<&str> = HashSet::new();
+    let _ = std::mem::take(&mut hash_set);
+
+    let mut btree_set: BTreeSet<&str> = BTreeSet::new();
+    let _ = std::mem::take(&mut btree_set);
+
+    let mut list: LinkedList<i32> = LinkedList::new();
+    let _ = std::mem::take(&mut list);
+
+    let mut binary_heap: BinaryHeap<i32> = BinaryHeap::new();
+    let _ = std::mem::take(&mut binary_heap);
 }
 
 fn main() {

diff --git a/tests/ui/mem_replace.rs b/tests/ui/mem_replace.rs
@@ -7,6 +7,7 @@
     clippy::mem_replace_with_default
 )]
 
+use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList, VecDeque};
 use std::mem;
 
 fn replace_option_with_none() {
@@ -19,9 +20,37 @@ fn replace_option_with_none() {
 fn replace_with_default() {
     let mut s = String::from("foo");
     let _ = std::mem::replace(&mut s, String::default());
+
     let s = &mut String::from("foo");
     let _ = std::mem::replace(s, String::default());
     let _ = std::mem::replace(s, Default::default());
+
+    let mut v = vec![123];
+    let _ = std::mem::replace(&mut v, Vec::default());
+    let _ = std::mem::replace(&mut v, Default::default());
+    let _ = std::mem::replace(&mut v, Vec::new());
+    let _ = std::mem::replace(&mut v, vec![]);
+
+    let mut hash_map: HashMap<i32, i32> = HashMap::new();
+    let _ = std::mem::replace(&mut hash_map, HashMap::new());
+
+    let mut btree_map: BTreeMap<i32, i32> = BTreeMap::new();
+    let _ = std::mem::replace(&mut btree_map, BTreeMap::new());
+
+    let mut vd: VecDeque<i32> = VecDeque::new();
+    let _ = std::mem::replace(&mut vd, VecDeque::new());
+
+    let mut hash_set: HashSet<&str> = HashSet::new();
+    let _ = std::mem::replace(&mut hash_set, HashSet::new());
+
+    let mut btree_set: BTreeSet<&str> = BTreeSet::new();
+    let _ = std::mem::replace(&mut btree_set, BTreeSet::new());
+
+    let mut list: LinkedList<i32> = LinkedList::new();
+    let _ = std::mem::replace(&mut list, LinkedList::new());
+
+    let mut binary_heap: BinaryHeap<i32> = BinaryHeap::new();
+    let _ = std::mem::replace(&mut binary_heap, BinaryHeap::new());
 }
 
 fn main() {

diff --git a/tests/ui/mem_replace.stderr b/tests/ui/mem_replace.stderr
@@ -1,36 +1,102 @@
 error: replacing an `Option` with `None`
-  --> $DIR/mem_replace.rs:14:13
+  --> $DIR/mem_replace.rs:15:13
    |
 LL |     let _ = mem::replace(&mut an_option, None);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `an_option.take()`
    |
    = note: `-D clippy::mem-replace-option-with-none` implied by `-D warnings`
 
 error: replacing an `Option` with `None`
-  --> $DIR/mem_replace.rs:16:13
+  --> $DIR/mem_replace.rs:17:13
    |
 LL |     let _ = mem::replace(an_option, None);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `an_option.take()`
 
 error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> $DIR/mem_replace.rs:21:13
+  --> $DIR/mem_replace.rs:22:13
    |
 LL |     let _ = std::mem::replace(&mut s, String::default());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut s)`
    |
    = note: `-D clippy::mem-replace-with-default` implied by `-D warnings`
 
 error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> $DIR/mem_replace.rs:23:13
+  --> $DIR/mem_replace.rs:25:13
    |
 LL |     let _ = std::mem::replace(s, String::default());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(s)`
 
 error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> $DIR/mem_replace.rs:24:13
+  --> $DIR/mem_replace.rs:26:13
    |
 LL |     let _ = std::mem::replace(s, Default::default());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(s)`
 
-error: aborting due to 5 previous errors
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace.rs:29:13
+   |
+LL |     let _ = std::mem::replace(&mut v, Vec::default());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut v)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace.rs:30:13
+   |
+LL |     let _ = std::mem::replace(&mut v, Default::default());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut v)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace.rs:31:13
+   |
+LL |     let _ = std::mem::replace(&mut v, Vec::new());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut v)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace.rs:32:13
+   |
+LL |     let _ = std::mem::replace(&mut v, vec![]);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut v)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace.rs:35:13
+   |
+LL |     let _ = std::mem::replace(&mut hash_map, HashMap::new());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut hash_map)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace.rs:38:13
+   |
+LL |     let _ = std::mem::replace(&mut btree_map, BTreeMap::new());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut btree_map)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace.rs:41:13
+   |
+LL |     let _ = std::mem::replace(&mut vd, VecDeque::new());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut vd)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace.rs:44:13
+   |
+LL |     let _ = std::mem::replace(&mut hash_set, HashSet::new());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut hash_set)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace.rs:47:13
+   |
+LL |     let _ = std::mem::replace(&mut btree_set, BTreeSet::new());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut btree_set)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace.rs:50:13
+   |
+LL |     let _ = std::mem::replace(&mut list, LinkedList::new());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut list)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> $DIR/mem_replace.rs:53:13
+   |
+LL |     let _ = std::mem::replace(&mut binary_heap, BinaryHeap::new());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut binary_heap)`
+
+error: aborting due to 16 previous errors