fix: multipart suggestions for derivable_impls

Author: scottgerring

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,
             );
         });

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

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