Make `derivable_impls` machine applicable

4 files changed, 278 insertions, 19 deletions

diff --git a/clippy_lints/src/derivable_impls.rs b/clippy_lints/src/derivable_impls.rs
index ef9eeecc6a9..06ae5abeaeb 100644
--- a/clippy_lints/src/derivable_impls.rs
+++ b/clippy_lints/src/derivable_impls.rs
@@ -1,5 +1,6 @@
-use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::{is_default_equivalent, peel_blocks};
+use rustc_errors::Applicability;
 use rustc_hir::{
     def::{DefKind, Res},
     Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, TyKind,
@@ -100,15 +101,28 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
                     ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)),
                     _ => false,
                 };
+
                 if should_emit {
-                    let path_string = cx.tcx.def_path_str(adt_def.did());
-                    span_lint_and_help(
+                    let struct_span = cx.tcx.def_span(adt_def.did());
+                    span_lint_and_then(
                         cx,
                         DERIVABLE_IMPLS,
                         item.span,
                         "this `impl` can be derived",
-                        None,
-                        &format!("try annotating `{}` with `#[derive(Default)]`", path_string),
+                        |diag| {
+                            diag.span_suggestion_hidden(
+                                item.span,
+                                "remove the manual implementation...",
+                                String::new(),
+                                Applicability::MachineApplicable
+                            );
+                            diag.span_suggestion(
+                                struct_span.shrink_to_lo(),
+                                "...and instead derive it",
+                                "#[derive(Default)]\n".to_string(),
+                                Applicability::MachineApplicable
+                            );
+                        }
                     );
                 }
             }
diff --git a/tests/ui/derivable_impls.fixed b/tests/ui/derivable_impls.fixed
new file mode 100644
index 00000000000..7dcdfb0937e
--- /dev/null
+++ b/tests/ui/derivable_impls.fixed
@@ -0,0 +1,213 @@
+// run-rustfix
+
+#![allow(dead_code)]
+
+use std::collections::HashMap;
+
+#[derive(Default)]
+struct FooDefault<'a> {
+    a: bool,
+    b: i32,
+    c: u64,
+    d: Vec<i32>,
+    e: FooND1,
+    f: FooND2,
+    g: HashMap<i32, i32>,
+    h: (i32, Vec<i32>),
+    i: [Vec<i32>; 3],
+    j: [i32; 5],
+    k: Option<i32>,
+    l: &'a [i32],
+}
+
+
+
+#[derive(Default)]
+struct TupleDefault(bool, i32, u64);
+
+
+
+struct FooND1 {
+    a: bool,
+}
+
+impl std::default::Default for FooND1 {
+    fn default() -> Self {
+        Self { a: true }
+    }
+}
+
+struct FooND2 {
+    a: i32,
+}
+
+impl std::default::Default for FooND2 {
+    fn default() -> Self {
+        Self { a: 5 }
+    }
+}
+
+struct FooNDNew {
+    a: bool,
+}
+
+impl FooNDNew {
+    fn new() -> Self {
+        Self { a: true }
+    }
+}
+
+impl Default for FooNDNew {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+struct FooNDVec(Vec<i32>);
+
+impl Default for FooNDVec {
+    fn default() -> Self {
+        Self(vec![5, 12])
+    }
+}
+
+#[derive(Default)]
+struct StrDefault<'a>(&'a str);
+
+
+
+#[derive(Default)]
+struct AlreadyDerived(i32, bool);
+
+macro_rules! mac {
+    () => {
+        0
+    };
+    ($e:expr) => {
+        struct X(u32);
+        impl Default for X {
+            fn default() -> Self {
+                Self($e)
+            }
+        }
+    };
+}
+
+mac!(0);
+
+#[derive(Default)]
+struct Y(u32);
+
+
+struct RustIssue26925<T> {
+    a: Option<T>,
+}
+
+// We should watch out for cases where a manual impl is needed because a
+// derive adds different type bounds (https://github.com/rust-lang/rust/issues/26925).
+// For example, a struct with Option<T> does not require T: Default, but a derive adds
+// that type bound anyways. So until #26925 get fixed we should disable lint
+// for the following case
+impl<T> Default for RustIssue26925<T> {
+    fn default() -> Self {
+        Self { a: None }
+    }
+}
+
+struct SpecializedImpl<A, B> {
+    a: A,
+    b: B,
+}
+
+impl<T: Default> Default for SpecializedImpl<T, T> {
+    fn default() -> Self {
+        Self {
+            a: T::default(),
+            b: T::default(),
+        }
+    }
+}
+
+#[derive(Default)]
+struct WithoutSelfCurly {
+    a: bool,
+}
+
+
+
+#[derive(Default)]
+struct WithoutSelfParan(bool);
+
+
+
+// https://github.com/rust-lang/rust-clippy/issues/7655
+
+pub struct SpecializedImpl2<T> {
+    v: Vec<T>,
+}
+
+impl Default for SpecializedImpl2<String> {
+    fn default() -> Self {
+        Self { v: Vec::new() }
+    }
+}
+
+// https://github.com/rust-lang/rust-clippy/issues/7654
+
+pub struct Color {
+    pub r: u8,
+    pub g: u8,
+    pub b: u8,
+}
+
+/// `#000000`
+impl Default for Color {
+    fn default() -> Self {
+        Color { r: 0, g: 0, b: 0 }
+    }
+}
+
+pub struct Color2 {
+    pub r: u8,
+    pub g: u8,
+    pub b: u8,
+}
+
+impl Default for Color2 {
+    /// `#000000`
+    fn default() -> Self {
+        Self { r: 0, g: 0, b: 0 }
+    }
+}
+
+#[derive(Default)]
+pub struct RepeatDefault1 {
+    a: [i8; 32],
+}
+
+
+
+pub struct RepeatDefault2 {
+    a: [i8; 33],
+}
+
+impl Default for RepeatDefault2 {
+    fn default() -> Self {
+        RepeatDefault2 { a: [0; 33] }
+    }
+}
+
+// https://github.com/rust-lang/rust-clippy/issues/7753
+
+pub enum IntOrString {
+    Int(i32),
+    String(String),
+}
+
+impl Default for IntOrString {
+    fn default() -> Self {
+        IntOrString::Int(0)
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/derivable_impls.rs b/tests/ui/derivable_impls.rs
index a6412004726..625cbcdde23 100644
--- a/tests/ui/derivable_impls.rs
+++ b/tests/ui/derivable_impls.rs
@@ -1,3 +1,7 @@
+// run-rustfix
+
+#![allow(dead_code)]
+
 use std::collections::HashMap;
 
 struct FooDefault<'a> {
diff --git a/tests/ui/derivable_impls.stderr b/tests/ui/derivable_impls.stderr
index 49fb471a219..c1db5a58b1f 100644
--- a/tests/ui/derivable_impls.stderr
+++ b/tests/ui/derivable_impls.stderr
@@ -1,5 +1,5 @@
 error: this `impl` can be derived
-  --> $DIR/derivable_impls.rs:18:1
+  --> $DIR/derivable_impls.rs:22:1
    |
 LL | / impl std::default::Default for FooDefault<'_> {
 LL | |     fn default() -> Self {
@@ -11,10 +11,14 @@ LL | | }
    | |_^
    |
    = note: `-D clippy::derivable-impls` implied by `-D warnings`
-   = help: try annotating `FooDefault` with `#[derive(Default)]`
+   = help: remove the manual implementation...
+help: ...and instead derive it
+   |
+LL | #[derive(Default)]
+   |
 
 error: this `impl` can be derived
-  --> $DIR/derivable_impls.rs:39:1
+  --> $DIR/derivable_impls.rs:43:1
    |
 LL | / impl std::default::Default for TupleDefault {
 LL | |     fn default() -> Self {
@@ -23,10 +27,14 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: try annotating `TupleDefault` with `#[derive(Default)]`
+   = help: remove the manual implementation...
+help: ...and instead derive it
+   |
+LL | #[derive(Default)]
+   |
 
 error: this `impl` can be derived
-  --> $DIR/derivable_impls.rs:91:1
+  --> $DIR/derivable_impls.rs:95:1
    |
 LL | / impl Default for StrDefault<'_> {
 LL | |     fn default() -> Self {
@@ -35,10 +43,14 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: try annotating `StrDefault` with `#[derive(Default)]`
+   = help: remove the manual implementation...
+help: ...and instead derive it
+   |
+LL | #[derive(Default)]
+   |
 
 error: this `impl` can be derived
-  --> $DIR/derivable_impls.rs:117:1
+  --> $DIR/derivable_impls.rs:121:1
    |
 LL | / impl Default for Y {
 LL | |     fn default() -> Self {
@@ -47,10 +59,14 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: try annotating `Y` with `#[derive(Default)]`
+   = help: remove the manual implementation...
+help: ...and instead derive it
+   |
+LL | #[derive(Default)]
+   |
 
 error: this `impl` can be derived
-  --> $DIR/derivable_impls.rs:156:1
+  --> $DIR/derivable_impls.rs:160:1
    |
 LL | / impl Default for WithoutSelfCurly {
 LL | |     fn default() -> Self {
@@ -59,10 +75,14 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: try annotating `WithoutSelfCurly` with `#[derive(Default)]`
+   = help: remove the manual implementation...
+help: ...and instead derive it
+   |
+LL | #[derive(Default)]
+   |
 
 error: this `impl` can be derived
-  --> $DIR/derivable_impls.rs:164:1
+  --> $DIR/derivable_impls.rs:168:1
    |
 LL | / impl Default for WithoutSelfParan {
 LL | |     fn default() -> Self {
@@ -71,10 +91,14 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: try annotating `WithoutSelfParan` with `#[derive(Default)]`
+   = help: remove the manual implementation...
+help: ...and instead derive it
+   |
+LL | #[derive(Default)]
+   |
 
 error: this `impl` can be derived
-  --> $DIR/derivable_impls.rs:214:1
+  --> $DIR/derivable_impls.rs:218:1
    |
 LL | / impl Default for RepeatDefault1 {
 LL | |     fn default() -> Self {
@@ -83,7 +107,11 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: try annotating `RepeatDefault1` with `#[derive(Default)]`
+   = help: remove the manual implementation...
+help: ...and instead derive it
+   |
+LL | #[derive(Default)]
+   |
 
 error: aborting due to 7 previous errors