resolve: Print import chains on privacy errors

petrochenkov · petrochenkov · commit 580c6a29d47a · 2020-03-11T21:17:32.000+03:00
diff --git 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,26 +937,62 @@ 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));
         if let Some(span) = ctor_fields_span {
             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
@@ -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
@@ -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 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
    |
 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
@@ -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
@@ -4,23 +4,33 @@ 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
    |
 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
 
