Lint structs with default field values and manual Default

Author: estebank

compiler/rustc_lint/src/default_could_be_derived.rs

@@ -58,21 +58,52 @@ impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived {
                     hir::ItemKind::Struct(hir::VariantData::Struct { fields, recovered: _ }, _generics),
                 ..
             })) => {
-                if fields.iter().all(|f| f.default.is_some()) {
+                let fields_with_default_value: Vec<_> =
+                    fields.iter().filter_map(|f| f.default).collect();
+                let fields_with_default_impl: Vec<_> = fields
+                    .iter()
+                    .filter_map(|f| match (f.ty.kind, f.default) {
+                        (hir::TyKind::Path(hir::QPath::Resolved(_, path)), None)
+                            if let Some(def_id) = path.res.opt_def_id()
+                                && let DefKind::Struct | DefKind::Enum =
+                                    cx.tcx.def_kind(def_id) =>
+                        {
+                            let ty = cx.tcx.type_of(def_id).instantiate_identity();
+                            let mut count = 0;
+                            cx.tcx.for_each_relevant_impl(default_def_id, ty, |_| count += 1);
+                            if count > 0 { Some(f.ty.span) } else { None }
+                        }
+                        _ => None,
+                    })
+                    .collect();
+                if !fields_with_default_value.is_empty()
+                    && fields.len()
+                        == fields_with_default_value.len() + fields_with_default_impl.len()
+                {
                     cx.tcx.node_span_lint(
                         DEFAULT_COULD_BE_DERIVED,
                         item.hir_id(),
                         item.span,
                         |diag| {
                             diag.primary_message("`impl Default` that could be derived");
+                            let msg = match (
+                                !fields_with_default_value.is_empty(),
+                                !fields_with_default_impl.is_empty(),
+                            ) {
+                                (true, true) => "default values or a type that impls `Default`",
+                                (true, false) => "default values",
+                                (false, true) => "a type that impls `Default`",
+                                (false, false) => unreachable!(),
+                            };
                             diag.span_label(
                                 cx.tcx.def_span(type_def_id),
-                                "all the fields in this struct have default values",
+                                format!("all the fields in this struct have {msg}"),
                             );
-                            for field in &fields[..] {
-                                if let Some(anon) = field.default {
-                                    diag.span_label(anon.span, "");
-                                }
+                            for anon in fields_with_default_value {
+                                diag.span_label(anon.span, "default value");
+                            }
+                            for field in fields_with_default_impl {
+                                diag.span_label(field, "implements `Default`");
                             }
                             diag.multipart_suggestion_verbose(
                                 "to avoid divergence in behavior between `Struct { .. }` and \

tests/ui/structs/manual-default-impl-could-be-derived.fixed

@@ -60,6 +60,14 @@
 }
 
 
+// Always lint against manual `Default` impl if all fields are defaulted.
+#[derive(PartialEq, Debug)]
+#[derive(Default)] struct I {
+    x: i32 = 101,
+    y: Option<i32>,
+}
+
+
 fn main() {
 //    let _ = A::default();
 //    let _ = B::default();

tests/ui/structs/manual-default-impl-could-be-derived.rs

@@ -94,6 +94,22 @@ impl Default for H { //~ ERROR
     }
 }
 
+// Always lint against manual `Default` impl if all fields are defaulted.
+#[derive(PartialEq, Debug)]
+struct I {
+    x: i32 = 101,
+    y: Option<i32>,
+}
+
+impl Default for I { //~ ERROR
+    fn default() -> Self {
+        I {
+            x: 1,
+            y: None,
+        }
+    }
+}
+
 fn main() {
 //    let _ = A::default();
 //    let _ = B::default();

tests/ui/structs/manual-default-impl-could-be-derived.stderr

@@ -76,7 +76,7 @@ error: `impl Default` that could be derived
 LL |   struct H {
    |   -------- all the fields in this struct have default values
 LL |       x: i32 = 101,
-   |                ---
+   |                --- default value
 ...
 LL | / impl Default for H {
 LL | |     fn default() -> Self {
@@ -92,5 +92,29 @@ help: to avoid divergence in behavior between `Struct { .. }` and `<Struct as De
 LL ~ #[derive(Default)] struct H {
    |
 
-error: aborting due to 5 previous errors
+error: `impl Default` that could be derived
+  --> $DIR/manual-default-impl-could-be-derived.rs:104:1
+   |
+LL |   struct I {
+   |   -------- all the fields in this struct have default values or a type that impls `Default`
+LL |       x: i32 = 101,
+   |                --- default value
+LL |       y: Option<i32>,
+   |          ----------- implements `Default`
+...
+LL | / impl Default for I {
+LL | |     fn default() -> Self {
+LL | |         I {
+LL | |             x: 1,
+...  |
+LL | |     }
+LL | | }
+   | |_^
+   |
+help: to avoid divergence in behavior between `Struct { .. }` and `<Struct as Default>::default()`, derive the `Default`
+   |
+LL ~ #[derive(Default)] struct I {
+   |
+
+error: aborting due to 6 previous errors