Skip to content

Commit b3a554d

Browse files
estebankestebank
committed
On privacy error caused by private reexport, use spans to show the use chain
Use full path for direct `use` of ADT instead of private re-export Point at enum defs and modules on private re-exports Use span notes to denote order Account for `use` of private `extern crate foo;`
1 parent a643ee8 commit b3a554d

File tree

12 files changed

+218
-76
lines changed

12 files changed

+218
-76
lines changed

‎src/librustc/hir/map/definitions.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,11 @@ impl DefPath {
202202
let mut s = String::with_capacity(self.data.len() * 16);
203203

204204
for component in &self.data {
205-
write!(s, "::{}[{}]", component.data.as_symbol(), component.disambiguator).unwrap();
205+
if component.disambiguator == 0 {
206+
write!(s, "::{}", component.data.as_symbol()).unwrap();
207+
} else {
208+
write!(s, "::{}[{}]", component.data.as_symbol(), component.disambiguator).unwrap();
209+
}
206210
}
207211

208212
s

‎src/librustc_resolve/diagnostics.rs‎

Lines changed: 106 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use crate::lifetimes::{ElisionFailureInfo, LifetimeContext};
2424
use crate::path_names_to_string;
2525
use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind};
2626
use crate::{BindingError, CrateLint, HasGenericParams, LegacyScope, Module, ModuleOrUniformRoot};
27+
use crate::{ModuleData, ModuleKind};
2728
use crate::{NameBinding, NameBindingKind, PrivacyError, VisResolutionError};
2829
use crate::{ParentScope, PathResult, ResolutionError, Resolver, Scope, ScopeSet, Segment};
2930

@@ -419,8 +420,7 @@ impl<'a> Resolver<'a> {
419420
self.session,
420421
span,
421422
E0128,
422-
"type parameters with a default cannot use \
423-
forward declared identifiers"
423+
"type parameters with a default cannot use forward declared identifiers"
424424
);
425425
err.span_label(
426426
span,
@@ -952,8 +952,7 @@ impl<'a> Resolver<'a> {
952952
err.emit();
953953
}
954954

955-
crate fn report_privacy_error(&self, privacy_error: &PrivacyError<'_>) {
956-
let PrivacyError { ident, binding, .. } = *privacy_error;
955+
crate fn report_privacy_error(&self, PrivacyError { ident, binding, .. }: &PrivacyError<'_>) {
957956
let session = &self.session;
958957
let mk_struct_span_error = |is_constructor| {
959958
let mut descr = binding.res().descr().to_string();
@@ -966,13 +965,7 @@ impl<'a> Resolver<'a> {
966965

967966
let mut err =
968967
struct_span_err!(session, ident.span, E0603, "{} `{}` is private", descr, ident);
969-
970968
err.span_label(ident.span, &format!("this {} is private", descr));
971-
err.span_note(
972-
session.source_map().def_span(binding.span),
973-
&format!("the {} `{}` is defined here", descr, ident),
974-
);
975-
976969
err
977970
};
978971

@@ -997,6 +990,109 @@ impl<'a> Resolver<'a> {
997990
mk_struct_span_error(false)
998991
};
999992

993+
// Display the chain of re-exports through to the original def for cases where the
994+
// `use` is private but the def is public.
995+
let mut imported = false;
996+
let mut binding = *binding;
997+
loop {
998+
let binding_span = session.source_map().def_span(binding.span);
999+
match binding.kind {
1000+
NameBindingKind::Res(res, _is_macro_export) => {
1001+
match (res, imported, binding.vis) {
1002+
(Res::Def(_, def_id), true, ty::Visibility::Public) => {
1003+
// FIXME: we should verify that this def is actually
1004+
// reachable from the user's crate, as the parent modules
1005+
// of this ADT might be private.
1006+
if def_id.is_local() {
1007+
err.span_help(
1008+
binding_span,
1009+
&format!(
1010+
"consider importing {} `{}` directly",
1011+
res.descr(),
1012+
self.definitions
1013+
.def_path(def_id.index)
1014+
.to_string_no_crate(),
1015+
),
1016+
);
1017+
} else {
1018+
err.span_help(
1019+
binding_span,
1020+
&format!(
1021+
"consider importing {} `{}` directly",
1022+
res.descr(),
1023+
ident.name
1024+
),
1025+
);
1026+
}
1027+
}
1028+
(Res::Def(_, def_id), true, _) if def_id.is_local() => {
1029+
err.span_help(
1030+
binding_span,
1031+
&format!("consider making {} `{}` public", res.descr(), ident.name),
1032+
);
1033+
}
1034+
_ => {
1035+
err.span_note(
1036+
binding_span,
1037+
&format!(
1038+
"the {}{} `{}` is defined here",
1039+
if imported { "re-exported " } else { "" },
1040+
res.descr(),
1041+
ident.name
1042+
),
1043+
);
1044+
}
1045+
}
1046+
break;
1047+
}
1048+
NameBindingKind::Module(ModuleData {
1049+
kind: ModuleKind::Def(DefKind::Mod, def_id, _),
1050+
..
1051+
}) if def_id.index == CRATE_DEF_INDEX && def_id.krate != LOCAL_CRATE => {
1052+
// Do not point at `extern crate foo;` twice.
1053+
break;
1054+
}
1055+
NameBindingKind::Module(ModuleData {
1056+
kind: ModuleKind::Def(kind, def_id, _),
1057+
..
1058+
}) => {
1059+
err.span_note(
1060+
binding_span,
1061+
&format!(
1062+
"the {}{} `{}` is defined here",
1063+
if imported { "re-exported " } else { "" },
1064+
kind.descr(*def_id),
1065+
ident.name,
1066+
),
1067+
);
1068+
break;
1069+
}
1070+
NameBindingKind::Module(_) => break,
1071+
NameBindingKind::Import { binding: inner_binding, .. } => {
1072+
err.span_note(
1073+
binding_span,
1074+
&format!(
1075+
"{} {} re-export of `{}`{}",
1076+
if imported { "...through this" } else { "the used" },
1077+
if binding.vis == ty::Visibility::Public {
1078+
"public"
1079+
} else {
1080+
"restricted"
1081+
},
1082+
ident.name,
1083+
if let NameBindingKind::Import { .. } = inner_binding.kind {
1084+
"..."
1085+
} else {
1086+
""
1087+
},
1088+
),
1089+
);
1090+
binding = inner_binding;
1091+
imported = true;
1092+
}
1093+
}
1094+
}
1095+
10001096
err.emit();
10011097
}
10021098
}

‎src/test/ui/extern/extern-crate-visibility.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ error[E0603]: crate import `core` is private
44
LL | use foo::core::cell;
55
| ^^^^ this crate import is private
66
|
7-
note: the crate import `core` is defined here
7+
note: the used restricted re-export of `core`
88
--> $DIR/extern-crate-visibility.rs:2:5
99
|
1010
LL | extern crate core;
@@ -16,7 +16,7 @@ error[E0603]: crate import `core` is private
1616
LL | foo::core::cell::Cell::new(0);
1717
| ^^^^ this crate import is private
1818
|
19-
note: the crate import `core` is defined here
19+
note: the used restricted re-export of `core`
2020
--> $DIR/extern-crate-visibility.rs:2:5
2121
|
2222
LL | extern crate core;

‎src/test/ui/import.stderr‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,12 @@ error[E0603]: unresolved item import `foo` is private
1919
LL | zed::foo();
2020
| ^^^ this unresolved item import is private
2121
|
22-
note: the unresolved item import `foo` is defined here
22+
note: the used restricted re-export of `foo`
2323
--> $DIR/import.rs:10:9
2424
|
2525
LL | use foo;
2626
| ^^^
27+
= note: the re-exported unresolved item `foo` is defined here
2728

2829
error: aborting due to 3 previous errors
2930

‎src/test/ui/imports/issue-55884-2.stderr‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,26 @@ error[E0603]: struct import `ParseOptions` is private
44
LL | pub use parser::ParseOptions;
55
| ^^^^^^^^^^^^ this struct import is private
66
|
7-
note: the struct import `ParseOptions` is defined here
7+
note: the used restricted re-export of `ParseOptions`...
88
--> $DIR/issue-55884-2.rs:9:9
99
|
1010
LL | use ParseOptions;
1111
| ^^^^^^^^^^^^
12+
note: ...through this public re-export of `ParseOptions`...
13+
--> $DIR/issue-55884-2.rs:12:9
14+
|
15+
LL | pub use parser::ParseOptions;
16+
| ^^^^^^^^^^^^^^^^^^^^
17+
note: ...through this public re-export of `ParseOptions`
18+
--> $DIR/issue-55884-2.rs:6:13
19+
|
20+
LL | pub use options::*;
21+
| ^^^^^^^^^^
22+
help: consider importing struct `::options::ParseOptions` directly
23+
--> $DIR/issue-55884-2.rs:2:5
24+
|
25+
LL | pub struct ParseOptions {}
26+
| ^^^^^^^^^^^^^^^^^^^^^^^
1227

1328
error: aborting due to previous error
1429

‎src/test/ui/imports/reexports.stderr‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,33 @@ error[E0603]: module import `foo` is private
1616
LL | use b::a::foo::S;
1717
| ^^^ this module import is private
1818
|
19-
note: the module import `foo` is defined here
19+
note: the used restricted re-export of `foo`
2020
--> $DIR/reexports.rs:21:17
2121
|
2222
LL | pub use super::foo; // This is OK since the value `foo` is visible enough.
2323
| ^^^^^^^^^^
24+
note: the re-exported module `foo` is defined here
25+
--> $DIR/reexports.rs:16:5
26+
|
27+
LL | mod foo {
28+
| ^^^^^^^
2429

2530
error[E0603]: module import `foo` is private
2631
--> $DIR/reexports.rs:34:15
2732
|
2833
LL | use b::b::foo::S as T;
2934
| ^^^ this module import is private
3035
|
31-
note: the module import `foo` is defined here
36+
note: the used restricted re-export of `foo`
3237
--> $DIR/reexports.rs:26:17
3338
|
3439
LL | pub use super::*; // This is also OK since the value `foo` is visible enough.
3540
| ^^^^^^^^
41+
note: the re-exported module `foo` is defined here
42+
--> $DIR/reexports.rs:16:5
43+
|
44+
LL | mod foo {
45+
| ^^^^^^^
3646

3747
warning: glob import doesn't reexport anything because no candidate is public enough
3848
--> $DIR/reexports.rs:9:17

‎src/test/ui/privacy/privacy2.stderr‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,16 @@ error[E0603]: function import `foo` is private
1010
LL | use bar::glob::foo;
1111
| ^^^ this function import is private
1212
|
13-
note: the function import `foo` is defined here
13+
note: the used restricted re-export of `foo`
1414
--> $DIR/privacy2.rs:10:13
1515
|
1616
LL | use foo;
1717
| ^^^
18+
help: consider importing function `::foo` directly
19+
--> $DIR/privacy2.rs:14:1
20+
|
21+
LL | pub fn foo() {}
22+
| ^^^^^^^^^^^^
1823

1924
error: requires `sized` lang_item
2025

0 commit comments

Comments
 (0)