Skip to content

Commit 4828fc9

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 bf5e6cc commit 4828fc9

File tree

11 files changed

+290
-36
lines changed

11 files changed

+290
-36
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ provide! { tcx, def_id, other, cdata,
413413

414414
crate_extern_paths => { cdata.source().paths().cloned().collect() }
415415
expn_that_defined => { cdata.get_expn_that_defined(def_id.index, tcx.sess) }
416+
default_field => { cdata.get_default_field(def_id.index) }
416417
is_doc_hidden => { cdata.get_attr_flags(def_id.index).contains(AttrFlags::IS_DOC_HIDDEN) }
417418
doc_link_resolutions => { tcx.arena.alloc(cdata.get_doc_link_resolutions(def_id.index)) }
418419
doc_link_traits_in_scope => {

compiler/rustc_middle/src/query/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1854,6 +1854,13 @@ rustc_queries! {
18541854
feedable
18551855
}
18561856

1857+
/// Returns whether the impl or associated function has the `default` keyword.
1858+
query default_field(def_id: DefId) -> Option<DefId> {
1859+
desc { |tcx| "looking up the `const` corresponding to the default for `{}`", tcx.def_path_str(def_id) }
1860+
separate_provide_extern
1861+
feedable
1862+
}
1863+
18571864
query check_well_formed(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
18581865
desc { |tcx| "checking that `{}` is well-formed", tcx.def_path_str(key) }
18591866
return_result_from_ensure_ok

compiler/rustc_resolve/src/build_reduced_graph.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -388,14 +388,18 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
388388
// The fields are not expanded yet.
389389
return;
390390
}
391-
let fields = fields
391+
let field_name = |i, field: &ast::FieldDef| {
392+
field.ident.unwrap_or_else(|| Ident::from_str_and_span(&format!("{i}"), field.span))
393+
};
394+
let field_names: Vec<_> =
395+
fields.iter().enumerate().map(|(i, field)| field_name(i, field)).collect();
396+
let defaults = fields
392397
.iter()
393398
.enumerate()
394-
.map(|(i, field)| {
395-
field.ident.unwrap_or_else(|| Ident::from_str_and_span(&format!("{i}"), field.span))
396-
})
399+
.filter_map(|(i, field)| field.default.as_ref().map(|_| field_name(i, field).name))
397400
.collect();
398-
self.r.field_names.insert(def_id, fields);
401+
self.r.field_names.insert(def_id, field_names);
402+
self.r.field_defaults.insert(def_id, defaults);
399403
}
400404

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

compiler/rustc_resolve/src/diagnostics.rs

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1950,8 +1950,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
19501950
}
19511951

19521952
fn report_privacy_error(&mut self, privacy_error: &PrivacyError<'ra>) {
1953-
let PrivacyError { ident, binding, outermost_res, parent_scope, single_nested, dedup_span } =
1954-
*privacy_error;
1953+
let PrivacyError {
1954+
ident,
1955+
binding,
1956+
outermost_res,
1957+
parent_scope,
1958+
single_nested,
1959+
dedup_span,
1960+
ref source,
1961+
} = *privacy_error;
19551962

19561963
let res = binding.res();
19571964
let ctor_fields_span = self.ctor_fields_span(binding);
@@ -1967,6 +1974,55 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
19671974
let mut err =
19681975
self.dcx().create_err(errors::IsPrivate { span: ident.span, ident_descr, ident });
19691976

1977+
if let Some(expr) = source
1978+
&& let ast::ExprKind::Struct(struct_expr) = &expr.kind
1979+
&& let Some(Res::Def(_, def_id)) = self.partial_res_map
1980+
[&struct_expr.path.segments.iter().last().unwrap().id]
1981+
.full_res()
1982+
&& let Some(default_fields) = self.field_defaults(def_id)
1983+
&& !struct_expr.fields.is_empty()
1984+
{
1985+
let last_span = struct_expr.fields.iter().last().unwrap().span;
1986+
let mut iter = struct_expr.fields.iter().peekable();
1987+
let mut prev: Option<Span> = None;
1988+
while let Some(field) = iter.next() {
1989+
if field.expr.span.overlaps(ident.span) {
1990+
err.span_label(field.ident.span, "while setting this field");
1991+
if default_fields.contains(&field.ident.name) {
1992+
let sugg = if last_span == field.span {
1993+
vec![(field.span, "..".to_string())]
1994+
} else {
1995+
vec![
1996+
(
1997+
// Account for trailing commas and ensure we remove them.
1998+
match (prev, iter.peek()) {
1999+
(_, Some(next)) => field.span.with_hi(next.span.lo()),
2000+
(Some(prev), _) => field.span.with_lo(prev.hi()),
2001+
(None, None) => field.span,
2002+
},
2003+
String::new(),
2004+
),
2005+
(last_span.shrink_to_hi(), ", ..".to_string()),
2006+
]
2007+
};
2008+
err.multipart_suggestion_verbose(
2009+
format!(
2010+
"the type `{ident}` of field `{}` is private, but you can \
2011+
construct the default value defined for it in `{}` using `..` in \
2012+
the struct initializer expression",
2013+
field.ident,
2014+
self.tcx.item_name(def_id),
2015+
),
2016+
sugg,
2017+
Applicability::MachineApplicable,
2018+
);
2019+
break;
2020+
}
2021+
}
2022+
prev = Some(field.span);
2023+
}
2024+
}
2025+
19702026
let mut not_publicly_reexported = false;
19712027
if let Some((this_res, outer_ident)) = outermost_res {
19722028
let import_suggestions = self.lookup_import_candidates(

compiler/rustc_resolve/src/ident.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1020,6 +1020,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
10201020
binding,
10211021
dedup_span: path_span,
10221022
outermost_res: None,
1023+
source: None,
10231024
parent_scope: *parent_scope,
10241025
single_nested: path_span != root_span,
10251026
});
@@ -1426,7 +1427,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14261427
parent_scope: &ParentScope<'ra>,
14271428
ignore_import: Option<Import<'ra>>,
14281429
) -> PathResult<'ra> {
1429-
self.resolve_path_with_ribs(path, opt_ns, parent_scope, None, None, None, ignore_import)
1430+
self.resolve_path_with_ribs(
1431+
path,
1432+
opt_ns,
1433+
parent_scope,
1434+
None,
1435+
None,
1436+
None,
1437+
None,
1438+
ignore_import,
1439+
)
14301440
}
14311441

14321442
#[instrument(level = "debug", skip(self))]
@@ -1443,6 +1453,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14431453
path,
14441454
opt_ns,
14451455
parent_scope,
1456+
None,
14461457
finalize,
14471458
None,
14481459
ignore_binding,
@@ -1455,6 +1466,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14551466
path: &[Segment],
14561467
opt_ns: Option<Namespace>, // `None` indicates a module path in import
14571468
parent_scope: &ParentScope<'ra>,
1469+
source: Option<PathSource<'_, '_, '_>>,
14581470
finalize: Option<Finalize>,
14591471
ribs: Option<&PerNS<Vec<Rib<'ra>>>>,
14601472
ignore_binding: Option<NameBinding<'ra>>,
@@ -1629,6 +1641,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
16291641
// the user it is not accessible.
16301642
for error in &mut self.privacy_errors[privacy_errors_len..] {
16311643
error.outermost_res = Some((res, ident));
1644+
error.source = match source {
1645+
Some(PathSource::Struct(Some(expr)))
1646+
| Some(PathSource::Expr(Some(expr))) => Some(expr.clone()),
1647+
_ => None,
1648+
};
16321649
}
16331650

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

compiler/rustc_resolve/src/late.rs

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ pub(crate) enum PathSource<'a, 'ast, 'ra> {
425425
/// Paths in path patterns `Path`.
426426
Pat,
427427
/// Paths in struct expressions and patterns `Path { .. }`.
428-
Struct,
428+
Struct(Option<&'a Expr>),
429429
/// Paths in tuple struct patterns `Path(..)`.
430430
TupleStruct(Span, &'ra [Span]),
431431
/// `m::A::B` in `<T as m::A>::B::C`.
@@ -448,7 +448,7 @@ impl PathSource<'_, '_, '_> {
448448
match self {
449449
PathSource::Type
450450
| PathSource::Trait(_)
451-
| PathSource::Struct
451+
| PathSource::Struct(_)
452452
| PathSource::DefineOpaques => TypeNS,
453453
PathSource::Expr(..)
454454
| PathSource::Pat
@@ -465,7 +465,7 @@ impl PathSource<'_, '_, '_> {
465465
PathSource::Type
466466
| PathSource::Expr(..)
467467
| PathSource::Pat
468-
| PathSource::Struct
468+
| PathSource::Struct(_)
469469
| PathSource::TupleStruct(..)
470470
| PathSource::ReturnTypeNotation => true,
471471
PathSource::Trait(_)
@@ -482,7 +482,7 @@ impl PathSource<'_, '_, '_> {
482482
PathSource::Type => "type",
483483
PathSource::Trait(_) => "trait",
484484
PathSource::Pat => "unit struct, unit variant or constant",
485-
PathSource::Struct => "struct, variant or union type",
485+
PathSource::Struct(_) => "struct, variant or union type",
486486
PathSource::TraitItem(ValueNS, PathSource::TupleStruct(..))
487487
| PathSource::TupleStruct(..) => "tuple struct or tuple variant",
488488
PathSource::TraitItem(ns, _) => match ns {
@@ -577,7 +577,7 @@ impl PathSource<'_, '_, '_> {
577577
|| matches!(res, Res::Def(DefKind::Const | DefKind::AssocConst, _))
578578
}
579579
PathSource::TupleStruct(..) => res.expected_in_tuple_struct_pat(),
580-
PathSource::Struct => matches!(
580+
PathSource::Struct(_) => matches!(
581581
res,
582582
Res::Def(
583583
DefKind::Struct
@@ -617,8 +617,8 @@ impl PathSource<'_, '_, '_> {
617617
(PathSource::Trait(_), false) => E0405,
618618
(PathSource::Type | PathSource::DefineOpaques, true) => E0573,
619619
(PathSource::Type | PathSource::DefineOpaques, false) => E0412,
620-
(PathSource::Struct, true) => E0574,
621-
(PathSource::Struct, false) => E0422,
620+
(PathSource::Struct(_), true) => E0574,
621+
(PathSource::Struct(_), false) => E0422,
622622
(PathSource::Expr(..), true) | (PathSource::Delegation, true) => E0423,
623623
(PathSource::Expr(..), false) | (PathSource::Delegation, false) => E0425,
624624
(PathSource::Pat | PathSource::TupleStruct(..), true) => E0532,
@@ -1511,11 +1511,13 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
15111511
path: &[Segment],
15121512
opt_ns: Option<Namespace>, // `None` indicates a module path in import
15131513
finalize: Option<Finalize>,
1514+
source: PathSource<'_, 'ast, 'ra>,
15141515
) -> PathResult<'ra> {
15151516
self.r.resolve_path_with_ribs(
15161517
path,
15171518
opt_ns,
15181519
&self.parent_scope,
1520+
Some(source),
15191521
finalize,
15201522
Some(&self.ribs),
15211523
None,
@@ -1995,7 +1997,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
19951997
&mut self,
19961998
partial_res: PartialRes,
19971999
path: &[Segment],
1998-
source: PathSource<'_, '_, '_>,
2000+
source: PathSource<'_, 'ast, 'ra>,
19992001
path_span: Span,
20002002
) {
20012003
let proj_start = path.len() - partial_res.unresolved_segments();
@@ -2048,7 +2050,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
20482050
| PathSource::ReturnTypeNotation => false,
20492051
PathSource::Expr(..)
20502052
| PathSource::Pat
2051-
| PathSource::Struct
2053+
| PathSource::Struct(_)
20522054
| PathSource::TupleStruct(..)
20532055
| PathSource::DefineOpaques
20542056
| PathSource::Delegation => true,
@@ -3878,7 +3880,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
38783880
self.smart_resolve_path(pat.id, qself, path, PathSource::Pat);
38793881
}
38803882
PatKind::Struct(ref qself, ref path, ref _fields, ref rest) => {
3881-
self.smart_resolve_path(pat.id, qself, path, PathSource::Struct);
3883+
self.smart_resolve_path(pat.id, qself, path, PathSource::Struct(None));
38823884
self.record_patterns_with_skipped_bindings(pat, rest);
38833885
}
38843886
PatKind::Or(ref ps) => {
@@ -4122,7 +4124,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
41224124
id: NodeId,
41234125
qself: &Option<P<QSelf>>,
41244126
path: &Path,
4125-
source: PathSource<'_, 'ast, '_>,
4127+
source: PathSource<'_, 'ast, 'ra>,
41264128
) {
41274129
self.smart_resolve_path_fragment(
41284130
qself,
@@ -4139,7 +4141,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
41394141
&mut self,
41404142
qself: &Option<P<QSelf>>,
41414143
path: &[Segment],
4142-
source: PathSource<'_, 'ast, '_>,
4144+
source: PathSource<'_, 'ast, 'ra>,
41434145
finalize: Finalize,
41444146
record_partial_res: RecordPartialRes,
41454147
parent_qself: Option<&QSelf>,
@@ -4369,7 +4371,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
43694371
std_path.push(Segment::from_ident(Ident::with_dummy_span(sym::std)));
43704372
std_path.extend(path);
43714373
if let PathResult::Module(_) | PathResult::NonModule(_) =
4372-
self.resolve_path(&std_path, Some(ns), None)
4374+
self.resolve_path(&std_path, Some(ns), None, source)
43734375
{
43744376
// Check if we wrote `str::from_utf8` instead of `std::str::from_utf8`
43754377
let item_span =
@@ -4443,7 +4445,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
44434445
span: Span,
44444446
defer_to_typeck: bool,
44454447
finalize: Finalize,
4446-
source: PathSource<'_, 'ast, '_>,
4448+
source: PathSource<'_, 'ast, 'ra>,
44474449
) -> Result<Option<PartialRes>, Spanned<ResolutionError<'ra>>> {
44484450
let mut fin_res = None;
44494451

@@ -4486,7 +4488,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
44864488
path: &[Segment],
44874489
ns: Namespace,
44884490
finalize: Finalize,
4489-
source: PathSource<'_, 'ast, '_>,
4491+
source: PathSource<'_, 'ast, 'ra>,
44904492
) -> Result<Option<PartialRes>, Spanned<ResolutionError<'ra>>> {
44914493
debug!(
44924494
"resolve_qpath(qself={:?}, path={:?}, ns={:?}, finalize={:?})",
@@ -4549,7 +4551,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
45494551
)));
45504552
}
45514553

4552-
let result = match self.resolve_path(path, Some(ns), Some(finalize)) {
4554+
let result = match self.resolve_path(path, Some(ns), Some(finalize), source) {
45534555
PathResult::NonModule(path_res) => path_res,
45544556
PathResult::Module(ModuleOrUniformRoot::Module(module)) if !module.is_normal() => {
45554557
PartialRes::new(module.res().unwrap())
@@ -4772,7 +4774,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
47724774
}
47734775

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

0 commit comments

Comments
 (0)