Skip to content

Commit 7d576f6

Browse files
committed
Detect struct construction with private field in field with default
When trying to construct a struct that has a public field of a private type, suggest using `..` if that field has a default value. ``` error[E0603]: struct `Priv1` is private --> $DIR/non-exhaustive-ctor.rs:25:39 | LL | let _ = S { field: (), field1: m::Priv1 {} }; | ------ ^^^^^ private struct | | | while setting this field | note: the struct `Priv1` is defined here --> $DIR/non-exhaustive-ctor.rs:14:4 | LL | struct Priv1 {} | ^^^^^^^^^^^^ help: the field `field1` you're trying to set has a default value, you can use `..` to use it | LL | let _ = S { field: (), .. }; | ~~ ```
1 parent 06a24e9 commit 7d576f6

File tree

11 files changed

+289
-32
lines changed

11 files changed

+289
-32
lines changed

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

+1
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,7 @@ provide! { tcx, def_id, other, cdata,
424424

425425
crate_extern_paths => { cdata.source().paths().cloned().collect() }
426426
expn_that_defined => { cdata.get_expn_that_defined(def_id.index, tcx.sess) }
427+
default_field => { cdata.get_default_field(def_id.index) }
427428
is_doc_hidden => { cdata.get_attr_flags(def_id.index).contains(AttrFlags::IS_DOC_HIDDEN) }
428429
doc_link_resolutions => { tcx.arena.alloc(cdata.get_doc_link_resolutions(def_id.index)) }
429430
doc_link_traits_in_scope => {

compiler/rustc_middle/src/query/mod.rs

+7
Original file line numberDiff line numberDiff line change
@@ -1719,6 +1719,13 @@ rustc_queries! {
17191719
feedable
17201720
}
17211721

1722+
/// Returns whether the impl or associated function has the `default` keyword.
1723+
query default_field(def_id: DefId) -> Option<DefId> {
1724+
desc { |tcx| "looking up the `const` corresponding to the default for `{}`", tcx.def_path_str(def_id) }
1725+
separate_provide_extern
1726+
feedable
1727+
}
1728+
17221729
query check_well_formed(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
17231730
desc { |tcx| "checking that `{}` is well-formed", tcx.def_path_str(key) }
17241731
return_result_from_ensure_ok

compiler/rustc_resolve/src/build_reduced_graph.rs

+9-5
Original file line numberDiff line numberDiff line change
@@ -396,14 +396,18 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
396396
// The fields are not expanded yet.
397397
return;
398398
}
399-
let fields = fields
399+
let field_name = |i, field: &ast::FieldDef| {
400+
field.ident.unwrap_or_else(|| Ident::from_str_and_span(&format!("{i}"), field.span))
401+
};
402+
let field_names: Vec<_> =
403+
fields.iter().enumerate().map(|(i, field)| field_name(i, field)).collect();
404+
let defaults = fields
400405
.iter()
401406
.enumerate()
402-
.map(|(i, field)| {
403-
field.ident.unwrap_or_else(|| Ident::from_str_and_span(&format!("{i}"), field.span))
404-
})
407+
.filter_map(|(i, field)| field.default.as_ref().map(|_| field_name(i, field).name))
405408
.collect();
406-
self.r.field_names.insert(def_id, fields);
409+
self.r.field_names.insert(def_id, field_names);
410+
self.r.field_defaults.insert(def_id, defaults);
407411
}
408412

409413
fn insert_field_visibilities_local(&mut self, def_id: DefId, fields: &[ast::FieldDef]) {

compiler/rustc_resolve/src/diagnostics.rs

+58-2
Original file line numberDiff line numberDiff line change
@@ -1750,8 +1750,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
17501750
}
17511751

17521752
fn report_privacy_error(&mut self, privacy_error: &PrivacyError<'ra>) {
1753-
let PrivacyError { ident, binding, outermost_res, parent_scope, single_nested, dedup_span } =
1754-
*privacy_error;
1753+
let PrivacyError {
1754+
ident,
1755+
binding,
1756+
outermost_res,
1757+
parent_scope,
1758+
single_nested,
1759+
dedup_span,
1760+
ref source,
1761+
} = *privacy_error;
17551762

17561763
let res = binding.res();
17571764
let ctor_fields_span = self.ctor_fields_span(binding);
@@ -1767,6 +1774,55 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
17671774
let mut err =
17681775
self.dcx().create_err(errors::IsPrivate { span: ident.span, ident_descr, ident });
17691776

1777+
if let Some(expr) = source
1778+
&& let ast::ExprKind::Struct(struct_expr) = &expr.kind
1779+
&& let Some(Res::Def(_, def_id)) = self.partial_res_map
1780+
[&struct_expr.path.segments.iter().last().unwrap().id]
1781+
.full_res()
1782+
&& let Some(default_fields) = self.field_defaults(def_id)
1783+
&& !struct_expr.fields.is_empty()
1784+
{
1785+
let last_span = struct_expr.fields.iter().last().unwrap().span;
1786+
let mut iter = struct_expr.fields.iter().peekable();
1787+
let mut prev: Option<Span> = None;
1788+
while let Some(field) = iter.next() {
1789+
if field.expr.span.overlaps(ident.span) {
1790+
err.span_label(field.ident.span, "while setting this field");
1791+
if default_fields.contains(&field.ident.name) {
1792+
let sugg = if last_span == field.span {
1793+
vec![(field.span, "..".to_string())]
1794+
} else {
1795+
vec![
1796+
(
1797+
// Account for trailing commas and ensure we remove them.
1798+
match (prev, iter.peek()) {
1799+
(_, Some(next)) => field.span.with_hi(next.span.lo()),
1800+
(Some(prev), _) => field.span.with_lo(prev.hi()),
1801+
(None, None) => field.span,
1802+
},
1803+
String::new(),
1804+
),
1805+
(last_span.shrink_to_hi(), ", ..".to_string()),
1806+
]
1807+
};
1808+
err.multipart_suggestion_verbose(
1809+
format!(
1810+
"the type `{ident}` of field `{}` is private, but you can \
1811+
construct the default value defined for it in `{}` using `..` in \
1812+
the struct initializer expression",
1813+
field.ident,
1814+
self.tcx.item_name(def_id),
1815+
),
1816+
sugg,
1817+
Applicability::MachineApplicable,
1818+
);
1819+
break;
1820+
}
1821+
}
1822+
prev = Some(field.span);
1823+
}
1824+
}
1825+
17701826
let mut not_publicly_reexported = false;
17711827
if let Some((this_res, outer_ident)) = outermost_res {
17721828
let import_suggestions = self.lookup_import_candidates(

compiler/rustc_resolve/src/ident.rs

+18-1
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
893893
binding,
894894
dedup_span: path_span,
895895
outermost_res: None,
896+
source: None,
896897
parent_scope: *parent_scope,
897898
single_nested: path_span != root_span,
898899
});
@@ -1402,7 +1403,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14021403
parent_scope: &ParentScope<'ra>,
14031404
ignore_import: Option<Import<'ra>>,
14041405
) -> PathResult<'ra> {
1405-
self.resolve_path_with_ribs(path, opt_ns, parent_scope, None, None, None, ignore_import)
1406+
self.resolve_path_with_ribs(
1407+
path,
1408+
opt_ns,
1409+
parent_scope,
1410+
None,
1411+
None,
1412+
None,
1413+
None,
1414+
ignore_import,
1415+
)
14061416
}
14071417

14081418
#[instrument(level = "debug", skip(self))]
@@ -1419,6 +1429,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14191429
path,
14201430
opt_ns,
14211431
parent_scope,
1432+
None,
14221433
finalize,
14231434
None,
14241435
ignore_binding,
@@ -1431,6 +1442,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14311442
path: &[Segment],
14321443
opt_ns: Option<Namespace>, // `None` indicates a module path in import
14331444
parent_scope: &ParentScope<'ra>,
1445+
source: Option<PathSource<'_>>,
14341446
finalize: Option<Finalize>,
14351447
ribs: Option<&PerNS<Vec<Rib<'ra>>>>,
14361448
ignore_binding: Option<NameBinding<'ra>>,
@@ -1605,6 +1617,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
16051617
// the user it is not accessible.
16061618
for error in &mut self.privacy_errors[privacy_errors_len..] {
16071619
error.outermost_res = Some((res, ident));
1620+
error.source = match source {
1621+
Some(PathSource::Struct(Some(expr)))
1622+
| Some(PathSource::Expr(Some(expr))) => Some(expr.clone()),
1623+
_ => None,
1624+
};
16081625
}
16091626

16101627
let maybe_assoc = opt_ns != Some(MacroNS) && PathSource::Type.is_expected(res);

compiler/rustc_resolve/src/late.rs

+18-13
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ pub(crate) enum PathSource<'a> {
403403
// Paths in path patterns `Path`.
404404
Pat,
405405
// Paths in struct expressions and patterns `Path { .. }`.
406-
Struct,
406+
Struct(Option<&'a Expr>),
407407
// Paths in tuple struct patterns `Path(..)`.
408408
TupleStruct(Span, &'a [Span]),
409409
// `m::A::B` in `<T as m::A>::B::C`.
@@ -419,7 +419,7 @@ pub(crate) enum PathSource<'a> {
419419
impl<'a> PathSource<'a> {
420420
fn namespace(self) -> Namespace {
421421
match self {
422-
PathSource::Type | PathSource::Trait(_) | PathSource::Struct => TypeNS,
422+
PathSource::Type | PathSource::Trait(_) | PathSource::Struct(_) => TypeNS,
423423
PathSource::Expr(..)
424424
| PathSource::Pat
425425
| PathSource::TupleStruct(..)
@@ -435,7 +435,7 @@ impl<'a> PathSource<'a> {
435435
PathSource::Type
436436
| PathSource::Expr(..)
437437
| PathSource::Pat
438-
| PathSource::Struct
438+
| PathSource::Struct(_)
439439
| PathSource::TupleStruct(..)
440440
| PathSource::ReturnTypeNotation => true,
441441
PathSource::Trait(_)
@@ -450,7 +450,7 @@ impl<'a> PathSource<'a> {
450450
PathSource::Type => "type",
451451
PathSource::Trait(_) => "trait",
452452
PathSource::Pat => "unit struct, unit variant or constant",
453-
PathSource::Struct => "struct, variant or union type",
453+
PathSource::Struct(_) => "struct, variant or union type",
454454
PathSource::TupleStruct(..) => "tuple struct or tuple variant",
455455
PathSource::TraitItem(ns) => match ns {
456456
TypeNS => "associated type",
@@ -531,7 +531,7 @@ impl<'a> PathSource<'a> {
531531
|| matches!(res, Res::Def(DefKind::Const | DefKind::AssocConst, _))
532532
}
533533
PathSource::TupleStruct(..) => res.expected_in_tuple_struct_pat(),
534-
PathSource::Struct => matches!(
534+
PathSource::Struct(_) => matches!(
535535
res,
536536
Res::Def(
537537
DefKind::Struct
@@ -571,8 +571,8 @@ impl<'a> PathSource<'a> {
571571
(PathSource::Trait(_), false) => E0405,
572572
(PathSource::Type, true) => E0573,
573573
(PathSource::Type, false) => E0412,
574-
(PathSource::Struct, true) => E0574,
575-
(PathSource::Struct, false) => E0422,
574+
(PathSource::Struct(_), true) => E0574,
575+
(PathSource::Struct(_), false) => E0422,
576576
(PathSource::Expr(..), true) | (PathSource::Delegation, true) => E0423,
577577
(PathSource::Expr(..), false) | (PathSource::Delegation, false) => E0425,
578578
(PathSource::Pat | PathSource::TupleStruct(..), true) => E0532,
@@ -1458,11 +1458,13 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
14581458
path: &[Segment],
14591459
opt_ns: Option<Namespace>, // `None` indicates a module path in import
14601460
finalize: Option<Finalize>,
1461+
source: PathSource<'ast>,
14611462
) -> PathResult<'ra> {
14621463
self.r.resolve_path_with_ribs(
14631464
path,
14641465
opt_ns,
14651466
&self.parent_scope,
1467+
Some(source),
14661468
finalize,
14671469
Some(&self.ribs),
14681470
None,
@@ -1967,7 +1969,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
19671969
| PathSource::ReturnTypeNotation => false,
19681970
PathSource::Expr(..)
19691971
| PathSource::Pat
1970-
| PathSource::Struct
1972+
| PathSource::Struct(_)
19711973
| PathSource::TupleStruct(..)
19721974
| PathSource::Delegation => true,
19731975
};
@@ -3805,7 +3807,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
38053807
self.smart_resolve_path(pat.id, qself, path, PathSource::Pat);
38063808
}
38073809
PatKind::Struct(ref qself, ref path, ref _fields, ref rest) => {
3808-
self.smart_resolve_path(pat.id, qself, path, PathSource::Struct);
3810+
self.smart_resolve_path(pat.id, qself, path, PathSource::Struct(None));
38093811
self.record_patterns_with_skipped_bindings(pat, rest);
38103812
}
38113813
PatKind::Or(ref ps) => {
@@ -4214,6 +4216,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
42144216
qself,
42154217
path,
42164218
ns,
4219+
source,
42174220
path_span,
42184221
source.defer_to_typeck(),
42194222
finalize,
@@ -4259,7 +4262,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
42594262
std_path.push(Segment::from_ident(Ident::with_dummy_span(sym::std)));
42604263
std_path.extend(path);
42614264
if let PathResult::Module(_) | PathResult::NonModule(_) =
4262-
self.resolve_path(&std_path, Some(ns), None)
4265+
self.resolve_path(&std_path, Some(ns), None, source)
42634266
{
42644267
// Check if we wrote `str::from_utf8` instead of `std::str::from_utf8`
42654268
let item_span =
@@ -4330,6 +4333,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
43304333
qself: &Option<P<QSelf>>,
43314334
path: &[Segment],
43324335
primary_ns: Namespace,
4336+
source: PathSource<'ast>,
43334337
span: Span,
43344338
defer_to_typeck: bool,
43354339
finalize: Finalize,
@@ -4338,7 +4342,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
43384342

43394343
for (i, &ns) in [primary_ns, TypeNS, ValueNS].iter().enumerate() {
43404344
if i == 0 || ns != primary_ns {
4341-
match self.resolve_qpath(qself, path, ns, finalize)? {
4345+
match self.resolve_qpath(qself, path, ns, source, finalize)? {
43424346
Some(partial_res)
43434347
if partial_res.unresolved_segments() == 0 || defer_to_typeck =>
43444348
{
@@ -4374,6 +4378,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
43744378
qself: &Option<P<QSelf>>,
43754379
path: &[Segment],
43764380
ns: Namespace,
4381+
source: PathSource<'ast>,
43774382
finalize: Finalize,
43784383
) -> Result<Option<PartialRes>, Spanned<ResolutionError<'ra>>> {
43794384
debug!(
@@ -4435,7 +4440,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
44354440
)));
44364441
}
44374442

4438-
let result = match self.resolve_path(path, Some(ns), Some(finalize)) {
4443+
let result = match self.resolve_path(path, Some(ns), Some(finalize), source) {
44394444
PathResult::NonModule(path_res) => path_res,
44404445
PathResult::Module(ModuleOrUniformRoot::Module(module)) if !module.is_normal() => {
44414446
PartialRes::new(module.res().unwrap())
@@ -4659,7 +4664,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
46594664
}
46604665

46614666
ExprKind::Struct(ref se) => {
4662-
self.smart_resolve_path(expr.id, &se.qself, &se.path, PathSource::Struct);
4667+
self.smart_resolve_path(expr.id, &se.qself, &se.path, PathSource::Struct(parent));
46634668
// This is the same as `visit::walk_expr(self, expr);`, but we want to pass the
46644669
// parent in for accurate suggestions when encountering `Foo { bar }` that should
46654670
// have been `Foo { bar: self.bar }`.

0 commit comments

Comments
 (0)