Skip to content

Commit 41e66d9

Browse files
estebankestebank
committed
review comments: Tweak output
* Account for `struct S(pub(super)Ty);` in suggestion * Suggest changing field visibility in E0603 too
1 parent eb83509 commit 41e66d9

8 files changed

+210
-14
lines changed

‎compiler/rustc_resolve/src/build_reduced_graph.rs

+13-1
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,15 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
331331
.iter()
332332
.map(|field| respan(field.span, field.ident.map_or(kw::Empty, |ident| ident.name)))
333333
.collect();
334-
let field_vis = vdata.fields().iter().map(|field| field.vis.span).collect();
335334
self.r.field_names.insert(def_id, field_names);
335+
}
336+
337+
fn insert_field_visibilities_local(&mut self, def_id: DefId, vdata: &ast::VariantData) {
338+
let field_vis = vdata
339+
.fields()
340+
.iter()
341+
.map(|field| field.vis.span.until(field.ident.map_or(field.ty.span, |i| i.span)))
342+
.collect();
336343
self.r.field_visibility_spans.insert(def_id, field_vis);
337344
}
338345

@@ -739,6 +746,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
739746

740747
// Record field names for error reporting.
741748
self.insert_field_names_local(def_id, vdata);
749+
self.insert_field_visibilities_local(def_id, vdata);
742750

743751
// If this is a tuple or unit struct, define a name
744752
// in the value namespace as well.
@@ -772,6 +780,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
772780
Res::Def(DefKind::Ctor(CtorOf::Struct, ctor_kind), ctor_def_id.to_def_id());
773781
self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, sp, expansion));
774782
self.r.visibilities.insert(ctor_def_id, ctor_vis);
783+
// We need the field visibility spans also for the constructor for E0603.
784+
self.insert_field_visibilities_local(ctor_def_id.to_def_id(), vdata);
775785

776786
self.r
777787
.struct_constructors
@@ -785,6 +795,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
785795

786796
// Record field names for error reporting.
787797
self.insert_field_names_local(def_id, vdata);
798+
self.insert_field_visibilities_local(def_id, vdata);
788799
}
789800

790801
ItemKind::Trait(..) => {
@@ -1512,6 +1523,7 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> {
15121523

15131524
// Record field names for error reporting.
15141525
self.insert_field_names_local(def_id.to_def_id(), &variant.data);
1526+
self.insert_field_visibilities_local(def_id.to_def_id(), &variant.data);
15151527

15161528
visit::walk_variant(self, variant);
15171529
}

‎compiler/rustc_resolve/src/diagnostics.rs

+13-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ use rustc_ast::{self as ast, Crate, ItemKind, ModKind, NodeId, Path, CRATE_NODE_
66
use rustc_ast_pretty::pprust;
77
use rustc_data_structures::fx::FxHashSet;
88
use rustc_errors::struct_span_err;
9-
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan};
9+
use rustc_errors::{
10+
pluralize, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan,
11+
};
1012
use rustc_feature::BUILTIN_ATTRIBUTES;
1113
use rustc_hir::def::Namespace::{self, *};
1214
use rustc_hir::def::{self, CtorKind, CtorOf, DefKind, NonMacroAttrKind, PerNS};
@@ -1604,6 +1606,16 @@ impl<'a> Resolver<'a> {
16041606
err.span_label(ident.span, &format!("private {}", descr));
16051607
if let Some(span) = ctor_fields_span {
16061608
err.span_label(span, "a constructor is private if any of the fields is private");
1609+
if let Res::Def(_, d) = res && let Some(fields) = self.field_visibility_spans.get(&d) {
1610+
err.multipart_suggestion_verbose(
1611+
&format!(
1612+
"consider making the field{} publicly accessible",
1613+
pluralize!(fields.len())
1614+
),
1615+
fields.iter().map(|span| (*span, "pub ".to_string())).collect(),
1616+
Applicability::MaybeIncorrect,
1617+
);
1618+
}
16071619
}
16081620

16091621
// Print the whole import chain to make it easier to see what happens.

‎compiler/rustc_resolve/src/late/diagnostics.rs

+3-12
Original file line numberDiff line numberDiff line change
@@ -1451,22 +1451,13 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
14511451
.collect();
14521452

14531453
if non_visible_spans.len() > 0 {
1454-
if let Some(visibility_spans) = self.r.field_visibility_spans.get(&def_id) {
1454+
if let Some(fields) = self.r.field_visibility_spans.get(&def_id) {
14551455
err.multipart_suggestion_verbose(
14561456
&format!(
14571457
"consider making the field{} publicly accessible",
1458-
pluralize!(visibility_spans.len())
1458+
pluralize!(fields.len())
14591459
),
1460-
visibility_spans
1461-
.iter()
1462-
.map(|span| {
1463-
(
1464-
*span,
1465-
if span.is_empty() { "pub " } else { "pub" }
1466-
.to_string(),
1467-
)
1468-
})
1469-
.collect(),
1460+
fields.iter().map(|span| (*span, "pub ".to_string())).collect(),
14701461
Applicability::MaybeIncorrect,
14711462
);
14721463
}

‎tests/ui/privacy/privacy5.stderr

+96
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ note: the tuple struct constructor `A` is defined here
1212
|
1313
LL | pub struct A(());
1414
| ^^^^^^^^^^^^^^^^^
15+
help: consider making the field publicly accessible
16+
|
17+
LL | pub struct A(pub ());
18+
| +++
1519

1620
error[E0603]: tuple struct constructor `B` is private
1721
--> $DIR/privacy5.rs:52:16
@@ -27,6 +31,10 @@ note: the tuple struct constructor `B` is defined here
2731
|
2832
LL | pub struct B(isize);
2933
| ^^^^^^^^^^^^^^^^^^^^
34+
help: consider making the field publicly accessible
35+
|
36+
LL | pub struct B(pub isize);
37+
| +++
3038

3139
error[E0603]: tuple struct constructor `C` is private
3240
--> $DIR/privacy5.rs:53:16
@@ -42,6 +50,10 @@ note: the tuple struct constructor `C` is defined here
4250
|
4351
LL | pub struct C(pub isize, isize);
4452
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
53+
help: consider making the fields publicly accessible
54+
|
55+
LL | pub struct C(pub isize, pub isize);
56+
| ~~~ +++
4557

4658
error[E0603]: tuple struct constructor `A` is private
4759
--> $DIR/privacy5.rs:56:12
@@ -57,6 +69,10 @@ note: the tuple struct constructor `A` is defined here
5769
|
5870
LL | pub struct A(());
5971
| ^^^^^^^^^^^^^^^^^
72+
help: consider making the field publicly accessible
73+
|
74+
LL | pub struct A(pub ());
75+
| +++
6076

6177
error[E0603]: tuple struct constructor `A` is private
6278
--> $DIR/privacy5.rs:57:12
@@ -72,6 +88,10 @@ note: the tuple struct constructor `A` is defined here
7288
|
7389
LL | pub struct A(());
7490
| ^^^^^^^^^^^^^^^^^
91+
help: consider making the field publicly accessible
92+
|
93+
LL | pub struct A(pub ());
94+
| +++
7595

7696
error[E0603]: tuple struct constructor `A` is private
7797
--> $DIR/privacy5.rs:58:18
@@ -87,6 +107,10 @@ note: the tuple struct constructor `A` is defined here
87107
|
88108
LL | pub struct A(());
89109
| ^^^^^^^^^^^^^^^^^
110+
help: consider making the field publicly accessible
111+
|
112+
LL | pub struct A(pub ());
113+
| +++
90114

91115
error[E0603]: tuple struct constructor `A` is private
92116
--> $DIR/privacy5.rs:59:18
@@ -102,6 +126,10 @@ note: the tuple struct constructor `A` is defined here
102126
|
103127
LL | pub struct A(());
104128
| ^^^^^^^^^^^^^^^^^
129+
help: consider making the field publicly accessible
130+
|
131+
LL | pub struct A(pub ());
132+
| +++
105133

106134
error[E0603]: tuple struct constructor `B` is private
107135
--> $DIR/privacy5.rs:61:12
@@ -117,6 +145,10 @@ note: the tuple struct constructor `B` is defined here
117145
|
118146
LL | pub struct B(isize);
119147
| ^^^^^^^^^^^^^^^^^^^^
148+
help: consider making the field publicly accessible
149+
|
150+
LL | pub struct B(pub isize);
151+
| +++
120152

121153
error[E0603]: tuple struct constructor `B` is private
122154
--> $DIR/privacy5.rs:62:12
@@ -132,6 +164,10 @@ note: the tuple struct constructor `B` is defined here
132164
|
133165
LL | pub struct B(isize);
134166
| ^^^^^^^^^^^^^^^^^^^^
167+
help: consider making the field publicly accessible
168+
|
169+
LL | pub struct B(pub isize);
170+
| +++
135171

136172
error[E0603]: tuple struct constructor `B` is private
137173
--> $DIR/privacy5.rs:63:18
@@ -147,6 +183,10 @@ note: the tuple struct constructor `B` is defined here
147183
|
148184
LL | pub struct B(isize);
149185
| ^^^^^^^^^^^^^^^^^^^^
186+
help: consider making the field publicly accessible
187+
|
188+
LL | pub struct B(pub isize);
189+
| +++
150190

151191
error[E0603]: tuple struct constructor `B` is private
152192
--> $DIR/privacy5.rs:64:18
@@ -162,6 +202,10 @@ note: the tuple struct constructor `B` is defined here
162202
|
163203
LL | pub struct B(isize);
164204
| ^^^^^^^^^^^^^^^^^^^^
205+
help: consider making the field publicly accessible
206+
|
207+
LL | pub struct B(pub isize);
208+
| +++
165209

166210
error[E0603]: tuple struct constructor `B` is private
167211
--> $DIR/privacy5.rs:65:18
@@ -177,6 +221,10 @@ note: the tuple struct constructor `B` is defined here
177221
|
178222
LL | pub struct B(isize);
179223
| ^^^^^^^^^^^^^^^^^^^^
224+
help: consider making the field publicly accessible
225+
|
226+
LL | pub struct B(pub isize);
227+
| +++
180228

181229
error[E0603]: tuple struct constructor `B` is private
182230
--> $DIR/privacy5.rs:65:32
@@ -192,6 +240,10 @@ note: the tuple struct constructor `B` is defined here
192240
|
193241
LL | pub struct B(isize);
194242
| ^^^^^^^^^^^^^^^^^^^^
243+
help: consider making the field publicly accessible
244+
|
245+
LL | pub struct B(pub isize);
246+
| +++
195247

196248
error[E0603]: tuple struct constructor `C` is private
197249
--> $DIR/privacy5.rs:68:12
@@ -207,6 +259,10 @@ note: the tuple struct constructor `C` is defined here
207259
|
208260
LL | pub struct C(pub isize, isize);
209261
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
262+
help: consider making the fields publicly accessible
263+
|
264+
LL | pub struct C(pub isize, pub isize);
265+
| ~~~ +++
210266

211267
error[E0603]: tuple struct constructor `C` is private
212268
--> $DIR/privacy5.rs:69:12
@@ -222,6 +278,10 @@ note: the tuple struct constructor `C` is defined here
222278
|
223279
LL | pub struct C(pub isize, isize);
224280
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
281+
help: consider making the fields publicly accessible
282+
|
283+
LL | pub struct C(pub isize, pub isize);
284+
| ~~~ +++
225285

226286
error[E0603]: tuple struct constructor `C` is private
227287
--> $DIR/privacy5.rs:70:12
@@ -237,6 +297,10 @@ note: the tuple struct constructor `C` is defined here
237297
|
238298
LL | pub struct C(pub isize, isize);
239299
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
300+
help: consider making the fields publicly accessible
301+
|
302+
LL | pub struct C(pub isize, pub isize);
303+
| ~~~ +++
240304

241305
error[E0603]: tuple struct constructor `C` is private
242306
--> $DIR/privacy5.rs:71:12
@@ -252,6 +316,10 @@ note: the tuple struct constructor `C` is defined here
252316
|
253317
LL | pub struct C(pub isize, isize);
254318
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
319+
help: consider making the fields publicly accessible
320+
|
321+
LL | pub struct C(pub isize, pub isize);
322+
| ~~~ +++
255323

256324
error[E0603]: tuple struct constructor `C` is private
257325
--> $DIR/privacy5.rs:72:18
@@ -267,6 +335,10 @@ note: the tuple struct constructor `C` is defined here
267335
|
268336
LL | pub struct C(pub isize, isize);
269337
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
338+
help: consider making the fields publicly accessible
339+
|
340+
LL | pub struct C(pub isize, pub isize);
341+
| ~~~ +++
270342

271343
error[E0603]: tuple struct constructor `C` is private
272344
--> $DIR/privacy5.rs:73:18
@@ -282,6 +354,10 @@ note: the tuple struct constructor `C` is defined here
282354
|
283355
LL | pub struct C(pub isize, isize);
284356
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
357+
help: consider making the fields publicly accessible
358+
|
359+
LL | pub struct C(pub isize, pub isize);
360+
| ~~~ +++
285361

286362
error[E0603]: tuple struct constructor `C` is private
287363
--> $DIR/privacy5.rs:74:18
@@ -297,6 +373,10 @@ note: the tuple struct constructor `C` is defined here
297373
|
298374
LL | pub struct C(pub isize, isize);
299375
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
376+
help: consider making the fields publicly accessible
377+
|
378+
LL | pub struct C(pub isize, pub isize);
379+
| ~~~ +++
300380

301381
error[E0603]: tuple struct constructor `C` is private
302382
--> $DIR/privacy5.rs:75:18
@@ -312,6 +392,10 @@ note: the tuple struct constructor `C` is defined here
312392
|
313393
LL | pub struct C(pub isize, isize);
314394
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
395+
help: consider making the fields publicly accessible
396+
|
397+
LL | pub struct C(pub isize, pub isize);
398+
| ~~~ +++
315399

316400
error[E0603]: tuple struct constructor `A` is private
317401
--> $DIR/privacy5.rs:83:17
@@ -327,6 +411,10 @@ note: the tuple struct constructor `A` is defined here
327411
|
328412
LL | pub struct A(());
329413
| ^^^^^^^^^^^^^^^^^
414+
help: consider making the field publicly accessible
415+
|
416+
LL | pub struct A(pub ());
417+
| +++
330418

331419
error[E0603]: tuple struct constructor `B` is private
332420
--> $DIR/privacy5.rs:84:17
@@ -342,6 +430,10 @@ note: the tuple struct constructor `B` is defined here
342430
|
343431
LL | pub struct B(isize);
344432
| ^^^^^^^^^^^^^^^^^^^^
433+
help: consider making the field publicly accessible
434+
|
435+
LL | pub struct B(pub isize);
436+
| +++
345437

346438
error[E0603]: tuple struct constructor `C` is private
347439
--> $DIR/privacy5.rs:85:17
@@ -357,6 +449,10 @@ note: the tuple struct constructor `C` is defined here
357449
|
358450
LL | pub struct C(pub isize, isize);
359451
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
452+
help: consider making the fields publicly accessible
453+
|
454+
LL | pub struct C(pub isize, pub isize);
455+
| ~~~ +++
360456

361457
error[E0603]: tuple struct constructor `A` is private
362458
--> $DIR/privacy5.rs:90:20

‎tests/ui/privacy/suggest-making-field-public.fixed

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// run-rustfix
2+
mod a {
3+
pub struct A(pub String);
4+
}
5+
6+
mod b {
7+
use crate::a::A;
8+
pub fn x() {
9+
A("".into()); //~ ERROR cannot initialize a tuple struct which contains private fields
10+
}
11+
}
12+
fn main() {
13+
a::A("a".into()); //~ ERROR tuple struct constructor `A` is private
14+
b::x();
15+
}

‎tests/ui/privacy/suggest-making-field-public.rs

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// run-rustfix
2+
mod a {
3+
pub struct A(pub(self)String);
4+
}
5+
6+
mod b {
7+
use crate::a::A;
8+
pub fn x() {
9+
A("".into()); //~ ERROR cannot initialize a tuple struct which contains private fields
10+
}
11+
}
12+
fn main() {
13+
a::A("a".into()); //~ ERROR tuple struct constructor `A` is private
14+
b::x();
15+
}

0 commit comments

Comments
 (0)