bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

review comments: Tweak output

Browse source 
* Account for `struct S(pub(super)Ty);` in suggestion
* Suggest changing field visibility in E0603 too
This commit is contained in:
Esteban Küber 2023-01-10 20:57:02 +00:00
parent eb835093a3
commit 41e66d9025
8 changed files with 210 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

14
compiler/rustc_resolve/src/build_reduced_graph.rs
View file

@ -331,8 +331,15 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
.iter()
.map(|field| respan(field.span, field.ident.map_or(kw::Empty, |ident| ident.name)))
.collect();
let field_vis = vdata.fields().iter().map(|field| field.vis.span).collect();
self.r.field_names.insert(def_id, field_names);
}
fn insert_field_visibilities_local(&mut self, def_id: DefId, vdata: &ast::VariantData) {
let field_vis = vdata
.fields()
.iter()
.map(|field| field.vis.span.until(field.ident.map_or(field.ty.span, |i| i.span)))
.collect();
self.r.field_visibility_spans.insert(def_id, field_vis);
}
@ -739,6 +746,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
// Record field names for error reporting.
self.insert_field_names_local(def_id, vdata);
self.insert_field_visibilities_local(def_id, vdata);
// If this is a tuple or unit struct, define a name
// in the value namespace as well.
@ -772,6 +780,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
Res::Def(DefKind::Ctor(CtorOf::Struct, ctor_kind), ctor_def_id.to_def_id());
self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, sp, expansion));
self.r.visibilities.insert(ctor_def_id, ctor_vis);
// We need the field visibility spans also for the constructor for E0603.
self.insert_field_visibilities_local(ctor_def_id.to_def_id(), vdata);
self.r
.struct_constructors
@ -785,6 +795,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
// Record field names for error reporting.
self.insert_field_names_local(def_id, vdata);
self.insert_field_visibilities_local(def_id, vdata);
}
ItemKind::Trait(..) => {
@ -1512,6 +1523,7 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> {
// Record field names for error reporting.
self.insert_field_names_local(def_id.to_def_id(), &variant.data);
self.insert_field_visibilities_local(def_id.to_def_id(), &variant.data);
visit::walk_variant(self, variant);
}

14
compiler/rustc_resolve/src/diagnostics.rs
View file

@ -6,7 +6,9 @@ use rustc_ast::{self as ast, Crate, ItemKind, ModKind, NodeId, Path, CRATE_NODE_
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::struct_span_err;
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan};
use rustc_errors::{
pluralize, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan,
};
use rustc_feature::BUILTIN_ATTRIBUTES;
use rustc_hir::def::Namespace::{self, *};
use rustc_hir::def::{self, CtorKind, CtorOf, DefKind, NonMacroAttrKind, PerNS};
@ -1604,6 +1606,16 @@ impl<'a> Resolver<'a> {
err.span_label(ident.span, &format!("private {}", descr));
if let Some(span) = ctor_fields_span {
err.span_label(span, "a constructor is private if any of the fields is private");
if let Res::Def(_, d) = res && let Some(fields) = self.field_visibility_spans.get(&d) {
err.multipart_suggestion_verbose(
&format!(
"consider making the field{} publicly accessible",
pluralize!(fields.len())
),
fields.iter().map(|span| (*span, "pub ".to_string())).collect(),
Applicability::MaybeIncorrect,
);
}
}
// Print the whole import chain to make it easier to see what happens.

15
compiler/rustc_resolve/src/late/diagnostics.rs
View file

@ -1451,22 +1451,13 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
.collect();
if non_visible_spans.len() > 0 {
if let Some(visibility_spans) = self.r.field_visibility_spans.get(&def_id) {
if let Some(fields) = self.r.field_visibility_spans.get(&def_id) {
err.multipart_suggestion_verbose(
&format!(
"consider making the field{} publicly accessible",
pluralize!(visibility_spans.len())
pluralize!(fields.len())
),
visibility_spans
.iter()
.map(|span| {
(
*span,
if span.is_empty() { "pub " } else { "pub" }
.to_string(),
)
})
.collect(),
fields.iter().map(|span| (*span, "pub ".to_string())).collect(),
Applicability::MaybeIncorrect,
);
}