Skip to content

Visually mark 👻hidden👻 items with document-hidden-items #122495

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 7 commits into from
Mar 15, 2024
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
16 changes: 13 additions & 3 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
@@ -62,6 +62,9 @@ pub(crate) fn try_inline(
attrs_without_docs.as_ref().map(|(attrs, def_id)| (&attrs[..], *def_id));

let import_def_id = attrs.and_then(|(_, def_id)| def_id);

let (attrs, cfg) = merge_attrs(cx, load_attrs(cx, did), attrs);

let kind = match res {
Res::Def(DefKind::Trait, did) => {
record_extern_fqn(cx, did, ItemType::Trait);
@@ -131,7 +134,7 @@ pub(crate) fn try_inline(
cx.with_param_env(did, |cx| clean::ConstantItem(build_const(cx, did)))
}
Res::Def(DefKind::Macro(kind), did) => {
let mac = build_macro(cx, did, name, import_def_id, kind);
let mac = build_macro(cx, did, name, import_def_id, kind, attrs.is_doc_hidden());

let type_kind = match kind {
MacroKind::Bang => ItemType::Macro,
@@ -144,7 +147,6 @@ pub(crate) fn try_inline(
_ => return None,
};

let (attrs, cfg) = merge_attrs(cx, load_attrs(cx, did), attrs);
cx.inlined.insert(did.into());
let mut item =
clean::Item::from_def_id_and_attrs_and_parts(did, Some(name), kind, Box::new(attrs), cfg);
@@ -751,14 +753,22 @@ fn build_macro(
name: Symbol,
import_def_id: Option<DefId>,
macro_kind: MacroKind,
is_doc_hidden: bool,
) -> clean::ItemKind {
match CStore::from_tcx(cx.tcx).load_macro_untracked(def_id, cx.tcx) {
LoadedMacro::MacroDef(item_def, _) => match macro_kind {
MacroKind::Bang => {
if let ast::ItemKind::MacroDef(ref def) = item_def.kind {
let vis = cx.tcx.visibility(import_def_id.unwrap_or(def_id));
clean::MacroItem(clean::Macro {
source: utils::display_macro_source(cx, name, def, def_id, vis),
source: utils::display_macro_source(
cx,
name,
def,
def_id,
vis,
is_doc_hidden,
),
})
} else {
unreachable!()
3 changes: 2 additions & 1 deletion src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
@@ -2796,7 +2796,8 @@ fn clean_maybe_renamed_item<'tcx>(
ItemKind::Macro(ref macro_def, MacroKind::Bang) => {
let ty_vis = cx.tcx.visibility(def_id);
MacroItem(Macro {
source: display_macro_source(cx, name, macro_def, def_id, ty_vis),
// FIXME this shouldn't be false
source: display_macro_source(cx, name, macro_def, def_id, ty_vis, false),
})
}
ItemKind::Macro(_, macro_kind) => clean_proc_macro(item, &mut name, macro_kind, cx),
2 changes: 1 addition & 1 deletion src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
@@ -1161,7 +1161,7 @@ impl Attributes {
false
}

fn is_doc_hidden(&self) -> bool {
pub(crate) fn is_doc_hidden(&self) -> bool {
self.has_doc_flag(sym::hidden)
}

5 changes: 3 additions & 2 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
@@ -625,6 +625,7 @@ pub(super) fn display_macro_source(
def: &ast::MacroDef,
def_id: DefId,
vis: ty::Visibility<DefId>,
is_doc_hidden: bool,
) -> String {
// Extract the spans of all matchers. They represent the "interface" of the macro.
let matchers = def.body.tokens.chunks(4).map(|arm| &arm[0]);
@@ -635,15 +636,15 @@ pub(super) fn display_macro_source(
if matchers.len() <= 1 {
format!(
"{vis}macro {name}{matchers} {{\n ...\n}}",
vis = visibility_to_src_with_space(Some(vis), cx.tcx, def_id),
vis = visibility_to_src_with_space(Some(vis), cx.tcx, def_id, is_doc_hidden),
matchers = matchers
.map(|matcher| render_macro_matcher(cx.tcx, matcher))
.collect::<String>(),
)
} else {
format!(
"{vis}macro {name} {{\n{arms}}}",
vis = visibility_to_src_with_space(Some(vis), cx.tcx, def_id),
vis = visibility_to_src_with_space(Some(vis), cx.tcx, def_id, is_doc_hidden),
arms = render_macro_arms(cx.tcx, matchers, ","),
)
}
31 changes: 21 additions & 10 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
@@ -29,8 +29,7 @@ use rustc_target::spec::abi::Abi;
use itertools::Itertools;

use crate::clean::{
self, types::ExternalLocation, utils::find_nearest_parent_module, ExternalCrate, ItemId,
PrimitiveType,
self, types::ExternalLocation, utils::find_nearest_parent_module, ExternalCrate, PrimitiveType,
};
use crate::formats::cache::Cache;
use crate::formats::item_type::ItemType;
@@ -1506,20 +1505,18 @@ impl clean::FnDecl {
}

pub(crate) fn visibility_print_with_space<'a, 'tcx: 'a>(
visibility: Option<ty::Visibility<DefId>>,
item_did: ItemId,
item: &clean::Item,
cx: &'a Context<'tcx>,
) -> impl Display + 'a + Captures<'tcx> {
use std::fmt::Write as _;

let to_print: Cow<'static, str> = match visibility {
let vis: Cow<'static, str> = match item.visibility(cx.tcx()) {
None => "".into(),
Some(ty::Visibility::Public) => "pub ".into(),
Some(ty::Visibility::Restricted(vis_did)) => {
// FIXME(camelid): This may not work correctly if `item_did` is a module.
// However, rustdoc currently never displays a module's
// visibility, so it shouldn't matter.
let parent_module = find_nearest_parent_module(cx.tcx(), item_did.expect_def_id());
let parent_module = find_nearest_parent_module(cx.tcx(), item.item_id.expect_def_id());

if vis_did.is_crate_root() {
"pub(crate) ".into()
@@ -1547,7 +1544,15 @@ pub(crate) fn visibility_print_with_space<'a, 'tcx: 'a>(
}
}
};
display_fn(move |f| f.write_str(&to_print))

let is_doc_hidden = item.is_doc_hidden();
display_fn(move |f| {
if is_doc_hidden {
f.write_str("#[doc(hidden)] ")?;
}

f.write_str(&vis)
})
}

/// This function is the same as print_with_space, except that it renders no links.
@@ -1557,8 +1562,9 @@ pub(crate) fn visibility_to_src_with_space<'a, 'tcx: 'a>(
visibility: Option<ty::Visibility<DefId>>,
tcx: TyCtxt<'tcx>,
item_did: DefId,
is_doc_hidden: bool,
) -> impl Display + 'a + Captures<'tcx> {
let to_print: Cow<'static, str> = match visibility {
let vis: Cow<'static, str> = match visibility {
None => "".into(),
Some(ty::Visibility::Public) => "pub ".into(),
Some(ty::Visibility::Restricted(vis_did)) => {
@@ -1582,7 +1588,12 @@ pub(crate) fn visibility_to_src_with_space<'a, 'tcx: 'a>(
}
}
};
display_fn(move |f| f.write_str(&to_print))
display_fn(move |f| {
if is_doc_hidden {
f.write_str("#[doc(hidden)] ")?;
}
f.write_str(&vis)
})
}

pub(crate) trait PrintWithSpace {
7 changes: 3 additions & 4 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
@@ -883,7 +883,7 @@ fn assoc_const(
w,
"{indent}{vis}const <a{href} class=\"constant\">{name}</a>{generics}: {ty}",
indent = " ".repeat(indent),
vis = visibility_print_with_space(it.visibility(tcx), it.item_id, cx),
vis = visibility_print_with_space(it, cx),
href = assoc_href_attr(it, link, cx),
name = it.name.as_ref().unwrap(),
generics = generics.print(cx),
@@ -912,12 +912,11 @@ fn assoc_type(
indent: usize,
cx: &Context<'_>,
) {
let tcx = cx.tcx();
write!(
w,
"{indent}{vis}type <a{href} class=\"associatedtype\">{name}</a>{generics}",
indent = " ".repeat(indent),
vis = visibility_print_with_space(it.visibility(tcx), it.item_id, cx),
vis = visibility_print_with_space(it, cx),
href = assoc_href_attr(it, link, cx),
name = it.name.as_ref().unwrap(),
generics = generics.print(cx),
@@ -945,7 +944,7 @@ fn assoc_method(
let tcx = cx.tcx();
let header = meth.fn_header(tcx).expect("Trying to get header from a non-function item");
let name = meth.name.as_ref().unwrap();
let vis = visibility_print_with_space(meth.visibility(tcx), meth.item_id, cx).to_string();
let vis = visibility_print_with_space(meth, cx).to_string();
let defaultness = print_default_space(meth.is_default());
// FIXME: Once https://github.com/rust-lang/rust/issues/67792 is implemented, we can remove
// this condition.
62 changes: 25 additions & 37 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
@@ -445,14 +445,14 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
Some(src) => write!(
w,
"<div class=\"item-name\"><code>{}extern crate {} as {};",
visibility_print_with_space(myitem.visibility(tcx), myitem.item_id, cx),
visibility_print_with_space(myitem, cx),
anchor(myitem.item_id.expect_def_id(), src, cx),
myitem.name.unwrap(),
),
None => write!(
w,
"<div class=\"item-name\"><code>{}extern crate {};",
visibility_print_with_space(myitem.visibility(tcx), myitem.item_id, cx),
visibility_print_with_space(myitem, cx),
anchor(myitem.item_id.expect_def_id(), myitem.name.unwrap(), cx),
),
}
@@ -491,7 +491,7 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
<code>{vis}{imp}</code>\
</div>\
{stab_tags_before}{stab_tags}{stab_tags_after}",
vis = visibility_print_with_space(myitem.visibility(tcx), myitem.item_id, cx),
vis = visibility_print_with_space(myitem, cx),
imp = import.print(cx),
);
w.write_str(ITEM_TABLE_ROW_CLOSE);
@@ -511,10 +511,16 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
_ => "",
};

let visibility_emoji = match myitem.visibility(tcx) {
let visibility_and_hidden = match myitem.visibility(tcx) {
Some(ty::Visibility::Restricted(_)) => {
"<span title=\"Restricted Visibility\">&nbsp;🔒</span> "
if myitem.is_doc_hidden() {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the condition reversed here? Also, if the item is both hidden and private, should we show that it is private?

Copy link
Member Author

Choose a reason for hiding this comment

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

oops. fixed. And yes, I think we should show both, though really hidden+private doesn't make sense.

Copy link
Member

Choose a reason for hiding this comment

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

Then what's the use case of showing both?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's not a large use case, but it may make sense for crates that more regularly use --document-private-items.

Copy link
Member

Choose a reason for hiding this comment

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

Well, we can always change it afterwards if we change our minds about it.

Copy link
Member

Choose a reason for hiding this comment

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

(I meant it's fine to leave it as is)

// Don't separate with a space when there are two of them
"<span title=\"Restricted Visibility\">&nbsp;🔒</span><span title=\"Hidden item\">👻</span> "
} else {
"<span title=\"Restricted Visibility\">&nbsp;🔒</span> "
}
}
_ if myitem.is_doc_hidden() => "<span title=\"Hidden item\">&nbsp;👻</span> ",
_ => "",
};

@@ -530,13 +536,13 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
w,
"<div class=\"item-name\">\
<a class=\"{class}\" href=\"{href}\" title=\"{title}\">{name}</a>\
{visibility_emoji}\
{visibility_and_hidden}\
{unsafety_flag}\
{stab_tags}\
</div>\
{docs_before}{docs}{docs_after}",
name = myitem.name.unwrap(),
visibility_emoji = visibility_emoji,
visibility_and_hidden = visibility_and_hidden,
stab_tags = extra_info_tags(myitem, item, tcx),
class = myitem.type_(),
unsafety_flag = unsafety_flag,
@@ -625,7 +631,7 @@ fn item_function(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, f: &cle
let unsafety = header.unsafety.print_with_space();
let abi = print_abi_with_space(header.abi).to_string();
let asyncness = header.asyncness.print_with_space();
let visibility = visibility_print_with_space(it.visibility(tcx), it.item_id, cx).to_string();
let visibility = visibility_print_with_space(it, cx).to_string();
let name = it.name.unwrap();

let generics_len = format!("{:#}", f.generics.print(cx)).len();
@@ -682,7 +688,7 @@ fn item_trait(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean:
w,
"{attrs}{vis}{unsafety}{is_auto}trait {name}{generics}{bounds}",
attrs = render_attributes_in_pre(it, "", cx),
vis = visibility_print_with_space(it.visibility(tcx), it.item_id, cx),
vis = visibility_print_with_space(it, cx),
unsafety = t.unsafety(tcx).print_with_space(),
is_auto = if t.is_auto(tcx) { "auto " } else { "" },
name = it.name.unwrap(),
@@ -1237,7 +1243,7 @@ fn item_type_alias(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &c
w,
"{attrs}{vis}type {name}{generics}{where_clause} = {type_};",
attrs = render_attributes_in_pre(it, "", cx),
vis = visibility_print_with_space(it.visibility(cx.tcx()), it.item_id, cx),
vis = visibility_print_with_space(it, cx),
name = it.name.unwrap(),
generics = t.generics.print(cx),
where_clause = print_where_clause(&t.generics, cx, 0, Ending::Newline),
@@ -1516,14 +1522,13 @@ fn print_tuple_struct_fields<'a, 'cx: 'a>(
}

fn item_enum(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, e: &clean::Enum) {
let tcx = cx.tcx();
let count_variants = e.variants().count();
wrap_item(w, |w| {
render_attributes_in_code(w, it, cx);
write!(
w,
"{}enum {}{}",
visibility_print_with_space(it.visibility(tcx), it.item_id, cx),
visibility_print_with_space(it, cx),
it.name.unwrap(),
e.generics.print(cx),
);
@@ -1854,7 +1859,7 @@ fn item_constant(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, c: &cle
write!(
w,
"{vis}const {name}{generics}: {typ}{where_clause}",
vis = visibility_print_with_space(it.visibility(tcx), it.item_id, cx),
vis = visibility_print_with_space(it, cx),
name = it.name.unwrap(),
generics = c.generics.print(cx),
typ = c.type_.print(cx),
@@ -1958,7 +1963,7 @@ fn item_static(w: &mut impl fmt::Write, cx: &mut Context<'_>, it: &clean::Item,
write!(
buffer,
"{vis}static {mutability}{name}: {typ}",
vis = visibility_print_with_space(it.visibility(cx.tcx()), it.item_id, cx),
vis = visibility_print_with_space(it, cx),
mutability = s.mutability.print_with_space(),
name = it.name.unwrap(),
typ = s.type_.print(cx)
@@ -1976,7 +1981,7 @@ fn item_foreign_type(w: &mut impl fmt::Write, cx: &mut Context<'_>, it: &clean::
write!(
buffer,
" {}type {};\n}}",
visibility_print_with_space(it.visibility(cx.tcx()), it.item_id, cx),
visibility_print_with_space(it, cx),
it.name.unwrap(),
)
.unwrap();
@@ -2133,13 +2138,7 @@ fn render_union<'a, 'cx: 'a>(
cx: &'a Context<'cx>,
) -> impl fmt::Display + 'a + Captures<'cx> {
display_fn(move |mut f| {
let tcx = cx.tcx();
write!(
f,
"{}union {}",
visibility_print_with_space(it.visibility(tcx), it.item_id, cx),
it.name.unwrap(),
)?;
write!(f, "{}union {}", visibility_print_with_space(it, cx), it.name.unwrap(),)?;

let where_displayed = g
.map(|g| {
@@ -2169,7 +2168,7 @@ fn render_union<'a, 'cx: 'a>(
write!(
f,
" {}{}: {},\n",
visibility_print_with_space(field.visibility(tcx), field.item_id, cx),
visibility_print_with_space(field, cx),
field.name.unwrap(),
ty.print(cx)
)?;
@@ -2197,11 +2196,10 @@ fn render_struct(
structhead: bool,
cx: &Context<'_>,
) {
let tcx = cx.tcx();
write!(
w,
"{}{}{}",
visibility_print_with_space(it.visibility(tcx), it.item_id, cx),
visibility_print_with_space(it, cx),
if structhead { "struct " } else { "" },
it.name.unwrap()
);
@@ -2230,7 +2228,6 @@ fn render_struct_fields(
has_stripped_entries: bool,
cx: &Context<'_>,
) {
let tcx = cx.tcx();
match ty {
None => {
let where_displayed =
@@ -2254,7 +2251,7 @@ fn render_struct_fields(
write!(
w,
"\n{tab} {vis}{name}: {ty},",
vis = visibility_print_with_space(field.visibility(tcx), field.item_id, cx),
vis = visibility_print_with_space(field, cx),
name = field.name.unwrap(),
ty = ty.print(cx),
);
@@ -2290,16 +2287,7 @@ fn render_struct_fields(
match *field.kind {
clean::StrippedItem(box clean::StructFieldItem(..)) => write!(w, "_"),
clean::StructFieldItem(ref ty) => {
write!(
w,
"{}{}",
visibility_print_with_space(
field.visibility(tcx),
field.item_id,
cx
),
ty.print(cx),
)
write!(w, "{}{}", visibility_print_with_space(field, cx), ty.print(cx),)
}
_ => unreachable!(),
}
12 changes: 8 additions & 4 deletions tests/rustdoc/display-hidden-items.rs
Original file line number Diff line number Diff line change
@@ -5,19 +5,22 @@
#![crate_name = "foo"]

// @has 'foo/index.html'
// @has - '//*[@id="reexport.hidden_reexport"]/code' 'pub use hidden::inside_hidden as hidden_reexport;'
// @has - '//*[@class="item-name"]/span[@title="Hidden item"]' '👻'

// @has - '//*[@id="reexport.hidden_reexport"]/code' '#[doc(hidden)] pub use hidden::inside_hidden as hidden_reexport;'
#[doc(hidden)]
pub use hidden::inside_hidden as hidden_reexport;

// @has - '//*[@class="item-name"]/a[@class="trait"]' 'TraitHidden'
// @has 'foo/trait.TraitHidden.html'
// @has - '//code' '#[doc(hidden)] pub trait TraitHidden'
#[doc(hidden)]
pub trait TraitHidden {}

// @has 'foo/index.html' '//*[@class="item-name"]/a[@class="trait"]' 'Trait'
pub trait Trait {
// @has 'foo/trait.Trait.html'
// @has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]' 'const BAR: u32 = 0u32'
// @has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]' '#[doc(hidden)] const BAR: u32 = 0u32'
#[doc(hidden)]
const BAR: u32 = 0;

@@ -41,14 +44,15 @@ impl Struct {
}

impl Trait for Struct {
// @has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]' 'const BAR: u32 = 0u32'
// @has - '//*[@id="method.foo"]/*[@class="code-header"]' 'fn foo()'
// @has - '//*[@id="associatedconstant.BAR"]/*[@class="code-header"]' '#[doc(hidden)] const BAR: u32 = 0u32'
// @has - '//*[@id="method.foo"]/*[@class="code-header"]' '#[doc(hidden)] fn foo()'
}
// @has - '//*[@id="impl-TraitHidden-for-Struct"]/*[@class="code-header"]' 'impl TraitHidden for Struct'
impl TraitHidden for Struct {}

// @has 'foo/index.html' '//*[@class="item-name"]/a[@class="enum"]' 'HiddenEnum'
// @has 'foo/enum.HiddenEnum.html'
// @has - '//code' '#[doc(hidden)] pub enum HiddenEnum'
#[doc(hidden)]
pub enum HiddenEnum {
A,