Skip to content

Commit 0930485

Browse files
estebankestebank
committed
Explicitly consider fields set to None as Default derivable
1 parent 3c6b94a commit 0930485

File tree

4 files changed

+93
-38
lines changed

4 files changed

+93
-38
lines changed

‎compiler/rustc_lint/src/default_could_be_derived.rs‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,9 @@ impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived {
133133
// }
134134
//
135135
// We suggest #[derive(Default)] if
136-
// - `val` is `Default::default()`
137-
// - `val` is `0`
138-
// - `val` is `false`
136+
// - `val` is `Default::default()`
137+
// - `val` is `0`
138+
// - `val` is `false`
139139
fn check_expr(tcx: TyCtxt<'_>, kind: hir::ExprKind<'_>) -> bool {
140140
match kind {
141141
hir::ExprKind::Lit(spanned_lit) => match spanned_lit.node {
@@ -152,9 +152,20 @@ impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived {
152152
// field: Default::default(),
153153
true
154154
}
155-
_ => {
156-
false
155+
hir::ExprKind::Path(hir::QPath::Resolved(_, path))
156+
if let Res::Def(
157+
DefKind::Ctor(CtorOf::Variant, CtorKind::Const),
158+
def_id,
159+
) = path.res
160+
&& let def_id = tcx.parent(def_id) // From Ctor to variant
161+
&& tcx.is_lang_item(def_id, hir::LangItem::OptionNone) =>
162+
{
163+
// FIXME: We should use a better check where we explore existing
164+
// `impl Default for def_id` of the found type and see compare them
165+
// against what we have here. For now, special case `Option::None`.
166+
true
157167
}
168+
_ => false,
158169
}
159170
}
160171
if fields.iter().all(|f| check_expr(cx.tcx, f.expr.kind)) {

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

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,36 +25,46 @@
2525
// }
2626
//
2727

28+
// Explicit check against numeric literals and `Default::default()` calls.
2829
#[derive(Default)] struct D {
2930
x: Option<i32>,
3031
y: i32,
3132
}
3233

33-
//
34-
// #[derive(Debug)]
35-
// struct E {
36-
// x: Option<i32>,
37-
// }
38-
//
39-
// impl Default for E {
40-
// fn default() -> Self {
41-
// E {
42-
// x: None,
43-
// }
44-
// }
45-
// }
4634

35+
// Explicit check against `None` literal, in the same way that we check against numeric literals.
36+
#[derive(Debug)]
37+
#[derive(Default)] struct E {
38+
x: Option<i32>,
39+
}
40+
41+
42+
// Detection of unit variant ctors that could have been marked `#[default]`.
4743
#[derive(Default)] enum F {
4844
#[default] Unit,
4945
Tuple(i32),
5046
}
5147

5248

49+
// Comparison of `impl` *fields* with their `Default::default()` bodies.
50+
// struct G {
51+
// f: F,
52+
// }
53+
54+
// impl Default for G {
55+
// fn default() -> Self {
56+
// G {
57+
// f: F::Unit,
58+
// }
59+
// }
60+
// }
61+
5362
fn main() {
5463
// let _ = A::default();
5564
// let _ = B::default();
5665
// let _ = C::default();
57-
// let _ = D::default();
58-
// let _ = E::default();
66+
let _ = D::default();
67+
let _ = E::default();
5968
let _ = F::default();
69+
// let _ = G::default();
6070
}

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

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
// }
2626
//
2727

28+
// Explicit check against numeric literals and `Default::default()` calls.
2829
struct D {
2930
x: Option<i32>,
3031
y: i32,
@@ -38,20 +39,22 @@ impl Default for D { //~ ERROR
3839
}
3940
}
4041
}
41-
//
42-
// #[derive(Debug)]
43-
// struct E {
44-
// x: Option<i32>,
45-
// }
46-
//
47-
// impl Default for E {
48-
// fn default() -> Self {
49-
// E {
50-
// x: None,
51-
// }
52-
// }
53-
// }
5442

43+
// Explicit check against `None` literal, in the same way that we check against numeric literals.
44+
#[derive(Debug)]
45+
struct E {
46+
x: Option<i32>,
47+
}
48+
49+
impl Default for E { //~ ERROR
50+
fn default() -> Self {
51+
E {
52+
x: None,
53+
}
54+
}
55+
}
56+
57+
// Detection of unit variant ctors that could have been marked `#[default]`.
5558
enum F {
5659
Unit,
5760
Tuple(i32),
@@ -63,11 +66,25 @@ impl Default for F { //~ ERROR
6366
}
6467
}
6568

69+
// Comparison of `impl` *fields* with their `Default::default()` bodies.
70+
// struct G {
71+
// f: F,
72+
// }
73+
74+
// impl Default for G {
75+
// fn default() -> Self {
76+
// G {
77+
// f: F::Unit,
78+
// }
79+
// }
80+
// }
81+
6682
fn main() {
6783
// let _ = A::default();
6884
// let _ = B::default();
6985
// let _ = C::default();
70-
// let _ = D::default();
71-
// let _ = E::default();
86+
let _ = D::default();
87+
let _ = E::default();
7288
let _ = F::default();
89+
// let _ = G::default();
7390
}

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

Lines changed: 20 additions & 3 deletions
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:33:1
2+
--> $DIR/manual-default-impl-could-be-derived.rs:34:1
33
|
44
LL | / impl Default for D {
55
LL | | fn default() -> Self {
@@ -21,7 +21,24 @@ LL ~ #[derive(Default)] struct D {
2121
|
2222

2323
error: `impl Default` that could be derived
24-
--> $DIR/manual-default-impl-could-be-derived.rs:60:1
24+
--> $DIR/manual-default-impl-could-be-derived.rs:49:1
25+
|
26+
LL | / impl Default for E {
27+
LL | | fn default() -> Self {
28+
LL | | E {
29+
LL | | x: None,
30+
LL | | }
31+
LL | | }
32+
LL | | }
33+
| |_^
34+
|
35+
help: you don't need to manually `impl Default`, you can derive it
36+
|
37+
LL ~ #[derive(Default)] struct E {
38+
|
39+
40+
error: `impl Default` that could be derived
41+
--> $DIR/manual-default-impl-could-be-derived.rs:63:1
2542
|
2643
LL | / impl Default for F {
2744
LL | | fn default() -> Self {
@@ -36,5 +53,5 @@ LL ~ #[derive(Default)] enum F {
3653
LL ~ #[default] Unit,
3754
|
3855

39-
error: aborting due to 2 previous errors
56+
error: aborting due to 3 previous errors
4057

0 commit comments

Comments
 (0)