Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit single privacy error for struct literal with multiple private fields and add test for default_field_values privacy #135700

Merged
merged 3 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions compiler/rustc_privacy/messages.ftl
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
privacy_field_is_private = field `{$field_name}` of {$variant_descr} `{$def_path_str}` is private
privacy_field_is_private_is_update_syntax_label = field `{$field_name}` is private
privacy_field_is_private =
{$len ->
[1] field
*[other] fields
} {$field_names} of {$variant_descr} `{$def_path_str}` {$len ->
[1] is
*[other] are
} private
.label = in this type
privacy_field_is_private_is_update_syntax_label = {$rest_len ->
[1] field
*[other] fields
} {$rest_field_names} {$rest_len ->
[1] is
*[other] are
} private
privacy_field_is_private_label = private field

privacy_from_private_dep_in_public_interface =
Expand Down
14 changes: 9 additions & 5 deletions compiler/rustc_privacy/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
use rustc_errors::DiagArgFromDisplay;
use rustc_errors::codes::*;
use rustc_errors::{DiagArgFromDisplay, MultiSpan};
use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
use rustc_span::{Span, Symbol};

#[derive(Diagnostic)]
#[diag(privacy_field_is_private, code = E0451)]
pub(crate) struct FieldIsPrivate {
#[primary_span]
pub span: Span,
pub field_name: Symbol,
pub span: MultiSpan,
#[label]
pub struct_span: Option<Span>,
pub field_names: String,
pub variant_descr: &'static str,
pub def_path_str: String,
#[subdiagnostic]
pub label: FieldIsPrivateLabel,
pub labels: Vec<FieldIsPrivateLabel>,
pub len: usize,
}

#[derive(Subdiagnostic)]
Expand All @@ -21,7 +24,8 @@ pub(crate) enum FieldIsPrivateLabel {
IsUpdateSyntax {
#[primary_span]
span: Span,
field_name: Symbol,
rest_field_names: String,
rest_len: usize,
},
#[label(privacy_field_is_private_label)]
Other {
Expand Down
151 changes: 120 additions & 31 deletions compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use rustc_ast::MacroDef;
use rustc_ast::visit::{VisitorResult, try_visit};
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::intern::Interned;
use rustc_errors::MultiSpan;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{CRATE_DEF_ID, DefId, LocalDefId, LocalModDefId};
use rustc_hir::intravisit::{self, Visitor};
Expand All @@ -38,7 +39,7 @@ use rustc_middle::ty::{
use rustc_middle::{bug, span_bug};
use rustc_session::lint;
use rustc_span::hygiene::Transparency;
use rustc_span::{Ident, Span, kw, sym};
use rustc_span::{Ident, Span, Symbol, kw, sym};
use tracing::debug;
use {rustc_attr_parsing as attr, rustc_hir as hir};

Expand Down Expand Up @@ -921,31 +922,95 @@ impl<'tcx> NamePrivacyVisitor<'tcx> {
&mut self,
hir_id: hir::HirId, // ID of the field use
use_ctxt: Span, // syntax context of the field name at the use site
span: Span, // span of the field pattern, e.g., `x: 0`
def: ty::AdtDef<'tcx>, // definition of the struct or enum
field: &'tcx ty::FieldDef,
in_update_syntax: bool,
) {
) -> bool {
if def.is_enum() {
return;
return true;
}

// definition of the field
let ident = Ident::new(kw::Empty, use_ctxt);
let def_id = self.tcx.adjust_ident_and_get_scope(ident, def.did(), hir_id).1;
if !field.vis.is_accessible_from(def_id, self.tcx) {
self.tcx.dcx().emit_err(FieldIsPrivate {
span,
field_name: field.name,
variant_descr: def.variant_descr(),
def_path_str: self.tcx.def_path_str(def.did()),
label: if in_update_syntax {
FieldIsPrivateLabel::IsUpdateSyntax { span, field_name: field.name }
} else {
FieldIsPrivateLabel::Other { span }
},
});
!field.vis.is_accessible_from(def_id, self.tcx)
}

// Checks that a field in a struct constructor (expression or pattern) is accessible.
fn emit_unreachable_field_error(
&mut self,
fields: Vec<(Symbol, Span, bool /* field is present */)>,
def: ty::AdtDef<'tcx>, // definition of the struct or enum
update_syntax: Option<Span>,
struct_span: Span,
) {
if def.is_enum() || fields.is_empty() {
return;
}

// error[E0451]: fields `beta` and `gamma` of struct `Alpha` are private
// --> $DIR/visibility.rs:18:13
// |
// LL | let _x = Alpha {
// | ----- in this type # from `def`
// LL | beta: 0,
// | ^^^^^^^ private field # `fields.2` is `true`
// LL | ..
// | ^^ field `gamma` is private # `fields.2` is `false`

// Get the list of all private fields for the main message.
let field_names: Vec<_> = fields.iter().map(|(name, _, _)| name).collect();
let field_names = match &field_names[..] {
[] => return,
[name] => format!("`{name}`"),
[fields @ .., last] => format!(
"{} and `{last}`",
fields.iter().map(|f| format!("`{f}`")).collect::<Vec<_>>().join(", "),
),
};
let span: MultiSpan = fields.iter().map(|(_, span, _)| *span).collect::<Vec<Span>>().into();

// Get the list of all private fields when pointing at the `..rest`.
let rest_field_names: Vec<_> =
fields.iter().filter(|(_, _, is_present)| !is_present).map(|(n, _, _)| n).collect();
let rest_len = rest_field_names.len();
let rest_field_names = match &rest_field_names[..] {
[] => String::new(),
[name] => format!("`{name}`"),
[fields @ .., last] => format!(
"{} and `{last}`",
fields.iter().map(|f| format!("`{f}`")).collect::<Vec<_>>().join(", "),
),
};
// Get all the labels for each field or `..rest` in the primary MultiSpan.
let labels = fields
.iter()
.filter(|(_, _, is_present)| *is_present)
.map(|(_, span, _)| FieldIsPrivateLabel::Other { span: *span })
.chain(update_syntax.iter().map(|span| FieldIsPrivateLabel::IsUpdateSyntax {
span: *span,
rest_field_names: rest_field_names.clone(),
rest_len,
}))
.collect();

self.tcx.dcx().emit_err(FieldIsPrivate {
span,
struct_span: if self
.tcx
.sess
.source_map()
.is_multiline(fields[0].1.between(struct_span))
{
Some(struct_span)
} else {
None
},
field_names: field_names.clone(),
variant_descr: def.variant_descr(),
def_path_str: self.tcx.def_path_str(def.did()),
labels,
len: fields.len(),
});
}

fn check_expanded_fields(
Expand All @@ -955,16 +1020,25 @@ impl<'tcx> NamePrivacyVisitor<'tcx> {
fields: &[hir::ExprField<'tcx>],
hir_id: hir::HirId,
span: Span,
struct_span: Span,
) {
let mut failed_fields = vec![];
for (vf_index, variant_field) in variant.fields.iter_enumerated() {
let field =
fields.iter().find(|f| self.typeck_results().field_index(f.hir_id) == vf_index);
let (hir_id, use_ctxt, span) = match field {
Some(field) => (field.hir_id, field.ident.span, field.span),
None => (hir_id, span, span),
};
self.check_field(hir_id, use_ctxt, span, adt, variant_field, true);
if self.check_field(hir_id, use_ctxt, adt, variant_field) {
let name = match field {
Some(field) => field.ident.name,
None => variant_field.name,
};
failed_fields.push((name, span, field.is_some()));
}
}
self.emit_unreachable_field_error(failed_fields, adt, Some(span), struct_span);
}
}

Expand All @@ -990,24 +1064,35 @@ impl<'tcx> Visitor<'tcx> for NamePrivacyVisitor<'tcx> {
// If the expression uses FRU we need to make sure all the unmentioned fields
// are checked for privacy (RFC 736). Rather than computing the set of
// unmentioned fields, just check them all.
self.check_expanded_fields(adt, variant, fields, base.hir_id, base.span);
self.check_expanded_fields(
adt,
variant,
fields,
base.hir_id,
base.span,
qpath.span(),
);
}
hir::StructTailExpr::DefaultFields(span) => {
self.check_expanded_fields(adt, variant, fields, expr.hir_id, span);
self.check_expanded_fields(
adt,
variant,
fields,
expr.hir_id,
span,
qpath.span(),
);
}
hir::StructTailExpr::None => {
let mut failed_fields = vec![];
for field in fields {
let (hir_id, use_ctxt, span) = (field.hir_id, field.ident.span, field.span);
let (hir_id, use_ctxt) = (field.hir_id, field.ident.span);
let index = self.typeck_results().field_index(field.hir_id);
self.check_field(
hir_id,
use_ctxt,
span,
adt,
&variant.fields[index],
false,
);
if self.check_field(hir_id, use_ctxt, adt, &variant.fields[index]) {
failed_fields.push((field.ident.name, field.ident.span, true));
}
}
self.emit_unreachable_field_error(failed_fields, adt, None, qpath.span());
}
}
}
Expand All @@ -1020,11 +1105,15 @@ impl<'tcx> Visitor<'tcx> for NamePrivacyVisitor<'tcx> {
let res = self.typeck_results().qpath_res(qpath, pat.hir_id);
let adt = self.typeck_results().pat_ty(pat).ty_adt_def().unwrap();
let variant = adt.variant_of_res(res);
let mut failed_fields = vec![];
for field in fields {
let (hir_id, use_ctxt, span) = (field.hir_id, field.ident.span, field.span);
let (hir_id, use_ctxt) = (field.hir_id, field.ident.span);
let index = self.typeck_results().field_index(field.hir_id);
self.check_field(hir_id, use_ctxt, span, adt, &variant.fields[index], false);
if self.check_field(hir_id, use_ctxt, adt, &variant.fields[index]) {
failed_fields.push((field.ident.name, field.ident.span, true));
}
}
self.emit_unreachable_field_error(failed_fields, adt, None, qpath.span());
}

intravisit::walk_pat(self, pat);
Expand Down
5 changes: 4 additions & 1 deletion tests/ui/deprecation/deprecation-lint.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -739,8 +739,11 @@ LL | _)
error[E0451]: field `i` of struct `this_crate::nested::DeprecatedStruct` is private
--> $DIR/deprecation-lint.rs:280:13
|
LL | let _ = nested::DeprecatedStruct {
| ------------------------ in this type
LL |
LL | i: 0
| ^^^^ private field
| ^ private field

error: aborting due to 123 previous errors

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/error-codes/E0451.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ error[E0451]: field `b` of struct `Foo` is private
--> $DIR/E0451.rs:18:29
|
LL | let f = bar::Foo{ a: 0, b: 0 };
| ^^^^ private field
| ^ private field

error: aborting due to 2 previous errors

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0451]: field `1` of struct `Box` is private
--> $DIR/issue-82772-match-box-as-struct.rs:4:15
|
LL | let Box { 1: _, .. }: Box<()>;
| ^^^^ private field
| ^ private field

error: aborting due to 1 previous error

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/privacy/private-struct-field-ctor.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0451]: field `x` of struct `Foo` is private
--> $DIR/private-struct-field-ctor.rs:8:22
|
LL | let s = a::Foo { x: 1 };
| ^^^^ private field
| ^ private field

error: aborting due to 1 previous error

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/privacy/private-struct-field-pattern.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0451]: field `x` of struct `Foo` is private
--> $DIR/private-struct-field-pattern.rs:15:15
|
LL | Foo { x: _ } => {}
| ^^^^ private field
| ^ private field

error: aborting due to 1 previous error

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/privacy/restricted/struct-literal-field.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0451]: field `x` of struct `S` is private
--> $DIR/struct-literal-field.rs:18:9
|
LL | S { x: 0 };
| ^^^^ private field
| ^ private field

error: aborting due to 1 previous error

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/privacy/union-field-privacy-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0451]: field `c` of union `U` is private
--> $DIR/union-field-privacy-1.rs:12:20
|
LL | let u = m::U { c: 0 };
| ^^^^ private field
| ^ private field

error[E0451]: field `c` of union `U` is private
--> $DIR/union-field-privacy-1.rs:16:16
Expand Down
42 changes: 42 additions & 0 deletions tests/ui/structs/default-field-values/visibility.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#![feature(default_field_values)]
pub mod foo {
#[derive(Default)]
pub struct Alpha {
beta: u8 = 42,
gamma: bool = true,
}
}

mod bar {
use crate::foo::Alpha;
fn baz() {
let _x = Alpha { .. };
//~^ ERROR fields `beta` and `gamma` of struct `Alpha` are private
let _x = Alpha {
beta: 0, //~ ERROR fields `beta` and `gamma` of struct `Alpha` are private
gamma: false,
};
let _x = Alpha {
beta: 0, //~ ERROR fields `beta` and `gamma` of struct `Alpha` are private
..
};
let _x = Alpha { beta: 0, .. };
//~^ ERROR fields `beta` and `gamma` of struct `Alpha` are private
let _x = Alpha { beta: 0, ..Default::default() };
//~^ ERROR fields `beta` and `gamma` of struct `Alpha` are private
}
}

pub mod baz {
pub struct S {
x: i32 = 1,
}
}
fn main() {
let _a = baz::S {
.. //~ ERROR field `x` of struct `S` is private
};
let _b = baz::S {
x: 0, //~ ERROR field `x` of struct `S` is private
};
}
Loading
Loading