Skip to content

Commit

Permalink
Rollup merge of #76524 - davidtwco:issue-76077-inaccessible-private-f…
Browse files Browse the repository at this point in the history
…ields, r=estebank

typeck: don't suggest inaccessible private fields

Fixes #76077.

This PR adjusts the missing field diagnostic logic in typeck so that when none of the missing fields in a struct expr are accessible then the error is less confusing.

r? @estebank
  • Loading branch information
tmandry authored Sep 10, 2020
2 parents 7565ccc + 409c141 commit 9f8a782
Show file tree
Hide file tree
Showing 7 changed files with 244 additions and 42 deletions.
118 changes: 85 additions & 33 deletions compiler/rustc_typeck/src/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1241,42 +1241,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
tcx.sess.span_err(span, "union expressions should have exactly one field");
}
} else if check_completeness && !error_happened && !remaining_fields.is_empty() {
let len = remaining_fields.len();

let mut displayable_field_names =
remaining_fields.keys().map(|ident| ident.as_str()).collect::<Vec<_>>();

displayable_field_names.sort();
let no_accessible_remaining_fields = remaining_fields
.iter()
.filter(|(_, (_, field))| {
field.vis.is_accessible_from(tcx.parent_module(expr_id).to_def_id(), tcx)
})
.next()
.is_none();

let truncated_fields_error = if len <= 3 {
String::new()
if no_accessible_remaining_fields {
self.report_no_accessible_fields(adt_ty, span);
} else {
format!(" and {} other field{}", (len - 3), if len - 3 == 1 { "" } else { "s" })
};

let remaining_fields_names = displayable_field_names
.iter()
.take(3)
.map(|n| format!("`{}`", n))
.collect::<Vec<_>>()
.join(", ");

struct_span_err!(
tcx.sess,
span,
E0063,
"missing field{} {}{} in initializer of `{}`",
pluralize!(remaining_fields.len()),
remaining_fields_names,
truncated_fields_error,
adt_ty
)
.span_label(
span,
format!("missing {}{}", remaining_fields_names, truncated_fields_error),
)
.emit();
self.report_missing_field(adt_ty, span, remaining_fields);
}
}

error_happened
}

Expand All @@ -1293,6 +1272,79 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

/// Report an error for a struct field expression when there are fields which aren't provided.
///
/// ```ignore (diagnostic)
/// error: missing field `you_can_use_this_field` in initializer of `foo::Foo`
/// --> src/main.rs:8:5
/// |
/// 8 | foo::Foo {};
/// | ^^^^^^^^ missing `you_can_use_this_field`
///
/// error: aborting due to previous error
/// ```
fn report_missing_field(
&self,
adt_ty: Ty<'tcx>,
span: Span,
remaining_fields: FxHashMap<Ident, (usize, &ty::FieldDef)>,
) {
let tcx = self.tcx;
let len = remaining_fields.len();

let mut displayable_field_names =
remaining_fields.keys().map(|ident| ident.as_str()).collect::<Vec<_>>();

displayable_field_names.sort();

let truncated_fields_error = if len <= 3 {
String::new()
} else {
format!(" and {} other field{}", (len - 3), if len - 3 == 1 { "" } else { "s" })
};

let remaining_fields_names = displayable_field_names
.iter()
.take(3)
.map(|n| format!("`{}`", n))
.collect::<Vec<_>>()
.join(", ");

struct_span_err!(
tcx.sess,
span,
E0063,
"missing field{} {}{} in initializer of `{}`",
pluralize!(remaining_fields.len()),
remaining_fields_names,
truncated_fields_error,
adt_ty
)
.span_label(span, format!("missing {}{}", remaining_fields_names, truncated_fields_error))
.emit();
}

/// Report an error for a struct field expression when there are no visible fields.
///
/// ```ignore (diagnostic)
/// error: cannot construct `Foo` with struct literal syntax due to inaccessible fields
/// --> src/main.rs:8:5
/// |
/// 8 | foo::Foo {};
/// | ^^^^^^^^
///
/// error: aborting due to previous error
/// ```
fn report_no_accessible_fields(&self, adt_ty: Ty<'tcx>, span: Span) {
self.tcx.sess.span_err(
span,
&format!(
"cannot construct `{}` with struct literal syntax due to inaccessible fields",
adt_ty,
),
);
}

fn report_unknown_field(
&self,
ty: Ty<'tcx>,
Expand Down
90 changes: 81 additions & 9 deletions compiler/rustc_typeck/src/check/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1078,8 +1078,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mut unmentioned_fields = variant
.fields
.iter()
.map(|field| field.ident.normalize_to_macros_2_0())
.filter(|ident| !used_fields.contains_key(&ident))
.map(|field| (field, field.ident.normalize_to_macros_2_0()))
.filter(|(_, ident)| !used_fields.contains_key(&ident))
.collect::<Vec<_>>();

let inexistent_fields_err = if !(inexistent_fields.is_empty() || variant.is_recovered()) {
Expand Down Expand Up @@ -1110,7 +1110,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
tcx.sess.struct_span_err(pat.span, "`..` cannot be used in union patterns").emit();
}
} else if !etc && !unmentioned_fields.is_empty() {
unmentioned_err = Some(self.error_unmentioned_fields(pat, &unmentioned_fields));
let no_accessible_unmentioned_fields = unmentioned_fields
.iter()
.filter(|(field, _)| {
field.vis.is_accessible_from(tcx.parent_module(pat.hir_id).to_def_id(), tcx)
})
.next()
.is_none();

if no_accessible_unmentioned_fields {
unmentioned_err = Some(self.error_no_accessible_fields(pat, &fields));
} else {
unmentioned_err = Some(self.error_unmentioned_fields(pat, &unmentioned_fields));
}
}
match (inexistent_fields_err, unmentioned_err) {
(Some(mut i), Some(mut u)) => {
Expand Down Expand Up @@ -1173,7 +1185,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
kind_name: &str,
inexistent_fields: &[Ident],
unmentioned_fields: &mut Vec<Ident>,
unmentioned_fields: &mut Vec<(&ty::FieldDef, Ident)>,
variant: &ty::VariantDef,
) -> DiagnosticBuilder<'tcx> {
let tcx = self.tcx;
Expand Down Expand Up @@ -1215,7 +1227,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
),
);
if plural == "" {
let input = unmentioned_fields.iter().map(|field| &field.name);
let input = unmentioned_fields.iter().map(|(_, field)| &field.name);
let suggested_name = find_best_match_for_name(input, ident.name, None);
if let Some(suggested_name) = suggested_name {
err.span_suggestion(
Expand All @@ -1232,7 +1244,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// `smart_resolve_context_dependent_help`.
if suggested_name.to_ident_string().parse::<usize>().is_err() {
// We don't want to throw `E0027` in case we have thrown `E0026` for them.
unmentioned_fields.retain(|&x| x.name != suggested_name);
unmentioned_fields.retain(|&(_, x)| x.name != suggested_name);
}
}
}
Expand Down Expand Up @@ -1300,17 +1312,77 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
None
}

/// Returns a diagnostic reporting a struct pattern which is missing an `..` due to
/// inaccessible fields.
///
/// ```ignore (diagnostic)
/// error: pattern requires `..` due to inaccessible fields
/// --> src/main.rs:10:9
/// |
/// LL | let foo::Foo {} = foo::Foo::default();
/// | ^^^^^^^^^^^
/// |
/// help: add a `..`
/// |
/// LL | let foo::Foo { .. } = foo::Foo::default();
/// | ^^^^^^
/// ```
fn error_no_accessible_fields(
&self,
pat: &Pat<'_>,
fields: &'tcx [hir::FieldPat<'tcx>],
) -> DiagnosticBuilder<'tcx> {
let mut err = self
.tcx
.sess
.struct_span_err(pat.span, "pattern requires `..` due to inaccessible fields");

if let Some(field) = fields.last() {
err.span_suggestion_verbose(
field.span.shrink_to_hi(),
"ignore the inaccessible and unused fields",
", ..".to_string(),
Applicability::MachineApplicable,
);
} else {
let qpath_span = if let PatKind::Struct(qpath, ..) = &pat.kind {
qpath.span()
} else {
bug!("`error_no_accessible_fields` called on non-struct pattern");
};

// Shrink the span to exclude the `foo:Foo` in `foo::Foo { }`.
let span = pat.span.with_lo(qpath_span.shrink_to_hi().hi());
err.span_suggestion_verbose(
span,
"ignore the inaccessible and unused fields",
" { .. }".to_string(),
Applicability::MachineApplicable,
);
}
err
}

/// Returns a diagnostic reporting a struct pattern which does not mention some fields.
///
/// ```ignore (diagnostic)
/// error[E0027]: pattern does not mention field `you_cant_use_this_field`
/// --> src/main.rs:15:9
/// |
/// LL | let foo::Foo {} = foo::Foo::new();
/// | ^^^^^^^^^^^ missing field `you_cant_use_this_field`
/// ```
fn error_unmentioned_fields(
&self,
pat: &Pat<'_>,
unmentioned_fields: &[Ident],
unmentioned_fields: &[(&ty::FieldDef, Ident)],
) -> DiagnosticBuilder<'tcx> {
let field_names = if unmentioned_fields.len() == 1 {
format!("field `{}`", unmentioned_fields[0])
format!("field `{}`", unmentioned_fields[0].1)
} else {
let fields = unmentioned_fields
.iter()
.map(|name| format!("`{}`", name))
.map(|(_, name)| format!("`{}`", name))
.collect::<Vec<String>>()
.join(", ");
format!("fields {}", fields)
Expand Down
18 changes: 18 additions & 0 deletions src/test/ui/issues/issue-76077-1.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// run-rustfix
#![allow(dead_code, unused_variables)]

pub mod foo {
#[derive(Default)]
pub struct Foo { invisible: bool, }

#[derive(Default)]
pub struct Bar { pub visible: bool, invisible: bool, }
}

fn main() {
let foo::Foo { .. } = foo::Foo::default();
//~^ ERROR pattern requires `..` due to inaccessible fields

let foo::Bar { visible, .. } = foo::Bar::default();
//~^ ERROR pattern requires `..` due to inaccessible fields
}
18 changes: 18 additions & 0 deletions src/test/ui/issues/issue-76077-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// run-rustfix
#![allow(dead_code, unused_variables)]

pub mod foo {
#[derive(Default)]
pub struct Foo { invisible: bool, }

#[derive(Default)]
pub struct Bar { pub visible: bool, invisible: bool, }
}

fn main() {
let foo::Foo {} = foo::Foo::default();
//~^ ERROR pattern requires `..` due to inaccessible fields

let foo::Bar { visible } = foo::Bar::default();
//~^ ERROR pattern requires `..` due to inaccessible fields
}
24 changes: 24 additions & 0 deletions src/test/ui/issues/issue-76077-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
error: pattern requires `..` due to inaccessible fields
--> $DIR/issue-76077-1.rs:13:9
|
LL | let foo::Foo {} = foo::Foo::default();
| ^^^^^^^^^^^
|
help: ignore the inaccessible and unused fields
|
LL | let foo::Foo { .. } = foo::Foo::default();
| ^^^^^^

error: pattern requires `..` due to inaccessible fields
--> $DIR/issue-76077-1.rs:16:9
|
LL | let foo::Bar { visible } = foo::Bar::default();
| ^^^^^^^^^^^^^^^^^^^^
|
help: ignore the inaccessible and unused fields
|
LL | let foo::Bar { visible, .. } = foo::Bar::default();
| ^^^^

error: aborting due to 2 previous errors

10 changes: 10 additions & 0 deletions src/test/ui/issues/issue-76077.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
pub mod foo {
pub struct Foo {
you_cant_use_this_field: bool,
}
}

fn main() {
foo::Foo {};
//~^ ERROR cannot construct `Foo` with struct literal syntax due to inaccessible fields
}
8 changes: 8 additions & 0 deletions src/test/ui/issues/issue-76077.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: cannot construct `Foo` with struct literal syntax due to inaccessible fields
--> $DIR/issue-76077.rs:8:5
|
LL | foo::Foo {};
| ^^^^^^^^

error: aborting due to previous error

0 comments on commit 9f8a782

Please sign in to comment.