]> git.lizzy.rs Git - rust.git/commitdiff
Auto merge of #50541 - QuietMisdreavus:rustdoc-errors, r=GuillaumeGomez
authorbors <bors@rust-lang.org>
Wed, 16 May 2018 01:43:26 +0000 (01:43 +0000)
committerbors <bors@rust-lang.org>
Wed, 16 May 2018 01:43:26 +0000 (01:43 +0000)
rustdoc: replace most (e)println! statements with structured warnings/errors

Turns out, the rustc diagnostic handler doesn't need a whole lot of setup that we weren't already doing. For errors that occur outside a "dealing with source code" context, we can just use the format/color config we were already parsing and make up a `Handler` that we can emit structured warnings/errors from. So i did that. This will make it way easier to test things with `rustdoc-ui` tests, since those require the JSON error output. (In fact, this PR is a yak shave for a different one where i was trying to do just that. `>_>`)

src/librustdoc/core.rs
src/librustdoc/externalfiles.rs
src/librustdoc/lib.rs
src/librustdoc/markdown.rs
src/librustdoc/theme.rs

index 6222edd5450f05cc1facd4fe4271816daab71d5a..c9a80d4779177bed04d448da3b747f940b408c61 100644 (file)
@@ -117,6 +117,57 @@ fn is_doc_reachable(&self, did: DefId) -> bool {
     }
 }
 
+/// Creates a new diagnostic `Handler` that can be used to emit warnings and errors.
+///
+/// If the given `error_format` is `ErrorOutputType::Json` and no `CodeMap` is given, a new one
+/// will be created for the handler.
+pub fn new_handler(error_format: ErrorOutputType, codemap: Option<Lrc<codemap::CodeMap>>)
+    -> errors::Handler
+{
+    // rustdoc doesn't override (or allow to override) anything from this that is relevant here, so
+    // stick to the defaults
+    let sessopts = config::basic_options();
+    let emitter: Box<dyn Emitter + sync::Send> = match error_format {
+        ErrorOutputType::HumanReadable(color_config) => Box::new(
+            EmitterWriter::stderr(
+                color_config,
+                codemap.map(|cm| cm as _),
+                false,
+                sessopts.debugging_opts.teach,
+            ).ui_testing(sessopts.debugging_opts.ui_testing)
+        ),
+        ErrorOutputType::Json(pretty) => {
+            let codemap = codemap.unwrap_or_else(
+                || Lrc::new(codemap::CodeMap::new(sessopts.file_path_mapping())));
+            Box::new(
+                JsonEmitter::stderr(
+                    None,
+                    codemap,
+                    pretty,
+                    sessopts.debugging_opts.suggestion_applicability,
+                ).ui_testing(sessopts.debugging_opts.ui_testing)
+            )
+        },
+        ErrorOutputType::Short(color_config) => Box::new(
+            EmitterWriter::stderr(
+                color_config,
+                codemap.map(|cm| cm as _),
+                true,
+                false)
+        ),
+    };
+
+    errors::Handler::with_emitter_and_flags(
+        emitter,
+        errors::HandlerFlags {
+            can_emit_warnings: true,
+            treat_err_as_bug: false,
+            external_macro_backtrace: false,
+            ..Default::default()
+        },
+    )
+}
+
 pub fn run_core(search_paths: SearchPaths,
                 cfgs: Vec<String>,
                 externs: config::Externs,
@@ -159,41 +210,11 @@ pub fn run_core(search_paths: SearchPaths,
         },
         error_format,
         edition,
-        ..config::basic_options().clone()
+        ..config::basic_options()
     };
     driver::spawn_thread_pool(sessopts, move |sessopts| {
         let codemap = Lrc::new(codemap::CodeMap::new(sessopts.file_path_mapping()));
-        let emitter: Box<dyn Emitter + sync::Send> = match error_format {
-            ErrorOutputType::HumanReadable(color_config) => Box::new(
-                EmitterWriter::stderr(
-                    color_config,
-                    Some(codemap.clone()),
-                    false,
-                    sessopts.debugging_opts.teach,
-                ).ui_testing(sessopts.debugging_opts.ui_testing)
-            ),
-            ErrorOutputType::Json(pretty) => Box::new(
-                JsonEmitter::stderr(
-                    None,
-                    codemap.clone(),
-                    pretty,
-                    sessopts.debugging_opts.suggestion_applicability,
-                ).ui_testing(sessopts.debugging_opts.ui_testing)
-            ),
-            ErrorOutputType::Short(color_config) => Box::new(
-                EmitterWriter::stderr(color_config, Some(codemap.clone()), true, false)
-            ),
-        };
-
-        let diagnostic_handler = errors::Handler::with_emitter_and_flags(
-            emitter,
-            errors::HandlerFlags {
-                can_emit_warnings: true,
-                treat_err_as_bug: false,
-                external_macro_backtrace: false,
-                ..Default::default()
-            },
-        );
+        let diagnostic_handler = new_handler(error_format, Some(codemap.clone()));
 
         let mut sess = session::build_session_(
             sessopts, cpath, diagnostic_handler, codemap,
index 6c328a87208aa73c211502192d1ad7273899bb6d..10b6c9850ae7773dc99d20972ce918c9f42570cc 100644 (file)
@@ -11,6 +11,7 @@
 use std::fs;
 use std::path::Path;
 use std::str;
+use errors;
 use html::markdown::Markdown;
 
 #[derive(Clone)]
@@ -28,23 +29,23 @@ pub struct ExternalHtml {
 
 impl ExternalHtml {
     pub fn load(in_header: &[String], before_content: &[String], after_content: &[String],
-                md_before_content: &[String], md_after_content: &[String])
+                md_before_content: &[String], md_after_content: &[String], diag: &errors::Handler)
             -> Option<ExternalHtml> {
-        load_external_files(in_header)
+        load_external_files(in_header, diag)
             .and_then(|ih|
-                load_external_files(before_content)
+                load_external_files(before_content, diag)
                     .map(|bc| (ih, bc))
             )
             .and_then(|(ih, bc)|
-                load_external_files(md_before_content)
+                load_external_files(md_before_content, diag)
                     .map(|m_bc| (ih, format!("{}{}", bc, Markdown(&m_bc, &[]))))
             )
             .and_then(|(ih, bc)|
-                load_external_files(after_content)
+                load_external_files(after_content, diag)
                     .map(|ac| (ih, bc, ac))
             )
             .and_then(|(ih, bc, ac)|
-                load_external_files(md_after_content)
+                load_external_files(md_after_content, diag)
                     .map(|m_ac| (ih, bc, format!("{}{}", ac, Markdown(&m_ac, &[]))))
             )
             .map(|(ih, bc, ac)|
@@ -62,28 +63,30 @@ pub enum LoadStringError {
     BadUtf8,
 }
 
-pub fn load_string<P: AsRef<Path>>(file_path: P) -> Result<String, LoadStringError> {
+pub fn load_string<P: AsRef<Path>>(file_path: P, diag: &errors::Handler)
+    -> Result<String, LoadStringError>
+{
     let file_path = file_path.as_ref();
     let contents = match fs::read(file_path) {
         Ok(bytes) => bytes,
         Err(e) => {
-            eprintln!("error reading `{}`: {}", file_path.display(), e);
+            diag.struct_err(&format!("error reading `{}`: {}", file_path.display(), e)).emit();
             return Err(LoadStringError::ReadFail);
         }
     };
     match str::from_utf8(&contents) {
         Ok(s) => Ok(s.to_string()),
         Err(_) => {
-            eprintln!("error reading `{}`: not UTF-8", file_path.display());
+            diag.struct_err(&format!("error reading `{}`: not UTF-8", file_path.display())).emit();
             Err(LoadStringError::BadUtf8)
         }
     }
 }
 
-fn load_external_files(names: &[String]) -> Option<String> {
+fn load_external_files(names: &[String], diag: &errors::Handler) -> Option<String> {
     let mut out = String::new();
     for name in names {
-        let s = match load_string(name) {
+        let s = match load_string(name, diag) {
             Ok(s) => s,
             Err(_) => return None,
         };
index d4244bfdba075fd2e84666489ab15b37a9a85f2a..7d98feaf539474856e043cd263a356020abffa6d 100644 (file)
 use std::collections::{BTreeMap, BTreeSet};
 use std::default::Default;
 use std::env;
-use std::fmt::Display;
-use std::io;
-use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::process;
 use std::sync::mpsc::channel;
 
 use syntax::edition::Edition;
 use externalfiles::ExternalHtml;
+use rustc::session::{early_warn, early_error};
 use rustc::session::search_paths::SearchPaths;
 use rustc::session::config::{ErrorOutputType, RustcOptGroup, Externs, CodegenOptions};
 use rustc::session::config::{nightly_options, build_codegen_options};
@@ -119,7 +117,8 @@ pub fn main() {
 fn get_args() -> Option<Vec<String>> {
     env::args_os().enumerate()
         .map(|(i, arg)| arg.into_string().map_err(|arg| {
-             print_error(format!("Argument {} is not valid Unicode: {:?}", i, arg));
+             early_warn(ErrorOutputType::default(),
+                        &format!("Argument {} is not valid Unicode: {:?}", i, arg));
         }).ok())
         .collect()
 }
@@ -324,16 +323,12 @@ pub fn main_args(args: &[String]) -> isize {
     let matches = match options.parse(&args[1..]) {
         Ok(m) => m,
         Err(err) => {
-            print_error(err);
-            return 1;
+            early_error(ErrorOutputType::default(), &err.to_string());
         }
     };
     // Check for unstable options.
     nightly_options::check_nightly_options(&matches, &opts());
 
-    // check for deprecated options
-    check_deprecated_options(&matches);
-
     if matches.opt_present("h") || matches.opt_present("help") {
         usage("rustdoc");
         return 0;
@@ -354,6 +349,35 @@ pub fn main_args(args: &[String]) -> isize {
         return 0;
     }
 
+    let color = match matches.opt_str("color").as_ref().map(|s| &s[..]) {
+        Some("auto") => ColorConfig::Auto,
+        Some("always") => ColorConfig::Always,
+        Some("never") => ColorConfig::Never,
+        None => ColorConfig::Auto,
+        Some(arg) => {
+            early_error(ErrorOutputType::default(),
+                        &format!("argument for --color must be `auto`, `always` or `never` \
+                                  (instead was `{}`)", arg));
+        }
+    };
+    let error_format = match matches.opt_str("error-format").as_ref().map(|s| &s[..]) {
+        Some("human") => ErrorOutputType::HumanReadable(color),
+        Some("json") => ErrorOutputType::Json(false),
+        Some("pretty-json") => ErrorOutputType::Json(true),
+        Some("short") => ErrorOutputType::Short(color),
+        None => ErrorOutputType::HumanReadable(color),
+        Some(arg) => {
+            early_error(ErrorOutputType::default(),
+                        &format!("argument for --error-format must be `human`, `json` or \
+                                  `short` (instead was `{}`)", arg));
+        }
+    };
+
+    let diag = core::new_handler(error_format, None);
+
+    // check for deprecated options
+    check_deprecated_options(&matches, &diag);
+
     let to_check = matches.opt_strs("theme-checker");
     if !to_check.is_empty() {
         let paths = theme::load_css_paths(include_bytes!("html/static/themes/light.css"));
@@ -362,7 +386,7 @@ pub fn main_args(args: &[String]) -> isize {
         println!("rustdoc: [theme-checker] Starting tests!");
         for theme_file in to_check.iter() {
             print!(" - Checking \"{}\"...", theme_file);
-            let (success, differences) = theme::test_theme_against(theme_file, &paths);
+            let (success, differences) = theme::test_theme_against(theme_file, &paths, &diag);
             if !differences.is_empty() || !success {
                 println!(" FAILED");
                 errors += 1;
@@ -380,39 +404,15 @@ pub fn main_args(args: &[String]) -> isize {
     }
 
     if matches.free.is_empty() {
-        print_error("missing file operand");
+        diag.struct_err("missing file operand").emit();
         return 1;
     }
     if matches.free.len() > 1 {
-        print_error("too many file operands");
+        diag.struct_err("too many file operands").emit();
         return 1;
     }
     let input = &matches.free[0];
 
-    let color = match matches.opt_str("color").as_ref().map(|s| &s[..]) {
-        Some("auto") => ColorConfig::Auto,
-        Some("always") => ColorConfig::Always,
-        Some("never") => ColorConfig::Never,
-        None => ColorConfig::Auto,
-        Some(arg) => {
-            print_error(&format!("argument for --color must be `auto`, `always` or `never` \
-                                  (instead was `{}`)", arg));
-            return 1;
-        }
-    };
-    let error_format = match matches.opt_str("error-format").as_ref().map(|s| &s[..]) {
-        Some("human") => ErrorOutputType::HumanReadable(color),
-        Some("json") => ErrorOutputType::Json(false),
-        Some("pretty-json") => ErrorOutputType::Json(true),
-        Some("short") => ErrorOutputType::Short(color),
-        None => ErrorOutputType::HumanReadable(color),
-        Some(arg) => {
-            print_error(&format!("argument for --error-format must be `human`, `json` or \
-                                  `short` (instead was `{}`)", arg));
-            return 1;
-        }
-    };
-
     let mut libs = SearchPaths::new();
     for s in &matches.opt_strs("L") {
         libs.add_path(s, error_format);
@@ -420,7 +420,7 @@ pub fn main_args(args: &[String]) -> isize {
     let externs = match parse_externs(&matches) {
         Ok(ex) => ex,
         Err(err) => {
-            print_error(err);
+            diag.struct_err(&err.to_string()).emit();
             return 1;
         }
     };
@@ -441,10 +441,7 @@ pub fn main_args(args: &[String]) -> isize {
 
     if let Some(ref p) = css_file_extension {
         if !p.is_file() {
-            writeln!(
-                &mut io::stderr(),
-                "rustdoc: option --extend-css argument must be a file."
-            ).unwrap();
+            diag.struct_err("option --extend-css argument must be a file").emit();
             return 1;
         }
     }
@@ -457,13 +454,14 @@ pub fn main_args(args: &[String]) -> isize {
                                             .iter()
                                             .map(|s| (PathBuf::from(&s), s.to_owned())) {
             if !theme_file.is_file() {
-                println!("rustdoc: option --themes arguments must all be files");
+                diag.struct_err("option --themes arguments must all be files").emit();
                 return 1;
             }
-            let (success, ret) = theme::test_theme_against(&theme_file, &paths);
+            let (success, ret) = theme::test_theme_against(&theme_file, &paths, &diag);
             if !success || !ret.is_empty() {
-                println!("rustdoc: invalid theme: \"{}\"", theme_s);
-                println!("         Check what's wrong with the \"theme-checker\" option");
+                diag.struct_err(&format!("invalid theme: \"{}\"", theme_s))
+                    .help("check what's wrong with the --theme-checker option")
+                    .emit();
                 return 1;
             }
             themes.push(theme_file);
@@ -475,7 +473,7 @@ pub fn main_args(args: &[String]) -> isize {
             &matches.opt_strs("html-before-content"),
             &matches.opt_strs("html-after-content"),
             &matches.opt_strs("markdown-before-content"),
-            &matches.opt_strs("markdown-after-content")) {
+            &matches.opt_strs("markdown-after-content"), &diag) {
         Some(eh) => eh,
         None => return 3,
     };
@@ -492,7 +490,7 @@ pub fn main_args(args: &[String]) -> isize {
     let edition = match edition.parse() {
         Ok(e) => e,
         Err(_) => {
-            print_error("could not parse edition");
+            diag.struct_err("could not parse edition").emit();
             return 1;
         }
     };
@@ -502,7 +500,7 @@ pub fn main_args(args: &[String]) -> isize {
     match (should_test, markdown_input) {
         (true, true) => {
             return markdown::test(input, cfgs, libs, externs, test_args, maybe_sysroot,
-                                  display_warnings, linker, edition, cg)
+                                  display_warnings, linker, edition, cg, &diag)
         }
         (true, false) => {
             return test::run(Path::new(input), cfgs, libs, externs, test_args, crate_name,
@@ -511,7 +509,7 @@ pub fn main_args(args: &[String]) -> isize {
         (false, true) => return markdown::render(Path::new(input),
                                                  output.unwrap_or(PathBuf::from("doc")),
                                                  &matches, &external_html,
-                                                 !matches.opt_present("markdown-no-toc")),
+                                                 !matches.opt_present("markdown-no-toc"), &diag),
         (false, false) => {}
     }
 
@@ -520,6 +518,7 @@ pub fn main_args(args: &[String]) -> isize {
     let res = acquire_input(PathBuf::from(input), externs, edition, cg, &matches, error_format,
                             move |out| {
         let Output { krate, passes, renderinfo } = out;
+        let diag = core::new_handler(error_format, None);
         info!("going to format");
         match output_format.as_ref().map(|s| &**s) {
             Some("html") | None => {
@@ -536,26 +535,17 @@ pub fn main_args(args: &[String]) -> isize {
                 0
             }
             Some(s) => {
-                print_error(format!("unknown output format: {}", s));
+                diag.struct_err(&format!("unknown output format: {}", s)).emit();
                 1
             }
         }
     });
     res.unwrap_or_else(|s| {
-        print_error(format!("input error: {}", s));
+        diag.struct_err(&format!("input error: {}", s)).emit();
         1
     })
 }
 
-/// Prints an uniformized error message on the standard error output
-fn print_error<T>(error_message: T) where T: Display {
-    writeln!(
-        &mut io::stderr(),
-        "rustdoc: {}\nTry 'rustdoc --help' for more information.",
-        error_message
-    ).unwrap();
-}
-
 /// Looks inside the command line arguments to extract the relevant input format
 /// and files and then generates the necessary rustdoc output for formatting.
 fn acquire_input<R, F>(input: PathBuf,
@@ -722,7 +712,7 @@ fn rust_input<R, F>(cratefile: PathBuf,
 }
 
 /// Prints deprecation warnings for deprecated options
-fn check_deprecated_options(matches: &getopts::Matches) {
+fn check_deprecated_options(matches: &getopts::Matches, diag: &errors::Handler) {
     let deprecated_flags = [
        "input-format",
        "output-format",
@@ -734,12 +724,15 @@ fn check_deprecated_options(matches: &getopts::Matches) {
 
     for flag in deprecated_flags.into_iter() {
         if matches.opt_present(flag) {
-            eprintln!("WARNING: the '{}' flag is considered deprecated", flag);
-            eprintln!("WARNING: please see https://github.com/rust-lang/rust/issues/44136");
-        }
-    }
+            let mut err = diag.struct_warn(&format!("the '{}' flag is considered deprecated",
+                                                    flag));
+            err.warn("please see https://github.com/rust-lang/rust/issues/44136");
 
-    if matches.opt_present("no-defaults") {
-        eprintln!("WARNING: (you may want to use --document-private-items)");
+            if *flag == "no-defaults" {
+                err.help("you may want to use --document-private-items");
+            }
+
+            err.emit();
+        }
     }
 }
index 8ada5ce1a4df9e83f74b068c903f2287038c083b..bf7b025884d5ab858628b5b18b320e7d98935f0e 100644 (file)
@@ -13,6 +13,7 @@
 use std::io::prelude::*;
 use std::path::{PathBuf, Path};
 
+use errors;
 use getopts;
 use testing;
 use rustc::session::search_paths::SearchPaths;
@@ -50,7 +51,7 @@ fn extract_leading_metadata<'a>(s: &'a str) -> (Vec<&'a str>, &'a str) {
 /// Render `input` (e.g. "foo.md") into an HTML file in `output`
 /// (e.g. output = "bar" => "bar/foo.html").
 pub fn render(input: &Path, mut output: PathBuf, matches: &getopts::Matches,
-              external_html: &ExternalHtml, include_toc: bool) -> isize {
+              external_html: &ExternalHtml, include_toc: bool, diag: &errors::Handler) -> isize {
     output.push(input.file_stem().unwrap());
     output.set_extension("html");
 
@@ -60,7 +61,7 @@ pub fn render(input: &Path, mut output: PathBuf, matches: &getopts::Matches,
         css.push_str(&s)
     }
 
-    let input_str = match load_string(input) {
+    let input_str = match load_string(input, diag) {
         Ok(s) => s,
         Err(LoadStringError::ReadFail) => return 1,
         Err(LoadStringError::BadUtf8) => return 2,
@@ -72,7 +73,7 @@ pub fn render(input: &Path, mut output: PathBuf, matches: &getopts::Matches,
 
     let mut out = match File::create(&output) {
         Err(e) => {
-            eprintln!("rustdoc: {}: {}", output.display(), e);
+            diag.struct_err(&format!("{}: {}", output.display(), e)).emit();
             return 4;
         }
         Ok(f) => f
@@ -80,7 +81,7 @@ pub fn render(input: &Path, mut output: PathBuf, matches: &getopts::Matches,
 
     let (metadata, text) = extract_leading_metadata(&input_str);
     if metadata.is_empty() {
-        eprintln!("rustdoc: invalid markdown file: no initial lines starting with `# ` or `%`");
+        diag.struct_err("invalid markdown file: no initial lines starting with `# ` or `%`").emit();
         return 5;
     }
     let title = metadata[0];
@@ -130,7 +131,7 @@ pub fn render(input: &Path, mut output: PathBuf, matches: &getopts::Matches,
 
     match err {
         Err(e) => {
-            eprintln!("rustdoc: cannot write to `{}`: {}", output.display(), e);
+            diag.struct_err(&format!("cannot write to `{}`: {}", output.display(), e)).emit();
             6
         }
         Ok(_) => 0,
@@ -141,8 +142,8 @@ pub fn render(input: &Path, mut output: PathBuf, matches: &getopts::Matches,
 pub fn test(input: &str, cfgs: Vec<String>, libs: SearchPaths, externs: Externs,
             mut test_args: Vec<String>, maybe_sysroot: Option<PathBuf>,
             display_warnings: bool, linker: Option<PathBuf>, edition: Edition,
-            cg: CodegenOptions) -> isize {
-    let input_str = match load_string(input) {
+            cg: CodegenOptions, diag: &errors::Handler) -> isize {
+    let input_str = match load_string(input, diag) {
         Ok(s) => s,
         Err(LoadStringError::ReadFail) => return 1,
         Err(LoadStringError::BadUtf8) => return 2,
index 1e4f64f5c52c9edd51e538a637866d85a0f5879a..96a67e078875800bbc06e84183ee88a242a4b7d5 100644 (file)
 use std::io::Read;
 use std::path::Path;
 
+use errors::Handler;
+
 macro_rules! try_something {
-    ($e:expr, $out:expr) => ({
+    ($e:expr, $diag:expr, $out:expr) => ({
         match $e {
             Ok(c) => c,
             Err(e) => {
-                eprintln!("rustdoc: got an error: {}", e);
+                $diag.struct_err(&e.to_string()).emit();
                 return $out;
             }
         }
@@ -273,11 +275,13 @@ pub fn get_differences(against: &CssPath, other: &CssPath, v: &mut Vec<String>)
     }
 }
 
-pub fn test_theme_against<P: AsRef<Path>>(f: &P, against: &CssPath) -> (bool, Vec<String>) {
-    let mut file = try_something!(File::open(f), (false, Vec::new()));
+pub fn test_theme_against<P: AsRef<Path>>(f: &P, against: &CssPath, diag: &Handler)
+    -> (bool, Vec<String>)
+{
+    let mut file = try_something!(File::open(f), diag, (false, Vec::new()));
     let mut data = Vec::with_capacity(1000);
 
-    try_something!(file.read_to_end(&mut data), (false, Vec::new()));
+    try_something!(file.read_to_end(&mut data), diag, (false, Vec::new()));
     let paths = load_css_paths(&data);
     let mut ret = Vec::new();
     get_differences(against, &paths, &mut ret);