From 580c6a29d47a9cd786c89df98d97e6de13f49291 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 7 Mar 2020 21:18:29 +0300 Subject: [PATCH] resolve: Print import chains on privacy errors --- src/librustc_resolve/diagnostics.rs | 61 +++++++++++++++---- src/test/ui/imports/issue-55884-2.stderr | 17 +++++- src/test/ui/imports/reexports.stderr | 14 ++++- src/test/ui/privacy/privacy2.stderr | 7 ++- .../shadowed/shadowed-use-visibility.stderr | 14 ++++- 5 files changed, 95 insertions(+), 18 deletions(-) diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index edbb2db3868d4..063c62ad9aac7 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -1,4 +1,5 @@ use std::cmp::Reverse; +use std::ptr; use log::debug; use rustc::bug; @@ -936,15 +937,17 @@ impl<'a> Resolver<'a> { crate fn report_privacy_error(&self, privacy_error: &PrivacyError<'_>) { let PrivacyError { ident, binding, .. } = *privacy_error; + let res = binding.res(); let ctor_fields_span = self.ctor_fields_span(binding); - let mut descr = binding.res().descr().to_string(); - if ctor_fields_span.is_some() { - descr += " constructor"; - } - if binding.is_import() { - descr += " import"; - } - + let plain_descr = res.descr().to_string(); + let nonimport_descr = + if ctor_fields_span.is_some() { plain_descr + " constructor" } else { plain_descr }; + let import_descr = nonimport_descr.clone() + " import"; + let get_descr = + |b: &NameBinding<'_>| if b.is_import() { &import_descr } else { &nonimport_descr }; + + // Print the primary message. + let descr = get_descr(binding); let mut err = struct_span_err!(self.session, ident.span, E0603, "{} `{}` is private", descr, ident); err.span_label(ident.span, &format!("this {} is private", descr)); @@ -952,10 +955,44 @@ impl<'a> Resolver<'a> { err.span_label(span, "a constructor is private if any of the fields is private"); } - err.span_note( - self.session.source_map().def_span(binding.span), - &format!("the {} `{}` is defined here", descr, ident), - ); + // Print the whole import chain to make it easier to see what happens. + let first_binding = binding; + let mut next_binding = Some(binding); + let mut next_ident = ident; + while let Some(binding) = next_binding { + let name = next_ident; + next_binding = match binding.kind { + _ if res == Res::Err => None, + NameBindingKind::Import { binding, import, .. } => match import.kind { + _ if binding.span.is_dummy() => None, + ImportKind::Single { source, .. } => { + next_ident = source; + Some(binding) + } + ImportKind::Glob { .. } | ImportKind::MacroUse => Some(binding), + ImportKind::ExternCrate { .. } => None, + }, + _ => None, + }; + + let first = ptr::eq(binding, first_binding); + let descr = get_descr(binding); + let msg = format!( + "{and_refers_to}the {item} `{name}`{which} is defined here{dots}", + and_refers_to = if first { "" } else { "...and refers to " }, + item = descr, + name = name, + which = if first { "" } else { " which" }, + dots = if next_binding.is_some() { "..." } else { "" }, + ); + let def_span = self.session.source_map().def_span(binding.span); + let mut note_span = MultiSpan::from_span(def_span); + if !first && next_binding.is_none() && binding.vis == ty::Visibility::Public { + note_span.push_span_label(def_span, "consider importing it directly".into()); + } + err.span_note(note_span, &msg); + } + err.emit(); } } diff --git a/src/test/ui/imports/issue-55884-2.stderr b/src/test/ui/imports/issue-55884-2.stderr index f16d2adb3656e..3eee7118e5a22 100644 --- a/src/test/ui/imports/issue-55884-2.stderr +++ b/src/test/ui/imports/issue-55884-2.stderr @@ -4,11 +4,26 @@ error[E0603]: struct import `ParseOptions` is private LL | pub use parser::ParseOptions; | ^^^^^^^^^^^^ this struct import is private | -note: the struct import `ParseOptions` is defined here +note: the struct import `ParseOptions` is defined here... --> $DIR/issue-55884-2.rs:9:9 | LL | use ParseOptions; | ^^^^^^^^^^^^ +note: ...and refers to the struct import `ParseOptions` which is defined here... + --> $DIR/issue-55884-2.rs:12:9 + | +LL | pub use parser::ParseOptions; + | ^^^^^^^^^^^^^^^^^^^^ +note: ...and refers to the struct import `ParseOptions` which is defined here... + --> $DIR/issue-55884-2.rs:6:13 + | +LL | pub use options::*; + | ^^^^^^^^^^ +note: ...and refers to the struct `ParseOptions` which is defined here + --> $DIR/issue-55884-2.rs:2:5 + | +LL | pub struct ParseOptions {} + | ^^^^^^^^^^^^^^^^^^^^^^^ consider importing it directly error: aborting due to previous error diff --git a/src/test/ui/imports/reexports.stderr b/src/test/ui/imports/reexports.stderr index 7b0d63574ec8e..d63fbc7ec6781 100644 --- a/src/test/ui/imports/reexports.stderr +++ b/src/test/ui/imports/reexports.stderr @@ -16,11 +16,16 @@ error[E0603]: module import `foo` is private LL | use b::a::foo::S; | ^^^ this module import is private | -note: the module import `foo` is defined here +note: the module import `foo` is defined here... --> $DIR/reexports.rs:21:17 | LL | pub use super::foo; // This is OK since the value `foo` is visible enough. | ^^^^^^^^^^ +note: ...and refers to the module `foo` which is defined here + --> $DIR/reexports.rs:16:5 + | +LL | mod foo { + | ^^^^^^^ error[E0603]: module import `foo` is private --> $DIR/reexports.rs:34:15 @@ -28,11 +33,16 @@ error[E0603]: module import `foo` is private LL | use b::b::foo::S as T; | ^^^ this module import is private | -note: the module import `foo` is defined here +note: the module import `foo` is defined here... --> $DIR/reexports.rs:26:17 | LL | pub use super::*; // This is also OK since the value `foo` is visible enough. | ^^^^^^^^ +note: ...and refers to the module `foo` which is defined here + --> $DIR/reexports.rs:16:5 + | +LL | mod foo { + | ^^^^^^^ warning: glob import doesn't reexport anything because no candidate is public enough --> $DIR/reexports.rs:9:17 diff --git a/src/test/ui/privacy/privacy2.stderr b/src/test/ui/privacy/privacy2.stderr index 719dc27ccf4d6..b10c3a5265971 100644 --- a/src/test/ui/privacy/privacy2.stderr +++ b/src/test/ui/privacy/privacy2.stderr @@ -10,11 +10,16 @@ error[E0603]: function import `foo` is private LL | use bar::glob::foo; | ^^^ this function import is private | -note: the function import `foo` is defined here +note: the function import `foo` is defined here... --> $DIR/privacy2.rs:10:13 | LL | use foo; | ^^^ +note: ...and refers to the function `foo` which is defined here + --> $DIR/privacy2.rs:14:1 + | +LL | pub fn foo() {} + | ^^^^^^^^^^^^ consider importing it directly error: requires `sized` lang_item diff --git a/src/test/ui/shadowed/shadowed-use-visibility.stderr b/src/test/ui/shadowed/shadowed-use-visibility.stderr index cd8ec13794c6f..2244f3a46b266 100644 --- a/src/test/ui/shadowed/shadowed-use-visibility.stderr +++ b/src/test/ui/shadowed/shadowed-use-visibility.stderr @@ -4,11 +4,16 @@ error[E0603]: module import `bar` is private LL | use foo::bar::f as g; | ^^^ this module import is private | -note: the module import `bar` is defined here +note: the module import `bar` is defined here... --> $DIR/shadowed-use-visibility.rs:4:9 | LL | use foo as bar; | ^^^^^^^^^^ +note: ...and refers to the module `foo` which is defined here + --> $DIR/shadowed-use-visibility.rs:1:1 + | +LL | mod foo { + | ^^^^^^^ error[E0603]: module import `f` is private --> $DIR/shadowed-use-visibility.rs:15:10 @@ -16,11 +21,16 @@ error[E0603]: module import `f` is private LL | use bar::f::f; | ^ this module import is private | -note: the module import `f` is defined here +note: the module import `f` is defined here... --> $DIR/shadowed-use-visibility.rs:11:9 | LL | use foo as f; | ^^^^^^^^ +note: ...and refers to the module `foo` which is defined here + --> $DIR/shadowed-use-visibility.rs:1:1 + | +LL | mod foo { + | ^^^^^^^ error: aborting due to 2 previous errors