Skip to content

Commit 059e825

Browse files
committed
resolve: Print import chains on privacy errors
1 parent 7819335 commit 059e825

File tree

5 files changed

+96
-18
lines changed

5 files changed

+96
-18
lines changed

src/librustc_resolve/diagnostics.rs

+50-12
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::cmp::Reverse;
2+
use std::ptr;
23

34
use log::debug;
45
use rustc::bug;
@@ -937,26 +938,63 @@ impl<'a> Resolver<'a> {
937938
crate fn report_privacy_error(&self, privacy_error: &PrivacyError<'_>) {
938939
let PrivacyError { ident, binding, .. } = *privacy_error;
939940

941+
let res = binding.res();
940942
let ctor_fields_span = self.ctor_fields_span(binding);
941-
let mut descr = binding.res().descr().to_string();
942-
if ctor_fields_span.is_some() {
943-
descr += " constructor";
944-
}
945-
if binding.is_import() {
946-
descr += " import";
947-
}
948-
943+
let plain_descr = res.descr().to_string();
944+
let nonimport_descr =
945+
if ctor_fields_span.is_some() { plain_descr + " constructor" } else { plain_descr };
946+
let import_descr = nonimport_descr.clone() + " import";
947+
let get_descr =
948+
|b: &NameBinding<'_>| if b.is_import() { &import_descr } else { &nonimport_descr };
949+
950+
// Print the primary message.
951+
let descr = get_descr(binding);
949952
let mut err =
950953
struct_span_err!(self.session, ident.span, E0603, "{} `{}` is private", descr, ident);
951954
err.span_label(ident.span, &format!("this {} is private", descr));
952955
if let Some(span) = ctor_fields_span {
953956
err.span_label(span, "a constructor is private if any of the fields is private");
954957
}
955958

956-
err.span_note(
957-
self.session.source_map().def_span(binding.span),
958-
&format!("the {} `{}` is defined here", descr, ident),
959-
);
959+
// Print the whole import chain to make it easier to see what happens.
960+
let first_binding = binding;
961+
let mut next_binding = Some(binding);
962+
let mut next_ident = ident;
963+
while let Some(binding) = next_binding {
964+
let name = next_ident;
965+
next_binding = match binding.kind {
966+
_ if res == Res::Err => None,
967+
NameBindingKind::Import { binding, directive, .. } => match directive.subclass {
968+
_ if binding.span.is_dummy() => None,
969+
ImportDirectiveSubclass::SingleImport { source, .. } => {
970+
next_ident = source;
971+
Some(binding)
972+
}
973+
ImportDirectiveSubclass::GlobImport { .. }
974+
| ImportDirectiveSubclass::MacroUse => Some(binding),
975+
ImportDirectiveSubclass::ExternCrate { .. } => None,
976+
},
977+
_ => None,
978+
};
979+
980+
let first = ptr::eq(binding, first_binding);
981+
let descr = get_descr(binding);
982+
let msg = format!(
983+
"{and_refers_to}the {item} `{name}`{which} is defined here{dots}",
984+
and_refers_to = if first { "" } else { "...and refers to " },
985+
item = descr,
986+
name = name,
987+
which = if first { "" } else { " which" },
988+
dots = if next_binding.is_some() { "..." } else { "" },
989+
);
990+
let def_span = self.session.source_map().def_span(binding.span);
991+
let mut note_span = MultiSpan::from_span(def_span);
992+
if !first && next_binding.is_none() && binding.vis == ty::Visibility::Public {
993+
note_span.push_span_label(def_span, "consider importing it directly".into());
994+
}
995+
err.span_note(note_span, &msg);
996+
}
997+
960998
err.emit();
961999
}
9621000
}

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

+16-1
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 struct import `ParseOptions` is defined here...
88
--> $DIR/issue-55884-2.rs:9:9
99
|
1010
LL | use ParseOptions;
1111
| ^^^^^^^^^^^^
12+
note: ...and refers to the struct import `ParseOptions` which is defined here...
13+
--> $DIR/issue-55884-2.rs:12:9
14+
|
15+
LL | pub use parser::ParseOptions;
16+
| ^^^^^^^^^^^^^^^^^^^^
17+
note: ...and refers to the struct import `ParseOptions` which is defined here...
18+
--> $DIR/issue-55884-2.rs:6:13
19+
|
20+
LL | pub use options::*;
21+
| ^^^^^^^^^^
22+
note: ...and refers to the struct `ParseOptions` which is defined here
23+
--> $DIR/issue-55884-2.rs:2:5
24+
|
25+
LL | pub struct ParseOptions {}
26+
| ^^^^^^^^^^^^^^^^^^^^^^^ consider importing it directly
1227

1328
error: aborting due to previous error
1429

src/test/ui/imports/reexports.stderr

+12-2
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 module import `foo` is defined here...
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: ...and refers to the module `foo` which 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 module import `foo` is defined here...
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: ...and refers to the module `foo` which 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

+6-1
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 function import `foo` is defined here...
1414
--> $DIR/privacy2.rs:10:13
1515
|
1616
LL | use foo;
1717
| ^^^
18+
note: ...and refers to the function `foo` which is defined here
19+
--> $DIR/privacy2.rs:14:1
20+
|
21+
LL | pub fn foo() {}
22+
| ^^^^^^^^^^^^ consider importing it directly
1823

1924
error: requires `sized` lang_item
2025

src/test/ui/shadowed/shadowed-use-visibility.stderr

+12-2
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,33 @@ error[E0603]: module import `bar` is private
44
LL | use foo::bar::f as g;
55
| ^^^ this module import is private
66
|
7-
note: the module import `bar` is defined here
7+
note: the module import `bar` is defined here...
88
--> $DIR/shadowed-use-visibility.rs:4:9
99
|
1010
LL | use foo as bar;
1111
| ^^^^^^^^^^
12+
note: ...and refers to the module `foo` which is defined here
13+
--> $DIR/shadowed-use-visibility.rs:1:1
14+
|
15+
LL | mod foo {
16+
| ^^^^^^^
1217

1318
error[E0603]: module import `f` is private
1419
--> $DIR/shadowed-use-visibility.rs:15:10
1520
|
1621
LL | use bar::f::f;
1722
| ^ this module import is private
1823
|
19-
note: the module import `f` is defined here
24+
note: the module import `f` is defined here...
2025
--> $DIR/shadowed-use-visibility.rs:11:9
2126
|
2227
LL | use foo as f;
2328
| ^^^^^^^^
29+
note: ...and refers to the module `foo` which is defined here
30+
--> $DIR/shadowed-use-visibility.rs:1:1
31+
|
32+
LL | mod foo {
33+
| ^^^^^^^
2434

2535
error: aborting due to 2 previous errors
2636

0 commit comments

Comments
 (0)