Skip to content

Commit be8b02f

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 e108481 commit be8b02f

File tree

30 files changed

+1237
-40
lines changed

30 files changed

+1237
-40
lines changed

‎compiler/rustc_ast/src/ast.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1455,8 +1455,8 @@ pub enum StructRest {
14551455
Base(P<Expr>),
14561456
/// `..`.
14571457
Rest(Span),
1458-
/// No trailing `..` or expression.
1459-
None,
1458+
/// No trailing `..` or expression. The `Span` points at the trailing `,` or spot before `}`.
1459+
None(Span),
14601460
}
14611461

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

‎compiler/rustc_ast/src/mut_visit.rs

+1-1
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

+1-1
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

+3-3
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

+1-1
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

+1-1
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

+10-10
Original file line numberDiff line numberDiff line change
@@ -2116,7 +2116,7 @@ impl Expr<'_> {
21162116
ExprKind::Struct(_, fields, init) => {
21172117
let init_side_effects = match init {
21182118
StructTailExpr::Base(init) => init.can_have_side_effects(),
2119-
StructTailExpr::DefaultFields(_) | StructTailExpr::None => false,
2119+
StructTailExpr::DefaultFields(_) | StructTailExpr::None(_) => false,
21202120
};
21212121
fields.iter().map(|field| field.expr).any(|e| e.can_have_side_effects())
21222122
|| init_side_effects
@@ -2191,48 +2191,48 @@ impl Expr<'_> {
21912191
ExprKind::Struct(
21922192
QPath::LangItem(LangItem::RangeTo, _),
21932193
[val1],
2194-
StructTailExpr::None,
2194+
StructTailExpr::None(_),
21952195
),
21962196
ExprKind::Struct(
21972197
QPath::LangItem(LangItem::RangeTo, _),
21982198
[val2],
2199-
StructTailExpr::None,
2199+
StructTailExpr::None(_),
22002200
),
22012201
)
22022202
| (
22032203
ExprKind::Struct(
22042204
QPath::LangItem(LangItem::RangeToInclusive, _),
22052205
[val1],
2206-
StructTailExpr::None,
2206+
StructTailExpr::None(_),
22072207
),
22082208
ExprKind::Struct(
22092209
QPath::LangItem(LangItem::RangeToInclusive, _),
22102210
[val2],
2211-
StructTailExpr::None,
2211+
StructTailExpr::None(_),
22122212
),
22132213
)
22142214
| (
22152215
ExprKind::Struct(
22162216
QPath::LangItem(LangItem::RangeFrom, _),
22172217
[val1],
2218-
StructTailExpr::None,
2218+
StructTailExpr::None(_),
22192219
),
22202220
ExprKind::Struct(
22212221
QPath::LangItem(LangItem::RangeFrom, _),
22222222
[val2],
2223-
StructTailExpr::None,
2223+
StructTailExpr::None(_),
22242224
),
22252225
) => val1.expr.equivalent_for_indexing(val2.expr),
22262226
(
22272227
ExprKind::Struct(
22282228
QPath::LangItem(LangItem::Range, _),
22292229
[val1, val3],
2230-
StructTailExpr::None,
2230+
StructTailExpr::None(_),
22312231
),
22322232
ExprKind::Struct(
22332233
QPath::LangItem(LangItem::Range, _),
22342234
[val2, val4],
2235-
StructTailExpr::None,
2235+
StructTailExpr::None(_),
22362236
),
22372237
) => {
22382238
val1.expr.equivalent_for_indexing(val2.expr)
@@ -2428,7 +2428,7 @@ pub enum ExprKind<'hir> {
24282428
#[derive(Debug, Clone, Copy, HashStable_Generic)]
24292429
pub enum StructTailExpr<'hir> {
24302430
/// A struct expression where all the fields are explicitly enumerated: `Foo { a, b }`.
2431-
None,
2431+
None(Span),
24322432
/// A struct expression with a "base", an expression of the same type as the outer struct that
24332433
/// will be used to populate any fields not explicitly mentioned: `Foo { ..base }`
24342434
Base(&'hir Expr<'hir>),

‎compiler/rustc_hir/src/intravisit.rs

+1-1
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

+1-1
Original file line numberDiff line numberDiff line change
@@ -1226,7 +1226,7 @@ impl<'a> State<'a> {
12261226
self.word("..");
12271227
self.end();
12281228
}
1229-
hir::StructTailExpr::None => {
1229+
hir::StructTailExpr::None(_) => {
12301230
if !fields.is_empty() {
12311231
self.word(",");
12321232
}

‎compiler/rustc_hir_typeck/src/expr_use_visitor.rs

+1-1
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)