Skip to content

Commit

Permalink
Rollup merge of #110450 - GuillaumeGomez:fix-nested-items-on-private-…
Browse files Browse the repository at this point in the history
…doc, r=notriddle,jyn514

rustdoc: Fix invalid handling of nested items with `--document-private-items`

Fixes #110422.

The problem is that only impl block and re-exported `macro_rules!` items are "visible" as nested items. This PR adds the missing checks to handle this correctly.

cc `@compiler-errors`
r? `@notriddle`
  • Loading branch information
matthiaskrgr committed Apr 18, 2023
2 parents 1e3a384 + c456e15 commit d646891
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 27 deletions.
48 changes: 38 additions & 10 deletions src/librustdoc/visit_ast.rs
Expand Up @@ -10,6 +10,7 @@ use rustc_hir::{Node, CRATE_HIR_ID};
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::TyCtxt;
use rustc_span::def_id::{CRATE_DEF_ID, LOCAL_CRATE};
use rustc_span::hygiene::MacroKind;
use rustc_span::symbol::{kw, sym, Symbol};
use rustc_span::Span;

Expand Down Expand Up @@ -87,6 +88,7 @@ pub(crate) struct RustdocVisitor<'a, 'tcx> {
inside_public_path: bool,
exact_paths: DefIdMap<Vec<Symbol>>,
modules: Vec<Module<'tcx>>,
is_importable_from_parent: bool,
}

impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
Expand All @@ -107,6 +109,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
inside_public_path: true,
exact_paths: Default::default(),
modules: vec![om],
is_importable_from_parent: true,
}
}

Expand Down Expand Up @@ -319,19 +322,31 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
renamed: Option<Symbol>,
parent_id: Option<LocalDefId>,
) {
self.modules
.last_mut()
.unwrap()
.items
.insert((item.owner_id.def_id, renamed), (item, renamed, parent_id));
if self.is_importable_from_parent
// If we're inside an item, only impl blocks and `macro_rules!` with the `macro_export`
// attribute can still be visible.
|| match item.kind {
hir::ItemKind::Impl(..) => true,
hir::ItemKind::Macro(_, MacroKind::Bang) => {
self.cx.tcx.has_attr(item.owner_id.def_id, sym::macro_export)
}
_ => false,
}
{
self.modules
.last_mut()
.unwrap()
.items
.insert((item.owner_id.def_id, renamed), (item, renamed, parent_id));
}
}

fn visit_item_inner(
&mut self,
item: &'tcx hir::Item<'_>,
renamed: Option<Symbol>,
import_id: Option<LocalDefId>,
) -> bool {
) {
debug!("visiting item {:?}", item);
let name = renamed.unwrap_or(item.ident.name);
let tcx = self.cx.tcx;
Expand Down Expand Up @@ -448,7 +463,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
}
}
}
true
}

fn visit_foreign_item_inner(
Expand Down Expand Up @@ -485,9 +499,23 @@ impl<'a, 'tcx> Visitor<'tcx> for RustdocVisitor<'a, 'tcx> {
}

fn visit_item(&mut self, i: &'tcx hir::Item<'tcx>) {
if self.visit_item_inner(i, None, None) {
walk_item(self, i);
}
self.visit_item_inner(i, None, None);
let new_value = if self.is_importable_from_parent {
matches!(
i.kind,
hir::ItemKind::Mod(..)
| hir::ItemKind::ForeignMod { .. }
| hir::ItemKind::Impl(..)
| hir::ItemKind::Trait(..)
)
} else {
// Whatever the context, if it's an impl block, the items inside it can be used so they
// should be visible.
matches!(i.kind, hir::ItemKind::Impl(..))
};
let prev = mem::replace(&mut self.is_importable_from_parent, new_value);
walk_item(self, i);
self.is_importable_from_parent = prev;
}

fn visit_mod(&mut self, _: &hir::Mod<'tcx>, _: Span, _: hir::HirId) {
Expand Down
5 changes: 4 additions & 1 deletion tests/rustdoc-ui/infinite-recursive-type-impl-trait.rs
@@ -1,6 +1,9 @@
// check-pass

fn f() -> impl Sized {
enum E { //~ ERROR
enum E {
V(E),
}

unimplemented!()
}
16 changes: 0 additions & 16 deletions tests/rustdoc-ui/infinite-recursive-type-impl-trait.stderr

This file was deleted.

64 changes: 64 additions & 0 deletions tests/rustdoc/issue-110422-inner-private.rs
@@ -0,0 +1,64 @@
// Regression test for <https://github.com/rust-lang/rust/issues/110422>.
// This test ensures that inner items (except for implementations and macros)
// don't appear in documentation.

// compile-flags: --document-private-items

#![crate_name = "foo"]

// @has 'foo/index.html'
// Checking there is no "trait" entry.
// @count - '//*[@id="main-content"]/*[@class="small-section-header"]' 4
// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Structs'
// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Constants'
// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Functions'
// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Macros'

// @has - '//a[@href="fn.foo.html"]' 'foo'
fn foo() {
fn bar() {}

// @has - '//a[@class="macro"]' 'visible_macro'
// @!has - '//a[@class="macro"]' 'non_visible_macro'
// @has 'foo/macro.visible_macro.html'
// @!has 'foo/macro.non_visible_macro.html'
#[macro_export]
macro_rules! visible_macro {
() => {}
}

macro_rules! non_visible_macro {
() => {}
}
}

// @has 'foo/index.html'
// @has - '//a[@href="struct.Bar.html"]' 'Bar'
struct Bar;

const BAR: i32 = {
// @!has - '//a[@href="fn.yo.html"]' 'yo'
// @!has 'foo/fn.yo.html'
fn yo() {}

// @!has 'foo/index.html' '//a[@href="trait.Foo.html"]' 'Foo'
// @!has 'foo/trait.Foo.html'
trait Foo {
fn babar() {}
}
impl Foo for Bar {}

// @has 'foo/struct.Bar.html'
// @has - '//*[@id="method.foo"]/*[@class="code-header"]' 'pub(crate) fn foo()'
// @count - '//*[@id="main-content"]/*[@class="small-section-header"]' 3
// We now check that the `Foo` trait is not documented nor visible on `Bar` page.
// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Implementations'
// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Auto Trait Implementations'
// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Blanket Implementations'
// @!has - '//*[@href="trait.Foo.html#method.babar"]/*[@class="code-header"]' 'fn babar()'
impl Bar {
fn foo() {}
}

1
};

0 comments on commit d646891

Please sign in to comment.