Use MIR body to identify more "default equivalent" calls for derivable_impls (#13988)

When looking for `Default` impls that could be derived, we look at the
body of their `fn default()` and if it is an fn call or literal we check
if they are equivalent to what `#[derive(Default)]` would have used.

Now, when checking those fn calls in the `fn default()` body, we also
compare against the corresponding type's `Default::default` body to see
if our call is equivalent to that one.

For example, given

```rust
struct S;

impl S {
    fn new() -> S { S }
}

impl Default for S {
    fn default() -> S { S::new() }
}
```

`<S as Default>::default()` and `S::new()` are considered equivalent.
Given that, if the user also writes

```rust
struct R {
    s: S,
}

impl Default for R {
    fn default() -> R {
        R { s: S::new() }
    }
}
```

the `derivable_impls` lint will now trigger.

changelog: [`derivable_impls`]: detect when a `Default` impl is using
the same fn call that that type's `Default::default` calls
changelog: [`mem_replace_with_default`]: detect when `std::mem::replace`
is being called with the same fn that that type's `Default::default`
calls, without the need of a manually maintained list

clippy_lints/src/methods/or_fun_call.rs

@@ -104,7 +104,7 @@ pub(super) fn check<'tcx>(
         if (is_new(fun) && output_type_implements_default(fun))
             || match call_expr {
                 Some(call_expr) => is_default_equivalent(cx, call_expr),
-                None => is_default_equivalent_call(cx, fun) || closure_body_returns_empty_to_string(cx, fun),
+                None => is_default_equivalent_call(cx, fun, None) || closure_body_returns_empty_to_string(cx, fun),
             }
         {
             span_lint_and_sugg(

clippy_utils/src/lib.rs

@@ -113,6 +113,7 @@ use rustc_hir::{
 use rustc_lexer::{TokenKind, tokenize};
 use rustc_lint::{LateContext, Level, Lint, LintContext};
 use rustc_middle::hir::place::PlaceBase;
+use rustc_middle::mir::{AggregateKind, Operand, RETURN_PLACE, Rvalue, StatementKind, TerminatorKind};
 use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
 use rustc_middle::ty::fast_reject::SimplifiedType;
 use rustc_middle::ty::layout::IntegerExt;
@@ -919,22 +920,101 @@ fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath<
 }
 
 /// Returns true if the expr is equal to `Default::default` when evaluated.
-pub fn is_default_equivalent_call(cx: &LateContext<'_>, repl_func: &Expr<'_>) -> bool {
+pub fn is_default_equivalent_call(
+    cx: &LateContext<'_>,
+    repl_func: &Expr<'_>,
+    whole_call_expr: Option<&Expr<'_>>,
+) -> bool {
     if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind
         && let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id()
         && (is_diag_trait_item(cx, repl_def_id, sym::Default)
             || is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath))
     {
-        true
-    } else {
-        false
+        return true;
     }
+
+    // Get the type of the whole method call expression, find the exact method definition, look at
+    // its body and check if it is similar to the corresponding `Default::default()` body.
+    let Some(e) = whole_call_expr else { return false };
+    let Some(default_fn_def_id) = cx.tcx.get_diagnostic_item(sym::default_fn) else {
+        return false;
+    };
+    let Some(ty) = cx.tcx.typeck(e.hir_id.owner.def_id).expr_ty_adjusted_opt(e) else {
+        return false;
+    };
+    let args = rustc_ty::GenericArgs::for_item(cx.tcx, default_fn_def_id, |param, _| {
+        if let rustc_ty::GenericParamDefKind::Lifetime = param.kind {
+            cx.tcx.lifetimes.re_erased.into()
+        } else if param.index == 0 && param.name == kw::SelfUpper {
+            ty.into()
+        } else {
+            param.to_error(cx.tcx)
+        }
+    });
+    let instance = rustc_ty::Instance::try_resolve(cx.tcx, cx.typing_env(), default_fn_def_id, args);
+
+    let Ok(Some(instance)) = instance else { return false };
+    if let rustc_ty::InstanceKind::Item(def) = instance.def
+        && !cx.tcx.is_mir_available(def)
+    {
+        return false;
+    }
+    let ExprKind::Path(ref repl_func_qpath) = repl_func.kind else {
+        return false;
+    };
+    let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id() else {
+        return false;
+    };
+
+    // Get the MIR Body for the `<Ty as Default>::default()` function.
+    // If it is a value or call (either fn or ctor), we compare its `DefId` against the one for the
+    // resolution of the expression we had in the path. This lets us identify, for example, that
+    // the body of `<Vec<T> as Default>::default()` is a `Vec::new()`, and the field was being
+    // initialized to `Vec::new()` as well.
+    let body = cx.tcx.instance_mir(instance.def);
+    for block_data in body.basic_blocks.iter() {
+        if block_data.statements.len() == 1
+            && let StatementKind::Assign(assign) = &block_data.statements[0].kind
+            && assign.0.local == RETURN_PLACE
+            && let Rvalue::Aggregate(kind, _places) = &assign.1
+            && let AggregateKind::Adt(did, variant_index, _, _, _) = &**kind
+            && let def = cx.tcx.adt_def(did)
+            && let variant = &def.variant(*variant_index)
+            && variant.fields.is_empty()
+            && let Some((_, did)) = variant.ctor
+            && did == repl_def_id
+        {
+            return true;
+        } else if block_data.statements.is_empty()
+            && let Some(term) = &block_data.terminator
+        {
+            match &term.kind {
+                TerminatorKind::Call {
+                    func: Operand::Constant(c),
+                    ..
+                } if let rustc_ty::FnDef(did, _args) = c.ty().kind()
+                    && *did == repl_def_id =>
+                {
+                    return true;
+                },
+                TerminatorKind::TailCall {
+                    func: Operand::Constant(c),
+                    ..
+                } if let rustc_ty::FnDef(did, _args) = c.ty().kind()
+                    && *did == repl_def_id =>
+                {
+                    return true;
+                },
+                _ => {},
+            }
+        }
+    }
+    false
 }
 
-/// Returns true if the expr is equal to `Default::default()` of it's type when evaluated.
+/// Returns true if the expr is equal to `Default::default()` of its type when evaluated.
 ///
-/// It doesn't cover all cases, for example indirect function calls (some of std
-/// functions are supported) but it is the best we have.
+/// It doesn't cover all cases, like struct literals, but it is a close approximation.
 pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
     match &e.kind {
         ExprKind::Lit(lit) => match lit.node {
@@ -955,7 +1035,7 @@ pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
                 false
             }
         },
-        ExprKind::Call(repl_func, []) => is_default_equivalent_call(cx, repl_func),
+        ExprKind::Call(repl_func, []) => is_default_equivalent_call(cx, repl_func, Some(e)),
         ExprKind::Call(from_func, [arg]) => is_default_equivalent_from(cx, from_func, arg),
         ExprKind::Path(qpath) => is_res_lang_ctor(cx, cx.qpath_res(qpath, e.hir_id), OptionNone),
         ExprKind::AddrOf(rustc_hir::BorrowKind::Ref, _, expr) => matches!(expr.kind, ExprKind::Array([])),

tests/ui/derivable_impls.fixed

@@ -144,6 +144,40 @@ impl Default for SpecializedImpl2<String> {
     }
 }
 
+#[derive(Default)]
+pub struct DirectDefaultDefaultCall {
+    v: Vec<i32>,
+}
+
+
+#[derive(Default)]
+pub struct EquivalentToDefaultDefaultCallVec {
+    v: Vec<i32>,
+}
+
+
+pub struct S {
+    x: i32,
+}
+
+impl S {
+    fn new() -> S {
+        S { x: 42 }
+    }
+}
+
+impl Default for S {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+#[derive(Default)]
+pub struct EquivalentToDefaultDefaultCallLocal {
+    v: S,
+}
+
+
 // https://github.com/rust-lang/rust-clippy/issues/7654
 
 pub struct Color {

tests/ui/derivable_impls.rs

@@ -181,6 +181,55 @@ impl Default for SpecializedImpl2<String> {
     }
 }
 
+pub struct DirectDefaultDefaultCall {
+    v: Vec<i32>,
+}
+
+impl Default for DirectDefaultDefaultCall {
+    fn default() -> Self {
+        // When calling `Default::default()` in all fields, we know it is the same as deriving.
+        Self { v: Default::default() }
+    }
+}
+
+pub struct EquivalentToDefaultDefaultCallVec {
+    v: Vec<i32>,
+}
+
+impl Default for EquivalentToDefaultDefaultCallVec {
+    fn default() -> Self {
+        // The body of `<Vec as Default>::default()` is `Vec::new()`, so they are equivalent.
+        Self { v: Vec::new() }
+    }
+}
+
+pub struct S {
+    x: i32,
+}
+
+impl S {
+    fn new() -> S {
+        S { x: 42 }
+    }
+}
+
+impl Default for S {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+pub struct EquivalentToDefaultDefaultCallLocal {
+    v: S,
+}
+
+impl Default for EquivalentToDefaultDefaultCallLocal {
+    fn default() -> Self {
+        // The body of `<S as Default>::default()` is `S::new()`, so they are equivalent.
+        Self { v: S::new() }
+    }
+}
+
 // https://github.com/rust-lang/rust-clippy/issues/7654
 
 pub struct Color {

tests/ui/derivable_impls.stderr

@@ -98,7 +98,58 @@ LL ~ struct WithoutSelfParan(bool);
    |
 
 error: this `impl` can be derived
-  --> tests/ui/derivable_impls.rs:216:1
+  --> tests/ui/derivable_impls.rs:188:1
+   |
+LL | / impl Default for DirectDefaultDefaultCall {
+LL | |     fn default() -> Self {
+LL | |         // When calling `Default::default()` in all fields, we know it is the same as deriving.
+LL | |         Self { v: Default::default() }
+LL | |     }
+LL | | }
+   | |_^
+   |
+help: replace the manual implementation with a derive attribute
+   |
+LL + #[derive(Default)]
+LL ~ pub struct DirectDefaultDefaultCall {
+   |
+
+error: this `impl` can be derived
+  --> tests/ui/derivable_impls.rs:199:1
+   |
+LL | / impl Default for EquivalentToDefaultDefaultCallVec {
+LL | |     fn default() -> Self {
+LL | |         // The body of `<Vec as Default>::default()` is `Vec::new()`, so they are equivalent.
+LL | |         Self { v: Vec::new() }
+LL | |     }
+LL | | }
+   | |_^
+   |
+help: replace the manual implementation with a derive attribute
+   |
+LL + #[derive(Default)]
+LL ~ pub struct EquivalentToDefaultDefaultCallVec {
+   |
+
+error: this `impl` can be derived
+  --> tests/ui/derivable_impls.rs:226:1
+   |
+LL | / impl Default for EquivalentToDefaultDefaultCallLocal {
+LL | |     fn default() -> Self {
+LL | |         // The body of `<S as Default>::default()` is `S::new()`, so they are equivalent.
+LL | |         Self { v: S::new() }
+LL | |     }
+LL | | }
+   | |_^
+   |
+help: replace the manual implementation with a derive attribute
+   |
+LL + #[derive(Default)]
+LL ~ pub struct EquivalentToDefaultDefaultCallLocal {
+   |
+
+error: this `impl` can be derived
+  --> tests/ui/derivable_impls.rs:265:1
    |
 LL | / impl Default for RepeatDefault1 {
 LL | |     fn default() -> Self {
@@ -114,7 +165,7 @@ LL ~ pub struct RepeatDefault1 {
    |
 
 error: this `impl` can be derived
-  --> tests/ui/derivable_impls.rs:250:1
+  --> tests/ui/derivable_impls.rs:299:1
    |
 LL | / impl Default for SimpleEnum {
 LL | |     fn default() -> Self {
@@ -132,5 +183,5 @@ LL ~     #[default]
 LL ~     Bar,
    |
 
-error: aborting due to 8 previous errors
+error: aborting due to 11 previous errors
 

tests/ui/mem_replace.fixed

@@ -18,10 +18,12 @@ fn replace_option_with_none() {
 fn replace_with_default() {
     let mut s = String::from("foo");
     let _ = std::mem::take(&mut s);
+    let _ = std::mem::take(&mut s);
 
     let s = &mut String::from("foo");
     let _ = std::mem::take(s);
     let _ = std::mem::take(s);
+    let _ = std::mem::take(s);
 
     let mut v = vec![123];
     let _ = std::mem::take(&mut v);

tests/ui/mem_replace.rs

@@ -18,9 +18,11 @@ fn replace_option_with_none() {
 fn replace_with_default() {
     let mut s = String::from("foo");
     let _ = std::mem::replace(&mut s, String::default());
+    let _ = std::mem::replace(&mut s, String::new());
 
     let s = &mut String::from("foo");
     let _ = std::mem::replace(s, String::default());
+    let _ = std::mem::replace(s, String::new());
     let _ = std::mem::replace(s, Default::default());
 
     let mut v = vec![123];

tests/ui/mem_replace.stderr

@@ -23,130 +23,142 @@ LL |     let _ = std::mem::replace(&mut s, String::default());
    = help: to override `-D warnings` add `#[allow(clippy::mem_replace_with_default)]`
 
 error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> tests/ui/mem_replace.rs:23:13
+  --> tests/ui/mem_replace.rs:21:13
+   |
+LL |     let _ = std::mem::replace(&mut s, String::new());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut s)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> tests/ui/mem_replace.rs:24: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`
-  --> tests/ui/mem_replace.rs:24:13
+  --> tests/ui/mem_replace.rs:25:13
+   |
+LL |     let _ = std::mem::replace(s, String::new());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(s)`
+
+error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
+  --> tests/ui/mem_replace.rs:26:13
    |
 LL |     let _ = std::mem::replace(s, Default::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`
-  --> tests/ui/mem_replace.rs:27:13
+  --> tests/ui/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`
-  --> tests/ui/mem_replace.rs:28:13
+  --> tests/ui/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`
-  --> tests/ui/mem_replace.rs:29:13
+  --> tests/ui/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`
-  --> tests/ui/mem_replace.rs:30:13
+  --> tests/ui/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`
-  --> tests/ui/mem_replace.rs:33:13
+  --> tests/ui/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`
-  --> tests/ui/mem_replace.rs:36:13
+  --> tests/ui/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`
-  --> tests/ui/mem_replace.rs:39:13
+  --> tests/ui/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`
-  --> tests/ui/mem_replace.rs:42:13
+  --> tests/ui/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`
-  --> tests/ui/mem_replace.rs:45:13
+  --> tests/ui/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`
-  --> tests/ui/mem_replace.rs:48:13
+  --> tests/ui/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`
-  --> tests/ui/mem_replace.rs:51:13
+  --> tests/ui/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: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> tests/ui/mem_replace.rs:54:13
+  --> tests/ui/mem_replace.rs:56:13
    |
 LL |     let _ = std::mem::replace(&mut tuple, (vec![], BinaryHeap::new()));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut tuple)`
 
 error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> tests/ui/mem_replace.rs:57:13
+  --> tests/ui/mem_replace.rs:59:13
    |
 LL |     let _ = std::mem::replace(&mut refstr, "");
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut refstr)`
 
 error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> tests/ui/mem_replace.rs:60:13
+  --> tests/ui/mem_replace.rs:62:13
    |
 LL |     let _ = std::mem::replace(&mut slice, &[]);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut slice)`
 
 error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> tests/ui/mem_replace.rs:96:13
+  --> tests/ui/mem_replace.rs:98:13
    |
 LL |     let _ = std::mem::replace(&mut s, String::default());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut s)`
 
 error: replacing an `Option` with `None`
-  --> tests/ui/mem_replace.rs:126:13
+  --> tests/ui/mem_replace.rs:128:13
    |
 LL |     let _ = std::mem::replace(&mut f.0, None);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `f.0.take()`
 
 error: replacing an `Option` with `None`
-  --> tests/ui/mem_replace.rs:127:13
+  --> tests/ui/mem_replace.rs:129:13
    |
 LL |     let _ = std::mem::replace(&mut *f, None);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `(*f).take()`
 
 error: replacing an `Option` with `None`
-  --> tests/ui/mem_replace.rs:128:13
+  --> tests/ui/mem_replace.rs:130:13
    |
 LL |     let _ = std::mem::replace(&mut b.opt, None);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Option::take()` instead: `b.opt.take()`
 
 error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`
-  --> tests/ui/mem_replace.rs:130:13
+  --> tests/ui/mem_replace.rs:132:13
    |
 LL |     let _ = std::mem::replace(&mut b.val, String::default());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut b.val)`
 
-error: aborting due to 24 previous errors
+error: aborting due to 26 previous errors