Auto merge of #6820 - mgacek8:issue_6562_enhance_mem_replace_with_default_with_other_ctors, r=phansch

mem_replace_with_default: recognize some std library ctors

fixes #6562
changelog: mem_replace_with_default: recognize some common constructors equivalent to `Default::default()`
This commit is contained in:
bors 2021-03-13 15:43:00 +00:00
commit 92b9677864
5 changed files with 166 additions and 8 deletions

View File

@ -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,

View File

@ -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

View File

@ -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() {

View File

@ -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() {

View File

@ -1,5 +1,5 @@
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()`
@ -7,13 +7,13 @@ LL | let _ = mem::replace(&mut an_option, None);
= 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)`
@ -21,16 +21,82 @@ LL | let _ = std::mem::replace(&mut s, String::default());
= 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