Skip to content

Commit

Permalink
Emit a single privacy error for multiple fields on the same struct ex…
Browse files Browse the repository at this point in the history
…pression

Collect all unreachable fields in a single struct literal struct and emit a single error, instead of one error per private field.

```
error[E0451]: fields `beta` and `gamma` of struct `Alpha` are private
  --> $DIR/visibility.rs:18:13
   |
LL |     let _x = Alpha {
   |              ----- in this type
LL |         beta: 0,
   |         ^^^^^^^ private field
LL |         ..
   |         ^^ field `gamma` is private
```
  • Loading branch information
estebank committed Jan 18, 2025
1 parent bbcf26f commit 0a1a1f7
Show file tree
Hide file tree
Showing 13 changed files with 199 additions and 82 deletions.
17 changes: 15 additions & 2 deletions compiler/rustc_privacy/messages.ftl
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
privacy_field_is_private = field `{$field_name}` of {$variant_descr} `{$def_path_str}` is private
privacy_field_is_private =

Check failure on line 1 in compiler/rustc_privacy/messages.ftl

View workflow job for this annotation

GitHub Actions / PR - mingw-check-tidy

trailing whitespace
{$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 = field `{$field_name}` is private
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
12 changes: 7 additions & 5 deletions compiler/rustc_privacy/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,20 +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 span: MultiSpan,
#[label]
pub struct_span: Option<Span>,
pub field_name: Symbol,
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 @@ -23,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
147 changes: 103 additions & 44 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,37 +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,
struct_span: Span,
) {
) -> 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,
struct_span: if self.tcx.sess.source_map().is_multiline(span.between(struct_span)) {
Some(struct_span)
} else {
None
},
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 @@ -963,15 +1022,23 @@ impl<'tcx> NamePrivacyVisitor<'tcx> {
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, struct_span);
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 Down Expand Up @@ -1017,19 +1084,15 @@ impl<'tcx> Visitor<'tcx> for NamePrivacyVisitor<'tcx> {
);
}
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,
qpath.span(),
);
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 @@ -1042,19 +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,
qpath.span(),
);
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
2 changes: 1 addition & 1 deletion tests/ui/deprecation/deprecation-lint.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,7 @@ 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
23 changes: 18 additions & 5 deletions tests/ui/structs/default-field-values/visibility.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,29 @@
#![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 = crate::foo::Alpha { .. };
//~^ ERROR field `beta` of struct `Alpha` is private
//~| ERROR field `gamma` of struct `Alpha` is private
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
}
}

Expand All @@ -20,10 +33,10 @@ pub mod baz {
}
}
fn main() {
let a = baz::S {
let _a = baz::S {
.. //~ ERROR field `x` of struct `S` is private
};
let b = baz::S {
let _b = baz::S {
x: 0, //~ ERROR field `x` of struct `S` is private
};
}
Loading

0 comments on commit 0a1a1f7

Please sign in to comment.