Rollup merge of rust-lang#69811 - petrochenkov:privdiag2, r=estebank

Centril · web-flow · commit 6a77a7537675 · 2020-03-10T10:41:17.000+01:00
resolve: Print import chains on privacy errors A part of rust-lang#67951 that doesn't require hacks. r? @estebank
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;
@@ -916,50 +917,82 @@ impl<'a> Resolver<'a> {
         err.emit();
     }
 
-    crate fn report_privacy_error(&self, privacy_error: &PrivacyError<'_>) {
-        let PrivacyError { ident, binding, .. } = *privacy_error;
-        let session = &self.session;
-        let mk_struct_span_error = |is_constructor| {
-            let mut descr = binding.res().descr().to_string();
-            if is_constructor {
-                descr += " constructor";
-            }
-            if binding.is_import() {
-                descr += " import";
-            }
-
-            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
-        };
-
-        let mut err = if let NameBindingKind::Res(
+    /// If the binding refers to a tuple struct constructor with fields,
+    /// returns the span of its fields.
+    fn ctor_fields_span(&self, binding: &NameBinding<'_>) -> Option<Span> {
+        if let NameBindingKind::Res(
             Res::Def(DefKind::Ctor(CtorOf::Struct, CtorKind::Fn), ctor_def_id),
             _,
         ) = binding.kind
         {
             let def_id = (&*self).parent(ctor_def_id).expect("no parent for a constructor");
             if let Some(fields) = self.field_names.get(&def_id) {
-                let mut err = mk_struct_span_error(true);
                 let first_field = fields.first().expect("empty field list in the map");
-                err.span_label(
-                    fields.iter().fold(first_field.span, |acc, field| acc.to(field.span)),
-                    "a constructor is private if any of the fields is private",
-                );
-                err
-            } else {
-                mk_struct_span_error(false)
+                return Some(fields.iter().fold(first_field.span, |acc, field| acc.to(field.span)));
             }
-        } else {
-            mk_struct_span_error(false)
-        };
+        }
+        None
+    }
+
+    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 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");
+        }
+
+        // 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, directive, .. } => match directive.subclass {
+                    _ if binding.span.is_dummy() => None,
+                    ImportDirectiveSubclass::SingleImport { source, .. } => {
+                        next_ident = source;
+                        Some(binding)
+                    }
+                    ImportDirectiveSubclass::GlobImport { .. }
+                    | ImportDirectiveSubclass::MacroUse => Some(binding),
+                    ImportDirectiveSubclass::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
 
