Skip to content

Commit 21c50ce

Browse files
borsbors
committed
Auto merge of #135846 - estebank:non-exhaustive-dfv-ctor-2, r=BoxyUwU
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-2.rs:19: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-2.rs:14:4 | LL | struct Priv1 {} | ^^^^^^^^^^^^ help: the type `Priv1` of field `field1` is private, but you can construct the default value defined for it in `S` using `..` in the struct initializer expression | LL | let _ = S { field: (), .. }; | ~~ ```
2 parents a955f1c + c41fee3 commit 21c50ce

File tree

11 files changed

+321
-36
lines changed

11 files changed

+321
-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: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1863,6 +1863,12 @@ rustc_queries! {
18631863
feedable
18641864
}
18651865

1866+
/// Returns whether the field corresponding to the `DefId` has a default field value.
1867+
query default_field(def_id: DefId) -> Option<DefId> {
1868+
desc { |tcx| "looking up the `const` corresponding to the default for `{}`", tcx.def_path_str(def_id) }
1869+
separate_provide_extern
1870+
}
1871+
18661872
query check_well_formed(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
18671873
desc { |tcx| "checking that `{}` is well-formed", tcx.def_path_str(key) }
18681874
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
@@ -391,14 +391,18 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
391391
// The fields are not expanded yet.
392392
return;
393393
}
394-
let fields = fields
394+
let field_name = |i, field: &ast::FieldDef| {
395+
field.ident.unwrap_or_else(|| Ident::from_str_and_span(&format!("{i}"), field.span))
396+
};
397+
let field_names: Vec<_> =
398+
fields.iter().enumerate().map(|(i, field)| field_name(i, field)).collect();
399+
let defaults = fields
395400
.iter()
396401
.enumerate()
397-
.map(|(i, field)| {
398-
field.ident.unwrap_or_else(|| Ident::from_str_and_span(&format!("{i}"), field.span))
399-
})
402+
.filter_map(|(i, field)| field.default.as_ref().map(|_| field_name(i, field).name))
400403
.collect();
401-
self.r.field_names.insert(def_id, fields);
404+
self.r.field_names.insert(def_id, field_names);
405+
self.r.field_defaults.insert(def_id, defaults);
402406
}
403407

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

‎compiler/rustc_resolve/src/diagnostics.rs‎

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1946,8 +1946,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
19461946
}
19471947

19481948
fn report_privacy_error(&mut self, privacy_error: &PrivacyError<'ra>) {
1949-
let PrivacyError { ident, binding, outermost_res, parent_scope, single_nested, dedup_span } =
1950-
*privacy_error;
1949+
let PrivacyError {
1950+
ident,
1951+
binding,
1952+
outermost_res,
1953+
parent_scope,
1954+
single_nested,
1955+
dedup_span,
1956+
ref source,
1957+
} = *privacy_error;
19511958

19521959
let res = binding.res();
19531960
let ctor_fields_span = self.ctor_fields_span(binding);
@@ -1963,6 +1970,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
19631970
let mut err =
19641971
self.dcx().create_err(errors::IsPrivate { span: ident.span, ident_descr, ident });
19651972

1973+
self.mention_default_field_values(source, ident, &mut err);
1974+
19661975
let mut not_publicly_reexported = false;
19671976
if let Some((this_res, outer_ident)) = outermost_res {
19681977
let import_suggestions = self.lookup_import_candidates(
@@ -2144,6 +2153,85 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
21442153
err.emit();
21452154
}
21462155

2156+
/// When a private field is being set that has a default field value, we suggest using `..` and
2157+
/// setting the value of that field implicitly with its default.
2158+
///
2159+
/// If we encounter code like
2160+
/// ```text
2161+
/// struct Priv;
2162+
/// pub struct S {
2163+
/// pub field: Priv = Priv,
2164+
/// }
2165+
/// ```
2166+
/// which is used from a place where `Priv` isn't accessible
2167+
/// ```text
2168+
/// let _ = S { field: m::Priv1 {} };
2169+
/// // ^^^^^ private struct
2170+
/// ```
2171+
/// we will suggest instead using the `default_field_values` syntax instead:
2172+
/// ```text
2173+
/// let _ = S { .. };
2174+
/// ```
2175+
fn mention_default_field_values(
2176+
&self,
2177+
source: &Option<ast::Expr>,
2178+
ident: Ident,
2179+
err: &mut Diag<'_>,
2180+
) {
2181+
let Some(expr) = source else { return };
2182+
let ast::ExprKind::Struct(struct_expr) = &expr.kind else { return };
2183+
// We don't have to handle type-relative paths because they're forbidden in ADT
2184+
// expressions, but that would change with `#[feature(more_qualified_paths)]`.
2185+
let Some(Res::Def(_, def_id)) =
2186+
self.partial_res_map[&struct_expr.path.segments.iter().last().unwrap().id].full_res()
2187+
else {
2188+
return;
2189+
};
2190+
let Some(default_fields) = self.field_defaults(def_id) else { return };
2191+
if struct_expr.fields.is_empty() {
2192+
return;
2193+
}
2194+
let last_span = struct_expr.fields.iter().last().unwrap().span;
2195+
let mut iter = struct_expr.fields.iter().peekable();
2196+
let mut prev: Option<Span> = None;
2197+
while let Some(field) = iter.next() {
2198+
if field.expr.span.overlaps(ident.span) {
2199+
err.span_label(field.ident.span, "while setting this field");
2200+
if default_fields.contains(&field.ident.name) {
2201+
let sugg = if last_span == field.span {
2202+
vec![(field.span, "..".to_string())]
2203+
} else {
2204+
vec![
2205+
(
2206+
// Account for trailing commas and ensure we remove them.
2207+
match (prev, iter.peek()) {
2208+
(_, Some(next)) => field.span.with_hi(next.span.lo()),
2209+
(Some(prev), _) => field.span.with_lo(prev.hi()),
2210+
(None, None) => field.span,
2211+
},
2212+
String::new(),
2213+
),
2214+
(last_span.shrink_to_hi(), ", ..".to_string()),
2215+
]
2216+
};
2217+
err.multipart_suggestion_verbose(
2218+
format!(
2219+
"the type `{ident}` of field `{}` is private, but you can construct \
2220+
the default value defined for it in `{}` using `..` in the struct \
2221+
initializer expression",
2222+
field.ident,
2223+
self.tcx.item_name(def_id),
2224+
),
2225+
sugg,
2226+
Applicability::MachineApplicable,
2227+
);
2228+
break;
2229+
}
2230+
}
2231+
prev = Some(field.span);
2232+
}
2233+
}
2234+
21472235
pub(crate) fn find_similarly_named_module_or_crate(
21482236
&self,
21492237
ident: Symbol,

‎compiler/rustc_resolve/src/ident.rs‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1005,6 +1005,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
10051005
binding,
10061006
dedup_span: path_span,
10071007
outermost_res: None,
1008+
source: None,
10081009
parent_scope: *parent_scope,
10091010
single_nested: path_span != root_span,
10101011
});
@@ -1411,7 +1412,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14111412
parent_scope: &ParentScope<'ra>,
14121413
ignore_import: Option<Import<'ra>>,
14131414
) -> PathResult<'ra> {
1414-
self.resolve_path_with_ribs(path, opt_ns, parent_scope, None, None, None, ignore_import)
1415+
self.resolve_path_with_ribs(
1416+
path,
1417+
opt_ns,
1418+
parent_scope,
1419+
None,
1420+
None,
1421+
None,
1422+
None,
1423+
ignore_import,
1424+
)
14151425
}
14161426

14171427
#[instrument(level = "debug", skip(self))]
@@ -1428,6 +1438,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14281438
path,
14291439
opt_ns,
14301440
parent_scope,
1441+
None,
14311442
finalize,
14321443
None,
14331444
ignore_binding,
@@ -1440,6 +1451,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14401451
path: &[Segment],
14411452
opt_ns: Option<Namespace>, // `None` indicates a module path in import
14421453
parent_scope: &ParentScope<'ra>,
1454+
source: Option<PathSource<'_, '_, '_>>,
14431455
finalize: Option<Finalize>,
14441456
ribs: Option<&PerNS<Vec<Rib<'ra>>>>,
14451457
ignore_binding: Option<NameBinding<'ra>>,
@@ -1615,6 +1627,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
16151627
// the user it is not accessible.
16161628
for error in &mut self.privacy_errors[privacy_errors_len..] {
16171629
error.outermost_res = Some((res, ident));
1630+
error.source = match source {
1631+
Some(PathSource::Struct(Some(expr)))
1632+
| Some(PathSource::Expr(Some(expr))) => Some(expr.clone()),
1633+
_ => None,
1634+
};
16181635
}
16191636

16201637
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,
@@ -3890,7 +3892,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
38903892
self.smart_resolve_path(pat.id, qself, path, PathSource::Pat);
38913893
}
38923894
PatKind::Struct(ref qself, ref path, ref _fields, ref rest) => {
3893-
self.smart_resolve_path(pat.id, qself, path, PathSource::Struct);
3895+
self.smart_resolve_path(pat.id, qself, path, PathSource::Struct(None));
38943896
self.record_patterns_with_skipped_bindings(pat, rest);
38953897
}
38963898
PatKind::Or(ref ps) => {
@@ -4134,7 +4136,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
41344136
id: NodeId,
41354137
qself: &Option<P<QSelf>>,
41364138
path: &Path,
4137-
source: PathSource<'_, 'ast, '_>,
4139+
source: PathSource<'_, 'ast, 'ra>,
41384140
) {
41394141
self.smart_resolve_path_fragment(
41404142
qself,
@@ -4151,7 +4153,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
41514153
&mut self,
41524154
qself: &Option<P<QSelf>>,
41534155
path: &[Segment],
4154-
source: PathSource<'_, 'ast, '_>,
4156+
source: PathSource<'_, 'ast, 'ra>,
41554157
finalize: Finalize,
41564158
record_partial_res: RecordPartialRes,
41574159
parent_qself: Option<&QSelf>,
@@ -4381,7 +4383,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
43814383
std_path.push(Segment::from_ident(Ident::with_dummy_span(sym::std)));
43824384
std_path.extend(path);
43834385
if let PathResult::Module(_) | PathResult::NonModule(_) =
4384-
self.resolve_path(&std_path, Some(ns), None)
4386+
self.resolve_path(&std_path, Some(ns), None, source)
43854387
{
43864388
// Check if we wrote `str::from_utf8` instead of `std::str::from_utf8`
43874389
let item_span =
@@ -4455,7 +4457,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
44554457
span: Span,
44564458
defer_to_typeck: bool,
44574459
finalize: Finalize,
4458-
source: PathSource<'_, 'ast, '_>,
4460+
source: PathSource<'_, 'ast, 'ra>,
44594461
) -> Result<Option<PartialRes>, Spanned<ResolutionError<'ra>>> {
44604462
let mut fin_res = None;
44614463

@@ -4498,7 +4500,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
44984500
path: &[Segment],
44994501
ns: Namespace,
45004502
finalize: Finalize,
4501-
source: PathSource<'_, 'ast, '_>,
4503+
source: PathSource<'_, 'ast, 'ra>,
45024504
) -> Result<Option<PartialRes>, Spanned<ResolutionError<'ra>>> {
45034505
debug!(
45044506
"resolve_qpath(qself={:?}, path={:?}, ns={:?}, finalize={:?})",
@@ -4561,7 +4563,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
45614563
)));
45624564
}
45634565

4564-
let result = match self.resolve_path(path, Some(ns), Some(finalize)) {
4566+
let result = match self.resolve_path(path, Some(ns), Some(finalize), source) {
45654567
PathResult::NonModule(path_res) => path_res,
45664568
PathResult::Module(ModuleOrUniformRoot::Module(module)) if !module.is_normal() => {
45674569
PartialRes::new(module.res().unwrap())
@@ -4784,7 +4786,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
47844786
}
47854787

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

0 commit comments

Comments
 (0)