Add SharedResource abstraction and use it in write_shared

jyn514 · jyn514 · commit 7c89cc4a6f68 · 2021-03-25T11:40:32.000-04:00
This cleans up the code quite a bit, and also makes the next commit much
easier.
diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs
@@ -79,17 +79,6 @@ crate struct Context<'tcx> {
 rustc_data_structures::static_assert_size!(Context<'_>, 152);
 
 impl<'tcx> Context<'tcx> {
-    pub(super) fn path(&self, filename: &str) -> PathBuf {
-        // We use splitn vs Path::extension here because we might get a filename
-        // like `style.min.css` and we want to process that into
-        // `style-suffix.min.css`.  Path::extension would just return `css`
-        // which would result in `style.min-suffix.css` which isn't what we
-        // want.
-        let (base, ext) = filename.split_once('.').unwrap();
-        let filename = format!("{}{}.{}", base, self.shared.resource_suffix, ext);
-        self.dst.join(&filename)
-    }
-
     pub(super) fn tcx(&self) -> TyCtxt<'tcx> {
         self.shared.tcx
     }
@@ -487,7 +476,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             |buf: &mut Buffer| all.print(buf),
             &self.shared.style_files,
         );
-        self.shared.fs.write(&final_file, v.as_bytes())?;
+        self.shared.fs.write(final_file, v.as_bytes())?;
 
         // Generating settings page.
         page.title = "Rustdoc settings";
diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs
@@ -14,7 +14,7 @@ use serde::Serialize;
 use super::{collect_paths_for_type, ensure_trailing_slash, Context, BASIC_KEYWORDS};
 use crate::clean::Crate;
 use crate::config::RenderOptions;
-use crate::docfs::{DocFS, PathError};
+use crate::docfs::PathError;
 use crate::error::Error;
 use crate::formats::FormatRenderer;
 use crate::html::{layout, static_files};
@@ -40,6 +40,81 @@ crate static FILES_UNVERSIONED: Lazy<FxHashMap<&str, &[u8]>> = Lazy::new(|| {
     }
 });
 
+enum SharedResource<'a> {
+    /// This file will never change, no matter what toolchain is used to build it.
+    ///
+    /// It does not have a resource suffix.
+    Unversioned { name: &'a str },
+    /// This file may change depending on the toolchain.
+    ///
+    /// It has a resource suffix.
+    ToolchainSpecific { basename: &'a str },
+    /// This file may change for any crate within a build.
+    ///
+    /// This differs from normal crate-specific files because it has a resource suffix.
+    CrateSpecific { basename: &'a str },
+}
+
+impl SharedResource<'_> {
+    fn extension(&self) -> Option<&OsStr> {
+        use SharedResource::*;
+        match self {
+            Unversioned { name }
+            | ToolchainSpecific { basename: name }
+            | CrateSpecific { basename: name } => Path::new(name).extension(),
+        }
+    }
+
+    fn path(&self, cx: &Context<'_>) -> PathBuf {
+        match self {
+            SharedResource::Unversioned { name } => cx.dst.join(name),
+            SharedResource::ToolchainSpecific { basename } => cx.suffix_path(basename),
+            SharedResource::CrateSpecific { basename } => cx.suffix_path(basename),
+        }
+    }
+}
+
+impl Context<'_> {
+    fn suffix_path(&self, filename: &str) -> PathBuf {
+        // We use splitn vs Path::extension here because we might get a filename
+        // like `style.min.css` and we want to process that into
+        // `style-suffix.min.css`.  Path::extension would just return `css`
+        // which would result in `style.min-suffix.css` which isn't what we
+        // want.
+        let (base, ext) = filename.split_once('.').unwrap();
+        let filename = format!("{}{}.{}", base, self.shared.resource_suffix, ext);
+        self.dst.join(&filename)
+    }
+
+    fn write_shared<C: AsRef<[u8]>>(&self, resource: SharedResource<'_>, contents: C) -> Result<(), Error>
+    {
+        self.shared.fs.write(resource.path(self), contents)
+    }
+
+    fn write_minify(
+        &self,
+        resource: SharedResource<'_>,
+        contents: &str,
+        minify: bool,
+    ) -> Result<(), Error> {
+        let tmp;
+        let contents = if minify {
+            tmp = if resource.extension() == Some(&OsStr::new("css")) {
+                minifier::css::minify(contents).map_err(|e| {
+                    Error::new(format!("failed to minify CSS file: {}", e), resource.path(self))
+                })?
+            } else {
+                minifier::js::minify(contents)
+            };
+            tmp.as_bytes()
+        } else {
+            contents.as_bytes()
+        };
+
+        self.write_shared(resource, contents)
+    }
+}
+
 pub(super) fn write_shared(
     cx: &Context<'_>,
     krate: &Crate,
@@ -52,27 +127,22 @@ pub(super) fn write_shared(
     let lock_file = cx.dst.join(".lock");
     let _lock = try_err!(flock::Lock::new(&lock_file, true, true, true), &lock_file);
 
+    // The weird `: &_` is to work around a borrowck bug: https://github.com/rust-lang/rust/issues/41078#issuecomment-293646723
+    let write_minify = |p, c: &_| {
+        cx.write_minify(
+            SharedResource::ToolchainSpecific { basename: p },
+            c,
+            options.enable_minification,
+        )
+    };
+    let write_toolchain =
+        |p: &_, c: &_| cx.write_shared(SharedResource::ToolchainSpecific { basename: p }, c);
+
     // Add all the static files. These may already exist, but we just
     // overwrite them anyway to make sure that they're fresh and up-to-date.
-
-    write_minify(
-        &cx.shared.fs,
-        cx.path("rustdoc.css"),
-        static_files::RUSTDOC_CSS,
-        options.enable_minification,
-    )?;
-    write_minify(
-        &cx.shared.fs,
-        cx.path("settings.css"),
-        static_files::SETTINGS_CSS,
-        options.enable_minification,
-    )?;
-    write_minify(
-        &cx.shared.fs,
-        cx.path("noscript.css"),
-        static_files::NOSCRIPT_CSS,
-        options.enable_minification,
-    )?;
+    write_minify("rustdoc.css", static_files::RUSTDOC_CSS)?;
+    write_minify("settings.css", static_files::SETTINGS_CSS)?;
+    write_minify("noscript.css", static_files::NOSCRIPT_CSS)?;
 
     // To avoid "light.css" to be overwritten, we'll first run over the received themes and only
     // then we'll run over the "official" styles.
@@ -85,106 +155,66 @@ pub(super) fn write_shared(
 
         // Handle the official themes
         match theme {
-            "light" => write_minify(
-                &cx.shared.fs,
-                cx.path("light.css"),
-                static_files::themes::LIGHT,
-                options.enable_minification,
-            )?,
-            "dark" => write_minify(
-                &cx.shared.fs,
-                cx.path("dark.css"),
-                static_files::themes::DARK,
-                options.enable_minification,
-            )?,
-            "ayu" => write_minify(
-                &cx.shared.fs,
-                cx.path("ayu.css"),
-                static_files::themes::AYU,
-                options.enable_minification,
-            )?,
+            "light" => write_minify("light.css", static_files::themes::LIGHT)?,
+            "dark" => write_minify("dark.css", static_files::themes::DARK)?,
+            "ayu" => write_minify("ayu.css", static_files::themes::AYU)?,
             _ => {
                 // Handle added third-party themes
                 let content = try_err!(fs::read(&entry.path), &entry.path);
-                cx.shared
-                    .fs
-                    .write(cx.path(&format!("{}.{}", theme, extension)), content.as_slice())?;
+                // This is not exactly right: if compiled a second time with the same toolchain but different CLI args, the file could be different.
+                // But docs.rs doesn't use this, so hopefully the issue doesn't come up.
+                write_toolchain(&format!("{}.{}", theme, extension), content.as_slice())?;
             }
         };
 
         themes.insert(theme.to_owned());
     }
 
-    let write = |p, c| cx.shared.fs.write(p, c);
     if (*cx.shared).layout.logo.is_empty() {
-        write(cx.path("rust-logo.png"), static_files::RUST_LOGO)?;
+        write_toolchain("rust-logo.png", static_files::RUST_LOGO)?;
     }
     if (*cx.shared).layout.favicon.is_empty() {
-        write(cx.path("favicon.svg"), static_files::RUST_FAVICON_SVG)?;
-        write(cx.path("favicon-16x16.png"), static_files::RUST_FAVICON_PNG_16)?;
-        write(cx.path("favicon-32x32.png"), static_files::RUST_FAVICON_PNG_32)?;
+        write_toolchain("favicon.svg", static_files::RUST_FAVICON_SVG)?;
+        write_toolchain("favicon-16x16.png", static_files::RUST_FAVICON_PNG_16)?;
+        write_toolchain("favicon-32x32.png", static_files::RUST_FAVICON_PNG_32)?;
     }
-    write(cx.path("brush.svg"), static_files::BRUSH_SVG)?;
-    write(cx.path("wheel.svg"), static_files::WHEEL_SVG)?;
-    write(cx.path("down-arrow.svg"), static_files::DOWN_ARROW_SVG)?;
+    write_toolchain("brush.svg", static_files::BRUSH_SVG)?;
+    write_toolchain("wheel.svg", static_files::WHEEL_SVG)?;
+    write_toolchain("down-arrow.svg", static_files::DOWN_ARROW_SVG)?;
 
     let mut themes: Vec<&String> = themes.iter().collect();
     themes.sort();
 
     write_minify(
-        &cx.shared.fs,
-        cx.path("main.js"),
+        "main.js",
         &static_files::MAIN_JS.replace(
             "/* INSERT THEMES HERE */",
             &format!(" = {}", serde_json::to_string(&themes).unwrap()),
         ),
-        options.enable_minification,
-    )?;
-    write_minify(
-        &cx.shared.fs,
-        cx.path("settings.js"),
-        static_files::SETTINGS_JS,
-        options.enable_minification,
     )?;
+    write_minify("settings.js", static_files::SETTINGS_JS)?;
     if cx.shared.include_sources {
-        write_minify(
-            &cx.shared.fs,
-            cx.path("source-script.js"),
-            static_files::sidebar::SOURCE_SCRIPT,
-            options.enable_minification,
-        )?;
+        write_minify("source-script.js", static_files::sidebar::SOURCE_SCRIPT)?;
     }
 
     {
         write_minify(
-            &cx.shared.fs,
-            cx.path("storage.js"),
+            "storage.js",
             &format!(
                 "var resourcesSuffix = \"{}\";{}",
                 cx.shared.resource_suffix,
                 static_files::STORAGE_JS
             ),
-            options.enable_minification,
         )?;
     }
 
     if let Some(ref css) = cx.shared.layout.css_file_extension {
-        let out = cx.path("theme.css");
         let buffer = try_err!(fs::read_to_string(css), css);
-        if !options.enable_minification {
-            cx.shared.fs.write(&out, &buffer)?;
-        } else {
-            write_minify(&cx.shared.fs, out, &buffer, options.enable_minification)?;
-        }
+        write_minify("theme.css", &buffer)?;
     }
-    write_minify(
-        &cx.shared.fs,
-        cx.path("normalize.css"),
-        static_files::NORMALIZE_CSS,
-        options.enable_minification,
-    )?;
-    for (file, contents) in &*FILES_UNVERSIONED {
-        write(cx.dst.join(file), contents)?;
+    write_minify("normalize.css", static_files::NORMALIZE_CSS)?;
+    for (name, contents) in &*FILES_UNVERSIONED {
+        cx.write_shared(SharedResource::Unversioned { name }, contents)?;
     }
 
     fn collect(path: &Path, krate: &str, key: &str) -> io::Result<(Vec<String>, Vec<String>)> {
@@ -324,7 +354,7 @@ pub(super) fn write_shared(
             "var N = null;var sourcesIndex = {{}};\n{}\ncreateSourceSidebar();\n",
             all_sources.join("\n")
         );
-        cx.shared.fs.write(&dst, v.as_bytes())?;
+        cx.write_shared(SharedResource::CrateSpecific { basename: "source-files.js" }, v)?;
     }
 
     // Update the search index and crate list.
@@ -341,13 +371,12 @@ pub(super) fn write_shared(
         let mut v = String::from("var searchIndex = JSON.parse('{\\\n");
         v.push_str(&all_indexes.join(",\\\n"));
         v.push_str("\\\n}');\ninitSearch(searchIndex);");
-        cx.shared.fs.write(&dst, &v)?;
+        cx.write_shared(SharedResource::CrateSpecific { basename: "search-index.js" }, v)?;
     }
 
-    let crate_list_dst = cx.dst.join(&format!("crates{}.js", cx.shared.resource_suffix));
     let crate_list =
         format!("window.ALL_CRATES = [{}];", krates.iter().map(|k| format!("\"{}\"", k)).join(","));
-    cx.shared.fs.write(&crate_list_dst, &crate_list)?;
+    cx.write_shared(SharedResource::CrateSpecific { basename: "crates.js" }, crate_list)?;
 
     if options.enable_index_page {
         if let Some(index_page) = options.index_page.clone() {
@@ -481,21 +510,3 @@ pub(super) fn write_shared(
     }
     Ok(())
 }
-
-fn write_minify(
-    fs: &DocFS,
-    dst: PathBuf,
-    contents: &str,
-    enable_minification: bool,
-) -> Result<(), Error> {
-    if enable_minification {
-        if dst.extension() == Some(&OsStr::new("css")) {
-            let res = try_none!(minifier::css::minify(contents).ok(), &dst);
-            fs.write(dst, res.as_bytes())
-        } else {
-            fs.write(dst, minifier::js::minify(contents).as_bytes())
-        }
-    } else {
-        fs.write(dst, contents.as_bytes())
-    }
-}
