review comments: Tweak output

* Account for `struct S(pub(super)Ty);` in suggestion
* Suggest changing field visibility in E0603 too

diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
index 928f372d948dd38fb126a16c0aa9647478023927..b1b04c92a75042cfa4af912ff5024c1acbde65f5 100644 (file)
--- a/compiler/rustc_resolve/src/build_reduced_graph.rs
+++ b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -331,8 +331,15 @@ fn insert_field_names_local(&mut self, def_id: DefId, vdata: &ast::VariantData)
             .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 @@ fn build_reduced_graph_for_item(&mut self, item: &'b Item) {
 
                 // 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 @@ fn build_reduced_graph_for_item(&mut self, item: &'b Item) {
                         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 @@ fn build_reduced_graph_for_item(&mut self, item: &'b Item) {
 
                 // 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 @@ fn visit_variant(&mut self, variant: &'b ast::Variant) {
 
         // 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);
     }

diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs
index 7d62d67d64f078256ed63ba4d1f211858fca1b09..1a852de8eed690d29e6e2cabebff7ead288bb83c 100644 (file)
--- a/compiler/rustc_resolve/src/diagnostics.rs
+++ b/compiler/rustc_resolve/src/diagnostics.rs
@@ -6,7 +6,9 @@
 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 @@ fn report_privacy_error(&self, privacy_error: &PrivacyError<'_>) {
         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.

diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs
index 6923ca7ef1bf4ed88b8450616cd1987371100a24..d92f5a7c05e6d8473ae85c0eff0107f9e6be3300 100644 (file)
--- a/compiler/rustc_resolve/src/late/diagnostics.rs
+++ b/compiler/rustc_resolve/src/late/diagnostics.rs
@@ -1451,22 +1451,13 @@ fn smart_resolve_context_dependent_help(
                         .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,
                             );
                         }

diff --git a/tests/ui/privacy/privacy5.stderr b/tests/ui/privacy/privacy5.stderr
index 680161272cefb79bee11c011c1025589baf822a4..615b0af2762d3de9e70a7d08a2f0a1b989f863cb 100644 (file)
--- a/tests/ui/privacy/privacy5.stderr
+++ b/tests/ui/privacy/privacy5.stderr
@@ -12,6 +12,10 @@ note: the tuple struct constructor `A` is defined here
    |
 LL |     pub struct A(());
    |     ^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct A(pub ());
+   |                  +++
 
 error[E0603]: tuple struct constructor `B` is private
   --> $DIR/privacy5.rs:52:16
@@ -27,6 +31,10 @@ note: the tuple struct constructor `B` is defined here
    |
 LL |     pub struct B(isize);
    |     ^^^^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct B(pub isize);
+   |                  +++
 
 error[E0603]: tuple struct constructor `C` is private
   --> $DIR/privacy5.rs:53:16
@@ -42,6 +50,10 @@ note: the tuple struct constructor `C` is defined here
    |
 LL |     pub struct C(pub isize, isize);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the fields publicly accessible
+   |
+LL |     pub struct C(pub isize, pub isize);
+   |                  ~~~        +++
 
 error[E0603]: tuple struct constructor `A` is private
   --> $DIR/privacy5.rs:56:12
@@ -57,6 +69,10 @@ note: the tuple struct constructor `A` is defined here
    |
 LL |     pub struct A(());
    |     ^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct A(pub ());
+   |                  +++
 
 error[E0603]: tuple struct constructor `A` is private
   --> $DIR/privacy5.rs:57:12
@@ -72,6 +88,10 @@ note: the tuple struct constructor `A` is defined here
    |
 LL |     pub struct A(());
    |     ^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct A(pub ());
+   |                  +++
 
 error[E0603]: tuple struct constructor `A` is private
   --> $DIR/privacy5.rs:58:18
@@ -87,6 +107,10 @@ note: the tuple struct constructor `A` is defined here
    |
 LL |     pub struct A(());
    |     ^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct A(pub ());
+   |                  +++
 
 error[E0603]: tuple struct constructor `A` is private
   --> $DIR/privacy5.rs:59:18
@@ -102,6 +126,10 @@ note: the tuple struct constructor `A` is defined here
    |
 LL |     pub struct A(());
    |     ^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct A(pub ());
+   |                  +++
 
 error[E0603]: tuple struct constructor `B` is private
   --> $DIR/privacy5.rs:61:12
@@ -117,6 +145,10 @@ note: the tuple struct constructor `B` is defined here
    |
 LL |     pub struct B(isize);
    |     ^^^^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct B(pub isize);
+   |                  +++
 
 error[E0603]: tuple struct constructor `B` is private
   --> $DIR/privacy5.rs:62:12
@@ -132,6 +164,10 @@ note: the tuple struct constructor `B` is defined here
    |
 LL |     pub struct B(isize);
    |     ^^^^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct B(pub isize);
+   |                  +++
 
 error[E0603]: tuple struct constructor `B` is private
   --> $DIR/privacy5.rs:63:18
@@ -147,6 +183,10 @@ note: the tuple struct constructor `B` is defined here
    |
 LL |     pub struct B(isize);
    |     ^^^^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct B(pub isize);
+   |                  +++
 
 error[E0603]: tuple struct constructor `B` is private
   --> $DIR/privacy5.rs:64:18
@@ -162,6 +202,10 @@ note: the tuple struct constructor `B` is defined here
    |
 LL |     pub struct B(isize);
    |     ^^^^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct B(pub isize);
+   |                  +++
 
 error[E0603]: tuple struct constructor `B` is private
   --> $DIR/privacy5.rs:65:18
@@ -177,6 +221,10 @@ note: the tuple struct constructor `B` is defined here
    |
 LL |     pub struct B(isize);
    |     ^^^^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct B(pub isize);
+   |                  +++
 
 error[E0603]: tuple struct constructor `B` is private
   --> $DIR/privacy5.rs:65:32
@@ -192,6 +240,10 @@ note: the tuple struct constructor `B` is defined here
    |
 LL |     pub struct B(isize);
    |     ^^^^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct B(pub isize);
+   |                  +++
 
 error[E0603]: tuple struct constructor `C` is private
   --> $DIR/privacy5.rs:68:12
@@ -207,6 +259,10 @@ note: the tuple struct constructor `C` is defined here
    |
 LL |     pub struct C(pub isize, isize);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the fields publicly accessible
+   |
+LL |     pub struct C(pub isize, pub isize);
+   |                  ~~~        +++
 
 error[E0603]: tuple struct constructor `C` is private
   --> $DIR/privacy5.rs:69:12
@@ -222,6 +278,10 @@ note: the tuple struct constructor `C` is defined here
    |
 LL |     pub struct C(pub isize, isize);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the fields publicly accessible
+   |
+LL |     pub struct C(pub isize, pub isize);
+   |                  ~~~        +++
 
 error[E0603]: tuple struct constructor `C` is private
   --> $DIR/privacy5.rs:70:12
@@ -237,6 +297,10 @@ note: the tuple struct constructor `C` is defined here
    |
 LL |     pub struct C(pub isize, isize);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the fields publicly accessible
+   |
+LL |     pub struct C(pub isize, pub isize);
+   |                  ~~~        +++
 
 error[E0603]: tuple struct constructor `C` is private
   --> $DIR/privacy5.rs:71:12
@@ -252,6 +316,10 @@ note: the tuple struct constructor `C` is defined here
    |
 LL |     pub struct C(pub isize, isize);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the fields publicly accessible
+   |
+LL |     pub struct C(pub isize, pub isize);
+   |                  ~~~        +++
 
 error[E0603]: tuple struct constructor `C` is private
   --> $DIR/privacy5.rs:72:18
@@ -267,6 +335,10 @@ note: the tuple struct constructor `C` is defined here
    |
 LL |     pub struct C(pub isize, isize);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the fields publicly accessible
+   |
+LL |     pub struct C(pub isize, pub isize);
+   |                  ~~~        +++
 
 error[E0603]: tuple struct constructor `C` is private
   --> $DIR/privacy5.rs:73:18
@@ -282,6 +354,10 @@ note: the tuple struct constructor `C` is defined here
    |
 LL |     pub struct C(pub isize, isize);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the fields publicly accessible
+   |
+LL |     pub struct C(pub isize, pub isize);
+   |                  ~~~        +++
 
 error[E0603]: tuple struct constructor `C` is private
   --> $DIR/privacy5.rs:74:18
@@ -297,6 +373,10 @@ note: the tuple struct constructor `C` is defined here
    |
 LL |     pub struct C(pub isize, isize);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the fields publicly accessible
+   |
+LL |     pub struct C(pub isize, pub isize);
+   |                  ~~~        +++
 
 error[E0603]: tuple struct constructor `C` is private
   --> $DIR/privacy5.rs:75:18
@@ -312,6 +392,10 @@ note: the tuple struct constructor `C` is defined here
    |
 LL |     pub struct C(pub isize, isize);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the fields publicly accessible
+   |
+LL |     pub struct C(pub isize, pub isize);
+   |                  ~~~        +++
 
 error[E0603]: tuple struct constructor `A` is private
   --> $DIR/privacy5.rs:83:17
@@ -327,6 +411,10 @@ note: the tuple struct constructor `A` is defined here
    |
 LL |     pub struct A(());
    |     ^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct A(pub ());
+   |                  +++
 
 error[E0603]: tuple struct constructor `B` is private
   --> $DIR/privacy5.rs:84:17
@@ -342,6 +430,10 @@ note: the tuple struct constructor `B` is defined here
    |
 LL |     pub struct B(isize);
    |     ^^^^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct B(pub isize);
+   |                  +++
 
 error[E0603]: tuple struct constructor `C` is private
   --> $DIR/privacy5.rs:85:17
@@ -357,6 +449,10 @@ note: the tuple struct constructor `C` is defined here
    |
 LL |     pub struct C(pub isize, isize);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the fields publicly accessible
+   |
+LL |     pub struct C(pub isize, pub isize);
+   |                  ~~~        +++
 
 error[E0603]: tuple struct constructor `A` is private
   --> $DIR/privacy5.rs:90:20

diff --git a/tests/ui/privacy/suggest-making-field-public.fixed b/tests/ui/privacy/suggest-making-field-public.fixed
new file mode 100644 (file)
index 0000000..78e335b
--- /dev/null
+++ b/tests/ui/privacy/suggest-making-field-public.fixed
@@ -0,0 +1,15 @@
+// run-rustfix
+mod a {
+    pub struct A(pub String);
+}
+
+mod b {
+    use crate::a::A;
+    pub fn x() {
+        A("".into()); //~ ERROR cannot initialize a tuple struct which contains private fields
+    }
+}
+fn main() {
+    a::A("a".into()); //~ ERROR tuple struct constructor `A` is private
+    b::x();
+}

diff --git a/tests/ui/privacy/suggest-making-field-public.rs b/tests/ui/privacy/suggest-making-field-public.rs
new file mode 100644 (file)
index 0000000..b65c801
--- /dev/null
+++ b/tests/ui/privacy/suggest-making-field-public.rs
@@ -0,0 +1,15 @@
+// run-rustfix
+mod a {
+    pub struct A(pub(self)String);
+}
+
+mod b {
+    use crate::a::A;
+    pub fn x() {
+        A("".into()); //~ ERROR cannot initialize a tuple struct which contains private fields
+    }
+}
+fn main() {
+    a::A("a".into()); //~ ERROR tuple struct constructor `A` is private
+    b::x();
+}

diff --git a/tests/ui/privacy/suggest-making-field-public.stderr b/tests/ui/privacy/suggest-making-field-public.stderr
new file mode 100644 (file)
index 0000000..e92e9aa
--- /dev/null
+++ b/tests/ui/privacy/suggest-making-field-public.stderr
@@ -0,0 +1,39 @@
+error[E0603]: tuple struct constructor `A` is private
+  --> $DIR/suggest-making-field-public.rs:13:8
+   |
+LL |     pub struct A(pub(self)String);
+   |                  --------------- a constructor is private if any of the fields is private
+...
+LL |     a::A("a".into());
+   |        ^ private tuple struct constructor
+   |
+note: the tuple struct constructor `A` is defined here
+  --> $DIR/suggest-making-field-public.rs:3:5
+   |
+LL |     pub struct A(pub(self)String);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct A(pub String);
+   |                  ~~~
+
+error[E0423]: cannot initialize a tuple struct which contains private fields
+  --> $DIR/suggest-making-field-public.rs:9:9
+   |
+LL |         A("".into());
+   |         ^
+   |
+note: constructor is not visible here due to private fields
+  --> $DIR/suggest-making-field-public.rs:3:18
+   |
+LL |     pub struct A(pub(self)String);
+   |                  ^^^^^^^^^^^^^^^ private field
+help: consider making the field publicly accessible
+   |
+LL |     pub struct A(pub String);
+   |                  ~~~
+
+error: aborting due to 2 previous errors
+
+Some errors have detailed explanations: E0423, E0603.
+For more information about an error, try `rustc --explain E0423`.

diff --git a/tests/ui/resolve/privacy-struct-ctor.stderr b/tests/ui/resolve/privacy-struct-ctor.stderr
index 17a666a401ce85a6387ab82d08d31f9aa7191132..c1fcaaf05738f0f348be28a8f5c9c85249f3002d 100644 (file)
--- a/tests/ui/resolve/privacy-struct-ctor.stderr
+++ b/tests/ui/resolve/privacy-struct-ctor.stderr
@@ -53,6 +53,10 @@ note: the tuple struct constructor `Z` is defined here
    |
 LL |         pub(in m) struct Z(pub(in m::n) u8);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |         pub(in m) struct Z(pub u8);
+   |                            ~~~
 
 error[E0603]: tuple struct constructor `S` is private
   --> $DIR/privacy-struct-ctor.rs:29:8
@@ -68,6 +72,10 @@ note: the tuple struct constructor `S` is defined here
    |
 LL |     pub struct S(u8);
    |     ^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct S(pub u8);
+   |                  +++
 
 error[E0603]: tuple struct constructor `S` is private
   --> $DIR/privacy-struct-ctor.rs:31:19
@@ -83,6 +91,10 @@ note: the tuple struct constructor `S` is defined here
    |
 LL |     pub struct S(u8);
    |     ^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |     pub struct S(pub u8);
+   |                  +++
 
 error[E0603]: tuple struct constructor `Z` is private
   --> $DIR/privacy-struct-ctor.rs:35:11
@@ -98,6 +110,10 @@ note: the tuple struct constructor `Z` is defined here
    |
 LL |         pub(in m) struct Z(pub(in m::n) u8);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider making the field publicly accessible
+   |
+LL |         pub(in m) struct Z(pub u8);
+   |                            ~~~
 
 error[E0603]: tuple struct constructor `S` is private
   --> $DIR/privacy-struct-ctor.rs:41:16