From 3f727aeeb7eb5a3d9a30131bdc462905900b6996 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 31 May 2019 22:01:27 +0200 Subject: [PATCH 1/5] Add new diagnostic writer using annotate-snippet library This adds a new diagnostic writer `AnnotateRsEmitterWriter` that uses the [`annotate-snippet`][as] library to print out the human readable diagnostics. The goal is to eventually switch over to using the library instead of maintaining our own diagnostics output. This commit does *not* add all the required features to the new diagnostics writer. It is only meant as a starting point so that other people can contribute as well. [as]: https://github.com/rust-lang/annotate-snippets-rs --- Cargo.lock | 1 + src/librustc_errors/Cargo.toml | 1 + src/librustc_errors/annotate_rs_emitter.rs | 210 +++++++++++++++++++++ src/librustc_errors/emitter.rs | 6 +- src/librustc_errors/lib.rs | 1 + src/tools/tidy/src/deps.rs | 2 + 6 files changed, 219 insertions(+), 2 deletions(-) create mode 100644 src/librustc_errors/annotate_rs_emitter.rs diff --git a/Cargo.lock b/Cargo.lock index a8bba30cf3339..5293391a07155 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2789,6 +2789,7 @@ dependencies = [ name = "rustc_errors" version = "0.0.0" dependencies = [ + "annotate-snippets 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_cratesio_shim 0.0.0", diff --git a/src/librustc_errors/Cargo.toml b/src/librustc_errors/Cargo.toml index 02c011857bd2a..3689a463a5c84 100644 --- a/src/librustc_errors/Cargo.toml +++ b/src/librustc_errors/Cargo.toml @@ -18,3 +18,4 @@ rustc_cratesio_shim = { path = "../librustc_cratesio_shim" } unicode-width = "0.1.4" atty = "0.2" termcolor = "1.0" +annotate-snippets = "0.5.0" diff --git a/src/librustc_errors/annotate_rs_emitter.rs b/src/librustc_errors/annotate_rs_emitter.rs new file mode 100644 index 0000000000000..172b8b5551fef --- /dev/null +++ b/src/librustc_errors/annotate_rs_emitter.rs @@ -0,0 +1,210 @@ +/// Emit diagnostics using the `annotate-snippets` library +/// +/// This is the equivalent of `./emitter.rs` but making use of the +/// [`annotate-snippets`][annotate_snippets] library instead of building the output ourselves. +/// +/// [annotate_snippets]: https://docs.rs/crate/annotate-snippets/ + +use syntax_pos::{SourceFile, MultiSpan, Loc}; +use crate::{ + Level, CodeSuggestion, DiagnosticBuilder, Emitter, + SourceMapperDyn, SubDiagnostic, DiagnosticId +}; +use crate::emitter::FileWithAnnotatedLines; +use rustc_data_structures::sync::Lrc; +use crate::snippet::Line; +use annotate_snippets::snippet::*; +use annotate_snippets::display_list::DisplayList; +use annotate_snippets::formatter::DisplayListFormatter; + + +/// Generates diagnostics using annotate-rs +pub struct AnnotateRsEmitterWriter { + source_map: Option>, + /// If true, hides the longer explanation text + short_message: bool, + /// If true, will normalize line numbers with LL to prevent noise in UI test diffs. + ui_testing: bool, +} + +impl Emitter for AnnotateRsEmitterWriter { + /// The entry point for the diagnostics generation + fn emit_diagnostic(&mut self, db: &DiagnosticBuilder<'_>) { + let primary_span = db.span.clone(); + let children = db.children.clone(); + // FIXME(#59346): Collect suggestions (see emitter.rs) + let suggestions: &[_] = &[]; + + // FIXME(#59346): Add `fix_multispans_in_std_macros` function from emitter.rs + + self.emit_messages_default(&db.level, + db.message(), + &db.code, + &primary_span, + &children, + &suggestions); + } + + fn should_show_explain(&self) -> bool { + !self.short_message + } +} + +/// Collects all the data needed to generate the data structures needed for the +/// `annotate-snippets` library. +struct DiagnosticConverter<'a> { + source_map: Option>, + level: Level, + message: String, + code: Option, + msp: MultiSpan, + #[allow(dead_code)] + children: &'a [SubDiagnostic], + #[allow(dead_code)] + suggestions: &'a [CodeSuggestion] +} + +impl<'a> DiagnosticConverter<'a> { + /// Turns rustc Diagnostic information into a `annotate_snippets::snippet::Snippet`. + fn to_annotation_snippet(&self) -> Option { + if let Some(source_map) = &self.source_map { + // Make sure our primary file comes first + let primary_lo = if let Some(ref primary_span) = + self.msp.primary_span().as_ref() { + source_map.lookup_char_pos(primary_span.lo()) + } else { + // FIXME(#59346): Not sure when this is the case and what + // should be done if it happens + return None + }; + let annotated_files = FileWithAnnotatedLines::collect_annotations( + &self.msp, + &self.source_map + ); + let slices = self.slices_for_files(annotated_files, primary_lo); + + Some(Snippet { + title: Some(Annotation { + label: Some(self.message.to_string()), + id: self.code.clone().map(|c| { + match c { + DiagnosticId::Error(val) | DiagnosticId::Lint(val) => val + } + }), + annotation_type: Self::annotation_type_for_level(self.level), + }), + footer: vec![], + slices: slices, + }) + } else { + // FIXME(#59346): Is it ok to return None if there's no source_map? + None + } + } + + fn slices_for_files( + &self, + annotated_files: Vec, + primary_lo: Loc + ) -> Vec { + // FIXME(#59346): Provide a test case where `annotated_files` is > 1 + annotated_files.iter().flat_map(|annotated_file| { + annotated_file.lines.iter().map(|line| { + let line_source = Self::source_string(annotated_file.file.clone(), &line); + Slice { + source: line_source, + line_start: line.line_index, + origin: Some(primary_lo.file.name.to_string()), + // FIXME(#59346): Not really sure when `fold` should be true or false + fold: false, + annotations: line.annotations.iter().map(|a| { + self.annotation_to_source_annotation(a.clone()) + }).collect(), + } + }).collect::>() + }).collect::>() + } + + /// Turns a `crate::snippet::Annotation` into a `SourceAnnotation` + fn annotation_to_source_annotation( + &self, + annotation: crate::snippet::Annotation + ) -> SourceAnnotation { + SourceAnnotation { + range: (annotation.start_col, annotation.end_col), + label: annotation.label.unwrap_or("".to_string()), + annotation_type: Self::annotation_type_for_level(self.level) + } + } + + /// Provides the source string for the given `line` of `file` + fn source_string(file: Lrc, + line: &Line) -> String { + let source_string = match file.get_line(line.line_index - 1) { + Some(s) => s.clone(), + None => return String::new(), + }; + source_string.to_string() + } + + /// Maps `Diagnostic::Level` to `snippet::AnnotationType` + fn annotation_type_for_level(level: Level) -> AnnotationType { + match level { + Level::Bug | Level::Fatal | Level::PhaseFatal | Level::Error => AnnotationType::Error, + Level::Warning => AnnotationType::Warning, + Level::Note => AnnotationType::Note, + Level::Help => AnnotationType::Help, + // FIXME(#59346): Not sure how to map these two levels + Level::Cancelled | Level::FailureNote => AnnotationType::Error + } + } +} + +impl AnnotateRsEmitterWriter { + pub fn new( + source_map: Option>, + short_message: bool + ) -> Self { + Self { + source_map, + short_message, + ui_testing: false, + } + } + + /// Allows to modify `Self` to enable or disable the `ui_testing` flag. + /// + /// If this is set to true, line numbers will be normalized as `LL` in the output. + // FIXME(#59346): This method is used via the public interface, but setting the `ui_testing` + // flag currently does not anonymize line numbers. We would have to add the `maybe_anonymized` + // method from `emitter.rs` and implement rust-lang/annotate-snippets-rs#2 in order to + // anonymize line numbers. + pub fn ui_testing(mut self, ui_testing: bool) -> Self { + self.ui_testing = ui_testing; + self + } + + fn emit_messages_default(&mut self, + level: &Level, + message: String, + code: &Option, + msp: &MultiSpan, + children: &[SubDiagnostic], + suggestions: &[CodeSuggestion] + ) { + let converter = DiagnosticConverter { + source_map: self.source_map.clone(), + level: level.clone(), + message: message.clone(), + code: code.clone(), + msp: msp.clone(), + children, + suggestions + }; + if let Some(snippet) = converter.to_annotation_snippet() { + let dl = DisplayList::from(snippet); + let dlf = DisplayListFormatter::new(true); + print!("{}", dlf.format(&dl)); + }; + } +} diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index e1112a1557771..fcc0358ea7c11 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -24,6 +24,7 @@ use termcolor::{WriteColor, Color, Buffer}; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum HumanReadableErrorType { Default(ColorConfig), + AnnotateRs(ColorConfig), Short(ColorConfig), } @@ -33,6 +34,7 @@ impl HumanReadableErrorType { match self { HumanReadableErrorType::Default(cc) => (false, cc), HumanReadableErrorType::Short(cc) => (true, cc), + HumanReadableErrorType::AnnotateRs(cc) => (false, cc), } } pub fn new_emitter( @@ -173,8 +175,8 @@ pub struct EmitterWriter { #[derive(Debug)] pub struct FileWithAnnotatedLines { - file: Lrc, - lines: Vec, + pub file: Lrc, + pub lines: Vec, multiline_depth: usize, } diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index 2dcf7be2aa8b1..7bc7d0ddaf243 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -33,6 +33,7 @@ use termcolor::{ColorSpec, Color}; mod diagnostic; mod diagnostic_builder; pub mod emitter; +pub mod annotate_rs_emitter; mod snippet; pub mod registry; mod styled_buffer; diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 7922cb14eeca8..11f37b2718869 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -62,6 +62,8 @@ const WHITELIST_CRATES: &[CrateVersion<'_>] = &[ const WHITELIST: &[Crate<'_>] = &[ Crate("adler32"), Crate("aho-corasick"), + Crate("annotate-snippets"), + Crate("ansi_term"), Crate("arrayvec"), Crate("atty"), Crate("autocfg"), From c04a2ccb3525ee55ab2484f0390eff2a047eecce Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 31 May 2019 21:15:59 +0200 Subject: [PATCH 2/5] Add new error-format value to use annotate-snippet output --- src/librustc/session/config.rs | 10 +++++ src/librustc/session/mod.rs | 39 +++++++++++++------- src/test/ui/annotate-snippet/missing-type.rs | 5 +++ 3 files changed, 40 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/annotate-snippet/missing-type.rs diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index d8efa17defe3d..6a35906d20c2f 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -2002,6 +2002,9 @@ pub fn build_session_options_and_crate_config( match matches.opt_str("error-format").as_ref().map(|s| &s[..]) { None | Some("human") => ErrorOutputType::HumanReadable(HumanReadableErrorType::Default(color)), + Some("human-annotate-rs") => { + ErrorOutputType::HumanReadable(HumanReadableErrorType::AnnotateRs(color)) + }, Some("json") => ErrorOutputType::Json { pretty: false, json_rendered }, Some("pretty-json") => ErrorOutputType::Json { pretty: true, json_rendered }, Some("short") => ErrorOutputType::HumanReadable(HumanReadableErrorType::Short(color)), @@ -2038,6 +2041,13 @@ pub fn build_session_options_and_crate_config( "--error-format=pretty-json is unstable", ); } + if let ErrorOutputType::HumanReadable(HumanReadableErrorType::AnnotateRs(_)) = + error_format { + early_error( + ErrorOutputType::Json { pretty: false, json_rendered }, + "--error-format=human-annotate-rs is unstable", + ); + } } if debugging_opts.pgo_gen.enabled() && debugging_opts.pgo_use.is_some() { diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 40af5b45f9c35..b3a9d764b1db0 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -23,6 +23,8 @@ use rustc_data_structures::sync::{ use errors::{DiagnosticBuilder, DiagnosticId, Applicability}; use errors::emitter::{Emitter, EmitterWriter}; +use errors::emitter::HumanReadableErrorType; +use errors::annotate_rs_emitter::{AnnotateRsEmitterWriter}; use syntax::ast::{self, NodeId}; use syntax::edition::Edition; use syntax::feature_gate::{self, AttributeType}; @@ -1031,22 +1033,31 @@ fn default_emitter( match (sopts.error_format, emitter_dest) { (config::ErrorOutputType::HumanReadable(kind), dst) => { let (short, color_config) = kind.unzip(); - let emitter = match dst { - None => EmitterWriter::stderr( - color_config, - Some(source_map.clone()), - short, - sopts.debugging_opts.teach, - ), - Some(dst) => EmitterWriter::new( - dst, + + if let HumanReadableErrorType::AnnotateRs(_) = kind { + let emitter = AnnotateRsEmitterWriter::new( Some(source_map.clone()), short, - false, // no teach messages when writing to a buffer - false, // no colors when writing to a buffer - ), - }; - Box::new(emitter.ui_testing(sopts.debugging_opts.ui_testing)) + ); + Box::new(emitter.ui_testing(sopts.debugging_opts.ui_testing)) + } else { + let emitter = match dst { + None => EmitterWriter::stderr( + color_config, + Some(source_map.clone()), + short, + sopts.debugging_opts.teach, + ), + Some(dst) => EmitterWriter::new( + dst, + Some(source_map.clone()), + short, + false, // no teach messages when writing to a buffer + false, // no colors when writing to a buffer + ), + }; + Box::new(emitter.ui_testing(sopts.debugging_opts.ui_testing)) + } }, (config::ErrorOutputType::Json { pretty, json_rendered }, None) => Box::new( JsonEmitter::stderr( diff --git a/src/test/ui/annotate-snippet/missing-type.rs b/src/test/ui/annotate-snippet/missing-type.rs new file mode 100644 index 0000000000000..e52a81ec161ba --- /dev/null +++ b/src/test/ui/annotate-snippet/missing-type.rs @@ -0,0 +1,5 @@ +// compile-flags: --error-format human-annotate-rs + +pub fn main() { + let x: Iter; +} From 94c8aa67917fe053c9d03d5be22ded221c0a5241 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Sat, 1 Jun 2019 08:29:12 +0200 Subject: [PATCH 3/5] Print to stderr and bless --- src/librustc_errors/annotate_rs_emitter.rs | 5 ++++- src/test/ui/annotate-snippet/missing-type.rs | 2 +- src/test/ui/annotate-snippet/missing-type.stderr | 6 ++++++ 3 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/annotate-snippet/missing-type.stderr diff --git a/src/librustc_errors/annotate_rs_emitter.rs b/src/librustc_errors/annotate_rs_emitter.rs index 172b8b5551fef..a5699035deac8 100644 --- a/src/librustc_errors/annotate_rs_emitter.rs +++ b/src/librustc_errors/annotate_rs_emitter.rs @@ -204,7 +204,10 @@ impl AnnotateRsEmitterWriter { if let Some(snippet) = converter.to_annotation_snippet() { let dl = DisplayList::from(snippet); let dlf = DisplayListFormatter::new(true); - print!("{}", dlf.format(&dl)); + // FIXME(#59346): Figure out if we can _always_ print to stderr or not. + // `emitter.rs` has the `Destination` enum that lists various possible output + // destinations. + eprint!("{}", dlf.format(&dl)); }; } } diff --git a/src/test/ui/annotate-snippet/missing-type.rs b/src/test/ui/annotate-snippet/missing-type.rs index e52a81ec161ba..d0814a9537d0c 100644 --- a/src/test/ui/annotate-snippet/missing-type.rs +++ b/src/test/ui/annotate-snippet/missing-type.rs @@ -1,5 +1,5 @@ // compile-flags: --error-format human-annotate-rs pub fn main() { - let x: Iter; + let x: Iter; //~ ERROR cannot find type `Iter` in this scope } diff --git a/src/test/ui/annotate-snippet/missing-type.stderr b/src/test/ui/annotate-snippet/missing-type.stderr new file mode 100644 index 0000000000000..1e84929d3c5be --- /dev/null +++ b/src/test/ui/annotate-snippet/missing-type.stderr @@ -0,0 +1,6 @@ +error[E0412]: cannot find type `Iter` in this scope + --> $DIR/missing-type.rs:4:11 + | +4 | let x: Iter; + | ^^^^ not found in this scope + | \ No newline at end of file From 4a6b91cd66e7293af2ff4120b510fab1818b45cf Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Mon, 3 Jun 2019 07:38:19 +0200 Subject: [PATCH 4/5] Simplify source_string and block-format methods --- src/librustc_errors/annotate_rs_emitter.rs | 27 +++++++++++----------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/librustc_errors/annotate_rs_emitter.rs b/src/librustc_errors/annotate_rs_emitter.rs index a5699035deac8..f3c55ae79f792 100644 --- a/src/librustc_errors/annotate_rs_emitter.rs +++ b/src/librustc_errors/annotate_rs_emitter.rs @@ -138,13 +138,11 @@ impl<'a> DiagnosticConverter<'a> { } /// Provides the source string for the given `line` of `file` - fn source_string(file: Lrc, - line: &Line) -> String { - let source_string = match file.get_line(line.line_index - 1) { - Some(s) => s.clone(), - None => return String::new(), - }; - source_string.to_string() + fn source_string( + file: Lrc, + line: &Line + ) -> String { + file.get_line(line.line_index - 1).map(|a| a.to_string()).unwrap_or(String::new()) } /// Maps `Diagnostic::Level` to `snippet::AnnotationType` @@ -184,13 +182,14 @@ impl AnnotateRsEmitterWriter { self } - fn emit_messages_default(&mut self, - level: &Level, - message: String, - code: &Option, - msp: &MultiSpan, - children: &[SubDiagnostic], - suggestions: &[CodeSuggestion] + fn emit_messages_default( + &mut self, + level: &Level, + message: String, + code: &Option, + msp: &MultiSpan, + children: &[SubDiagnostic], + suggestions: &[CodeSuggestion] ) { let converter = DiagnosticConverter { source_map: self.source_map.clone(), From bfe5d9796b38fdecab6cd7afd28e0a7f23e4915a Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Mon, 3 Jun 2019 07:38:55 +0200 Subject: [PATCH 5/5] eprint -> eprintln to add trailing newline --- src/librustc_errors/annotate_rs_emitter.rs | 2 +- src/test/ui/annotate-snippet/missing-type.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_errors/annotate_rs_emitter.rs b/src/librustc_errors/annotate_rs_emitter.rs index f3c55ae79f792..de42389de74ba 100644 --- a/src/librustc_errors/annotate_rs_emitter.rs +++ b/src/librustc_errors/annotate_rs_emitter.rs @@ -206,7 +206,7 @@ impl AnnotateRsEmitterWriter { // FIXME(#59346): Figure out if we can _always_ print to stderr or not. // `emitter.rs` has the `Destination` enum that lists various possible output // destinations. - eprint!("{}", dlf.format(&dl)); + eprintln!("{}", dlf.format(&dl)); }; } } diff --git a/src/test/ui/annotate-snippet/missing-type.stderr b/src/test/ui/annotate-snippet/missing-type.stderr index 1e84929d3c5be..8857ae7d85034 100644 --- a/src/test/ui/annotate-snippet/missing-type.stderr +++ b/src/test/ui/annotate-snippet/missing-type.stderr @@ -3,4 +3,4 @@ error[E0412]: cannot find type `Iter` in this scope | 4 | let x: Iter; | ^^^^ not found in this scope - | \ No newline at end of file + |