Skip to content

Commit

Permalink
Rollup merge of #93576 - jsha:fix-rustdoc-html, r=GuillaumeGomez
Browse files Browse the repository at this point in the history
Emit more valid HTML from rustdoc

Previously, tidy-html5 (`tidy`) would complain about a few things in our HTML. The main thing is that `<summary>` tags can't contain `<div>`s. That's easily fixed by changing out the `<div>`s for `<span>`s with `display: block`.

However, there's also a rule that `<span>`s can't contain heading elements. `<span>` permits only "phrasing content" https://developer.mozilla.org/en-US/docs/Web/HTML/Element/span, and `<h3>` (and friends) are "Flow content, heading content, palpable content". https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements

We have a wrapping `<div>` that goes around each `<h3>`/`<h4>`, etc. We turn that into a `<section>` rather than a `<span>` because `<section>` permits "flow content". https://developer.mozilla.org/en-US/docs/Web/HTML/Element/section

After this change we get only three warnings from tidy, run on struct.String.html:

line 6 column 10790 - Warning: trimming empty <span>
line 1 column 1118 - Warning: <link> proprietary attribute "disabled"
line 1 column 1193 - Warning: <link> proprietary attribute "disabled"

The empty `<span>` is a known issue - there's a span in front of the search box to work around a strange Safari issue.

The `<link>` attributes are the non-default stylesheets. We can probably refactor theme application to avoid using this proprietary "disabled" attribute.

We can suppress those warnings with flags to tidy, and get a run that returns 0 (success):

```
tidy -o /dev/null -quiet --drop-empty-elements no --warn-proprietary-attributes no build/x86_64-unknown-linux-gnu/doc/std/string/trait.ToString.html
```

Note: this requires the latest version of tidy-html5, built from https://github.com/htacg/tidy-html5. Older versions (including the default version on Ubuntu 21.10) think `<section>` can't occur inside `<summary>`.

Demo: https://rustdoc.crud.net/jsha/fix-rustdoc-html/std/string/struct.String.html

r? `@GuillaumeGomez`
  • Loading branch information
matthiaskrgr authored Feb 4, 2022
2 parents af2886e + 32f6260 commit 3edec80
Show file tree
Hide file tree
Showing 27 changed files with 127 additions and 133 deletions.
62 changes: 27 additions & 35 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1265,7 +1265,7 @@ fn notable_traits_decl(decl: &clean::FnDecl, cx: &Context<'_>) -> String {
if out.is_empty() {
write!(
&mut out,
"<div class=\"notable\">Notable traits for {}</div>\
"<span class=\"notable\">Notable traits for {}</span>\
<code class=\"content\">",
impl_.for_.print(cx)
);
Expand Down Expand Up @@ -1297,9 +1297,9 @@ fn notable_traits_decl(decl: &clean::FnDecl, cx: &Context<'_>) -> String {
out.insert_str(
0,
"<span class=\"notable-traits\"><span class=\"notable-traits-tooltip\">ⓘ\
<div class=\"notable-traits-tooltiptext\"><span class=\"docblock\">",
<span class=\"notable-traits-tooltiptext\"><span class=\"docblock\">",
);
out.push_str("</code></span></div></span></span>");
out.push_str("</code></span></span></span></span>");
}

out.into_inner()
Expand Down Expand Up @@ -1431,7 +1431,7 @@ fn render_impl(
.map(|item| format!("{}.{}", item.type_(), name));
write!(
w,
"<div id=\"{}\" class=\"{}{} has-srclink\">",
"<section id=\"{}\" class=\"{}{} has-srclink\">",
id, item_type, in_trait_class,
);
render_rightside(w, cx, item, containing_item, render_mode);
Expand All @@ -1446,15 +1446,15 @@ fn render_impl(
render_mode,
);
w.write_str("</h4>");
w.write_str("</div>");
w.write_str("</section>");
}
}
clean::TypedefItem(ref tydef, _) => {
let source_id = format!("{}.{}", ItemType::AssocType, name);
let id = cx.derive_id(source_id.clone());
write!(
w,
"<div id=\"{}\" class=\"{}{} has-srclink\">",
"<section id=\"{}\" class=\"{}{} has-srclink\">",
id, item_type, in_trait_class
);
write!(w, "<a href=\"#{}\" class=\"anchor\"></a>", id);
Expand All @@ -1469,14 +1469,14 @@ fn render_impl(
cx,
);
w.write_str("</h4>");
w.write_str("</div>");
w.write_str("</section>");
}
clean::AssocConstItem(ref ty, _) => {
let source_id = format!("{}.{}", item_type, name);
let id = cx.derive_id(source_id.clone());
write!(
w,
"<div id=\"{}\" class=\"{}{} has-srclink\">",
"<section id=\"{}\" class=\"{}{} has-srclink\">",
id, item_type, in_trait_class
);
render_rightside(w, cx, item, containing_item, render_mode);
Expand All @@ -1491,12 +1491,12 @@ fn render_impl(
cx,
);
w.write_str("</h4>");
w.write_str("</div>");
w.write_str("</section>");
}
clean::AssocTypeItem(ref bounds, ref default) => {
let source_id = format!("{}.{}", item_type, name);
let id = cx.derive_id(source_id.clone());
write!(w, "<div id=\"{}\" class=\"{}{}\">", id, item_type, in_trait_class,);
write!(w, "<section id=\"{}\" class=\"{}{}\">", id, item_type, in_trait_class,);
write!(w, "<a href=\"#{}\" class=\"anchor\"></a>", id);
w.write_str("<h4 class=\"code-header\">");
assoc_type(
Expand All @@ -1509,7 +1509,7 @@ fn render_impl(
cx,
);
w.write_str("</h4>");
w.write_str("</div>");
w.write_str("</section>");
}
clean::StrippedItem(..) => return,
_ => panic!("can't make docs for trait item with name {:?}", item.name),
Expand Down Expand Up @@ -1668,21 +1668,23 @@ fn render_rightside(
RenderMode::ForDeref { .. } => (None, None),
};

write!(w, "<div class=\"rightside\">");
let mut rightside = Buffer::new();
let has_stability = render_stability_since_raw(
w,
&mut rightside,
item.stable_since(tcx),
const_stability,
containing_item.stable_since(tcx),
const_stable_since,
);
let mut tmp_buf = Buffer::empty_from(w);
write_srclink(cx, item, &mut tmp_buf);
if has_stability && !tmp_buf.is_empty() {
w.write_str(" · ");
let mut srclink = Buffer::empty_from(w);
write_srclink(cx, item, &mut srclink);
if has_stability && !srclink.is_empty() {
rightside.write_str(" · ");
}
rightside.push_buffer(srclink);
if !rightside.is_empty() {
write!(w, "<span class=\"rightside\">{}</span>", rightside.into_inner());
}
w.push_buffer(tmp_buf);
w.write_str("</div>");
}

pub(crate) fn render_impl_summary(
Expand Down Expand Up @@ -1713,7 +1715,7 @@ pub(crate) fn render_impl_summary(
} else {
format!(" data-aliases=\"{}\"", aliases.join(","))
};
write!(w, "<div id=\"{}\" class=\"impl has-srclink\"{}>", id, aliases);
write!(w, "<section id=\"{}\" class=\"impl has-srclink\"{}>", id, 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 in-band\">");
Expand All @@ -1737,11 +1739,11 @@ pub(crate) fn render_impl_summary(
let is_trait = i.inner_impl().trait_.is_some();
if is_trait {
if let Some(portability) = portability(&i.impl_item, Some(parent)) {
write!(w, "<div class=\"item-info\">{}</div>", portability);
write!(w, "<span class=\"item-info\">{}</span>", portability);
}
}

w.write_str("</div>");
w.write_str("</section>");
}

fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buffer) {
Expand Down Expand Up @@ -1802,19 +1804,9 @@ fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buffer) {
// to navigate the documentation (though slightly inefficiently).

if !it.is_mod() {
buffer.write_str("<h2 class=\"location\">In ");
for (i, name) in cx.current.iter().take(parentlen).enumerate() {
if i > 0 {
buffer.write_str("::<wbr>");
}
write!(
buffer,
"<a href=\"{}index.html\">{}</a>",
&cx.root_path()[..(cx.current.len() - i - 1) * 3],
*name
);
}
buffer.write_str("</h2>");
let path: String = cx.current.iter().map(|s| s.as_str()).intersperse("::").collect();

write!(buffer, "<h2 class=\"location\"><a href=\"index.html\">In {}</a></h2>", path);
}

// Sidebar refers to the enclosing module, not this module.
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/html/static/css/rustdoc.css
Original file line number Diff line number Diff line change
Expand Up @@ -1336,6 +1336,7 @@ h3.variant {
margin-bottom: 13px;
font-size: 1.1875rem;
font-weight: 600;
display: block;
}

.notable-traits .docblock code.content{
Expand Down
6 changes: 3 additions & 3 deletions src/test/rustdoc/async-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ struct AsyncFdReadyGuard<'a, T> { x: &'a T }

impl Foo {
// @has async_fn/struct.Foo.html
// @has - '//div[@class="method has-srclink"]' 'pub async fn complicated_lifetimes( &self, context: &impl Bar) -> impl Iterator<Item = &usize>'
// @has - '//*[@class="method has-srclink"]' 'pub async fn complicated_lifetimes( &self, context: &impl Bar) -> impl Iterator<Item = &usize>'
pub async fn complicated_lifetimes(&self, context: &impl Bar) -> impl Iterator<Item = &usize> {}
// taken from `tokio` as an example of a method that was particularly bad before
// @has - '//div[@class="method has-srclink"]' "pub async fn readable<T>(&self) -> Result<AsyncFdReadyGuard<'_, T>, ()>"
// @has - '//*[@class="method has-srclink"]' "pub async fn readable<T>(&self) -> Result<AsyncFdReadyGuard<'_, T>, ()>"
pub async fn readable<T>(&self) -> Result<AsyncFdReadyGuard<'_, T>, ()> {}
// @has - '//div[@class="method has-srclink"]' "pub async fn mut_self(&mut self)"
// @has - '//*[@class="method has-srclink"]' "pub async fn mut_self(&mut self)"
pub async fn mut_self(&mut self) {}
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc/auto_aliases.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![feature(auto_traits)]

// @has auto_aliases/trait.Bar.html '//div[@data-aliases="auto_aliases::Foo"]' 'impl Bar for Foo'
// @has auto_aliases/trait.Bar.html '//*[@data-aliases="auto_aliases::Foo"]' 'impl Bar for Foo'
pub struct Foo;

pub auto trait Bar {}
2 changes: 1 addition & 1 deletion src/test/rustdoc/blanket-reexport-item.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![crate_name = "foo"]

// @has foo/struct.S.html '//div[@id="impl-Into%3CU%3E"]//h3[@class="code-header in-band"]' 'impl<T, U> Into<U> for T'
// @has foo/struct.S.html '//*[@id="impl-Into%3CU%3E"]//h3[@class="code-header in-band"]' 'impl<T, U> Into<U> for T'
pub struct S2 {}
mod m {
pub struct S {}
Expand Down
6 changes: 3 additions & 3 deletions src/test/rustdoc/const-display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,19 @@ pub const unsafe fn bar_not_gated() -> u32 { 42 }
pub struct Foo;

impl Foo {
// @has 'foo/struct.Foo.html' '//div[@id="method.gated"]/h4[@class="code-header"]' 'pub fn gated() -> u32'
// @has 'foo/struct.Foo.html' '//*[@id="method.gated"]/h4[@class="code-header"]' 'pub fn gated() -> u32'
// @has - '//span[@class="since"]' '1.0.0 (const: unstable)'
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature="foo", issue = "none")]
pub const fn gated() -> u32 { 42 }

// @has 'foo/struct.Foo.html' '//div[@id="method.gated_unsafe"]/h4[@class="code-header"]' 'pub unsafe fn gated_unsafe() -> u32'
// @has 'foo/struct.Foo.html' '//*[@id="method.gated_unsafe"]/h4[@class="code-header"]' 'pub unsafe fn gated_unsafe() -> u32'
// @has - '//span[@class="since"]' '1.0.0 (const: unstable)'
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature="foo", issue = "none")]
pub const unsafe fn gated_unsafe() -> u32 { 42 }

// @has 'foo/struct.Foo.html' '//div[@id="method.stable_impl"]/h4[@class="code-header"]' 'pub const fn stable_impl() -> u32'
// @has 'foo/struct.Foo.html' '//*[@id="method.stable_impl"]/h4[@class="code-header"]' 'pub const fn stable_impl() -> u32'
// @has - '//span[@class="since"]' '1.0.0 (const: 1.2.0)'
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "rust1", since = "1.2.0")]
Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc/const-generics/add-impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub struct Simd<T, const WIDTH: usize> {
inner: T,
}

// @has foo/struct.Simd.html '//div[@id="trait-implementations-list"]//div/h3[@class="code-header in-band"]' 'impl Add<Simd<u8, 16_usize>> for Simd<u8, 16>'
// @has foo/struct.Simd.html '//div[@id="trait-implementations-list"]//h3[@class="code-header in-band"]' 'impl Add<Simd<u8, 16_usize>> for Simd<u8, 16>'
impl Add for Simd<u8, 16> {
type Output = Self;

Expand Down
4 changes: 2 additions & 2 deletions src/test/rustdoc/const-generics/const-generic-slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pub trait Array {
}

// @has foo/trait.Array.html
// @has - '//div[@class="impl has-srclink"]' 'impl<T, const N: usize> Array for [T; N]'
impl <T, const N: usize> Array for [T; N] {
// @has - '//*[@class="impl has-srclink"]' 'impl<T, const N: usize> Array for [T; N]'
impl<T, const N: usize> Array for [T; N] {
type Item = T;
}
4 changes: 2 additions & 2 deletions src/test/rustdoc/const-generics/const-generics-docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub struct Foo<const N: usize> where u8: Trait<N>;
// @has foo/struct.Bar.html '//pre[@class="rust struct"]' 'pub struct Bar<T, const N: usize>(_)'
pub struct Bar<T, const N: usize>([T; N]);

// @has foo/struct.Foo.html '//div[@id="impl"]/h3[@class="code-header in-band"]' 'impl<const M: usize> Foo<M> where u8: Trait<M>'
// @has foo/struct.Foo.html '//*[@id="impl"]/h3[@class="code-header in-band"]' 'impl<const M: usize> Foo<M> where u8: Trait<M>'
impl<const M: usize> Foo<M> where u8: Trait<M> {
// @has - '//*[@id="associatedconstant.FOO_ASSOC"]' 'pub const FOO_ASSOC: usize'
pub const FOO_ASSOC: usize = M + 13;
Expand All @@ -47,7 +47,7 @@ impl<const M: usize> Foo<M> where u8: Trait<M> {
}
}

// @has foo/struct.Bar.html '//div[@id="impl"]/h3[@class="code-header in-band"]' 'impl<const M: usize> Bar<u8, M>'
// @has foo/struct.Bar.html '//*[@id="impl"]/h3[@class="code-header in-band"]' 'impl<const M: usize> Bar<u8, M>'
impl<const M: usize> Bar<u8, M> {
// @has - '//*[@id="method.hey"]' \
// 'pub fn hey<const N: usize>(&self) -> Foo<N> where u8: Trait<N>'
Expand Down
12 changes: 5 additions & 7 deletions src/test/rustdoc/const-generics/const-impl.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
#![allow(incomplete_features)]

#![feature(adt_const_params)]

#![crate_name = "foo"]

#[derive(PartialEq, Eq)]
Expand All @@ -11,20 +9,20 @@ pub enum Order {
}

// @has foo/struct.VSet.html '//pre[@class="rust struct"]' 'pub struct VSet<T, const ORDER: Order>'
// @has foo/struct.VSet.html '//div[@id="impl-Send"]/h3[@class="code-header in-band"]' 'impl<T, const ORDER: Order> Send for VSet<T, ORDER>'
// @has foo/struct.VSet.html '//div[@id="impl-Sync"]/h3[@class="code-header in-band"]' 'impl<T, const ORDER: Order> Sync for VSet<T, ORDER>'
// @has foo/struct.VSet.html '//*[@id="impl-Send"]/h3[@class="code-header in-band"]' 'impl<T, const ORDER: Order> Send for VSet<T, ORDER>'
// @has foo/struct.VSet.html '//*[@id="impl-Sync"]/h3[@class="code-header in-band"]' 'impl<T, const ORDER: Order> Sync for VSet<T, ORDER>'
pub struct VSet<T, const ORDER: Order> {
inner: Vec<T>,
}

// @has foo/struct.VSet.html '//div[@id="impl"]/h3[@class="code-header in-band"]' 'impl<T> VSet<T, { Order::Sorted }>'
// @has foo/struct.VSet.html '//*[@id="impl"]/h3[@class="code-header in-band"]' 'impl<T> VSet<T, { Order::Sorted }>'
impl<T> VSet<T, { Order::Sorted }> {
pub fn new() -> Self {
Self { inner: Vec::new() }
}
}

// @has foo/struct.VSet.html '//div[@id="impl-1"]/h3[@class="code-header in-band"]' 'impl<T> VSet<T, { Order::Unsorted }>'
// @has foo/struct.VSet.html '//*[@id="impl-1"]/h3[@class="code-header in-band"]' 'impl<T> VSet<T, { Order::Unsorted }>'
impl<T> VSet<T, { Order::Unsorted }> {
pub fn new() -> Self {
Self { inner: Vec::new() }
Expand All @@ -33,7 +31,7 @@ impl<T> VSet<T, { Order::Unsorted }> {

pub struct Escape<const S: &'static str>;

// @has foo/struct.Escape.html '//div[@id="impl"]/h3[@class="code-header in-band"]' 'impl Escape<r#"<script>alert("Escape");</script>"#>'
// @has foo/struct.Escape.html '//*[@id="impl"]/h3[@class="code-header in-band"]' 'impl Escape<r#"<script>alert("Escape");</script>"#>'
impl Escape<r#"<script>alert("Escape");</script>"#> {
pub fn f() {}
}
6 changes: 3 additions & 3 deletions src/test/rustdoc/empty-impls.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
#![crate_name = "foo"]

// @has foo/struct.Foo.html
// @has - '//div[@id="synthetic-implementations-list"]/div[@id="impl-Send"]' 'impl Send for Foo'
// @has - '//div[@id="synthetic-implementations-list"]/*[@id="impl-Send"]' 'impl Send for Foo'
pub struct Foo;

pub trait EmptyTrait {}

// @has - '//div[@id="trait-implementations-list"]/div[@id="impl-EmptyTrait"]' 'impl EmptyTrait for Foo'
// @has - '//div[@id="trait-implementations-list"]/*[@id="impl-EmptyTrait"]' 'impl EmptyTrait for Foo'
impl EmptyTrait for Foo {}

pub trait NotEmpty {
fn foo(&self);
}

// @has - '//div[@id="trait-implementations-list"]/details/summary/div[@id="impl-NotEmpty"]' 'impl NotEmpty for Foo'
// @has - '//div[@id="trait-implementations-list"]/details/summary/*[@id="impl-NotEmpty"]' 'impl NotEmpty for Foo'
impl NotEmpty for Foo {
fn foo(&self) {}
}
2 changes: 1 addition & 1 deletion src/test/rustdoc/ensure-src-link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

// This test ensures that the [src] link is present on traits items.

// @has foo/trait.Iterator.html '//div[@id="method.zip"]//a[@class="srclink"]' "source"
// @has foo/trait.Iterator.html '//*[@id="method.zip"]//a[@class="srclink"]' "source"
pub use std::iter::Iterator;
4 changes: 2 additions & 2 deletions src/test/rustdoc/extern-default-method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
extern crate rustdoc_extern_default_method as ext;

// @count extern_default_method/struct.Struct.html '//*[@id="method.provided"]' 1
// @has extern_default_method/struct.Struct.html '//div[@id="method.provided"]//a[@class="fnname"]/@href' #method.provided
// @has extern_default_method/struct.Struct.html '//div[@id="method.provided"]//a[@class="anchor"]/@href' #method.provided
// @has extern_default_method/struct.Struct.html '//*[@id="method.provided"]//a[@class="fnname"]/@href' #method.provided
// @has extern_default_method/struct.Struct.html '//*[@id="method.provided"]//a[@class="anchor"]/@href' #method.provided
pub use ext::Struct;
4 changes: 2 additions & 2 deletions src/test/rustdoc/generic-impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

use std::fmt;

// @!has foo/struct.Bar.html '//div[@id="impl-ToString"]//h3[@class="code-header in-band"]' 'impl<T> ToString for T'
// @!has foo/struct.Bar.html '//*[@id="impl-ToString"]//h3[@class="code-header in-band"]' 'impl<T> ToString for T'
pub struct Bar;

// @has foo/struct.Foo.html '//div[@id="impl-ToString"]//h3[@class="code-header in-band"]' 'impl<T> ToString for T'
// @has foo/struct.Foo.html '//*[@id="impl-ToString"]//h3[@class="code-header in-band"]' 'impl<T> ToString for T'
pub struct Foo;
// @has foo/struct.Foo.html '//div[@class="sidebar-links"]/a[@href="#impl-ToString"]' 'ToString'

Expand Down
10 changes: 6 additions & 4 deletions src/test/rustdoc/issue-29503.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ pub trait MyTrait {
fn my_string(&self) -> String;
}

// @has - "//div[@id='implementors-list']//div[@id='impl-MyTrait']//h3[@class='code-header in-band']" "impl<T> MyTrait for T where T: Debug"
impl<T> MyTrait for T where T: fmt::Debug {
// @has - "//div[@id='implementors-list']//*[@id='impl-MyTrait']//h3[@class='code-header in-band']" "impl<T> MyTrait for T where T: Debug"
impl<T> MyTrait for T
where
T: fmt::Debug,
{
fn my_string(&self) -> String {
format!("{:?}", self)
}
}

pub fn main() {
}
pub fn main() {}
Loading

0 comments on commit 3edec80

Please sign in to comment.