Auto merge of #71900 - GuillaumeGomez:clean-up-rustdoc, r=ollie27,kin…

…nison Clean up rustdoc source code Fixes #70498. r? @kinnison cc @rust-lang/rustdoc
rust-lang · May 26, 2020 · aeca4d6 · aeca4d6
2 parents cb272d5 + c8fb0d7
commit aeca4d6
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 50 deletions.
diff --git a/src/librustdoc/docfs.rs b/src/librustdoc/docfs.rs
@@ -12,6 +12,7 @@
 use std::fs;
 use std::io;
 use std::path::Path;
+use std::string::ToString;
 use std::sync::mpsc::{channel, Receiver, Sender};
 use std::sync::Arc;
 
@@ -25,7 +26,9 @@ macro_rules! try_err {
 }
 
 pub trait PathError {
-    fn new<P: AsRef<Path>>(e: io::Error, path: P) -> Self;
+    fn new<S, P: AsRef<Path>>(e: S, path: P) -> Self
+    where
+        S: ToString + Sized;
 }
 
 pub struct ErrorStorage {

diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs
@@ -31,7 +31,6 @@ use std::cmp::Ordering;
 use std::collections::{BTreeMap, VecDeque};
 use std::default::Default;
 use std::error;
-
 use std::ffi::OsStr;
 use std::fmt::{self, Formatter, Write};
 use std::fs::{self, File};
@@ -40,6 +39,7 @@ use std::io::{self, BufReader};
 use std::path::{Component, Path, PathBuf};
 use std::rc::Rc;
 use std::str;
+use std::string::ToString;
 use std::sync::Arc;
 
 use rustc_ast_pretty::pprust;
@@ -92,7 +92,7 @@ crate fn ensure_trailing_slash(v: &str) -> impl fmt::Display + '_ {
 #[derive(Debug)]
 pub struct Error {
     pub file: PathBuf,
-    pub error: io::Error,
+    pub error: String,
 }
 
 impl error::Error for Error {}
@@ -109,8 +109,11 @@ impl std::fmt::Display for Error {
 }
 
 impl PathError for Error {
-    fn new<P: AsRef<Path>>(e: io::Error, path: P) -> Error {
-        Error { file: path.as_ref().to_path_buf(), error: e }
+    fn new<S, P: AsRef<Path>>(e: S, path: P) -> Error
+    where
+        S: ToString + Sized,
+    {
+        Error { file: path.as_ref().to_path_buf(), error: e.to_string() }
     }
 }
 
@@ -562,7 +565,7 @@ pub fn run(
 
     // Write shared runs within a flock; disable thread dispatching of IO temporarily.
     Arc::get_mut(&mut cx.shared).unwrap().fs.set_sync_only(true);
-    write_shared(&cx, &krate, index, &md_opts, diag)?;
+    write_shared(&cx, &krate, index, &md_opts)?;
     Arc::get_mut(&mut cx.shared).unwrap().fs.set_sync_only(false);
 
     // And finally render the whole crate's documentation
@@ -582,7 +585,6 @@ fn write_shared(
     krate: &clean::Crate,
     search_index: String,
     options: &RenderOptions,
-    diag: &rustc_errors::Handler,
 ) -> Result<(), Error> {
     // Write out the shared files. Note that these are shared among all rustdoc
     // docs placed in the output directory, so this needs to be a synchronized
@@ -928,7 +930,8 @@ themePicker.onblur = handleThemeButtonsBlur;
             md_opts.output = cx.dst.clone();
             md_opts.external_html = (*cx.shared).layout.external_html.clone();
 
-            crate::markdown::render(index_page, md_opts, diag, cx.shared.edition);
+            crate::markdown::render(&index_page, md_opts, cx.shared.edition)
+                .map_err(|e| Error::new(e, &index_page))?;
         } else {
             let dst = cx.dst.join("index.html");
             let page = layout::Page {

diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs
@@ -450,14 +450,29 @@ fn main_args(args: &[String]) -> i32 {
     rustc_interface::interface::default_thread_pool(options.edition, move || main_options(options))
 }
 
+fn wrap_return(diag: &rustc_errors::Handler, res: Result<(), String>) -> i32 {
+    match res {
+        Ok(()) => 0,
+        Err(err) => {
+            if !err.is_empty() {
+                diag.struct_err(&err).emit();
+            }
+            1
+        }
+    }
+}
+
 fn main_options(options: config::Options) -> i32 {
     let diag = core::new_handler(options.error_format, None, &options.debugging_options);
 
     match (options.should_test, options.markdown_input()) {
-        (true, true) => return markdown::test(options, &diag),
-        (true, false) => return test::run(options),
+        (true, true) => return wrap_return(&diag, markdown::test(options)),
+        (true, false) => return wrap_return(&diag, test::run(options)),
         (false, true) => {
-            return markdown::render(options.input, options.render_options, &diag, options.edition);
+            return wrap_return(
+                &diag,
+                markdown::render(&options.input, options.render_options, options.edition),
+            );
         }
         (false, false) => {}
     }

diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs
@@ -1,13 +1,12 @@
-use std::fs::{create_dir_all, File};
+use std::fs::{create_dir_all, read_to_string, File};
 use std::io::prelude::*;
-use std::path::PathBuf;
+use std::path::Path;
 
 use rustc_feature::UnstableFeatures;
 use rustc_span::edition::Edition;
 use rustc_span::source_map::DUMMY_SP;
 
 use crate::config::{Options, RenderOptions};
-use crate::externalfiles::{load_string, LoadStringError};
 use crate::html::escape::Escape;
 use crate::html::markdown;
 use crate::html::markdown::{find_testable_code, ErrorCodes, IdMap, Markdown, MarkdownWithToc};
@@ -34,17 +33,16 @@ fn extract_leading_metadata(s: &str) -> (Vec<&str>, &str) {
 
 /// Render `input` (e.g., "foo.md") into an HTML file in `output`
 /// (e.g., output = "bar" => "bar/foo.html").
-pub fn render(
-    input: PathBuf,
+pub fn render<P: AsRef<Path>>(
+    input: P,
     options: RenderOptions,
-    diag: &rustc_errors::Handler,
     edition: Edition,
-) -> i32 {
+) -> Result<(), String> {
     if let Err(e) = create_dir_all(&options.output) {
-        diag.struct_err(&format!("{}: {}", options.output.display(), e)).emit();
-        return 4;
+        return Err(format!("{}: {}", options.output.display(), e));
     }
 
+    let input = input.as_ref();
     let mut output = options.output;
     output.push(input.file_name().unwrap());
     output.set_extension("html");
@@ -55,26 +53,15 @@ pub fn render(
         css.push_str(&s)
     }
 
-    let input_str = match load_string(&input, diag) {
-        Ok(s) => s,
-        Err(LoadStringError::ReadFail) => return 1,
-        Err(LoadStringError::BadUtf8) => return 2,
-    };
+    let input_str = read_to_string(input).map_err(|err| format!("{}: {}", input.display(), err))?;
     let playground_url = options.markdown_playground_url.or(options.playground_url);
     let playground = playground_url.map(|url| markdown::Playground { crate_name: None, url });
 
-    let mut out = match File::create(&output) {
-        Err(e) => {
-            diag.struct_err(&format!("{}: {}", output.display(), e)).emit();
-            return 4;
-        }
-        Ok(f) => f,
-    };
+    let mut out = File::create(&output).map_err(|e| format!("{}: {}", output.display(), e))?;
 
     let (metadata, text) = extract_leading_metadata(&input_str);
     if metadata.is_empty() {
-        diag.struct_err("invalid markdown file: no initial lines starting with `# ` or `%`").emit();
-        return 5;
+        return Err("invalid markdown file: no initial lines starting with `# ` or `%`".to_owned());
     }
     let title = metadata[0];
 
@@ -122,22 +109,15 @@ pub fn render(
     );
 
     match err {
-        Err(e) => {
-            diag.struct_err(&format!("cannot write to `{}`: {}", output.display(), e)).emit();
-            6
-        }
-        Ok(_) => 0,
+        Err(e) => Err(format!("cannot write to `{}`: {}", output.display(), e)),
+        Ok(_) => Ok(()),
     }
 }
 
 /// Runs any tests/code examples in the markdown file `input`.
-pub fn test(mut options: Options, diag: &rustc_errors::Handler) -> i32 {
-    let input_str = match load_string(&options.input, diag) {
-        Ok(s) => s,
-        Err(LoadStringError::ReadFail) => return 1,
-        Err(LoadStringError::BadUtf8) => return 2,
-    };
-
+pub fn test(mut options: Options) -> Result<(), String> {
+    let input_str = read_to_string(&options.input)
+        .map_err(|err| format!("{}: {}", options.input.display(), err))?;
     let mut opts = TestOptions::default();
     opts.no_crate_inject = true;
     opts.display_warnings = options.display_warnings;
@@ -161,5 +141,5 @@ pub fn test(mut options: Options, diag: &rustc_errors::Handler) -> i32 {
         collector.tests,
         Some(testing::Options::new().display_output(options.display_warnings)),
     );
-    0
+    Ok(())
 }
diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs
@@ -43,7 +43,7 @@ pub struct TestOptions {
     pub attrs: Vec<String>,
 }
 
-pub fn run(options: Options) -> i32 {
+pub fn run(options: Options) -> Result<(), String> {
     let input = config::Input::File(options.input.clone());
 
     let invalid_codeblock_attribute_name = rustc_lint::builtin::INVALID_CODEBLOCK_ATTRIBUTE.name;
@@ -151,7 +151,7 @@ pub fn run(options: Options) -> i32 {
     });
     let tests = match tests {
         Ok(tests) => tests,
-        Err(ErrorReported) => return 1,
+        Err(ErrorReported) => return Err(String::new()),
     };
 
     test_args.insert(0, "rustdoctest".to_string());
@@ -162,7 +162,7 @@ pub fn run(options: Options) -> i32 {
         Some(testing::Options::new().display_output(display_warnings)),
     );
 
-    0
+    Ok(())
 }
 
 // Look for `#![doc(test(no_crate_inject))]`, used by crates in the std facade.