Skip to content

Commit e690992

Browse files
estebankestebank
committed
Implement default_could_be_derived and default_overrides_default_fields lints
Detect when a manual `Default` implementation isn't using the existing default field values and suggest using `..` instead: ``` error: `Default` impl doesn't use the declared default field values --> $DIR/manual-default-impl-could-be-derived.rs:13:1 | LL | / impl Default for A { LL | | fn default() -> Self { LL | | A { LL | | x: S, LL | | y: 0, | | - this field has a default value ... | LL | | } | |_^ | note: the lint level is defined here --> $DIR/manual-default-impl-could-be-derived.rs:4:35 | LL | #![deny(default_could_be_derived, default_overrides_default_fields)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the default values in the `impl` to avoid them diverging over time | LL - x: S, LL - y: 0, LL + x: S, .. | ``` Detect when a manual `Default` implementation for a type containing at least one default field value has *all* fields that could be derived and suggest `#[derive(Default)]`: ``` error: `Default` impl that could be derived --> $DIR/manual-default-impl-could-be-derived.rs:27:1 | LL | struct B { | -------- all the fields in this struct have default values LL | x: S = S, | - default value LL | y: i32 = 1, | - default value ... LL | / impl Default for B { LL | | fn default() -> Self { LL | | B { LL | | x: S, ... | LL | | } | |_^ | note: the lint level is defined here --> $DIR/manual-default-impl-could-be-derived.rs:4:9 | LL | #![deny(default_could_be_derived, default_overrides_default_fields)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: to avoid divergence in behavior between `Struct { .. }` and `<Struct as Default>::default()`, derive the `Default` | LL ~ #[derive(Default)] struct B { | ``` Store a mapping between the `DefId` for an `impl Default for Ty {}` and the `DefId` of either a Unit variant/struct or an fn with no arguments that is called within `<Ty as Default>::default()`. When linting `impl`s, if it is for `Default`, we evaluate the contents of their `fn default()`. If it is *only* an ADT literal for `Self` and every field is either a "known to be defaulted" value (`0` or `false`), an explicit `Default::default()` call or a call or path to the same "equivalent" `DefId` from that field's type's `Default::default()` implementation.
1 parent f2b91cc commit e690992

File tree

26 files changed

+945
-30
lines changed

26 files changed

+945
-30
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,8 +1449,8 @@ pub enum StructRest {
14491449
Base(P<Expr>),
14501450
/// `..`.
14511451
Rest(Span),
1452-
/// No trailing `..` or expression.
1453-
None,
1452+
/// No trailing `..` or expression. The `Span` points at the trailing `,` or spot before `}`.
1453+
None(Span),
14541454
}
14551455

14561456
#[derive(Clone, Encodable, Decodable, Debug)]

‎compiler/rustc_ast/src/mut_visit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1772,7 +1772,7 @@ pub fn walk_expr<T: MutVisitor>(vis: &mut T, Expr { kind, id, span, attrs, token
17721772
match rest {
17731773
StructRest::Base(expr) => vis.visit_expr(expr),
17741774
StructRest::Rest(_span) => {}
1775-
StructRest::None => {}
1775+
StructRest::None(_span) => {}
17761776
}
17771777
}
17781778
ExprKind::Paren(expr) => {

‎compiler/rustc_ast/src/visit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1095,7 +1095,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) -> V
10951095
match rest {
10961096
StructRest::Base(expr) => try_visit!(visitor.visit_expr(expr)),
10971097
StructRest::Rest(_span) => {}
1098-
StructRest::None => {}
1098+
StructRest::None(_span) => {}
10991099
}
11001100
}
11011101
ExprKind::Tup(subexpressions) => {

‎compiler/rustc_ast_lowering/src/expr.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
359359
let rest = match &se.rest {
360360
StructRest::Base(e) => hir::StructTailExpr::Base(self.lower_expr(e)),
361361
StructRest::Rest(sp) => hir::StructTailExpr::DefaultFields(*sp),
362-
StructRest::None => hir::StructTailExpr::None,
362+
StructRest::None(sp) => hir::StructTailExpr::None(*sp),
363363
};
364364
hir::ExprKind::Struct(
365365
self.arena.alloc(self.lower_qpath(
@@ -1420,7 +1420,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
14201420
true
14211421
}
14221422
StructRest::Rest(_) => true,
1423-
StructRest::None => false,
1423+
StructRest::None(_) => false,
14241424
};
14251425
let struct_pat = hir::PatKind::Struct(qpath, field_pats, fields_omitted);
14261426
return self.pat_without_dbm(lhs.span, struct_pat);
@@ -1531,7 +1531,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
15311531
hir::ExprKind::Struct(
15321532
self.arena.alloc(hir::QPath::LangItem(lang_item, self.lower_span(span))),
15331533
fields,
1534-
hir::StructTailExpr::None,
1534+
hir::StructTailExpr::None(DUMMY_SP),
15351535
)
15361536
}
15371537

‎compiler/rustc_ast_pretty/src/pprust/state/expr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ impl<'a> State<'a> {
161161
self.word("{");
162162
let has_rest = match rest {
163163
ast::StructRest::Base(_) | ast::StructRest::Rest(_) => true,
164-
ast::StructRest::None => false,
164+
ast::StructRest::None(_) => false,
165165
};
166166
if fields.is_empty() && !has_rest {
167167
self.word("}");

‎compiler/rustc_expand/src/build.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ impl<'a> ExtCtxt<'a> {
368368
qself: None,
369369
path,
370370
fields,
371-
rest: ast::StructRest::None,
371+
rest: ast::StructRest::None(DUMMY_SP),
372372
})),
373373
)
374374
}

‎compiler/rustc_hir/src/hir.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2112,7 +2112,7 @@ impl Expr<'_> {
21122112
ExprKind::Struct(_, fields, init) => {
21132113
let init_side_effects = match init {
21142114
StructTailExpr::Base(init) => init.can_have_side_effects(),
2115-
StructTailExpr::DefaultFields(_) | StructTailExpr::None => false,
2115+
StructTailExpr::DefaultFields(_) | StructTailExpr::None(_) => false,
21162116
};
21172117
fields.iter().map(|field| field.expr).any(|e| e.can_have_side_effects())
21182118
|| init_side_effects
@@ -2187,48 +2187,48 @@ impl Expr<'_> {
21872187
ExprKind::Struct(
21882188
QPath::LangItem(LangItem::RangeTo, _),
21892189
[val1],
2190-
StructTailExpr::None,
2190+
StructTailExpr::None(_),
21912191
),
21922192
ExprKind::Struct(
21932193
QPath::LangItem(LangItem::RangeTo, _),
21942194
[val2],
2195-
StructTailExpr::None,
2195+
StructTailExpr::None(_),
21962196
),
21972197
)
21982198
| (
21992199
ExprKind::Struct(
22002200
QPath::LangItem(LangItem::RangeToInclusive, _),
22012201
[val1],
2202-
StructTailExpr::None,
2202+
StructTailExpr::None(_),
22032203
),
22042204
ExprKind::Struct(
22052205
QPath::LangItem(LangItem::RangeToInclusive, _),
22062206
[val2],
2207-
StructTailExpr::None,
2207+
StructTailExpr::None(_),
22082208
),
22092209
)
22102210
| (
22112211
ExprKind::Struct(
22122212
QPath::LangItem(LangItem::RangeFrom, _),
22132213
[val1],
2214-
StructTailExpr::None,
2214+
StructTailExpr::None(_),
22152215
),
22162216
ExprKind::Struct(
22172217
QPath::LangItem(LangItem::RangeFrom, _),
22182218
[val2],
2219-
StructTailExpr::None,
2219+
StructTailExpr::None(_),
22202220
),
22212221
) => val1.expr.equivalent_for_indexing(val2.expr),
22222222
(
22232223
ExprKind::Struct(
22242224
QPath::LangItem(LangItem::Range, _),
22252225
[val1, val3],
2226-
StructTailExpr::None,
2226+
StructTailExpr::None(_),
22272227
),
22282228
ExprKind::Struct(
22292229
QPath::LangItem(LangItem::Range, _),
22302230
[val2, val4],
2231-
StructTailExpr::None,
2231+
StructTailExpr::None(_),
22322232
),
22332233
) => {
22342234
val1.expr.equivalent_for_indexing(val2.expr)
@@ -2424,7 +2424,7 @@ pub enum ExprKind<'hir> {
24242424
#[derive(Debug, Clone, Copy, HashStable_Generic)]
24252425
pub enum StructTailExpr<'hir> {
24262426
/// A struct expression where all the fields are explicitly enumerated: `Foo { a, b }`.
2427-
None,
2427+
None(Span),
24282428
/// A struct expression with a "base", an expression of the same type as the outer struct that
24292429
/// will be used to populate any fields not explicitly mentioned: `Foo { ..base }`
24302430
Base(&'hir Expr<'hir>),

‎compiler/rustc_hir/src/intravisit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)
750750
walk_list!(visitor, visit_expr_field, fields);
751751
match optional_base {
752752
StructTailExpr::Base(base) => try_visit!(visitor.visit_expr(base)),
753-
StructTailExpr::None | StructTailExpr::DefaultFields(_) => {}
753+
StructTailExpr::None(_) | StructTailExpr::DefaultFields(_) => {}
754754
}
755755
}
756756
ExprKind::Tup(subexpressions) => {

‎compiler/rustc_hir_pretty/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1225,7 +1225,7 @@ impl<'a> State<'a> {
12251225
self.word("..");
12261226
self.end();
12271227
}
1228-
hir::StructTailExpr::None => {
1228+
hir::StructTailExpr::None(_) => {
12291229
if !fields.is_empty() {
12301230
self.word(",");
12311231
}

‎compiler/rustc_hir_typeck/src/expr_use_visitor.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -707,7 +707,7 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx
707707

708708
let with_expr = match *opt_with {
709709
hir::StructTailExpr::Base(w) => &*w,
710-
hir::StructTailExpr::DefaultFields(_) | hir::StructTailExpr::None => {
710+
hir::StructTailExpr::DefaultFields(_) | hir::StructTailExpr::None(_) => {
711711
return Ok(());
712712
}
713713
};

0 commit comments

Comments
 (0)