Rollup merge of #114129 - GuillaumeGomez:rustdoc-cleanup, r=notriddle

Rustdoc small cleanups

Each commit does some little cleanups:

 * We had some `Res` comparisons in multiple places (and still do, but unless we use a macro, it's not possible to "merge" any further) so I moved it into a function.
 * It was weird to have some utils function used everywhere in `visit_ast` so I instead moved it into `clean/utils.rs`.
 * In HTML rendering, we had some write "issues":
   * Multiple calls that could be merged into one.
   * Some `write!` that could be `write_str`.
   * We didn't use the new `format!` args much.

r? `@notriddle`
This commit is contained in:
Matthias Krüger 2023-07-30 01:22:42 +02:00 committed by GitHub
commit fa94851b07
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 123 additions and 109 deletions

View File

@ -2809,7 +2809,7 @@ fn clean_use_statement_inner<'tcx>(
cx: &mut DocContext<'tcx>,
inlined_names: &mut FxHashSet<(ItemType, Symbol)>,
) -> Vec<Item> {
if let Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) = path.res {
if should_ignore_res(path.res) {
return Vec::new();
}
// We need this comparison because some imports (for std types for example)

View File

@ -13,7 +13,7 @@ use rustc_ast as ast;
use rustc_ast::tokenstream::TokenTree;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE};
use rustc_middle::mir;
use rustc_middle::mir::interpret::ConstValue;
use rustc_middle::ty::{self, GenericArgKind, GenericArgsRef, TyCtxt};
@ -629,3 +629,35 @@ pub(super) fn display_macro_source(
}
}
}
pub(crate) fn inherits_doc_hidden(
tcx: TyCtxt<'_>,
mut def_id: LocalDefId,
stop_at: Option<LocalDefId>,
) -> bool {
let hir = tcx.hir();
while let Some(id) = tcx.opt_local_parent(def_id) {
if let Some(stop_at) = stop_at && id == stop_at {
return false;
}
def_id = id;
if tcx.is_doc_hidden(def_id.to_def_id()) {
return true;
} else if let Some(node) = hir.find_by_def_id(def_id) &&
matches!(
node,
hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(_), .. }),
)
{
// `impl` blocks stand a bit on their own: unless they have `#[doc(hidden)]` directly
// on them, they don't inherit it from the parent context.
return false;
}
}
false
}
#[inline]
pub(crate) fn should_ignore_res(res: Res) -> bool {
matches!(res, Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..))
}

View File

@ -268,7 +268,7 @@ impl AllTypes {
fn append(&mut self, item_name: String, item_type: &ItemType) {
let mut url: Vec<_> = item_name.split("::").skip(1).collect();
if let Some(name) = url.pop() {
let new_url = format!("{}/{}.{}.html", url.join("/"), item_type, name);
let new_url = format!("{}/{item_type}.{name}.html", url.join("/"));
url.push(name);
let name = url.join("::");
match *item_type {
@ -385,16 +385,17 @@ impl AllTypes {
fn scrape_examples_help(shared: &SharedContext<'_>) -> String {
let mut content = SCRAPE_EXAMPLES_HELP_MD.to_owned();
content.push_str(&format!(
"## More information\n\n\
If you want more information about this feature, please read the [corresponding chapter in the Rustdoc book]({}/rustdoc/scraped-examples.html).",
DOC_RUST_LANG_ORG_CHANNEL));
"## More information\n\n\
If you want more information about this feature, please read the [corresponding chapter in \
the Rustdoc book]({DOC_RUST_LANG_ORG_CHANNEL}/rustdoc/scraped-examples.html)."
));
let mut ids = IdMap::default();
format!(
"<div class=\"main-heading\">\
<h1>About scraped examples</h1>\
</div>\
<div>{}</div>",
<h1>About scraped examples</h1>\
</div>\
<div>{}</div>",
Markdown {
content: &content,
links: &[],
@ -473,7 +474,7 @@ fn document_short<'a, 'cx: 'a>(
MarkdownSummaryLine(&s, &item.links(cx)).into_string_with_has_more_content();
if has_more_content {
let link = format!(r#" <a{}>Read more</a>"#, assoc_href_attr(item, link, cx));
let link = format!(" <a{}>Read more</a>", assoc_href_attr(item, link, cx));
if let Some(idx) = summary_html.rfind("</p>") {
summary_html.insert_str(idx, &link);
@ -482,7 +483,7 @@ fn document_short<'a, 'cx: 'a>(
}
}
write!(f, "<div class='docblock'>{}</div>", summary_html)?;
write!(f, "<div class='docblock'>{summary_html}</div>")?;
}
Ok(())
})
@ -517,9 +518,9 @@ fn document_full_inner<'a, 'cx: 'a>(
write!(
f,
"<details class=\"toggle top-doc\" open>\
<summary class=\"hideme\">\
<summary class=\"hideme\">\
<span>Expand description</span>\
</summary>{}</details>",
</summary>{}</details>",
render_markdown(cx, &s, item.links(cx), heading_offset)
)?;
} else {
@ -701,8 +702,8 @@ fn assoc_href_attr(it: &clean::Item, link: AssocItemLink<'_>, cx: &Context<'_>)
let item_type = it.type_();
let href = match link {
AssocItemLink::Anchor(Some(ref id)) => Some(format!("#{}", id)),
AssocItemLink::Anchor(None) => Some(format!("#{}.{}", item_type, name)),
AssocItemLink::Anchor(Some(ref id)) => Some(format!("#{id}")),
AssocItemLink::Anchor(None) => Some(format!("#{item_type}.{name}")),
AssocItemLink::GotoSource(did, provided_methods) => {
// We're creating a link from the implementation of an associated item to its
// declaration in the trait declaration.
@ -722,7 +723,7 @@ fn assoc_href_attr(it: &clean::Item, link: AssocItemLink<'_>, cx: &Context<'_>)
};
match href(did.expect_def_id(), cx) {
Ok((url, ..)) => Some(format!("{}#{}.{}", url, item_type, name)),
Ok((url, ..)) => Some(format!("{url}#{item_type}.{name}")),
// The link is broken since it points to an external crate that wasn't documented.
// Do not create any link in such case. This is better than falling back to a
// dummy anchor like `#{item_type}.{name}` representing the `id` of *this* impl item
@ -735,14 +736,14 @@ fn assoc_href_attr(it: &clean::Item, link: AssocItemLink<'_>, cx: &Context<'_>)
// In this scenario, the actual `id` of this impl item would be
// `#{item_type}.{name}-{n}` for some number `n` (a disambiguator).
Err(HrefError::DocumentationNotBuilt) => None,
Err(_) => Some(format!("#{}.{}", item_type, name)),
Err(_) => Some(format!("#{item_type}.{name}")),
}
}
};
// If there is no `href` for the reason explained above, simply do not render it which is valid:
// https://html.spec.whatwg.org/multipage/links.html#links-created-by-a-and-area-elements
href.map(|href| format!(" href=\"{}\"", href)).unwrap_or_default()
href.map(|href| format!(" href=\"{href}\"")).unwrap_or_default()
}
fn assoc_const(
@ -767,7 +768,7 @@ fn assoc_const(
ty = ty.print(cx),
);
if let Some(default) = default {
write!(w, " = ");
w.write_str(" = ");
// FIXME: `.value()` uses `clean::utils::format_integer_with_underscore_sep` under the
// hood which adds noisy underscores and a type suffix to number literals.
@ -910,39 +911,41 @@ fn render_stability_since_raw_with_extra(
if let Some(ver) = stable_version {
stability.push_str(ver.as_str());
title.push_str(&format!("Stable since Rust version {}", ver));
title.push_str(&format!("Stable since Rust version {ver}"));
}
let const_title_and_stability = match const_stability {
Some(ConstStability { level: StabilityLevel::Stable { since, .. }, .. })
if Some(since) != containing_const_ver =>
{
Some((format!("const since {}", since), format!("const: {}", since)))
Some((format!("const since {since}"), format!("const: {since}")))
}
Some(ConstStability { level: StabilityLevel::Unstable { issue, .. }, feature, .. }) => {
let unstable = if let Some(n) = issue {
format!(
r#"<a href="https://github.com/rust-lang/rust/issues/{}" title="Tracking issue for {}">unstable</a>"#,
n, feature
"<a \
href=\"https://github.com/rust-lang/rust/issues/{n}\" \
title=\"Tracking issue for {feature}\"\
>unstable</a>"
)
} else {
String::from("unstable")
};
Some((String::from("const unstable"), format!("const: {}", unstable)))
Some((String::from("const unstable"), format!("const: {unstable}")))
}
_ => None,
};
if let Some((const_title, const_stability)) = const_title_and_stability {
if !title.is_empty() {
title.push_str(&format!(", {}", const_title));
title.push_str(&format!(", {const_title}"));
} else {
title.push_str(&const_title);
}
if !stability.is_empty() {
stability.push_str(&format!(" ({})", const_stability));
stability.push_str(&format!(" ({const_stability})"));
} else {
stability.push_str(&const_stability);
}
@ -1091,7 +1094,7 @@ pub(crate) fn render_all_impls(
let impls = impls.into_inner();
if !impls.is_empty() {
write_impl_section_heading(&mut w, "Trait Implementations", "trait-implementations");
write!(w, "<div id=\"trait-implementations-list\">{}</div>", impls).unwrap();
write!(w, "<div id=\"trait-implementations-list\">{impls}</div>").unwrap();
}
if !synthetic.is_empty() {
@ -1189,10 +1192,13 @@ fn render_assoc_items_inner(
);
}
if !impls_buf.is_empty() {
write!(w, "{}", tmp_buf.into_inner()).unwrap();
write!(w, "<div id=\"{id}\"{class_html}>").unwrap();
write!(w, "{}", impls_buf.into_inner()).unwrap();
w.write_str("</div>").unwrap();
write!(
w,
"{}<div id=\"{id}\"{class_html}>{}</div>",
tmp_buf.into_inner(),
impls_buf.into_inner()
)
.unwrap();
}
}
@ -1392,7 +1398,7 @@ fn notable_traits_decl(ty: &clean::Type, cx: &Context<'_>) -> (String, String) {
}
}
if out.is_empty() {
write!(&mut out, "</code></pre>",);
out.write_str("</code></pre>");
}
(format!("{:#}", ty.print(cx)), out.into_inner())
@ -1538,25 +1544,25 @@ fn render_impl(
let toggled = !doc_buffer.is_empty();
if toggled {
let method_toggle_class = if item_type.is_method() { " method-toggle" } else { "" };
write!(w, "<details class=\"toggle{}\" open><summary>", method_toggle_class);
write!(w, "<details class=\"toggle{method_toggle_class}\" open><summary>");
}
match &*item.kind {
clean::MethodItem(..) | clean::TyMethodItem(_) => {
// Only render when the method is not static or we allow static methods
if render_method_item {
let id = cx.derive_id(format!("{}.{}", item_type, name));
let id = cx.derive_id(format!("{item_type}.{name}"));
let source_id = trait_
.and_then(|trait_| {
trait_.items.iter().find(|item| {
item.name.map(|n| n.as_str().eq(name.as_str())).unwrap_or(false)
})
})
.map(|item| format!("{}.{}", item.type_(), name));
write!(w, "<section id=\"{}\" class=\"{}{}\">", id, item_type, in_trait_class,);
.map(|item| format!("{}.{name}", item.type_()));
write!(w, "<section id=\"{id}\" class=\"{item_type}{in_trait_class}\">");
render_rightside(w, cx, item, containing_item, render_mode);
if trait_.is_some() {
// Anchors are only used on trait impls.
write!(w, "<a href=\"#{}\" class=\"anchor\">§</a>", id);
write!(w, "<a href=\"#{id}\" class=\"anchor\">§</a>");
}
w.write_str("<h4 class=\"code-header\">");
render_assoc_item(
@ -1567,19 +1573,18 @@ fn render_impl(
cx,
render_mode,
);
w.write_str("</h4>");
w.write_str("</section>");
w.write_str("</h4></section>");
}
}
kind @ (clean::TyAssocConstItem(generics, ty)
| clean::AssocConstItem(generics, ty, _)) => {
let source_id = format!("{}.{}", item_type, name);
let source_id = format!("{item_type}.{name}");
let id = cx.derive_id(source_id.clone());
write!(w, "<section id=\"{}\" class=\"{}{}\">", id, item_type, in_trait_class);
write!(w, "<section id=\"{id}\" class=\"{item_type}{in_trait_class}\">");
render_rightside(w, cx, item, containing_item, render_mode);
if trait_.is_some() {
// Anchors are only used on trait impls.
write!(w, "<a href=\"#{}\" class=\"anchor\">§</a>", id);
write!(w, "<a href=\"#{id}\" class=\"anchor\">§</a>");
}
w.write_str("<h4 class=\"code-header\">");
assoc_const(
@ -1596,16 +1601,15 @@ fn render_impl(
0,
cx,
);
w.write_str("</h4>");
w.write_str("</section>");
w.write_str("</h4></section>");
}
clean::TyAssocTypeItem(generics, bounds) => {
let source_id = format!("{}.{}", item_type, name);
let source_id = format!("{item_type}.{name}");
let id = cx.derive_id(source_id.clone());
write!(w, "<section id=\"{}\" class=\"{}{}\">", id, item_type, in_trait_class);
write!(w, "<section id=\"{id}\" class=\"{item_type}{in_trait_class}\">");
if trait_.is_some() {
// Anchors are only used on trait impls.
write!(w, "<a href=\"#{}\" class=\"anchor\">§</a>", id);
write!(w, "<a href=\"#{id}\" class=\"anchor\">§</a>");
}
w.write_str("<h4 class=\"code-header\">");
assoc_type(
@ -1618,16 +1622,15 @@ fn render_impl(
0,
cx,
);
w.write_str("</h4>");
w.write_str("</section>");
w.write_str("</h4></section>");
}
clean::AssocTypeItem(tydef, _bounds) => {
let source_id = format!("{}.{}", item_type, name);
let source_id = format!("{item_type}.{name}");
let id = cx.derive_id(source_id.clone());
write!(w, "<section id=\"{}\" class=\"{}{}\">", id, item_type, in_trait_class);
write!(w, "<section id=\"{id}\" class=\"{item_type}{in_trait_class}\">");
if trait_.is_some() {
// Anchors are only used on trait impls.
write!(w, "<a href=\"#{}\" class=\"anchor\">§</a>", id);
write!(w, "<a href=\"#{id}\" class=\"anchor\">§</a>");
}
w.write_str("<h4 class=\"code-header\">");
assoc_type(
@ -1640,8 +1643,7 @@ fn render_impl(
0,
cx,
);
w.write_str("</h4>");
w.write_str("</section>");
w.write_str("</h4></section>");
}
clean::StrippedItem(..) => return,
_ => panic!("can't make docs for trait item with name {:?}", item.name),
@ -1744,10 +1746,10 @@ fn render_impl(
close_tags.insert_str(0, "</details>");
write!(
w,
"<details class=\"toggle implementors-toggle\"{}>",
"<details class=\"toggle implementors-toggle\"{}>\
<summary>",
if rendering_params.toggle_open_by_default { " open" } else { "" }
);
write!(w, "<summary>")
}
render_impl_summary(
w,
@ -1760,15 +1762,15 @@ fn render_impl(
aliases,
);
if toggled {
write!(w, "</summary>")
w.write_str("</summary>");
}
if let Some(ref dox) = i.impl_item.opt_doc_value() {
if trait_.is_none() && i.inner_impl().items.is_empty() {
w.write_str(
"<div class=\"item-info\">\
<div class=\"stab empty-impl\">This impl block contains no items.</div>\
</div>",
<div class=\"stab empty-impl\">This impl block contains no items.</div>\
</div>",
);
}
write!(
@ -1827,11 +1829,11 @@ fn render_rightside(
const_stable_since,
if has_src_ref { "" } else { " rightside" },
);
if let Some(l) = src_href {
if let Some(link) = src_href {
if has_stability {
write!(rightside, " · <a class=\"src\" href=\"{}\">source</a>", l)
write!(rightside, " · <a class=\"src\" href=\"{link}\">source</a>")
} else {
write!(rightside, "<a class=\"src rightside\" href=\"{}\">source</a>", l)
write!(rightside, "<a class=\"src rightside\" href=\"{link}\">source</a>")
}
}
if has_stability && has_src_ref {
@ -1860,10 +1862,13 @@ pub(crate) fn render_impl_summary(
} else {
format!(" data-aliases=\"{}\"", aliases.join(","))
};
write!(w, "<section id=\"{}\" class=\"impl\"{}>", id, aliases);
write!(w, "<section id=\"{id}\" class=\"impl\"{aliases}>");
render_rightside(w, cx, &i.impl_item, containing_item, RenderMode::Normal);
write!(w, "<a href=\"#{}\" class=\"anchor\">§</a>", id);
write!(w, "<h3 class=\"code-header\">");
write!(
w,
"<a href=\"#{id}\" class=\"anchor\">§</a>\
<h3 class=\"code-header\">"
);
if let Some(use_absolute) = use_absolute {
write!(w, "{}", inner_impl.print(use_absolute, cx));
@ -1888,15 +1893,16 @@ pub(crate) fn render_impl_summary(
} else {
write!(w, "{}", inner_impl.print(false, cx));
}
write!(w, "</h3>");
w.write_str("</h3>");
let is_trait = inner_impl.trait_.is_some();
if is_trait {
if let Some(portability) = portability(&i.impl_item, Some(parent)) {
write!(
w,
"<span class=\"item-info\"><div class=\"stab portability\">{}</div></span>",
portability
"<span class=\"item-info\">\
<div class=\"stab portability\">{portability}</div>\
</span>",
);
}
}
@ -1949,7 +1955,7 @@ pub(crate) fn small_url_encode(s: String) -> String {
// consistent with itself when encoding them.
st += "+";
} else {
write!(st, "%{:02X}", b).unwrap();
write!(st, "%{b:02X}").unwrap();
}
// Invariant: if the current byte is not at the start of a multi-byte character,
// we need to get down here so that when the next turn of the loop comes around,
@ -2264,7 +2270,7 @@ fn render_call_locations<W: fmt::Write>(mut w: W, cx: &mut Context<'_>, item: &c
format!("lines {}-{}", line_lo + 1, line_hi + 1),
)
};
let url = format!("{}{}#{}", cx.root_path(), call_data.url, anchor);
let url = format!("{}{}#{anchor}", cx.root_path(), call_data.url);
(url, title)
};
@ -2274,7 +2280,7 @@ fn render_call_locations<W: fmt::Write>(mut w: W, cx: &mut Context<'_>, item: &c
Ok(contents) => contents,
Err(err) => {
let span = item.span(tcx).map_or(rustc_span::DUMMY_SP, |span| span.inner());
tcx.sess.span_err(span, format!("failed to read file {}: {}", path.display(), err));
tcx.sess.span_err(span, format!("failed to read file {}: {err}", path.display()));
return false;
}
};
@ -2333,7 +2339,7 @@ fn render_call_locations<W: fmt::Write>(mut w: W, cx: &mut Context<'_>, item: &c
.unwrap();
if line_ranges.len() > 1 {
write!(w, r#"<button class="prev">&pr;</button> <button class="next">&sc;</button>"#)
w.write_str(r#"<button class="prev">&pr;</button> <button class="next">&sc;</button>"#)
.unwrap();
}
@ -2369,7 +2375,7 @@ fn render_call_locations<W: fmt::Write>(mut w: W, cx: &mut Context<'_>, item: &c
highlight::DecorationInfo(decoration_info),
sources::SourceContext::Embedded { offset: line_min, needs_expansion },
);
write!(w, "</div></div>").unwrap();
w.write_str("</div></div>").unwrap();
true
};
@ -2436,8 +2442,10 @@ fn render_call_locations<W: fmt::Write>(mut w: W, cx: &mut Context<'_>, item: &c
// For the remaining examples, generate a <ul> containing links to the source files.
if it.peek().is_some() {
write!(w, r#"<div class="example-links">Additional examples can be found in:<br><ul>"#)
.unwrap();
w.write_str(
r#"<div class="example-links">Additional examples can be found in:<br><ul>"#,
)
.unwrap();
it.for_each(|(_, call_data)| {
let (url, _) = link_to_loc(call_data, &call_data.locations[0]);
write!(
@ -2448,11 +2456,11 @@ fn render_call_locations<W: fmt::Write>(mut w: W, cx: &mut Context<'_>, item: &c
)
.unwrap();
});
write!(w, "</ul></div>").unwrap();
w.write_str("</ul></div>").unwrap();
}
write!(w, "</div></details>").unwrap();
w.write_str("</div></details>").unwrap();
}
write!(w, "</div>").unwrap();
w.write_str("</div>").unwrap();
}

View File

@ -7,11 +7,11 @@
use super::Pass;
use crate::clean;
use crate::clean::utils::inherits_doc_hidden;
use crate::clean::*;
use crate::core::DocContext;
use crate::html::markdown::{find_testable_code, ErrorCodes, Ignore, LangString};
use crate::visit::DocVisitor;
use crate::visit_ast::inherits_doc_hidden;
use rustc_hir as hir;
use rustc_middle::lint::LintLevelSource;
use rustc_session::lint;

View File

@ -6,11 +6,11 @@ use rustc_span::symbol::sym;
use std::mem;
use crate::clean;
use crate::clean::utils::inherits_doc_hidden;
use crate::clean::{Item, ItemIdSet};
use crate::core::DocContext;
use crate::fold::{strip_item, DocFolder};
use crate::passes::{ImplStripper, Pass};
use crate::visit_ast::inherits_doc_hidden;
pub(crate) const STRIP_HIDDEN: Pass = Pass {
name: "strip-hidden",

View File

@ -3,10 +3,10 @@ use rustc_hir::def_id::DefId;
use rustc_middle::ty::{TyCtxt, Visibility};
use std::mem;
use crate::clean::utils::inherits_doc_hidden;
use crate::clean::{self, Item, ItemId, ItemIdSet};
use crate::fold::{strip_item, DocFolder};
use crate::formats::cache::Cache;
use crate::visit_ast::inherits_doc_hidden;
use crate::visit_lib::RustdocEffectiveVisibilities;
pub(crate) struct Stripper<'a, 'tcx> {

View File

@ -16,6 +16,7 @@ use rustc_span::Span;
use std::mem;
use crate::clean::utils::{inherits_doc_hidden, should_ignore_res};
use crate::clean::{cfg::Cfg, reexport_chain, AttributesExt, NestedAttributesExt};
use crate::core;
@ -73,33 +74,6 @@ fn def_id_to_path(tcx: TyCtxt<'_>, did: DefId) -> Vec<Symbol> {
std::iter::once(crate_name).chain(relative).collect()
}
pub(crate) fn inherits_doc_hidden(
tcx: TyCtxt<'_>,
mut def_id: LocalDefId,
stop_at: Option<LocalDefId>,
) -> bool {
let hir = tcx.hir();
while let Some(id) = tcx.opt_local_parent(def_id) {
if let Some(stop_at) = stop_at && id == stop_at {
return false;
}
def_id = id;
if tcx.is_doc_hidden(def_id.to_def_id()) {
return true;
} else if let Some(node) = hir.find_by_def_id(def_id) &&
matches!(
node,
hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(_), .. }),
)
{
// `impl` blocks stand a bit on their own: unless they have `#[doc(hidden)]` directly
// on them, they don't inherit it from the parent context.
return false;
}
}
false
}
pub(crate) struct RustdocVisitor<'a, 'tcx> {
cx: &'a mut core::DocContext<'tcx>,
view_item_stack: LocalDefIdSet,
@ -466,7 +440,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
for &res in &path.res {
// Struct and variant constructors and proc macro stubs always show up alongside
// their definitions, we've already processed them so just discard these.
if let Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) = res {
if should_ignore_res(res) {
continue;
}