On privacy error caused by private reexport, use spans to show the `u…

…se` 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;`

src/librustc/hir/map/definitions.rs

@@ -202,7 +202,11 @@ impl DefPath {
         let mut s = String::with_capacity(self.data.len() * 16);
 
         for component in &self.data {
-            write!(s, "::{}[{}]", component.data.as_symbol(), component.disambiguator).unwrap();
+            if component.disambiguator == 0 {
+                write!(s, "::{}", component.data.as_symbol()).unwrap();
+            } else {
+                write!(s, "::{}[{}]", component.data.as_symbol(), component.disambiguator).unwrap();
+            }
         }
 
         s

src/librustc_resolve/diagnostics.rs

@@ -24,6 +24,7 @@ use crate::lifetimes::{ElisionFailureInfo, LifetimeContext};
 use crate::path_names_to_string;
 use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind};
 use crate::{BindingError, CrateLint, HasGenericParams, LegacyScope, Module, ModuleOrUniformRoot};
+use crate::{ModuleData, ModuleKind};
 use crate::{NameBinding, NameBindingKind, PrivacyError, VisResolutionError};
 use crate::{ParentScope, PathResult, ResolutionError, Resolver, Scope, ScopeSet, Segment};
 
@@ -419,8 +420,7 @@ impl<'a> Resolver<'a> {
                     self.session,
                     span,
                     E0128,
-                    "type parameters with a default cannot use \
-                                                forward declared identifiers"
+                    "type parameters with a default cannot use forward declared identifiers"
                 );
                 err.span_label(
                     span,
@@ -952,8 +952,7 @@ impl<'a> Resolver<'a> {
         err.emit();
     }
 
-    crate fn report_privacy_error(&self, privacy_error: &PrivacyError<'_>) {
-        let PrivacyError { ident, binding, .. } = *privacy_error;
+    crate fn report_privacy_error(&self, PrivacyError { ident, binding, .. }: &PrivacyError<'_>) {
         let session = &self.session;
         let mk_struct_span_error = |is_constructor| {
             let mut descr = binding.res().descr().to_string();
@@ -966,13 +965,7 @@ impl<'a> Resolver<'a> {
 
             let mut err =
                 struct_span_err!(session, ident.span, E0603, "{} `{}` is private", descr, ident);
-
             err.span_label(ident.span, &format!("this {} is private", descr));
-            err.span_note(
-                session.source_map().def_span(binding.span),
-                &format!("the {} `{}` is defined here", descr, ident),
-            );
-
             err
         };
 
@@ -997,6 +990,109 @@ impl<'a> Resolver<'a> {
             mk_struct_span_error(false)
         };
 
+        // Display the chain of re-exports through to the original def for cases where the
+        // `use` is private but the def is public.
+        let mut imported = false;
+        let mut binding = *binding;
+        loop {
+            let binding_span = session.source_map().def_span(binding.span);
+            match binding.kind {
+                NameBindingKind::Res(res, _is_macro_export) => {
+                    match (res, imported, binding.vis) {
+                        (Res::Def(_, def_id), true, ty::Visibility::Public) => {
+                            // FIXME: we should verify that this def is actually
+                            // reachable from the user's crate, as the parent modules
+                            // of this ADT might be private.
+                            if def_id.is_local() {
+                                err.span_help(
+                                    binding_span,
+                                    &format!(
+                                        "consider importing {} `{}` directly",
+                                        res.descr(),
+                                        self.definitions
+                                            .def_path(def_id.index)
+                                            .to_string_no_crate(),
+                                    ),
+                                );
+                            } else {
+                                err.span_help(
+                                    binding_span,
+                                    &format!(
+                                        "consider importing {} `{}` directly",
+                                        res.descr(),
+                                        ident.name
+                                    ),
+                                );
+                            }
+                        }
+                        (Res::Def(_, def_id), true, _) if def_id.is_local() => {
+                            err.span_help(
+                                binding_span,
+                                &format!("consider making {} `{}` public", res.descr(), ident.name),
+                            );
+                        }
+                        _ => {
+                            err.span_note(
+                                binding_span,
+                                &format!(
+                                    "the {}{} `{}` is defined here",
+                                    if imported { "re-exported " } else { "" },
+                                    res.descr(),
+                                    ident.name
+                                ),
+                            );
+                        }
+                    }
+                    break;
+                }
+                NameBindingKind::Module(ModuleData {
+                    kind: ModuleKind::Def(DefKind::Mod, def_id, _),
+                    ..
+                }) if def_id.index == CRATE_DEF_INDEX && def_id.krate != LOCAL_CRATE => {
+                    // Do not point at `extern crate foo;` twice.
+                    break;
+                }
+                NameBindingKind::Module(ModuleData {
+                    kind: ModuleKind::Def(kind, def_id, _),
+                    ..
+                }) => {
+                    err.span_note(
+                        binding_span,
+                        &format!(
+                            "the {}{} `{}` is defined here",
+                            if imported { "re-exported " } else { "" },
+                            kind.descr(*def_id),
+                            ident.name,
+                        ),
+                    );
+                    break;
+                }
+                NameBindingKind::Module(_) => break,
+                NameBindingKind::Import { binding: inner_binding, .. } => {
+                    err.span_note(
+                        binding_span,
+                        &format!(
+                            "{} {} re-export of `{}`{}",
+                            if imported { "...through this" } else { "the used" },
+                            if binding.vis == ty::Visibility::Public {
+                                "public"
+                            } else {
+                                "restricted"
+                            },
+                            ident.name,
+                            if let NameBindingKind::Import { .. } = inner_binding.kind {
+                                "..."
+                            } else {
+                                ""
+                            },
+                        ),
+                    );
+                    binding = inner_binding;
+                    imported = true;
+                }
+            }
+        }
+
         err.emit();
     }
 }

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

@@ -4,7 +4,7 @@ error[E0603]: crate import `core` is private
 LL | use foo::core::cell;
    |          ^^^^ this crate import is private
    |
-note: the crate import `core` is defined here
+note: the used restricted re-export of `core`
   --> $DIR/extern-crate-visibility.rs:2:5
    |
 LL |     extern crate core;
@@ -16,7 +16,7 @@ error[E0603]: crate import `core` is private
 LL |     foo::core::cell::Cell::new(0);
    |          ^^^^ this crate import is private
    |
-note: the crate import `core` is defined here
+note: the used restricted re-export of `core`
   --> $DIR/extern-crate-visibility.rs:2:5
    |
 LL |     extern crate core;

src/test/ui/import.stderr

@@ -19,11 +19,12 @@ error[E0603]: unresolved item import `foo` is private
 LL |     zed::foo();
    |          ^^^ this unresolved item import is private
    |
-note: the unresolved item import `foo` is defined here
+note: the used restricted re-export of `foo`
   --> $DIR/import.rs:10:9
    |
 LL |     use foo;
    |         ^^^
+   = note: the re-exported unresolved item `foo` is defined here
 
 error: aborting due to 3 previous errors
 

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 used restricted re-export of `ParseOptions`...
   --> $DIR/issue-55884-2.rs:9:9
    |
 LL |     use ParseOptions;
    |         ^^^^^^^^^^^^
+note: ...through this public re-export of `ParseOptions`...
+  --> $DIR/issue-55884-2.rs:12:9
+   |
+LL | pub use parser::ParseOptions;
+   |         ^^^^^^^^^^^^^^^^^^^^
+note: ...through this public re-export of `ParseOptions`
+  --> $DIR/issue-55884-2.rs:6:13
+   |
+LL |     pub use options::*;
+   |             ^^^^^^^^^^
+help: consider importing struct `::options::ParseOptions` directly
+  --> $DIR/issue-55884-2.rs:2:5
+   |
+LL |     pub struct ParseOptions {}
+   |     ^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/imports/reexports.stderr

@@ -16,23 +16,33 @@ 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 used restricted re-export of `foo`
   --> $DIR/reexports.rs:21:17
    |
 LL |         pub use super::foo; // This is OK since the value `foo` is visible enough.
    |                 ^^^^^^^^^^
+note: the re-exported module `foo` is defined here
+  --> $DIR/reexports.rs:16:5
+   |
+LL |     mod foo {
+   |     ^^^^^^^
 
 error[E0603]: module import `foo` is private
   --> $DIR/reexports.rs:34:15
    |
 LL |     use b::b::foo::S as T;
    |               ^^^ this module import is private
    |
-note: the module import `foo` is defined here
+note: the used restricted re-export of `foo`
   --> $DIR/reexports.rs:26:17
    |
 LL |         pub use super::*; // This is also OK since the value `foo` is visible enough.
    |                 ^^^^^^^^
+note: the re-exported module `foo` 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

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 used restricted re-export of `foo`
   --> $DIR/privacy2.rs:10:13
    |
 LL |         use foo;
    |             ^^^
+help: consider importing function `::foo` directly
+  --> $DIR/privacy2.rs:14:1
+   |
+LL | pub fn foo() {}
+   | ^^^^^^^^^^^^
 
 error: requires `sized` lang_item