Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jul 27, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 121 additions & 3 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
@@ -1496,8 +1496,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 {
Copy link
Member

@fmease fmease Jul 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, we could at some point support Ctor but that's low priority. This would only be reachable with const generics (and const items if/once we render their body). E.g. Container<{ Alias }> with use Type::Ctor as Alias.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now it's something we exclude everywhere in rustdoc when looking for reexports. We can indeed add support for it later on.

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 {
@@ -1514,7 +1627,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)
}
}
@@ -1665,7 +1783,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 {
11 changes: 11 additions & 0 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
@@ -2203,6 +2203,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,
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub struct Ident;
13 changes: 13 additions & 0 deletions tests/rustdoc/issue-81141-private-reexport-in-public-api-2.rs
Original file line number Diff line number Diff line change
@@ -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 }
}
Original file line number Diff line number Diff line change
@@ -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)
}
16 changes: 16 additions & 0 deletions tests/rustdoc/issue-81141-private-reexport-in-public-api-hidden.rs
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -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
}
124 changes: 124 additions & 0 deletions tests/rustdoc/issue-81141-private-reexport-in-public-api.rs
Original file line number Diff line number Diff line change
@@ -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 }
}
13 changes: 13 additions & 0 deletions tests/rustdoc/private-use.rs
Original file line number Diff line number Diff line change
@@ -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::*;