Fix: Use multipart_suggestion for derivable_impls (#13717)

This should address #13099 for the `derivable_impls` test. As I've not
contributed to clippy before, I'd like to make sure i'm on the right
track before doing more :)

changelog: [`derivable_impls`]: Use multipart_suggestion to aggregate
feedback

4 files changed, 343 insertions, 62 deletions

diff --git a/clippy_lints/src/derivable_impls.rs b/clippy_lints/src/derivable_impls.rs
index 2b264421322..9569081ad08 100644
--- a/clippy_lints/src/derivable_impls.rs
+++ b/clippy_lints/src/derivable_impls.rs
@@ -132,17 +132,15 @@ fn check_struct<'tcx>(
 
     if should_emit {
         let struct_span = cx.tcx.def_span(adt_def.did());
+        let suggestions = vec![
+            (item.span, String::new()), // Remove the manual implementation
+            (struct_span.shrink_to_lo(), "#[derive(Default)]\n".to_string()), // Add the derive attribute
+        ];
+
         span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |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(),
+            diag.multipart_suggestion(
+                "replace the manual implementation with a derive attribute",
+                suggestions,
                 Applicability::MachineApplicable,
             );
         });
@@ -161,23 +159,23 @@ fn check_enum<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>, func_expr: &Ex
         let indent_enum = indent_of(cx, enum_span).unwrap_or(0);
         let variant_span = cx.tcx.def_span(variant_def.def_id);
         let indent_variant = indent_of(cx, variant_span).unwrap_or(0);
-        span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |diag| {
-            diag.span_suggestion_hidden(
-                item.span,
-                "remove the manual implementation...",
-                String::new(),
-                Applicability::MachineApplicable,
-            );
-            diag.span_suggestion(
+
+        let suggestions = vec![
+            (item.span, String::new()), // Remove the manual implementation
+            (
                 enum_span.shrink_to_lo(),
-                "...and instead derive it...",
-                format!("#[derive(Default)]\n{indent}", indent = " ".repeat(indent_enum),),
-                Applicability::MachineApplicable,
-            );
-            diag.span_suggestion(
+                format!("#[derive(Default)]\n{}", " ".repeat(indent_enum)),
+            ), // Add the derive attribute
+            (
                 variant_span.shrink_to_lo(),
-                "...and mark the default variant",
-                format!("#[default]\n{indent}", indent = " ".repeat(indent_variant),),
+                format!("#[default]\n{}", " ".repeat(indent_variant)),
+            ), // Mark the default variant
+        ];
+
+        span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |diag| {
+            diag.multipart_suggestion(
+                "replace the manual implementation with a derive attribute and mark the default variant",
+                suggestions,
                 Applicability::MachineApplicable,
             );
         });
diff --git a/tests/ui/derivable_impls.fixed b/tests/ui/derivable_impls.fixed
new file mode 100644
index 00000000000..c85f384fd6e
--- /dev/null
+++ b/tests/ui/derivable_impls.fixed
@@ -0,0 +1,295 @@
+#![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)
+    }
+}
+
+#[derive(Default)]
+pub enum SimpleEnum {
+    Foo,
+    #[default]
+    Bar,
+}
+
+
+pub enum NonExhaustiveEnum {
+    Foo,
+    #[non_exhaustive]
+    Bar,
+}
+
+impl Default for NonExhaustiveEnum {
+    fn default() -> Self {
+        NonExhaustiveEnum::Bar
+    }
+}
+
+// https://github.com/rust-lang/rust-clippy/issues/10396
+
+#[derive(Default)]
+struct DefaultType;
+
+struct GenericType<T = DefaultType> {
+    t: T,
+}
+
+impl Default for GenericType {
+    fn default() -> Self {
+        Self { t: Default::default() }
+    }
+}
+
+struct InnerGenericType<T> {
+    t: T,
+}
+
+impl Default for InnerGenericType<DefaultType> {
+    fn default() -> Self {
+        Self { t: Default::default() }
+    }
+}
+
+struct OtherGenericType<T = DefaultType> {
+    inner: InnerGenericType<T>,
+}
+
+impl Default for OtherGenericType {
+    fn default() -> Self {
+        Self {
+            inner: Default::default(),
+        }
+    }
+}
+
+mod issue10158 {
+    pub trait T {}
+
+    #[derive(Default)]
+    pub struct S {}
+    impl T for S {}
+
+    pub struct Outer {
+        pub inner: Box<dyn T>,
+    }
+
+    impl Default for Outer {
+        fn default() -> Self {
+            Outer {
+                // Box::<S>::default() adjusts to Box<dyn T>
+                inner: Box::<S>::default(),
+            }
+        }
+    }
+}
+
+mod issue11368 {
+    pub struct A {
+        a: u32,
+    }
+
+    impl Default for A {
+        #[track_caller]
+        fn default() -> Self {
+            Self { a: 0 }
+        }
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/derivable_impls.rs b/tests/ui/derivable_impls.rs
index 58f7771b627..21d73ba8b77 100644
--- a/tests/ui/derivable_impls.rs
+++ b/tests/ui/derivable_impls.rs
@@ -1,7 +1,5 @@
 #![allow(dead_code)]
 
-//@no-rustfix: need to change the suggestion to a multipart suggestion
-
 use std::collections::HashMap;
 
 struct FooDefault<'a> {
diff --git a/tests/ui/derivable_impls.stderr b/tests/ui/derivable_impls.stderr
index d3adfa60e10..c22569145bd 100644
--- a/tests/ui/derivable_impls.stderr
+++ b/tests/ui/derivable_impls.stderr
@@ -1,5 +1,5 @@
 error: this `impl` can be derived
-  --> tests/ui/derivable_impls.rs:22:1
+  --> tests/ui/derivable_impls.rs:20:1
    |
 LL | / impl std::default::Default for FooDefault<'_> {
 LL | |     fn default() -> Self {
@@ -12,15 +12,14 @@ LL | | }
    |
    = note: `-D clippy::derivable-impls` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::derivable_impls)]`
-   = help: remove the manual implementation...
-help: ...and instead derive it
+help: replace the manual implementation with a derive attribute
    |
 LL + #[derive(Default)]
-LL | struct FooDefault<'a> {
+LL ~ struct FooDefault<'a> {
    |
 
 error: this `impl` can be derived
-  --> tests/ui/derivable_impls.rs:43:1
+  --> tests/ui/derivable_impls.rs:41:1
    |
 LL | / impl std::default::Default for TupleDefault {
 LL | |     fn default() -> Self {
@@ -29,15 +28,14 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: remove the manual implementation...
-help: ...and instead derive it
+help: replace the manual implementation with a derive attribute
    |
 LL + #[derive(Default)]
-LL | struct TupleDefault(bool, i32, u64);
+LL ~ struct TupleDefault(bool, i32, u64);
    |
 
 error: this `impl` can be derived
-  --> tests/ui/derivable_impls.rs:95:1
+  --> tests/ui/derivable_impls.rs:93:1
    |
 LL | / impl Default for StrDefault<'_> {
 LL | |     fn default() -> Self {
@@ -46,15 +44,14 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: remove the manual implementation...
-help: ...and instead derive it
+help: replace the manual implementation with a derive attribute
    |
 LL + #[derive(Default)]
-LL | struct StrDefault<'a>(&'a str);
+LL ~ struct StrDefault<'a>(&'a str);
    |
 
 error: this `impl` can be derived
-  --> tests/ui/derivable_impls.rs:121:1
+  --> tests/ui/derivable_impls.rs:119:1
    |
 LL | / impl Default for Y {
 LL | |     fn default() -> Self {
@@ -63,15 +60,14 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: remove the manual implementation...
-help: ...and instead derive it
+help: replace the manual implementation with a derive attribute
    |
 LL + #[derive(Default)]
-LL | struct Y(u32);
+LL ~ struct Y(u32);
    |
 
 error: this `impl` can be derived
-  --> tests/ui/derivable_impls.rs:160:1
+  --> tests/ui/derivable_impls.rs:158:1
    |
 LL | / impl Default for WithoutSelfCurly {
 LL | |     fn default() -> Self {
@@ -80,15 +76,14 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: remove the manual implementation...
-help: ...and instead derive it
+help: replace the manual implementation with a derive attribute
    |
 LL + #[derive(Default)]
-LL | struct WithoutSelfCurly {
+LL ~ struct WithoutSelfCurly {
    |
 
 error: this `impl` can be derived
-  --> tests/ui/derivable_impls.rs:168:1
+  --> tests/ui/derivable_impls.rs:166:1
    |
 LL | / impl Default for WithoutSelfParan {
 LL | |     fn default() -> Self {
@@ -97,15 +92,14 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: remove the manual implementation...
-help: ...and instead derive it
+help: replace the manual implementation with a derive attribute
    |
 LL + #[derive(Default)]
-LL | struct WithoutSelfParan(bool);
+LL ~ struct WithoutSelfParan(bool);
    |
 
 error: this `impl` can be derived
-  --> tests/ui/derivable_impls.rs:218:1
+  --> tests/ui/derivable_impls.rs:216:1
    |
 LL | / impl Default for RepeatDefault1 {
 LL | |     fn default() -> Self {
@@ -114,15 +108,14 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: remove the manual implementation...
-help: ...and instead derive it
+help: replace the manual implementation with a derive attribute
    |
 LL + #[derive(Default)]
-LL | pub struct RepeatDefault1 {
+LL ~ pub struct RepeatDefault1 {
    |
 
 error: this `impl` can be derived
-  --> tests/ui/derivable_impls.rs:252:1
+  --> tests/ui/derivable_impls.rs:250:1
    |
 LL | / impl Default for SimpleEnum {
 LL | |     fn default() -> Self {
@@ -131,14 +124,11 @@ LL | |     }
 LL | | }
    | |_^
    |
-   = help: remove the manual implementation...
-help: ...and instead derive it...
+help: replace the manual implementation with a derive attribute and mark the default variant
    |
 LL + #[derive(Default)]
-LL | pub enum SimpleEnum {
-   |
-help: ...and mark the default variant
-   |
+LL ~ pub enum SimpleEnum {
+LL |     Foo,
 LL ~     #[default]
 LL ~     Bar,
    |