Skip to content

Commit 3c6b94a

Browse files
estebankestebank
committed
Detect impl Default where fields are literals and Default::default() calls
``` error: `impl Default` that could be derived --> $DIR/manual-default-impl-could-be-derived.rs:33:1 | LL | / impl Default for D { LL | | fn default() -> Self { LL | | D { LL | | x: Default::default(), ... | LL | | } LL | | } | |_^ | help: you don't need to manually `impl Default`, you can derive it | LL ~ #[derive(Default)] struct D { | ```
1 parent 3fe72a4 commit 3c6b94a

File tree

4 files changed

+105
-28
lines changed

4 files changed

+105
-28
lines changed

‎compiler/rustc_lint/src/default_could_be_derived.rs‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
use rustc_ast::LitKind;
12
use rustc_errors::Applicability;
23
use rustc_hir as hir;
34
use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
5+
use rustc_middle::ty::TyCtxt;
46
use rustc_session::{declare_lint, declare_lint_pass};
57
use rustc_span::symbol::{kw, sym};
68

@@ -115,6 +117,68 @@ impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived {
115117
},
116118
);
117119
}
120+
hir::ExprKind::Struct(_qpath, fields, _tail) => {
121+
// We have a struct literal
122+
//
123+
// struct Foo {
124+
// field: Type,
125+
// }
126+
//
127+
// impl Default for Foo {
128+
// fn default() -> Foo {
129+
// Foo {
130+
// field: val,
131+
// }
132+
// }
133+
// }
134+
//
135+
// We suggest #[derive(Default)] if
136+
// - `val` is `Default::default()`
137+
// - `val` is `0`
138+
// - `val` is `false`
139+
fn check_expr(tcx: TyCtxt<'_>, kind: hir::ExprKind<'_>) -> bool {
140+
match kind {
141+
hir::ExprKind::Lit(spanned_lit) => match spanned_lit.node {
142+
LitKind::Int(val, _) if val == 0 => true, // field: 0,
143+
LitKind::Bool(false) => true, // field: false,
144+
_ => false,
145+
},
146+
hir::ExprKind::Call(expr, [])
147+
if let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) =
148+
expr.kind
149+
&& let Some(def_id) = path.res.opt_def_id()
150+
&& tcx.is_diagnostic_item(sym::default_fn, def_id) =>
151+
{
152+
// field: Default::default(),
153+
true
154+
}
155+
_ => {
156+
false
157+
}
158+
}
159+
}
160+
if fields.iter().all(|f| check_expr(cx.tcx, f.expr.kind)) {
161+
cx.tcx.node_span_lint(
162+
DEFAULT_COULD_BE_DERIVED,
163+
item.hir_id(),
164+
item.span,
165+
|diag| {
166+
diag.primary_message("`impl Default` that could be derived");
167+
diag.multipart_suggestion_verbose(
168+
"you don't need to manually `impl Default`, you can derive it",
169+
vec![
170+
(
171+
cx.tcx.def_span(type_def_id).shrink_to_lo(),
172+
"#[derive(Default)] ".to_string(),
173+
),
174+
(item.span, String::new()),
175+
],
176+
Applicability::MachineApplicable,
177+
);
178+
},
179+
);
180+
}
181+
}
118182
_ => {}
119183
}
120184
}

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,12 @@
2424
// fn default() -> Self { C(None) }
2525
// }
2626
//
27-
// #[derive(Debug)]
28-
// struct D {
29-
// x: Option<i32>,
30-
// }
31-
//
32-
// impl Default for D {
33-
// fn default() -> Self {
34-
// D {
35-
// x: Default::default(),
36-
// }
37-
// }
38-
// }
27+
28+
#[derive(Default)] struct D {
29+
x: Option<i32>,
30+
y: i32,
31+
}
32+
3933
//
4034
// #[derive(Debug)]
4135
// struct E {

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

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,20 @@
2424
// fn default() -> Self { C(None) }
2525
// }
2626
//
27-
// #[derive(Debug)]
28-
// struct D {
29-
// x: Option<i32>,
30-
// }
31-
//
32-
// impl Default for D {
33-
// fn default() -> Self {
34-
// D {
35-
// x: Default::default(),
36-
// }
37-
// }
38-
// }
27+
28+
struct D {
29+
x: Option<i32>,
30+
y: i32,
31+
}
32+
33+
impl Default for D { //~ ERROR
34+
fn default() -> Self {
35+
D {
36+
x: Default::default(),
37+
y: 0,
38+
}
39+
}
40+
}
3941
//
4042
// #[derive(Debug)]
4143
// struct E {

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

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
error: `impl Default` that could be derived
2-
--> $DIR/manual-default-impl-could-be-derived.rs:58:1
2+
--> $DIR/manual-default-impl-could-be-derived.rs:33:1
33
|
4-
LL | / impl Default for F {
4+
LL | / impl Default for D {
55
LL | | fn default() -> Self {
6-
LL | | F::Unit
6+
LL | | D {
7+
LL | | x: Default::default(),
8+
... |
79
LL | | }
810
LL | | }
911
| |_^
@@ -15,9 +17,24 @@ LL | #![deny(default_could_be_derived)]
1517
| ^^^^^^^^^^^^^^^^^^^^^^^^
1618
help: you don't need to manually `impl Default`, you can derive it
1719
|
20+
LL ~ #[derive(Default)] struct D {
21+
|
22+
23+
error: `impl Default` that could be derived
24+
--> $DIR/manual-default-impl-could-be-derived.rs:60:1
25+
|
26+
LL | / impl Default for F {
27+
LL | | fn default() -> Self {
28+
LL | | F::Unit
29+
LL | | }
30+
LL | | }
31+
| |_^
32+
|
33+
help: you don't need to manually `impl Default`, you can derive it
34+
|
1835
LL ~ #[derive(Default)] enum F {
1936
LL ~ #[default] Unit,
2037
|
2138

22-
error: aborting due to 1 previous error
39+
error: aborting due to 2 previous errors
2340

0 commit comments

Comments
 (0)