Auto merge of rust-lang#113374 - GuillaumeGomez:private-to-public-path, r=notriddle,fmease

[rustdoc] If re-export is private, get the next item until a public one is found or expose the private item directly

Fixes rust-lang#81141.

If we have:

```rust
use Private as Something;

pub fn foo() -> Something {}
```

Then `Something` will be replaced by `Private`.

r? `@notriddle`

Author: bors

src/librustdoc/clean/mod.rs

@@ -1514,8 +1514,121 @@ pub(crate) fn clean_middle_assoc_item<'tcx>(
     Item::from_def_id_and_parts(assoc_item.def_id, Some(assoc_item.name), kind, cx)
 }
 
+fn first_non_private_clean_path<'tcx>(
+    cx: &mut DocContext<'tcx>,
+    path: &hir::Path<'tcx>,
+    new_path_segments: &'tcx [hir::PathSegment<'tcx>],
+    new_path_span: rustc_span::Span,
+) -> Path {
+    let new_hir_path =
+        hir::Path { segments: new_path_segments, res: path.res, span: new_path_span };
+    let mut new_clean_path = clean_path(&new_hir_path, cx);
+    // In here we need to play with the path data one last time to provide it the
+    // missing `args` and `res` of the final `Path` we get, which, since it comes
+    // from a re-export, doesn't have the generics that were originally there, so
+    // we add them by hand.
+    if let Some(path_last) = path.segments.last().as_ref()
+        && let Some(new_path_last) = new_clean_path.segments[..].last_mut()
+        && let Some(path_last_args) = path_last.args.as_ref()
+        && path_last.args.is_some()
+    {
+        assert!(new_path_last.args.is_empty());
+        new_path_last.args = clean_generic_args(path_last_args, cx);
+    }
+    new_clean_path
+}
+
+/// The goal of this function is to return the first `Path` which is not private (ie not private
+/// or `doc(hidden)`). If it's not possible, it'll return the "end type".
+///
+/// If the path is not a re-export or is public, it'll return `None`.
+fn first_non_private<'tcx>(
+    cx: &mut DocContext<'tcx>,
+    hir_id: hir::HirId,
+    path: &hir::Path<'tcx>,
+) -> Option<Path> {
+    let target_def_id = path.res.opt_def_id()?;
+    let (parent_def_id, ident) = match &path.segments[..] {
+        [] => return None,
+        // Relative paths are available in the same scope as the owner.
+        [leaf] => (cx.tcx.local_parent(hir_id.owner.def_id), leaf.ident),
+        // So are self paths.
+        [parent, leaf] if parent.ident.name == kw::SelfLower => {
+            (cx.tcx.local_parent(hir_id.owner.def_id), leaf.ident)
+        }
+        // Crate paths are not. We start from the crate root.
+        [parent, leaf] if matches!(parent.ident.name, kw::Crate | kw::PathRoot) => {
+            (LOCAL_CRATE.as_def_id().as_local()?, leaf.ident)
+        }
+        [parent, leaf] if parent.ident.name == kw::Super => {
+            let parent_mod = cx.tcx.parent_module(hir_id);
+            if let Some(super_parent) = cx.tcx.opt_local_parent(parent_mod) {
+                (super_parent, leaf.ident)
+            } else {
+                // If we can't find the parent of the parent, then the parent is already the crate.
+                (LOCAL_CRATE.as_def_id().as_local()?, leaf.ident)
+            }
+        }
+        // Absolute paths are not. We start from the parent of the item.
+        [.., parent, leaf] => (parent.res.opt_def_id()?.as_local()?, leaf.ident),
+    };
+    let hir = cx.tcx.hir();
+    // First we try to get the `DefId` of the item.
+    for child in
+        cx.tcx.module_children_local(parent_def_id).iter().filter(move |c| c.ident == ident)
+    {
+        if let Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) = child.res {
+            continue;
+        }
+
+        if let Some(def_id) = child.res.opt_def_id() && target_def_id == def_id {
+            let mut last_path_res = None;
+            'reexps: for reexp in child.reexport_chain.iter() {
+                if let Some(use_def_id) = reexp.id() &&
+                    let Some(local_use_def_id) = use_def_id.as_local() &&
+                    let Some(hir::Node::Item(item)) = hir.find_by_def_id(local_use_def_id) &&
+                    !item.ident.name.is_empty() &&
+                    let hir::ItemKind::Use(path, _) = item.kind
+                {
+                    for res in &path.res {
+                        if let Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) = res {
+                            continue;
+                        }
+                        if (cx.render_options.document_hidden ||
+                            !cx.tcx.is_doc_hidden(use_def_id)) &&
+                            // We never check for "cx.render_options.document_private"
+                            // because if a re-export is not fully public, it's never
+                            // documented.
+                            cx.tcx.local_visibility(local_use_def_id).is_public() {
+                            break 'reexps;
+                        }
+                        last_path_res = Some((path, res));
+                        continue 'reexps;
+                    }
+                }
+            }
+            if !child.reexport_chain.is_empty() {
+                // So in here, we use the data we gathered from iterating the reexports. If
+                // `last_path_res` is set, it can mean two things:
+                //
+                // 1. We found a public reexport.
+                // 2. We didn't find a public reexport so it's the "end type" path.
+                if let Some((new_path, _)) = last_path_res {
+                    return Some(first_non_private_clean_path(cx, path, new_path.segments, new_path.span));
+                }
+                // If `last_path_res` is `None`, it can mean two things:
+                //
+                // 1. The re-export is public, no need to change anything, just use the path as is.
+                // 2. Nothing was found, so let's just return the original path.
+                return None;
+            }
+        }
+    }
+    None
+}
+
 fn clean_qpath<'tcx>(hir_ty: &hir::Ty<'tcx>, cx: &mut DocContext<'tcx>) -> Type {
-    let hir::Ty { hir_id: _, span, ref kind } = *hir_ty;
+    let hir::Ty { hir_id, span, ref kind } = *hir_ty;
     let hir::TyKind::Path(qpath) = kind else { unreachable!() };
 
     match qpath {
@@ -1532,7 +1645,12 @@ fn clean_qpath<'tcx>(hir_ty: &hir::Ty<'tcx>, cx: &mut DocContext<'tcx>) -> Type
             if let Some(expanded) = maybe_expand_private_type_alias(cx, path) {
                 expanded
             } else {
-                let path = clean_path(path, cx);
+                // First we check if it's a private re-export.
+                let path = if let Some(path) = first_non_private(cx, hir_id, &path) {
+                    path
+                } else {
+                    clean_path(path, cx)
+                };
                 resolve_type(cx, path)
             }
         }
@@ -1683,7 +1801,7 @@ fn maybe_expand_private_type_alias<'tcx>(
         }
     }
 
-    Some(cx.enter_alias(args, def_id.to_def_id(), |cx| clean_ty(ty, cx)))
+    Some(cx.enter_alias(args, def_id.to_def_id(), |cx| clean_ty(&ty, cx)))
 }
 
 pub(crate) fn clean_ty<'tcx>(ty: &hir::Ty<'tcx>, cx: &mut DocContext<'tcx>) -> Type {

src/librustdoc/clean/types.rs

@@ -2212,6 +2212,17 @@ pub(crate) enum GenericArgs {
     Parenthesized { inputs: Box<[Type]>, output: Option<Box<Type>> },
 }
 
+impl GenericArgs {
+    pub(crate) fn is_empty(&self) -> bool {
+        match self {
+            GenericArgs::AngleBracketed { args, bindings } => {
+                args.is_empty() && bindings.is_empty()
+            }
+            GenericArgs::Parenthesized { inputs, output } => inputs.is_empty() && output.is_none(),
+        }
+    }
+}
+
 #[derive(Clone, PartialEq, Eq, Debug, Hash)]
 pub(crate) struct PathSegment {
     pub(crate) name: Symbol,

tests/rustdoc/auxiliary/jump-to-def-res-err-handling-aux.rs

@@ -0,0 +1 @@
+pub struct Ident;

tests/rustdoc/issue-81141-private-reexport-in-public-api-2.rs

@@ -0,0 +1,13 @@
+// edition:2015
+
+#![crate_name = "foo"]
+
+use external::Public as Private;
+
+pub mod external {
+    pub struct Public;
+
+    // @has 'foo/external/fn.make.html'
+    // @has - '//*[@class="rust item-decl"]/code' 'pub fn make() -> Public'
+    pub fn make() -> ::Private { super::Private }
+}

tests/rustdoc/issue-81141-private-reexport-in-public-api-generics.rs

@@ -0,0 +1,13 @@
+#![crate_name = "foo"]
+
+use crate::bar::Foo as Alias;
+
+pub mod bar {
+    pub struct Foo<'a, T>(&'a T);
+}
+
+// @has "foo/fn.foo.html"
+// @has - '//*[@class="rust item-decl"]/code' "pub fn foo<'a, T>(f: Foo<'a, T>) -> Foo<'a, usize>"
+pub fn foo<'a, T>(f: Alias<'a, T>) -> Alias<'a, usize> {
+    Alias(&0)
+}

tests/rustdoc/issue-81141-private-reexport-in-public-api-hidden.rs

@@ -0,0 +1,16 @@
+// compile-flags: -Z unstable-options --document-hidden-items
+
+#![crate_name = "foo"]
+
+#[doc(hidden)]
+pub use crate::bar::Bar as Alias;
+
+mod bar {
+    pub struct Bar;
+}
+
+// @has 'foo/fn.bar.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar() -> Alias'
+pub fn bar() -> Alias {
+    Alias
+}

tests/rustdoc/issue-81141-private-reexport-in-public-api-private.rs

@@ -0,0 +1,32 @@
+// compile-flags: --document-private-items
+
+#![crate_name = "foo"]
+
+use crate::bar::Bar as Alias;
+pub(crate) use crate::bar::Bar as CrateAlias;
+
+mod bar {
+    pub struct Bar;
+    pub use self::Bar as Inner;
+}
+
+// It's a fully private re-export so it should not be displayed.
+// @has 'foo/fn.bar.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar() -> Bar'
+pub fn bar() -> Alias {
+    Alias
+}
+
+// It's public re-export inside a private module so it should be visible.
+// @has 'foo/fn.bar2.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar2() -> Inner'
+pub fn bar2() -> crate::bar::Inner {
+    Alias
+}
+
+// It's a non-public, so it doesn't appear in documentation so it should not be visible.
+// @has 'foo/fn.bar3.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar3() -> Bar'
+pub fn bar3() -> CrateAlias {
+    Alias
+}

tests/rustdoc/issue-81141-private-reexport-in-public-api.rs

@@ -0,0 +1,124 @@
+// This test ensures that if a private re-export is present in a public API, it'll be
+// replaced by the first public item in the re-export chain or by the private item.
+
+#![crate_name = "foo"]
+
+use crate::bar::Bar as Alias;
+
+pub use crate::bar::Bar as Whatever;
+use crate::Whatever as Whatever2;
+use crate::Whatever2 as Whatever3;
+pub use crate::bar::Inner as Whatever4;
+
+mod bar {
+    pub struct Bar;
+    pub use self::Bar as Inner;
+}
+
+// @has 'foo/fn.bar.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar() -> Bar'
+pub fn bar() -> Alias {
+    Alias
+}
+
+// @has 'foo/fn.bar2.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar2() -> Whatever'
+pub fn bar2() -> Whatever3 {
+    Whatever
+}
+
+// @has 'foo/fn.bar3.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar3() -> Whatever4'
+pub fn bar3() -> Whatever4 {
+    Whatever
+}
+
+// @has 'foo/fn.bar4.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar4() -> Bar'
+pub fn bar4() -> crate::Alias {
+    Alias
+}
+
+// @has 'foo/fn.bar5.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar5() -> Whatever'
+pub fn bar5() -> crate::Whatever3 {
+    Whatever
+}
+
+// @has 'foo/fn.bar6.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar6() -> Whatever4'
+pub fn bar6() -> crate::Whatever4 {
+    Whatever
+}
+
+
+// @has 'foo/fn.bar7.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar7() -> Bar'
+pub fn bar7() -> self::Alias {
+    Alias
+}
+
+// @has 'foo/fn.bar8.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar8() -> Whatever'
+pub fn bar8() -> self::Whatever3 {
+    Whatever
+}
+
+// @has 'foo/fn.bar9.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar9() -> Whatever4'
+pub fn bar9() -> self::Whatever4 {
+    Whatever
+}
+
+mod nested {
+    pub(crate) use crate::Alias;
+    pub(crate) use crate::Whatever3;
+    pub(crate) use crate::Whatever4;
+    pub(crate) use crate::nested as nested2;
+}
+
+// @has 'foo/fn.bar10.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar10() -> Bar'
+pub fn bar10() -> nested::Alias {
+    Alias
+}
+
+// @has 'foo/fn.bar11.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar11() -> Whatever'
+pub fn bar11() -> nested::Whatever3 {
+    Whatever
+}
+
+// @has 'foo/fn.bar12.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar12() -> Whatever4'
+pub fn bar12() -> nested::Whatever4 {
+    Whatever
+}
+
+// @has 'foo/fn.bar13.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar13() -> Bar'
+pub fn bar13() -> nested::nested2::Alias {
+    Alias
+}
+
+// @has 'foo/fn.bar14.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar14() -> Whatever'
+pub fn bar14() -> nested::nested2::Whatever3 {
+    Whatever
+}
+
+// @has 'foo/fn.bar15.html'
+// @has - '//*[@class="rust item-decl"]/code' 'pub fn bar15() -> Whatever4'
+pub fn bar15() -> nested::nested2::Whatever4 {
+    Whatever
+}
+
+use external::Public as Private;
+
+pub mod external {
+    pub struct Public;
+
+    // @has 'foo/external/fn.make.html'
+    // @has - '//*[@class="rust item-decl"]/code' 'pub fn make() -> Public'
+    pub fn make() -> super::Private { super::Private }
+}

tests/rustdoc/private-use.rs

@@ -0,0 +1,13 @@
+// Regression test for <https://github.com/rust-lang/rust/pull/113374> to
+// ensure it doesn't panic.
+
+mod generics {
+    pub enum WherePredicate {
+        EqPredicate,
+    }
+}
+pub mod visit {
+    use *;
+    pub fn visit_where_predicate<V>(_visitor: &mut V, _i: &WherePredicate) {}
+}
+pub use generics::*;