From e72f307f15138a8bb4fd1ae9ef3256fe1fa9dd94 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Sat, 8 Feb 2020 22:47:48 -0600 Subject: [PATCH] fix: backport parse bug fix Backport the fix for the parser bug where the messages from fatal/non-recoverable parser errors were being silently eaten by rustfmt. --- src/formatting.rs | 253 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 216 insertions(+), 37 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index 0c273400695..bed87748253 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -715,37 +715,43 @@ fn parse_crate( struct SilentOnIgnoredFilesEmitter { ignore_path_set: Rc, source_map: Rc, - emitter: EmitterWriter, + emitter: Box, has_non_ignorable_parser_errors: bool, can_reset: Rc>, } +impl SilentOnIgnoredFilesEmitter { + fn handle_non_ignoreable_error(&mut self, db: &Diagnostic) { + self.has_non_ignorable_parser_errors = true; + *self.can_reset.borrow_mut() = false; + self.emitter.emit_diagnostic(db); + } +} + impl Emitter for SilentOnIgnoredFilesEmitter { fn source_map(&self) -> Option<&Lrc> { None } + fn emit_diagnostic(&mut self, db: &Diagnostic) { + if db.level == DiagnosticLevel::Fatal { + return self.handle_non_ignoreable_error(db); + } if let Some(primary_span) = &db.span.primary_span() { let file_name = self.source_map.span_to_filename(*primary_span); - match file_name { - rustc_span::FileName::Real(ref path) => { - if self - .ignore_path_set - .is_match(&FileName::Real(path.to_path_buf())) - { - if !self.has_non_ignorable_parser_errors { - *self.can_reset.borrow_mut() = true; - } - return; + if let rustc_span::FileName::Real(ref path) = file_name { + if self + .ignore_path_set + .is_match(&FileName::Real(path.to_path_buf())) + { + if !self.has_non_ignorable_parser_errors { + *self.can_reset.borrow_mut() = true; } + return; } - _ => (), }; } - - self.has_non_ignorable_parser_errors = true; - *self.can_reset.borrow_mut() = false; - self.emitter.emit_diagnostic(db); + self.handle_non_ignoreable_error(db); } } @@ -759,7 +765,7 @@ impl Emitter for SilentEmitter { fn emit_diagnostic(&mut self, _db: &Diagnostic) {} } -fn silent_emitter() -> Box { +fn silent_emitter() -> Box { Box::new(SilentEmitter {}) } @@ -769,36 +775,38 @@ fn make_parse_sess( ignore_path_set: Rc, can_reset: Rc>, ) -> ParseSess { - let tty_handler = if config.hide_parse_errors() { - let silent_emitter = silent_emitter(); - Handler::with_emitter(true, None, silent_emitter) + let supports_color = term::stderr().map_or(false, |term| term.supports_color()); + let color_cfg = if supports_color { + ColorConfig::Auto } else { - let supports_color = term::stderr().map_or(false, |term| term.supports_color()); - let color_cfg = if supports_color { - ColorConfig::Auto - } else { - ColorConfig::Never - }; + ColorConfig::Never + }; - let emitter_writer = EmitterWriter::stderr( + let emitter = if config.hide_parse_errors() { + silent_emitter() + } else { + Box::new(EmitterWriter::stderr( color_cfg, Some(source_map.clone()), false, false, None, false, - ); - let emitter = Box::new(SilentOnIgnoredFilesEmitter { - has_non_ignorable_parser_errors: false, - ignore_path_set: ignore_path_set, - source_map: Rc::clone(&source_map), - emitter: emitter_writer, - can_reset, - }); - Handler::with_emitter(true, None, emitter) + )) }; + let handler = Handler::with_emitter( + true, + None, + Box::new(SilentOnIgnoredFilesEmitter { + has_non_ignorable_parser_errors: false, + source_map: source_map.clone(), + emitter, + ignore_path_set, + can_reset, + }), + ); - ParseSess::with_span_handler(tty_handler, source_map) + ParseSess::with_span_handler(handler, source_map) } fn should_emit_verbose(is_stdin: bool, config: &Config, f: F) @@ -809,3 +817,174 @@ where f(); } } + +#[cfg(test)] +mod tests { + use super::*; + + mod emitter { + use super::*; + use crate::config::IgnoreList; + use crate::is_nightly_channel; + use crate::utils::mk_sp; + use rustc_span::{BytePos, FileName as SourceMapFileName, MultiSpan, DUMMY_SP}; + use std::path::{Path, PathBuf}; + + struct TestEmitter { + num_emitted_errors: Rc>, + } + + impl Emitter for TestEmitter { + fn source_map(&self) -> Option<&Lrc> { + None + } + fn emit_diagnostic(&mut self, _db: &Diagnostic) { + *self.num_emitted_errors.borrow_mut() += 1; + } + } + + fn build_diagnostic(level: DiagnosticLevel, span: Option) -> Diagnostic { + Diagnostic { + level, + code: None, + message: vec![], + children: vec![], + suggestions: vec![], + span: span.unwrap_or_else(MultiSpan::new), + sort_span: DUMMY_SP, + } + } + + fn build_emitter( + num_emitted_errors: Rc>, + can_reset: Rc>, + source_map: Option>, + ignore_list: Option, + ) -> SilentOnIgnoredFilesEmitter { + let emitter_writer = TestEmitter { num_emitted_errors }; + let source_map = + source_map.unwrap_or_else(|| Rc::new(SourceMap::new(FilePathMapping::empty()))); + let ignore_path_set = + Rc::new(IgnorePathSet::from_ignore_list(&ignore_list.unwrap_or_default()).unwrap()); + SilentOnIgnoredFilesEmitter { + has_non_ignorable_parser_errors: false, + source_map, + emitter: Box::new(emitter_writer), + ignore_path_set, + can_reset, + } + } + + fn get_ignore_list(config: &str) -> IgnoreList { + Config::from_toml(config, Path::new("")).unwrap().ignore() + } + + #[test] + fn handles_fatal_parse_error_in_ignored_file() { + let num_emitted_errors = Rc::new(RefCell::new(0)); + let can_reset_errors = Rc::new(RefCell::new(false)); + let ignore_list = get_ignore_list(r#"ignore = ["foo.rs"]"#); + let source_map = Rc::new(SourceMap::new(FilePathMapping::empty())); + let source = + String::from(r#"extern "system" fn jni_symbol!( funcName ) ( ... ) -> {} "#); + source_map.new_source_file(SourceMapFileName::Real(PathBuf::from("foo.rs")), source); + let mut emitter = build_emitter( + Rc::clone(&num_emitted_errors), + Rc::clone(&can_reset_errors), + Some(Rc::clone(&source_map)), + Some(ignore_list), + ); + let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1))); + let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, Some(span)); + emitter.emit_diagnostic(&fatal_diagnostic); + assert_eq!(*num_emitted_errors.borrow(), 1); + assert_eq!(*can_reset_errors.borrow(), false); + } + + #[test] + fn handles_recoverable_parse_error_in_ignored_file() { + if !is_nightly_channel!() { + return; + } + let num_emitted_errors = Rc::new(RefCell::new(0)); + let can_reset_errors = Rc::new(RefCell::new(false)); + let ignore_list = get_ignore_list(r#"ignore = ["foo.rs"]"#); + let source_map = Rc::new(SourceMap::new(FilePathMapping::empty())); + let source = String::from(r#"pub fn bar() { 1x; }"#); + source_map.new_source_file(SourceMapFileName::Real(PathBuf::from("foo.rs")), source); + let mut emitter = build_emitter( + Rc::clone(&num_emitted_errors), + Rc::clone(&can_reset_errors), + Some(Rc::clone(&source_map)), + Some(ignore_list), + ); + let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1))); + let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span)); + emitter.emit_diagnostic(&non_fatal_diagnostic); + assert_eq!(*num_emitted_errors.borrow(), 0); + assert_eq!(*can_reset_errors.borrow(), true); + } + + #[test] + fn handles_recoverable_parse_error_in_non_ignored_file() { + if !is_nightly_channel!() { + return; + } + let num_emitted_errors = Rc::new(RefCell::new(0)); + let can_reset_errors = Rc::new(RefCell::new(false)); + let source_map = Rc::new(SourceMap::new(FilePathMapping::empty())); + let source = String::from(r#"pub fn bar() { 1x; }"#); + source_map.new_source_file(SourceMapFileName::Real(PathBuf::from("foo.rs")), source); + let mut emitter = build_emitter( + Rc::clone(&num_emitted_errors), + Rc::clone(&can_reset_errors), + Some(Rc::clone(&source_map)), + None, + ); + let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1))); + let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span)); + emitter.emit_diagnostic(&non_fatal_diagnostic); + assert_eq!(*num_emitted_errors.borrow(), 1); + assert_eq!(*can_reset_errors.borrow(), false); + } + + #[test] + fn handles_mix_of_recoverable_parse_error() { + if !is_nightly_channel!() { + return; + } + let num_emitted_errors = Rc::new(RefCell::new(0)); + let can_reset_errors = Rc::new(RefCell::new(false)); + let source_map = Rc::new(SourceMap::new(FilePathMapping::empty())); + let ignore_list = get_ignore_list(r#"ignore = ["foo.rs"]"#); + let bar_source = String::from(r#"pub fn bar() { 1x; }"#); + let foo_source = String::from(r#"pub fn foo() { 1x; }"#); + let fatal_source = + String::from(r#"extern "system" fn jni_symbol!( funcName ) ( ... ) -> {} "#); + source_map + .new_source_file(SourceMapFileName::Real(PathBuf::from("bar.rs")), bar_source); + source_map + .new_source_file(SourceMapFileName::Real(PathBuf::from("foo.rs")), foo_source); + source_map.new_source_file( + SourceMapFileName::Real(PathBuf::from("fatal.rs")), + fatal_source, + ); + let mut emitter = build_emitter( + Rc::clone(&num_emitted_errors), + Rc::clone(&can_reset_errors), + Some(Rc::clone(&source_map)), + Some(ignore_list), + ); + let bar_span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1))); + let foo_span = MultiSpan::from_span(mk_sp(BytePos(21), BytePos(22))); + let bar_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(bar_span)); + let foo_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(foo_span)); + let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, None); + emitter.emit_diagnostic(&bar_diagnostic); + emitter.emit_diagnostic(&foo_diagnostic); + emitter.emit_diagnostic(&fatal_diagnostic); + assert_eq!(*num_emitted_errors.borrow(), 2); + assert_eq!(*can_reset_errors.borrow(), false); + } + } +}