Skip to content

Commit b63e7dc

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 9e136a3 commit b63e7dc

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
@@ -1450,8 +1450,8 @@ pub enum StructRest {
14501450
Base(P<Expr>),
14511451
/// `..`.
14521452
Rest(Span),
1453-
/// No trailing `..` or expression.
1454-
None,
1453+
/// No trailing `..` or expression. The `Span` points at the trailing `,` or spot before `}`.
1454+
None(Span),
14551455
}
14561456

14571457
#[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
@@ -1771,7 +1771,7 @@ pub fn walk_expr<T: MutVisitor>(vis: &mut T, Expr { kind, id, span, attrs, token
17711771
match rest {
17721772
StructRest::Base(expr) => vis.visit_expr(expr),
17731773
StructRest::Rest(_span) => {}
1774-
StructRest::None => {}
1774+
StructRest::None(_span) => {}
17751775
}
17761776
}
17771777
ExprKind::Paren(expr) => {

‎compiler/rustc_ast/src/visit.rs

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

‎compiler/rustc_ast_lowering/src/expr.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
358358
let rest = match &se.rest {
359359
StructRest::Base(e) => hir::StructTailExpr::Base(self.lower_expr(e)),
360360
StructRest::Rest(sp) => hir::StructTailExpr::DefaultFields(*sp),
361-
StructRest::None => hir::StructTailExpr::None,
361+
StructRest::None(sp) => hir::StructTailExpr::None(*sp),
362362
};
363363
hir::ExprKind::Struct(
364364
self.arena.alloc(self.lower_qpath(
@@ -1419,7 +1419,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
14191419
true
14201420
}
14211421
StructRest::Rest(_) => true,
1422-
StructRest::None => false,
1422+
StructRest::None(_) => false,
14231423
};
14241424
let struct_pat = hir::PatKind::Struct(qpath, field_pats, fields_omitted);
14251425
return self.pat_without_dbm(lhs.span, struct_pat);
@@ -1530,7 +1530,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
15301530
hir::ExprKind::Struct(
15311531
self.arena.alloc(hir::QPath::LangItem(lang_item, self.lower_span(span))),
15321532
fields,
1533-
hir::StructTailExpr::None,
1533+
hir::StructTailExpr::None(DUMMY_SP),
15341534
)
15351535
}
15361536

‎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
@@ -367,7 +367,7 @@ impl<'a> ExtCtxt<'a> {
367367
qself: None,
368368
path,
369369
fields,
370-
rest: ast::StructRest::None,
370+
rest: ast::StructRest::None(DUMMY_SP),
371371
})),
372372
)
373373
}

‎compiler/rustc_hir/src/hir.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2111,7 +2111,7 @@ impl Expr<'_> {
21112111
ExprKind::Struct(_, fields, init) => {
21122112
let init_side_effects = match init {
21132113
StructTailExpr::Base(init) => init.can_have_side_effects(),
2114-
StructTailExpr::DefaultFields(_) | StructTailExpr::None => false,
2114+
StructTailExpr::DefaultFields(_) | StructTailExpr::None(_) => false,
21152115
};
21162116
fields.iter().map(|field| field.expr).any(|e| e.can_have_side_effects())
21172117
|| init_side_effects
@@ -2186,48 +2186,48 @@ impl Expr<'_> {
21862186
ExprKind::Struct(
21872187
QPath::LangItem(LangItem::RangeTo, _),
21882188
[val1],
2189-
StructTailExpr::None,
2189+
StructTailExpr::None(_),
21902190
),
21912191
ExprKind::Struct(
21922192
QPath::LangItem(LangItem::RangeTo, _),
21932193
[val2],
2194-
StructTailExpr::None,
2194+
StructTailExpr::None(_),
21952195
),
21962196
)
21972197
| (
21982198
ExprKind::Struct(
21992199
QPath::LangItem(LangItem::RangeToInclusive, _),
22002200
[val1],
2201-
StructTailExpr::None,
2201+
StructTailExpr::None(_),
22022202
),
22032203
ExprKind::Struct(
22042204
QPath::LangItem(LangItem::RangeToInclusive, _),
22052205
[val2],
2206-
StructTailExpr::None,
2206+
StructTailExpr::None(_),
22072207
),
22082208
)
22092209
| (
22102210
ExprKind::Struct(
22112211
QPath::LangItem(LangItem::RangeFrom, _),
22122212
[val1],
2213-
StructTailExpr::None,
2213+
StructTailExpr::None(_),
22142214
),
22152215
ExprKind::Struct(
22162216
QPath::LangItem(LangItem::RangeFrom, _),
22172217
[val2],
2218-
StructTailExpr::None,
2218+
StructTailExpr::None(_),
22192219
),
22202220
) => val1.expr.equivalent_for_indexing(val2.expr),
22212221
(
22222222
ExprKind::Struct(
22232223
QPath::LangItem(LangItem::Range, _),
22242224
[val1, val3],
2225-
StructTailExpr::None,
2225+
StructTailExpr::None(_),
22262226
),
22272227
ExprKind::Struct(
22282228
QPath::LangItem(LangItem::Range, _),
22292229
[val2, val4],
2230-
StructTailExpr::None,
2230+
StructTailExpr::None(_),
22312231
),
22322232
) => {
22332233
val1.expr.equivalent_for_indexing(val2.expr)
@@ -2423,7 +2423,7 @@ pub enum ExprKind<'hir> {
24232423
#[derive(Debug, Clone, Copy, HashStable_Generic)]
24242424
pub enum StructTailExpr<'hir> {
24252425
/// A struct expression where all the fields are explicitly enumerated: `Foo { a, b }`.
2426-
None,
2426+
None(Span),
24272427
/// A struct expression with a "base", an expression of the same type as the outer struct that
24282428
/// will be used to populate any fields not explicitly mentioned: `Foo { ..base }`
24292429
Base(&'hir Expr<'hir>),

‎compiler/rustc_hir/src/intravisit.rs

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

‎compiler/rustc_hir_pretty/src/lib.rs

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

‎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)