Tweak private re-export errors to point at correct span and have more context

compiler/rustc_resolve/src/diagnostics/mod.rs

@@ -831,22 +831,26 @@ pub(crate) struct CannotBeReexportedCratePublic {
 
 #[derive(Diagnostic)]
 #[diag("`{$ident}` is private, and cannot be re-exported", code = E0365)]
-#[note("consider declaring type or module `{$ident}` with `pub`")]
 pub(crate) struct CannotBeReexportedPrivateNS {
     #[primary_span]
     #[label("re-export of private `{$ident}`")]
     pub(crate) span: Span,
     pub(crate) ident: Ident,
+    #[note("consider declaring {$descr} `{$ident}` with `pub`")]
+    pub(crate) parent: Span,
+    pub(crate) descr: &'static str,
 }
 
 #[derive(Diagnostic)]
 #[diag("`{$ident}` is only public within the crate, and cannot be re-exported outside", code = E0365)]
-#[note("consider declaring type or module `{$ident}` with `pub`")]
 pub(crate) struct CannotBeReexportedCratePublicNS {
     #[primary_span]
     #[label("re-export of crate public `{$ident}`")]
     pub(crate) span: Span,
     pub(crate) ident: Ident,
+    #[note("consider declaring {$descr} `{$ident}` with `pub`")]
+    pub(crate) parent: Span,
+    pub(crate) descr: &'static str,
 }
 
 #[derive(Diagnostic)]
@@ -881,11 +885,12 @@ pub(crate) struct ConsiderMarkingAsPubCrate {
 }
 
 #[derive(Subdiagnostic)]
-#[note("consider marking `{$ident}` as `pub` in the imported module")]
+#[note("consider marking `{$ident}` as `pub` in the imported {$descr}")]
 pub(crate) struct ConsiderMarkingAsPub {
     #[primary_span]
-    pub(crate) span: Span,
+    pub(crate) span: MultiSpan,
     pub(crate) ident: Ident,
+    pub(crate) descr: &'static str,
 }
 
 #[derive(Diagnostic)]

compiler/rustc_resolve/src/imports.rs

@@ -6,10 +6,10 @@ use std::mem;
 use rustc_ast::NodeId;
 use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
 use rustc_data_structures::intern::Interned;
-use rustc_errors::{Applicability, BufferedEarlyLint, Diagnostic};
+use rustc_errors::{Applicability, BufferedEarlyLint, Diagnostic, MultiSpan};
 use rustc_expand::base::SyntaxExtensionKind;
 use rustc_hir::def::{self, DefKind};
-use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdMap};
+use rustc_hir::def_id::{CRATE_DEF_ID, DefId, LocalDefId, LocalDefIdMap};
 use rustc_lint_defs::LintId;
 use rustc_lint_defs::builtin::{
     AMBIGUOUS_GLOB_REEXPORTS, EXPORTED_PRIVATE_DEPENDENCIES, HIDDEN_GLOB_REEXPORTS,
@@ -20,7 +20,7 @@ use rustc_middle::ty::Visibility;
 use rustc_session::diagnostics::feature_err;
 use rustc_span::edit_distance::find_best_match_for_name;
 use rustc_span::hygiene::LocalExpnId;
-use rustc_span::{Ident, OrdSpan, Span, Symbol, kw, span_bug, sym};
+use rustc_span::{DUMMY_SP, Ident, OrdSpan, Span, Symbol, kw, span_bug, sym};
 use tracing::debug;
 
 use crate::Namespace::{self, *};
@@ -1685,10 +1685,24 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 diagnostic: diagnostic.into(),
             });
         } else if ns == TypeNS {
+            let def_id = decl.res().def_id();
+            let parent = if !def_id.is_crate_root() { self.def_span(def_id) } else { DUMMY_SP };
+            let descr = self.tcx().def_descr(def_id);
+
             if crate_private_reexport {
-                self.dcx().emit_err(CannotBeReexportedCratePublicNS { span: import.span, ident });
+                self.dcx().emit_err(CannotBeReexportedCratePublicNS {
+                    span: import.span,
+                    ident,
+                    parent,
+                    descr,
+                });
             } else {
-                self.dcx().emit_err(CannotBeReexportedPrivateNS { span: import.span, ident });
+                self.dcx().emit_err(CannotBeReexportedPrivateNS {
+                    span: import.span,
+                    ident,
+                    parent,
+                    descr,
+                });
             }
         } else {
             let mut err = if crate_private_reexport {
@@ -1708,7 +1722,30 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                     err.subdiagnostic(ConsiderMarkingAsPubCrate { vis_span: import.vis_span });
                 }
                 _ => {
-                    err.subdiagnostic(ConsiderMarkingAsPub { span: import.span, ident });
+                    let mut span: MultiSpan = decl.span.into();
+                    let descr = if decl.is_import() {
+                        "re-export"
+                    } else if let Some(def_id) = decl.res().opt_def_id() {
+                        let parent = self.tcx().parent(def_id);
+
+                        if let DefKind::Variant = self.tcx().def_kind(parent) {
+                            // We had the DefId for the variant's constructor, but we want to point at the enum.
+                            span.push_span_context(
+                                self.def_span(self.tcx().parent(parent)).shrink_to_lo(),
+                            );
+                            self.tcx().def_descr(self.tcx().parent(parent))
+                        } else if CRATE_DEF_ID.to_def_id() != parent {
+                            span.push_span_context(self.def_span(parent).shrink_to_lo());
+                            self.tcx().def_descr(parent)
+                        } else {
+                            self.tcx().def_descr(parent)
+                        }
+                    } else {
+                        // unreachable, but provide fallback just in case
+                        debug_assert!(false, "we have a DefId in {decl:?} with no parent?");
+                        "module"
+                    };
+                    err.subdiagnostic(ConsiderMarkingAsPub { span, ident, descr });
                 }
             }
             err.emit();

tests/ui/error-codes/E0365.stderr

@@ -4,7 +4,13 @@ error[E0365]: `foo` is only public within the crate, and cannot be re-exported o
 LL | pub use foo as foo2;
    |         ^^^^^^^^^^^ re-export of crate public `foo`
    |
-   = note: consider declaring type or module `foo` with `pub`
+note: consider declaring module `foo` with `pub`
+  --> $DIR/E0365.rs:1:1
+   |
+LL | / mod foo {
+LL | |     pub const X: u32 = 1;
+LL | | }
+   | |_^
 
 error: aborting due to 1 previous error
 

tests/ui/hygiene/privacy-early.stderr

@@ -9,13 +9,11 @@ LL |     foo::m!();
    |     --------- in this macro invocation
    |
 note: consider marking `f` as `pub` in the imported module
-  --> $DIR/privacy-early.rs:10:13
+  --> $DIR/privacy-early.rs:6:5
    |
-LL |         use f as g;
-   |             ^^^^^^
-...
-LL |     foo::m!();
-   |     --------- in this macro invocation
+LL | mod foo {
+LL |     fn f() {}
+   |     ^^^^^^^^^
 
 error[E0364]: `f` is private, and cannot be re-exported
   --> $DIR/privacy-early.rs:10:13
@@ -28,14 +26,12 @@ LL |     foo::m!();
    |     --------- in this macro invocation
    |
 note: consider marking `f` as `pub` in the imported module
-  --> $DIR/privacy-early.rs:10:13
+  --> $DIR/privacy-early.rs:7:5
    |
-LL |         use f as g;
-   |             ^^^^^^
-...
-LL |     foo::m!();
-   |     --------- in this macro invocation
-   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+LL | mod foo {
+LL |     fn f() {}
+LL |     macro f() {}
+   |     ^^^^^^^^^^^^
 
 error: aborting due to 2 previous errors
 

tests/ui/imports/private-from-decl-macro.fail.stderr

@@ -10,13 +10,12 @@ LL |     crate::m::mac_glob!();
    |     --------------------- in this macro invocation
    |
 note: consider marking `S` as `pub` in the imported module
-  --> $DIR/private-from-decl-macro.rs:18:13
+  --> $DIR/private-from-decl-macro.rs:10:5
    |
-LL |         use crate::m::*;
-   |             ^^^^^^^^^^^
+LL | mod m {
 ...
-LL |     crate::m::mac_glob!();
-   |     --------------------- in this macro invocation
+LL |     const S: u8 = 0;
+   |     ^^^^^^^^^^^^^^^^
 
 error[E0423]: cannot find value `S` in this scope
   --> $DIR/private-from-decl-macro.rs:28:17

tests/ui/imports/reexports.stderr

@@ -5,10 +5,11 @@ LL |         pub use super::foo;
    |                 ^^^^^^^^^^
    |
 note: consider marking `foo` as `pub` in the imported module
-  --> $DIR/reexports.rs:8:17
+  --> $DIR/reexports.rs:4:5
    |
-LL |         pub use super::foo;
-   |                 ^^^^^^^^^^
+LL | mod a {
+LL |     fn foo() {}
+   |     ^^^^^^^^^^^
 
 error[E0603]: module import `foo` is private
   --> $DIR/reexports.rs:36:22

tests/ui/modules/issue-56411.stderr

@@ -27,7 +27,14 @@ LL |             pub use self::$name;
 LL | import!(("issue-56411-aux.rs", issue_56411_aux));
    | ------------------------------------------------ in this macro invocation
    |
-   = note: consider declaring type or module `issue_56411_aux` with `pub`
+note: consider declaring module `issue_56411_aux` with `pub`
+  --> $DIR/issue-56411.rs:5:13
+   |
+LL |             mod $name;
+   |             ^^^^^^^^^^
+...
+LL | import!(("issue-56411-aux.rs", issue_56411_aux));
+   | ------------------------------------------------ in this macro invocation
 
 error: aborting due to 2 previous errors