Skip to content

Commit 4db9d0c

Browse files
estebankestebank
committed
Lint against manual Default impl if all fields are defaulted
1 parent 92eb997 commit 4db9d0c

4 files changed

+98
-10
lines changed

‎compiler/rustc_lint/src/default_could_be_derived.rs

+42-1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ declare_lint_pass!(DefaultCouldBeDerived => [DEFAULT_COULD_BE_DERIVED]);
3838

3939
impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived {
4040
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
41+
let hir = cx.tcx.hir();
4142
let hir::ItemKind::Impl(data) = item.kind else { return };
4243
let Some(trait_ref) = data.of_trait else { return };
4344
let Res::Def(DefKind::Trait, def_id) = trait_ref.path.res else { return };
@@ -51,6 +52,47 @@ impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived {
5152
let hir_self_ty = data.self_ty;
5253
let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = hir_self_ty.kind else { return };
5354
let Res::Def(def_kind, type_def_id) = path.res else { return };
55+
match hir.get_if_local(type_def_id) {
56+
Some(hir::Node::Item(hir::Item {
57+
kind:
58+
hir::ItemKind::Struct(hir::VariantData::Struct { fields, recovered: _ }, _generics),
59+
..
60+
})) => {
61+
if fields.iter().all(|f| f.default.is_some()) {
62+
cx.tcx.node_span_lint(
63+
DEFAULT_COULD_BE_DERIVED,
64+
item.hir_id(),
65+
item.span,
66+
|diag| {
67+
diag.primary_message("`impl Default` that could be derived");
68+
diag.span_label(
69+
cx.tcx.def_span(type_def_id),
70+
"all the fields in this struct have default values",
71+
);
72+
for field in &fields[..] {
73+
if let Some(anon) = field.default {
74+
diag.span_label(anon.span, "");
75+
}
76+
}
77+
diag.multipart_suggestion_verbose(
78+
"to avoid divergence in behavior between `Struct { .. }` and \
79+
`<Struct as Default>::default()`, derive the `Default`",
80+
vec![
81+
(
82+
cx.tcx.def_span(type_def_id).shrink_to_lo(),
83+
"#[derive(Default)] ".to_string(),
84+
),
85+
(item.span, String::new()),
86+
],
87+
Applicability::MachineApplicable,
88+
);
89+
},
90+
);
91+
return;
92+
}
93+
}
94+
_ => {}
95+
}
5496
let generics = cx.tcx.generics_of(type_def_id);
5597
if !generics.own_params.is_empty() && def_kind != DefKind::Enum {
5698
// For enums, `#[derive(Default)]` forces you to select a unit variant to avoid
@@ -61,7 +103,6 @@ impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived {
61103
}
62104
// We have a manual `impl Default for Ty {}` item, where `Ty` has no type parameters.
63105

64-
let hir = cx.tcx.hir();
65106
for assoc in data.items {
66107
let hir::AssocItemKind::Fn { has_self: false } = assoc.kind else { continue };
67108
if assoc.ident.name != kw::Default {

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

+13-4
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
//@ run-rustfix
33
#![allow(dead_code)]
44
#![deny(default_could_be_derived)]
5+
#![feature(default_field_values)]
56

67
// #[derive(Debug)]
78
// struct A;
@@ -40,15 +41,22 @@
4041

4142

4243
// Detection of unit variant ctors that could have been marked `#[default]`.
43-
#[derive(Default)] enum F {
44+
#[derive(Default)] enum F<T> {
4445
#[default] Unit,
45-
Tuple(i32),
46+
Tuple(T),
4647
}
4748

4849

4950
// Comparison of `impl` *fields* with their `Default::default()` bodies.
5051
#[derive(Default)] struct G {
51-
f: F,
52+
f: F<i32>,
53+
}
54+
55+
56+
// Always lint against manual `Default` impl if all fields are defaulted.
57+
#[derive(PartialEq, Debug)]
58+
#[derive(Default)] struct H {
59+
x: i32 = 101,
5260
}
5361

5462

@@ -58,6 +66,7 @@ fn main() {
5866
// let _ = C::default();
5967
let _ = D::default();
6068
let _ = E::default();
61-
let _ = F::default();
69+
let _ = F::<i32>::default();
6270
let _ = G::default();
71+
assert_eq!(H::default(), H { .. });
6372
}

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

+16
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
//@ run-rustfix
33
#![allow(dead_code)]
44
#![deny(default_could_be_derived)]
5+
#![feature(default_field_values)]
56

67
// #[derive(Debug)]
78
// struct A;
@@ -79,6 +80,20 @@ impl Default for G { //~ ERROR
7980
}
8081
}
8182

83+
// Always lint against manual `Default` impl if all fields are defaulted.
84+
#[derive(PartialEq, Debug)]
85+
struct H {
86+
x: i32 = 101,
87+
}
88+
89+
impl Default for H { //~ ERROR
90+
fn default() -> Self {
91+
H {
92+
x: 1,
93+
}
94+
}
95+
}
96+
8297
fn main() {
8398
// let _ = A::default();
8499
// let _ = B::default();
@@ -87,4 +102,5 @@ fn main() {
87102
let _ = E::default();
88103
let _ = F::<i32>::default();
89104
let _ = G::default();
105+
assert_eq!(H::default(), H { .. });
90106
}

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

+27-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: `impl Default` that could be derived
2-
--> $DIR/manual-default-impl-could-be-derived.rs:34:1
2+
--> $DIR/manual-default-impl-could-be-derived.rs:35:1
33
|
44
LL | / impl Default for D {
55
LL | | fn default() -> Self {
@@ -21,7 +21,7 @@ LL ~ #[derive(Default)] struct D {
2121
|
2222

2323
error: `impl Default` that could be derived
24-
--> $DIR/manual-default-impl-could-be-derived.rs:49:1
24+
--> $DIR/manual-default-impl-could-be-derived.rs:50:1
2525
|
2626
LL | / impl Default for E {
2727
LL | | fn default() -> Self {
@@ -38,7 +38,7 @@ LL ~ #[derive(Default)] struct E {
3838
|
3939

4040
error: `impl Default` that could be derived
41-
--> $DIR/manual-default-impl-could-be-derived.rs:63:1
41+
--> $DIR/manual-default-impl-could-be-derived.rs:64:1
4242
|
4343
LL | / impl<T> Default for F<T> {
4444
LL | | fn default() -> Self {
@@ -54,7 +54,7 @@ LL ~ #[default] Unit,
5454
|
5555

5656
error: `impl Default` that could be derived
57-
--> $DIR/manual-default-impl-could-be-derived.rs:74:1
57+
--> $DIR/manual-default-impl-could-be-derived.rs:75:1
5858
|
5959
LL | / impl Default for G {
6060
LL | | fn default() -> Self {
@@ -70,5 +70,27 @@ help: you don't need to manually `impl Default`, you can derive it
7070
LL ~ #[derive(Default)] struct G {
7171
|
7272

73-
error: aborting due to 4 previous errors
73+
error: `impl Default` that could be derived
74+
--> $DIR/manual-default-impl-could-be-derived.rs:89:1
75+
|
76+
LL | struct H {
77+
| -------- all the fields in this struct have default values
78+
LL | x: i32 = 101,
79+
| ---
80+
...
81+
LL | / impl Default for H {
82+
LL | | fn default() -> Self {
83+
LL | | H {
84+
LL | | x: 1,
85+
LL | | }
86+
LL | | }
87+
LL | | }
88+
| |_^
89+
|
90+
help: to avoid divergence in behavior between `Struct { .. }` and `<Struct as Default>::default()`, derive the `Default`
91+
|
92+
LL ~ #[derive(Default)] struct H {
93+
|
94+
95+
error: aborting due to 5 previous errors
7496

0 commit comments

Comments
 (0)