Auto merge of #64044 - Mark-Simulacrum:rustdoc-clean-2, r=GuillaumeGomez

Rustdoc: formatting to buffers

This should be reviewed commit-by-commit.

I've not attempted to fully flesh out what the end state of this PR could look like yet as I wanted to get it up for some early feedback (I already think this has some wins, too, so we could land it as-is).

The primary idea with `Buffer` is that it internally tracks whether we're printing to HTML or text, and the goal is that eventually instead of branch on `fmt.alternate()` anywhere, we'd call a helper like `buf.nbsp()` which would either return ` ` or ` ` depending on the target we're printing to. Obviously, that's not included in this PR, in part because it was already getting quite big.

src/librustdoc/html/format.rs

@@ -18,6 +18,100 @@ use crate::clean::{self, PrimitiveType};
 use crate::html::item_type::ItemType;
 use crate::html::render::{self, cache, CURRENT_DEPTH};
 
+pub trait Print {
+    fn print(self, buffer: &mut Buffer);
+}
+
+impl<F> Print for F
+    where F: FnOnce(&mut Buffer),
+{
+    fn print(self, buffer: &mut Buffer) {
+        (self)(buffer)
+    }
+}
+
+impl Print for String {
+    fn print(self, buffer: &mut Buffer) {
+        buffer.write_str(&self);
+    }
+}
+
+impl Print for &'_ str {
+    fn print(self, buffer: &mut Buffer) {
+        buffer.write_str(self);
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct Buffer {
+    for_html: bool,
+    buffer: String,
+}
+
+impl Buffer {
+    crate fn empty_from(v: &Buffer) -> Buffer {
+        Buffer {
+            for_html: v.for_html,
+            buffer: String::new(),
+        }
+    }
+
+    crate fn html() -> Buffer {
+        Buffer {
+            for_html: true,
+            buffer: String::new(),
+        }
+    }
+
+    crate fn is_empty(&self) -> bool {
+        self.buffer.is_empty()
+    }
+
+    crate fn into_inner(self) -> String {
+        self.buffer
+    }
+
+    crate fn insert_str(&mut self, idx: usize, s: &str) {
+        self.buffer.insert_str(idx, s);
+    }
+
+    crate fn push_str(&mut self, s: &str) {
+        self.buffer.push_str(s);
+    }
+
+    // Intended for consumption by write! and writeln! (std::fmt) but without
+    // the fmt::Result return type imposed by fmt::Write (and avoiding the trait
+    // import).
+    crate fn write_str(&mut self, s: &str) {
+        self.buffer.push_str(s);
+    }
+
+    // Intended for consumption by write! and writeln! (std::fmt) but without
+    // the fmt::Result return type imposed by fmt::Write (and avoiding the trait
+    // import).
+    crate fn write_fmt(&mut self, v: fmt::Arguments<'_>) {
+        use fmt::Write;
+        self.buffer.write_fmt(v).unwrap();
+    }
+
+    crate fn to_display<T: Print>(mut self, t: T) -> String {
+        t.print(&mut self);
+        self.into_inner()
+    }
+
+    crate fn with_formatter<T: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result>(&mut self, t: T) {
+        self.from_display(display_fn(move |f| (t)(f)));
+    }
+
+    crate fn from_display<T: std::fmt::Display>(&mut self, t: T) {
+        if self.for_html {
+            write!(self, "{}", t);
+        } else {
+            write!(self, "{:#}", t);
+        }
+    }
+}
+
 /// Helper to render an optional visibility with a space after it (if the
 /// visibility is preset)
 #[derive(Copy, Clone)]
@@ -778,9 +872,9 @@ impl fmt::Display for clean::Impl {
 
 // The difference from above is that trait is not hyperlinked.
 pub fn fmt_impl_for_trait_page(i: &clean::Impl,
-                               f: &mut fmt::Formatter<'_>,
+                               f: &mut Buffer,
-                               use_absolute: bool) -> fmt::Result {
+                               use_absolute: bool) {
-    fmt_impl(i, f, false, use_absolute)
+    f.with_formatter(|f| fmt_impl(i, f, false, use_absolute))
 }
 
 impl fmt::Display for clean::Arguments {

src/librustdoc/html/layout.rs

@@ -1,9 +1,8 @@
-use std::fmt;
-use std::io;
 use std::path::PathBuf;
 
 use crate::externalfiles::ExternalHtml;
 use crate::html::render::SlashChecker;
+use crate::html::format::{Buffer, Print};
 
 #[derive(Clone)]
 pub struct Layout {
@@ -11,6 +10,12 @@ pub struct Layout {
     pub favicon: String,
     pub external_html: ExternalHtml,
     pub krate: String,
+    /// The given user css file which allow to customize the generated
+    /// documentation theme.
+    pub css_file_extension: Option<PathBuf>,
+    /// If false, the `select` element to have search filtering by crates on rendered docs
+    /// won't be generated.
+    pub generate_search_filter: bool,
 }
 
 pub struct Page<'a> {
@@ -25,19 +30,15 @@ pub struct Page<'a> {
     pub static_extra_scripts: &'a [&'a str],
 }
 
-pub fn render<T: fmt::Display, S: fmt::Display>(
+pub fn render<T: Print, S: Print>(
-    dst: &mut dyn io::Write,
     layout: &Layout,
     page: &Page<'_>,
-    sidebar: &S,
+    sidebar: S,
-    t: &T,
+    t: T,
-    css_file_extension: bool,
     themes: &[PathBuf],
-    generate_search_filter: bool,
+) -> String {
-) -> io::Result<()> {
     let static_root_path = page.static_root_path.unwrap_or(page.root_path);
-    write!(dst,
+    format!("<!DOCTYPE html>\
-"<!DOCTYPE html>\
 <html lang=\"en\">\
 <head>\
     <meta charset=\"utf-8\">\
@@ -164,7 +165,7 @@ pub fn render<T: fmt::Display, S: fmt::Display>(
     <script defer src=\"{root_path}search-index{suffix}.js\"></script>\
 </body>\
 </html>",
-    css_extension = if css_file_extension {
+    css_extension = if layout.css_file_extension.is_some() {
         format!("<link rel=\"stylesheet\" \
                        type=\"text/css\" \
                        href=\"{static_root_path}theme{suffix}.css\">",
@@ -173,7 +174,7 @@ pub fn render<T: fmt::Display, S: fmt::Display>(
     } else {
         String::new()
     },
-    content   = *t,
+    content   = Buffer::html().to_display(t),
     static_root_path = static_root_path,
     root_path = page.root_path,
     css_class = page.css_class,
@@ -207,7 +208,7 @@ pub fn render<T: fmt::Display, S: fmt::Display>(
     in_header = layout.external_html.in_header,
     before_content = layout.external_html.before_content,
     after_content = layout.external_html.after_content,
-    sidebar   = *sidebar,
+    sidebar   = Buffer::html().to_display(sidebar),
     krate     = layout.krate,
     themes = themes.iter()
                    .filter_map(|t| t.file_stem())
@@ -228,7 +229,7 @@ pub fn render<T: fmt::Display, S: fmt::Display>(
                 root_path=page.root_path,
                 extra_script=e)
     }).collect::<String>(),
-    filter_crates=if generate_search_filter {
+    filter_crates=if layout.generate_search_filter {
         "<select id=\"crate-search\">\
             <option value=\"All crates\">All crates</option>\
         </select>"
@@ -238,9 +239,9 @@ pub fn render<T: fmt::Display, S: fmt::Display>(
     )
 }
 
-pub fn redirect(dst: &mut dyn io::Write, url: &str) -> io::Result<()> {
+pub fn redirect(url: &str) -> String {
     // <script> triggers a redirect before refresh, so this is fine.
-    write!(dst,
+    format!(
 r##"<!DOCTYPE html>
 <html lang="en">
 <head>

src/librustdoc/html/sources.rs

@@ -4,10 +4,10 @@ use crate::fold::DocFolder;
 use crate::html::layout;
 use crate::html::render::{Error, SharedContext, BASIC_KEYWORDS};
 use crate::html::highlight;
+use crate::html::format::Buffer;
 use std::ffi::OsStr;
 use std::fs;
 use std::path::{Component, Path, PathBuf};
-use std::fmt;
 use syntax::source_map::FileName;
 
 crate fn render(dst: &Path, scx: &mut SharedContext,
@@ -105,7 +105,6 @@ impl<'a> SourceCollector<'a> {
         cur.push(&fname);
         href.push_str(&fname.to_string_lossy());
 
-        let mut v = Vec::new();
         let title = format!("{} -- source", cur.file_name().expect("failed to get file name")
                                                .to_string_lossy());
         let desc = format!("Source to the Rust file `{}`.", filename);
@@ -120,15 +119,10 @@ impl<'a> SourceCollector<'a> {
             extra_scripts: &[&format!("source-files{}", self.scx.resource_suffix)],
             static_extra_scripts: &[&format!("source-script{}", self.scx.resource_suffix)],
         };
-        let result = layout::render(&mut v, &self.scx.layout,
+        let v = layout::render(&self.scx.layout,
-                       &page, &(""), &Source(contents),
+                       &page, "", |buf: &mut _| print_src(buf, &contents),
-                       self.scx.css_file_extension.is_some(),
+                       &self.scx.themes);
-                       &self.scx.themes,
+        self.scx.fs.write(&cur, v.as_bytes())?;
-                       self.scx.generate_search_filter);
-        if let Err(e) = result {
-            return Err(Error::new(e, &cur));
-        }
-        self.scx.fs.write(&cur, &v)?;
         self.scx.local_sources.insert(p.clone(), href);
         Ok(())
     }
@@ -163,11 +157,7 @@ where
 
 /// Wrapper struct to render the source code of a file. This will do things like
 /// adding line numbers to the left-hand side.
-struct Source<'a>(&'a str);
+fn print_src(buf: &mut Buffer, s: &str) {
-
-impl<'a> fmt::Display for Source<'a> {
-    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let Source(s) = *self;
     let lines = s.lines().count();
     let mut cols = 0;
     let mut tmp = lines;
@@ -175,13 +165,11 @@ impl<'a> fmt::Display for Source<'a> {
         cols += 1;
         tmp /= 10;
     }
-        write!(fmt, "<pre class=\"line-numbers\">")?;
+    write!(buf, "<pre class=\"line-numbers\">");
     for i in 1..=lines {
-            write!(fmt, "<span id=\"{0}\">{0:1$}</span>\n", i, cols)?;
+        write!(buf, "<span id=\"{0}\">{0:1$}</span>\n", i, cols);
-        }
-        write!(fmt, "</pre>")?;
-        write!(fmt, "{}",
-               highlight::render_with_highlighting(s, None, None, None))?;
-        Ok(())
     }
+    write!(buf, "</pre>");
+    write!(buf, "{}",
+            highlight::render_with_highlighting(s, None, None, None));
 }