Rollup merge of #153630 - arferreira:fix-doc-hidden-reexport-diagnostic-path, r=jackh726

Deprioritize doc(hidden) re-exports in diagnostic paths

Fixes #153477.

This is the other half of #99698, which fixed the case where the *parent module* is `#[doc(hidden)]` but left the case where the re-export itself is `#[doc(hidden)]` as a FIXME (with a tracking test in `dont-suggest-doc-hidden-variant-for-enum/hidden-child.rs`).

The problem: when a crate does `#[doc(hidden)] pub use core::error::Error`, diagnostics pick up the hidden re-export path instead of the canonical one. For example, `snafu::Error` instead of `std::error::Error`.

Two changes:

In `visible_parent_map`, the `add_child` closure now checks whether the re-export itself is `#[doc(hidden)]` via `reexport_chain` and sends it to `fallback_map`, same treatment as doc-hidden parents and underscore re-exports.

`should_encode_attrs` now returns `true` for `DefKind::Use`. Without this, `#[doc(hidden)]` on `use` items was never written to crate metadata, so `is_doc_hidden` always returned `false` cross-crate. This was the actual root cause, the check in `visible_parent_map` alone isn't enough if the attribute isn't in the metadata.

The existing FIXME test now serves as the regression test. The `.stderr` goes from suggesting `hidden_child::__private::Some(1i32)` to just `Some(1i32)`.

cc @eggyal

Author: matthiaskrgr

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -455,10 +455,11 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
 
             let mut visible_parent_map: DefIdMap<DefId> = Default::default();
             // This is a secondary visible_parent_map, storing the DefId of
-            // parents that re-export the child as `_` or module parents
-            // which are `#[doc(hidden)]`. Since we prefer paths that don't
-            // do this, merge this map at the end, only if we're missing
-            // keys from the former.
+            // parents that re-export the child as `_`, module parents
+            // which are `#[doc(hidden)]`, or `use` items that are themselves
+            // `#[doc(hidden)]`. Since we prefer paths that don't do this,
+            // merge this map at the end, only if we're missing keys from
+            // the former.
             // This is a rudimentary check that does not catch all cases,
             // just the easiest.
             let mut fallback_map: Vec<(DefId, DefId)> = Default::default();
@@ -500,6 +501,18 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
                         return;
                     }
 
+                    // If the re-export itself is `#[doc(hidden)]`, deprioritize it.
+                    // See PR #99698 for the case where the *parent* is doc-hidden.
+                    if child
+                        .reexport_chain
+                        .first()
+                        .and_then(|r| r.id())
+                        .is_some_and(|id| tcx.is_doc_hidden(id))
+                    {
+                        fallback_map.push((def_id, parent));
+                        return;
+                    }
+
                     match visible_parent_map.entry(def_id) {
                         Entry::Occupied(mut entry) => {
                             // If `child` is defined in crate `cnum`, ensure

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -943,6 +943,10 @@ fn should_encode_attrs(def_kind: DefKind) -> bool {
         | DefKind::Macro(_)
         | DefKind::Field
         | DefKind::Impl { .. } => true,
+        // Encoding attrs for `Use` items allows `#[doc(hidden)]` on re-exports
+        // to be read cross-crate, which is needed for diagnostic path selection
+        // in `visible_parent_map`. See #153477.
+        DefKind::Use => true,
         // Tools may want to be able to detect their tool lints on
         // closures from upstream crates, too. This is used by
         // https://github.com/model-checking/kani and is not a performance
@@ -953,7 +957,6 @@ fn should_encode_attrs(def_kind: DefKind) -> bool {
         | DefKind::ConstParam
         | DefKind::Ctor(..)
         | DefKind::ExternCrate
-        | DefKind::Use
         | DefKind::ForeignMod
         | DefKind::AnonConst
         | DefKind::InlineConst

tests/ui/suggestions/dont-suggest-doc-hidden-variant-for-enum/hidden-child.rs

@@ -1,7 +1,9 @@
 //@ aux-build:hidden-child.rs
 
-// FIXME(compiler-errors): This currently suggests the wrong thing.
-// UI test exists to track the problem.
+// Regression test for #153477.
+// When a re-export is #[doc(hidden)], diagnostics should prefer
+// the canonical path (e.g. `Some`) over the hidden re-export path
+// (e.g. `hidden_child::__private::Some`).
 
 extern crate hidden_child;
 

tests/ui/suggestions/dont-suggest-doc-hidden-variant-for-enum/hidden-child.stderr

@@ -1,5 +1,5 @@
 error[E0308]: mismatched types
-  --> $DIR/hidden-child.rs:9:26
+  --> $DIR/hidden-child.rs:11:26
    |
 LL |     let x: Option<i32> = 1i32;
    |            -----------   ^^^^ expected `Option<i32>`, found `i32`
@@ -8,10 +8,10 @@ LL |     let x: Option<i32> = 1i32;
    |
    = note: expected enum `Option<i32>`
               found type `i32`
-help: try wrapping the expression in `hidden_child::__private::Some`
+help: try wrapping the expression in `Some`
    |
-LL |     let x: Option<i32> = hidden_child::__private::Some(1i32);
-   |                          ++++++++++++++++++++++++++++++    +
+LL |     let x: Option<i32> = Some(1i32);
+   |                          +++++    +
 
 error: aborting due to 1 previous error