Skip to content
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

[rustdoc] partially fix invalid files creation #112836

Merged
merged 3 commits into from
Jun 21, 2023
Merged
Show file tree
Hide file tree
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
22 changes: 13 additions & 9 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,15 +358,15 @@ fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool {

impl Item {
pub(crate) fn stability<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<Stability> {
self.item_id.as_def_id().and_then(|did| tcx.lookup_stability(did))
self.def_id().and_then(|did| tcx.lookup_stability(did))
}

pub(crate) fn const_stability<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<ConstStability> {
self.item_id.as_def_id().and_then(|did| tcx.lookup_const_stability(did))
self.def_id().and_then(|did| tcx.lookup_const_stability(did))
}

pub(crate) fn deprecation(&self, tcx: TyCtxt<'_>) -> Option<Deprecation> {
self.item_id.as_def_id().and_then(|did| tcx.lookup_deprecation(did))
self.def_id().and_then(|did| tcx.lookup_deprecation(did))
}

pub(crate) fn inner_docs(&self, tcx: TyCtxt<'_>) -> bool {
Expand All @@ -391,7 +391,7 @@ impl Item {
panic!("blanket impl item has non-blanket ID")
}
}
_ => self.item_id.as_def_id().map(|did| rustc_span(did, tcx)),
_ => self.def_id().map(|did| rustc_span(did, tcx)),
}
}

Expand Down Expand Up @@ -501,7 +501,7 @@ impl Item {
}

pub(crate) fn is_crate(&self) -> bool {
self.is_mod() && self.item_id.as_def_id().map_or(false, |did| did.is_crate_root())
self.is_mod() && self.def_id().map_or(false, |did| did.is_crate_root())
}
pub(crate) fn is_mod(&self) -> bool {
self.type_() == ItemType::Module
Expand Down Expand Up @@ -638,11 +638,11 @@ impl Item {
}
let header = match *self.kind {
ItemKind::ForeignFunctionItem(_) => {
let def_id = self.item_id.as_def_id().unwrap();
let def_id = self.def_id().unwrap();
let abi = tcx.fn_sig(def_id).skip_binder().abi();
hir::FnHeader {
unsafety: if abi == Abi::RustIntrinsic {
intrinsic_operation_unsafety(tcx, self.item_id.as_def_id().unwrap())
intrinsic_operation_unsafety(tcx, self.def_id().unwrap())
} else {
hir::Unsafety::Unsafe
},
Expand All @@ -659,7 +659,7 @@ impl Item {
}
}
ItemKind::FunctionItem(_) | ItemKind::MethodItem(_, _) | ItemKind::TyMethodItem(_) => {
let def_id = self.item_id.as_def_id().unwrap();
let def_id = self.def_id().unwrap();
build_fn_header(def_id, tcx, tcx.asyncness(def_id))
}
_ => return None,
Expand Down Expand Up @@ -738,7 +738,7 @@ impl Item {
}
})
.collect();
if let Some(def_id) = self.item_id.as_def_id() &&
if let Some(def_id) = self.def_id() &&
!def_id.is_local() &&
// This check is needed because `adt_def` will panic if not a compatible type otherwise...
matches!(self.type_(), ItemType::Struct | ItemType::Enum | ItemType::Union)
Expand Down Expand Up @@ -787,6 +787,10 @@ impl Item {
pub fn is_doc_hidden(&self) -> bool {
self.attrs.is_doc_hidden()
}

pub fn def_id(&self) -> Option<DefId> {
self.item_id.as_def_id()
}
}

#[derive(Clone, Debug)]
Expand Down
5 changes: 5 additions & 0 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ pub(crate) struct Cache {
pub(crate) intra_doc_links: FxHashMap<ItemId, FxIndexSet<clean::ItemLink>>,
/// Cfg that have been hidden via #![doc(cfg_hide(...))]
pub(crate) hidden_cfg: FxHashSet<clean::cfg::Cfg>,

/// Contains the list of `DefId`s which have been inlined. It is used when generating files
/// to check if a stripped item should get its file generated or not: if it's inside a
/// `#[doc(hidden)]` item or a private one and not inlined, it shouldn't get a file.
pub(crate) inlined_items: DefIdSet,
}

/// This struct is used to wrap the `cache` and `tcx` in order to run `DocFolder`.
Expand Down
41 changes: 34 additions & 7 deletions src/librustdoc/html/render/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ pub(crate) struct Context<'tcx> {
pub(crate) include_sources: bool,
/// Collection of all types with notable traits referenced in the current module.
pub(crate) types_with_notable_traits: FxHashSet<clean::Type>,
/// Field used during rendering, to know if we're inside an inlined item.
pub(crate) is_inside_inlined_module: bool,
}

// `Context` is cloned a lot, so we don't want the size to grow unexpectedly.
Expand Down Expand Up @@ -171,6 +173,19 @@ impl<'tcx> Context<'tcx> {
}

fn render_item(&mut self, it: &clean::Item, is_module: bool) -> String {
let mut render_redirect_pages = self.render_redirect_pages;
// If the item is stripped but inlined, links won't point to the item so no need to generate
// a file for it.
if it.is_stripped() &&
let Some(def_id) = it.def_id() &&
def_id.is_local()
{
if self.is_inside_inlined_module || self.shared.cache.inlined_items.contains(&def_id) {
// For now we're forced to generate a redirect page for stripped items until
// `record_extern_fqn` correctly points to external items.
render_redirect_pages = true;
}
}
let mut title = String::new();
if !is_module {
title.push_str(it.name.unwrap().as_str());
Expand Down Expand Up @@ -205,7 +220,7 @@ impl<'tcx> Context<'tcx> {
tyname.as_str()
};

if !self.render_redirect_pages {
if !render_redirect_pages {
let clone_shared = Rc::clone(&self.shared);
let page = layout::Page {
css_class: tyname_s,
Expand Down Expand Up @@ -545,6 +560,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
shared: Rc::new(scx),
include_sources,
types_with_notable_traits: FxHashSet::default(),
is_inside_inlined_module: false,
};

if emit_crate {
Expand Down Expand Up @@ -574,6 +590,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
shared: Rc::clone(&self.shared),
include_sources: self.include_sources,
types_with_notable_traits: FxHashSet::default(),
is_inside_inlined_module: self.is_inside_inlined_module,
}
}

Expand Down Expand Up @@ -768,12 +785,22 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {

info!("Recursing into {}", self.dst.display());

let buf = self.render_item(item, true);
// buf will be empty if the module is stripped and there is no redirect for it
if !buf.is_empty() {
self.shared.ensure_dir(&self.dst)?;
let joint_dst = self.dst.join("index.html");
self.shared.fs.write(joint_dst, buf)?;
if !item.is_stripped() {
let buf = self.render_item(item, true);
// buf will be empty if the module is stripped and there is no redirect for it
if !buf.is_empty() {
self.shared.ensure_dir(&self.dst)?;
let joint_dst = self.dst.join("index.html");
self.shared.fs.write(joint_dst, buf)?;
}
}
if !self.is_inside_inlined_module {
if let Some(def_id) = item.def_id() && self.cache().inlined_items.contains(&def_id) {
self.is_inside_inlined_module = true;
}
} else if item.is_doc_hidden() {
// We're not inside an inlined module anymore since this one cannot be re-exported.
self.is_inside_inlined_module = false;
}

// Render sidebar-items.js used throughout this module.
Expand Down
7 changes: 5 additions & 2 deletions src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
return false;
}

let ret = match tcx.hir().get_by_def_id(res_did) {
let inlined = match tcx.hir().get_by_def_id(res_did) {
// Bang macros are handled a bit on their because of how they are handled by the
// compiler. If they have `#[doc(hidden)]` and the re-export doesn't have
// `#[doc(inline)]`, then we don't inline it.
Expand Down Expand Up @@ -344,7 +344,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
_ => false,
};
self.view_item_stack.remove(&res_did);
ret
if inlined {
self.cx.cache.inlined_items.insert(res_did.to_def_id());
}
inlined
}

/// Returns `true` if the item is visible, meaning it's not `#[doc(hidden)]` or private.
Expand Down
23 changes: 23 additions & 0 deletions tests/rustdoc/files-creation-hidden.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#![crate_name="foo"]

// @!has "foo/struct.Foo.html"
#[doc(hidden)]
pub struct Foo;

// @!has "foo/struct.Bar.html"
pub use crate::Foo as Bar;

// @!has "foo/struct.Baz.html"
#[doc(hidden)]
pub use crate::Foo as Baz;

// @!has "foo/foo/index.html"
#[doc(hidden)]
pub mod foo {}

// @!has "foo/bar/index.html"
pub use crate::foo as bar;

// @!has "foo/baz/index.html"
#[doc(hidden)]
pub use crate::foo as baz;
18 changes: 18 additions & 0 deletions tests/rustdoc/files-creation-private.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#![crate_name="foo"]

// @!has "foo/priv/index.html"
// @!has "foo/priv/struct.Foo.html"
mod private {
pub struct Foo;
}

// @has "foo/struct.Bar.html"
pub use crate::private::Foo as Bar;

// @!has "foo/foo/index.html"
mod foo {
pub mod subfoo {}
}

// @has "foo/bar/index.html"
pub use crate::foo::subfoo as bar;
8 changes: 4 additions & 4 deletions tests/rustdoc/issue-111064-reexport-trait-from-hidden-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
#![crate_name = "foo"]

// @!has 'foo/hidden/index.html'
// FIXME: add missing `@` for the two next tests once issue is fixed!
// To be done in <https://github.com/rust-lang/rust/issues/111249>.
// !has 'foo/hidden/inner/index.html'
// !has 'foo/hidden/inner/trait.Foo.html'
// @!has 'foo/hidden/inner/index.html'
// FIXME: Should be `@!has`: https://github.com/rust-lang/rust/issues/111249
// @has 'foo/hidden/inner/trait.Foo.html'
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
// @matchesraw - '<meta http-equiv="refresh" content="0;URL=../../../foo/visible/trait.Foo.html">'
#[doc(hidden)]
pub mod hidden {
pub mod inner {
Expand Down
34 changes: 34 additions & 0 deletions tests/rustdoc/issue-111249-file-creation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#![crate_name = "foo"]
#![feature(no_core)]
#![no_core]

// The following five should not fail!
// @!has 'foo/hidden/index.html'
// @!has 'foo/hidden/inner/index.html'
// FIXME: Should be `@!has`: https://github.com/rust-lang/rust/issues/111249
// @has 'foo/hidden/inner/trait.Foo.html'
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
// @matchesraw - '<meta http-equiv="refresh" content="0;URL=../../../foo/visible/trait.Foo.html">'
// @!has 'foo/hidden/inner/inner_hidden/index.html'
// @!has 'foo/hidden/inner/inner_hidden/trait.HiddenFoo.html'
#[doc(hidden)]
pub mod hidden {
pub mod inner {
pub trait Foo {}

#[doc(hidden)]
pub mod inner_hidden {
pub trait HiddenFoo {}
}
}
}

// @has 'foo/visible/index.html'
// @has 'foo/visible/trait.Foo.html'
#[doc(inline)]
pub use hidden::inner as visible;

// @has 'foo/struct.Bar.html'
// @count - '//*[@id="impl-Foo-for-Bar"]' 1
pub struct Bar;

impl visible::Foo for Bar {}
2 changes: 2 additions & 0 deletions tests/rustdoc/redirect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ pub trait Foo {}
// @has - '//code' 'pub use reexp_stripped::Bar'
// @has - '//code/a' 'Bar'
// @has - '//a[@href="../reexp_stripped/hidden/struct.Bar.html"]' 'Bar'
// FIXME: Should be `@!has`: https://github.com/rust-lang/rust/issues/111249
// @has reexp_stripped/hidden/struct.Bar.html
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
// @matchesraw - '<meta http-equiv="refresh" content="0;URL=../../reexp_stripped/struct.Bar.html">'
// @has 'reexp_stripped/struct.Bar.html'
// @has - '//a[@href="struct.Bar.html"]' 'Bar'
#[doc(no_inline)]
Expand Down
Loading