Skip to content

Commit 464a6b1

Browse files
estebankestebank
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 18eeac0 commit 464a6b1

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
@@ -395,6 +395,7 @@ provide! { tcx, def_id, other, cdata,
395395

396396
crate_extern_paths => { cdata.source().paths().cloned().collect() }
397397
expn_that_defined => { cdata.get_expn_that_defined(def_id.index, tcx.sess) }
398+
default_field => { cdata.get_default_field(def_id.index) }
398399
is_doc_hidden => { cdata.get_attr_flags(def_id.index).contains(AttrFlags::IS_DOC_HIDDEN) }
399400
doc_link_resolutions => { tcx.arena.alloc(cdata.get_doc_link_resolutions(def_id.index)) }
400401
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
@@ -1864,6 +1864,13 @@ rustc_queries! {
18641864
feedable
18651865
}
18661866

1867+
/// Returns whether the impl or associated function has the `default` keyword.
1868+
query default_field(def_id: DefId) -> Option<DefId> {
1869+
desc { |tcx| "looking up the `const` corresponding to the default for `{}`", tcx.def_path_str(def_id) }
1870+
separate_provide_extern
1871+
feedable
1872+
}
1873+
18671874
query check_well_formed(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
18681875
desc { |tcx| "checking that `{}` is well-formed", tcx.def_path_str(key) }
18691876
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
@@ -420,14 +420,18 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
420420
// The fields are not expanded yet.
421421
return;
422422
}
423-
let fields = fields
423+
let field_name = |i, field: &ast::FieldDef| {
424+
field.ident.unwrap_or_else(|| Ident::from_str_and_span(&format!("{i}"), field.span))
425+
};
426+
let field_names: Vec<_> =
427+
fields.iter().enumerate().map(|(i, field)| field_name(i, field)).collect();
428+
let defaults = fields
424429
.iter()
425430
.enumerate()
426-
.map(|(i, field)| {
427-
field.ident.unwrap_or_else(|| Ident::from_str_and_span(&format!("{i}"), field.span))
428-
})
431+
.filter_map(|(i, field)| field.default.as_ref().map(|_| field_name(i, field).name))
429432
.collect();
430-
self.r.field_names.insert(def_id, fields);
433+
self.r.field_names.insert(def_id, field_names);
434+
self.r.field_defaults.insert(def_id, defaults);
431435
}
432436

433437
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
@@ -1943,8 +1943,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
19431943
}
19441944

19451945
fn report_privacy_error(&mut self, privacy_error: &PrivacyError<'ra>) {
1946-
let PrivacyError { ident, binding, outermost_res, parent_scope, single_nested, dedup_span } =
1947-
*privacy_error;
1946+
let PrivacyError {
1947+
ident,
1948+
binding,
1949+
outermost_res,
1950+
parent_scope,
1951+
single_nested,
1952+
dedup_span,
1953+
ref source,
1954+
} = *privacy_error;
19481955

19491956
let res = binding.res();
19501957
let ctor_fields_span = self.ctor_fields_span(binding);
@@ -1960,6 +1967,55 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
19601967
let mut err =
19611968
self.dcx().create_err(errors::IsPrivate { span: ident.span, ident_descr, ident });
19621969

1970+
if let Some(expr) = source
1971+
&& let ast::ExprKind::Struct(struct_expr) = &expr.kind
1972+
&& let Some(Res::Def(_, def_id)) = self.partial_res_map
1973+
[&struct_expr.path.segments.iter().last().unwrap().id]
1974+
.full_res()
1975+
&& let Some(default_fields) = self.field_defaults(def_id)
1976+
&& !struct_expr.fields.is_empty()
1977+
{
1978+
let last_span = struct_expr.fields.iter().last().unwrap().span;
1979+
let mut iter = struct_expr.fields.iter().peekable();
1980+
let mut prev: Option<Span> = None;
1981+
while let Some(field) = iter.next() {
1982+
if field.expr.span.overlaps(ident.span) {
1983+
err.span_label(field.ident.span, "while setting this field");
1984+
if default_fields.contains(&field.ident.name) {
1985+
let sugg = if last_span == field.span {
1986+
vec![(field.span, "..".to_string())]
1987+
} else {
1988+
vec![
1989+
(
1990+
// Account for trailing commas and ensure we remove them.
1991+
match (prev, iter.peek()) {
1992+
(_, Some(next)) => field.span.with_hi(next.span.lo()),
1993+
(Some(prev), _) => field.span.with_lo(prev.hi()),
1994+
(None, None) => field.span,
1995+
},
1996+
String::new(),
1997+
),
1998+
(last_span.shrink_to_hi(), ", ..".to_string()),
1999+
]
2000+
};
2001+
err.multipart_suggestion_verbose(
2002+
format!(
2003+
"the type `{ident}` of field `{}` is private, but you can \
2004+
construct the default value defined for it in `{}` using `..` in \
2005+
the struct initializer expression",
2006+
field.ident,
2007+
self.tcx.item_name(def_id),
2008+
),
2009+
sugg,
2010+
Applicability::MachineApplicable,
2011+
);
2012+
break;
2013+
}
2014+
}
2015+
prev = Some(field.span);
2016+
}
2017+
}
2018+
19632019
let mut not_publicly_reexported = false;
19642020
if let Some((this_res, outer_ident)) = outermost_res {
19652021
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
@@ -1029,6 +1029,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
10291029
binding,
10301030
dedup_span: path_span,
10311031
outermost_res: None,
1032+
source: None,
10321033
parent_scope: *parent_scope,
10331034
single_nested: path_span != root_span,
10341035
});
@@ -1435,7 +1436,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14351436
parent_scope: &ParentScope<'ra>,
14361437
ignore_import: Option<Import<'ra>>,
14371438
) -> PathResult<'ra> {
1438-
self.resolve_path_with_ribs(path, opt_ns, parent_scope, None, None, None, ignore_import)
1439+
self.resolve_path_with_ribs(
1440+
path,
1441+
opt_ns,
1442+
parent_scope,
1443+
None,
1444+
None,
1445+
None,
1446+
None,
1447+
ignore_import,
1448+
)
14391449
}
14401450
#[instrument(level = "debug", skip(self))]
14411451
pub(crate) fn resolve_path<'r>(
@@ -1451,6 +1461,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14511461
path,
14521462
opt_ns,
14531463
parent_scope,
1464+
None,
14541465
finalize,
14551466
None,
14561467
ignore_binding,
@@ -1463,6 +1474,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14631474
path: &[Segment],
14641475
opt_ns: Option<Namespace>, // `None` indicates a module path in import
14651476
parent_scope: &ParentScope<'ra>,
1477+
source: Option<PathSource<'_, '_, '_>>,
14661478
finalize: Option<Finalize>,
14671479
ribs: Option<&PerNS<Vec<Rib<'ra>>>>,
14681480
ignore_binding: Option<NameBinding<'ra>>,
@@ -1645,6 +1657,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
16451657
if finalize.is_some() {
16461658
for error in &mut self.get_mut().privacy_errors[privacy_errors_len..] {
16471659
error.outermost_res = Some((res, ident));
1660+
error.source = match source {
1661+
Some(PathSource::Struct(Some(expr)))
1662+
| Some(PathSource::Expr(Some(expr))) => Some(expr.clone()),
1663+
_ => None,
1664+
};
16481665
}
16491666
}
16501667

‎compiler/rustc_resolve/src/late.rs‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ pub(crate) enum PathSource<'a, 'ast, 'ra> {
424424
/// Paths in path patterns `Path`.
425425
Pat,
426426
/// Paths in struct expressions and patterns `Path { .. }`.
427-
Struct,
427+
Struct(Option<&'a Expr>),
428428
/// Paths in tuple struct patterns `Path(..)`.
429429
TupleStruct(Span, &'ra [Span]),
430430
/// `m::A::B` in `<T as m::A>::B::C`.
@@ -447,7 +447,7 @@ impl PathSource<'_, '_, '_> {
447447
match self {
448448
PathSource::Type
449449
| PathSource::Trait(_)
450-
| PathSource::Struct
450+
| PathSource::Struct(_)
451451
| PathSource::DefineOpaques => TypeNS,
452452
PathSource::Expr(..)
453453
| PathSource::Pat
@@ -464,7 +464,7 @@ impl PathSource<'_, '_, '_> {
464464
PathSource::Type
465465
| PathSource::Expr(..)
466466
| PathSource::Pat
467-
| PathSource::Struct
467+
| PathSource::Struct(_)
468468
| PathSource::TupleStruct(..)
469469
| PathSource::ReturnTypeNotation => true,
470470
PathSource::Trait(_)
@@ -481,7 +481,7 @@ impl PathSource<'_, '_, '_> {
481481
PathSource::Type => "type",
482482
PathSource::Trait(_) => "trait",
483483
PathSource::Pat => "unit struct, unit variant or constant",
484-
PathSource::Struct => "struct, variant or union type",
484+
PathSource::Struct(_) => "struct, variant or union type",
485485
PathSource::TraitItem(ValueNS, PathSource::TupleStruct(..))
486486
| PathSource::TupleStruct(..) => "tuple struct or tuple variant",
487487
PathSource::TraitItem(ns, _) => match ns {
@@ -576,7 +576,7 @@ impl PathSource<'_, '_, '_> {
576576
|| matches!(res, Res::Def(DefKind::Const | DefKind::AssocConst, _))
577577
}
578578
PathSource::TupleStruct(..) => res.expected_in_tuple_struct_pat(),
579-
PathSource::Struct => matches!(
579+
PathSource::Struct(_) => matches!(
580580
res,
581581
Res::Def(
582582
DefKind::Struct
@@ -616,8 +616,8 @@ impl PathSource<'_, '_, '_> {
616616
(PathSource::Trait(_), false) => E0405,
617617
(PathSource::Type | PathSource::DefineOpaques, true) => E0573,
618618
(PathSource::Type | PathSource::DefineOpaques, false) => E0412,
619-
(PathSource::Struct, true) => E0574,
620-
(PathSource::Struct, false) => E0422,
619+
(PathSource::Struct(_), true) => E0574,
620+
(PathSource::Struct(_), false) => E0422,
621621
(PathSource::Expr(..), true) | (PathSource::Delegation, true) => E0423,
622622
(PathSource::Expr(..), false) | (PathSource::Delegation, false) => E0425,
623623
(PathSource::Pat | PathSource::TupleStruct(..), true) => E0532,
@@ -1482,11 +1482,13 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
14821482
path: &[Segment],
14831483
opt_ns: Option<Namespace>, // `None` indicates a module path in import
14841484
finalize: Option<Finalize>,
1485+
source: PathSource<'_, 'ast, 'ra>,
14851486
) -> PathResult<'ra> {
14861487
self.r.cm().resolve_path_with_ribs(
14871488
path,
14881489
opt_ns,
14891490
&self.parent_scope,
1491+
Some(source),
14901492
finalize,
14911493
Some(&self.ribs),
14921494
None,
@@ -1966,7 +1968,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
19661968
&mut self,
19671969
partial_res: PartialRes,
19681970
path: &[Segment],
1969-
source: PathSource<'_, '_, '_>,
1971+
source: PathSource<'_, 'ast, 'ra>,
19701972
path_span: Span,
19711973
) {
19721974
let proj_start = path.len() - partial_res.unresolved_segments();
@@ -2019,7 +2021,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
20192021
| PathSource::ReturnTypeNotation => false,
20202022
PathSource::Expr(..)
20212023
| PathSource::Pat
2022-
| PathSource::Struct
2024+
| PathSource::Struct(_)
20232025
| PathSource::TupleStruct(..)
20242026
| PathSource::DefineOpaques
20252027
| PathSource::Delegation => true,
@@ -3866,7 +3868,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
38663868
self.smart_resolve_path(pat.id, qself, path, PathSource::Pat);
38673869
}
38683870
PatKind::Struct(ref qself, ref path, ref _fields, ref rest) => {
3869-
self.smart_resolve_path(pat.id, qself, path, PathSource::Struct);
3871+
self.smart_resolve_path(pat.id, qself, path, PathSource::Struct(None));
38703872
self.record_patterns_with_skipped_bindings(pat, rest);
38713873
}
38723874
PatKind::Or(ref ps) => {
@@ -4110,7 +4112,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
41104112
id: NodeId,
41114113
qself: &Option<Box<QSelf>>,
41124114
path: &Path,
4113-
source: PathSource<'_, 'ast, '_>,
4115+
source: PathSource<'_, 'ast, 'ra>,
41144116
) {
41154117
self.smart_resolve_path_fragment(
41164118
qself,
@@ -4127,7 +4129,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
41274129
&mut self,
41284130
qself: &Option<Box<QSelf>>,
41294131
path: &[Segment],
4130-
source: PathSource<'_, 'ast, '_>,
4132+
source: PathSource<'_, 'ast, 'ra>,
41314133
finalize: Finalize,
41324134
record_partial_res: RecordPartialRes,
41334135
parent_qself: Option<&QSelf>,
@@ -4365,7 +4367,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
43654367
std_path.push(Segment::from_ident(Ident::with_dummy_span(sym::std)));
43664368
std_path.extend(path);
43674369
if let PathResult::Module(_) | PathResult::NonModule(_) =
4368-
self.resolve_path(&std_path, Some(ns), None)
4370+
self.resolve_path(&std_path, Some(ns), None, source)
43694371
{
43704372
// Check if we wrote `str::from_utf8` instead of `std::str::from_utf8`
43714373
let item_span =
@@ -4439,7 +4441,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
44394441
span: Span,
44404442
defer_to_typeck: bool,
44414443
finalize: Finalize,
4442-
source: PathSource<'_, 'ast, '_>,
4444+
source: PathSource<'_, 'ast, 'ra>,
44434445
) -> Result<Option<PartialRes>, Spanned<ResolutionError<'ra>>> {
44444446
let mut fin_res = None;
44454447

@@ -4488,7 +4490,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
44884490
path: &[Segment],
44894491
ns: Namespace,
44904492
finalize: Finalize,
4491-
source: PathSource<'_, 'ast, '_>,
4493+
source: PathSource<'_, 'ast, 'ra>,
44924494
) -> Result<Option<PartialRes>, Spanned<ResolutionError<'ra>>> {
44934495
debug!(
44944496
"resolve_qpath(qself={:?}, path={:?}, ns={:?}, finalize={:?})",
@@ -4551,7 +4553,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
45514553
)));
45524554
}
45534555

4554-
let result = match self.resolve_path(path, Some(ns), Some(finalize)) {
4556+
let result = match self.resolve_path(path, Some(ns), Some(finalize), source) {
45554557
PathResult::NonModule(path_res) => path_res,
45564558
PathResult::Module(ModuleOrUniformRoot::Module(module)) if !module.is_normal() => {
45574559
PartialRes::new(module.res().unwrap())
@@ -4774,7 +4776,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
47744776
}
47754777

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

0 commit comments

Comments
 (0)