From 10b9afa9061899f5e2db4be864655ee9d3a110d4 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sat, 8 Dec 2018 22:32:35 +0100 Subject: [PATCH 1/3] Add test asserting we catch Rust parser panics --- src/test/mod.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/mod.rs b/src/test/mod.rs index 599f83cd49c..77abc3c8558 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -288,6 +288,18 @@ fn stdin_formatting_smoke_test() { assert_eq!(buf, "fn main() {}\r\n".as_bytes()); } +#[test] +fn stdin_parser_panic_caught() { + // https://github.com/rust-lang/rustfmt/issues/3239 + for text in ["{", "}"].iter().cloned().map(String::from) { + let mut buf = vec![]; + let mut session = Session::new(Default::default(), Some(&mut buf)); + let _ = session.format(Input::Text(text)); + + assert!(session.has_parsing_errors()); + } +} + #[test] fn stdin_disable_all_formatting_test() { match option_env!("CFG_RELEASE_CHANNEL") { From a2042a64700840310be96041e039877383db14f2 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sat, 8 Dec 2018 23:22:23 +0100 Subject: [PATCH 2/3] Use non-panicking maybe_parser_from_source_str --- src/formatting.rs | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index 039276ba6e8..326059c7357 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -8,7 +8,7 @@ use std::time::{Duration, Instant}; use syntax::ast; use syntax::errors::emitter::{ColorConfig, EmitterWriter}; -use syntax::errors::Handler; +use syntax::errors::{DiagnosticBuilder, Handler}; use syntax::parse::{self, ParseSess}; use syntax::source_map::{FilePathMapping, SourceMap, Span}; @@ -604,22 +604,33 @@ fn parse_crate( ) -> Result { let input_is_stdin = input.is_text(); - let mut parser = match input { - Input::File(file) => parse::new_parser_from_file(parse_session, &file), - Input::Text(text) => parse::new_parser_from_source_str( + let parser = match input { + Input::File(file) => Ok(parse::new_parser_from_file(parse_session, &file)), + Input::Text(text) => parse::maybe_new_parser_from_source_str( parse_session, syntax::source_map::FileName::Custom("stdin".to_owned()), text, - ), + ) + .map_err(|diags| { + diags + .into_iter() + .map(|d| DiagnosticBuilder::new_diagnostic(&parse_session.span_diagnostic, d)) + .collect::>() + }), }; - parser.cfg_mods = false; - if config.skip_children() { - parser.recurse_into_file_modules = false; - } + let result = match parser { + Ok(mut parser) => { + parser.cfg_mods = false; + if config.skip_children() { + parser.recurse_into_file_modules = false; + } - let mut parser = AssertUnwindSafe(parser); - let result = catch_unwind(move || parser.0.parse_crate_mod()); + let mut parser = AssertUnwindSafe(parser); + catch_unwind(move || parser.0.parse_crate_mod().map_err(|d| vec![d])) + } + Err(db) => Ok(Err(db)), + }; match result { Ok(Ok(c)) => { @@ -627,7 +638,7 @@ fn parse_crate( return Ok(c); } } - Ok(Err(mut e)) => e.emit(), + Ok(Err(mut diagnostics)) => diagnostics.iter_mut().for_each(|d| d.emit()), Err(_) => { // Note that if you see this message and want more information, // then run the `parse_crate_mod` function above without From c7ee2a2857b0b8f1346bb02aabf19ce46184530f Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Thu, 13 Dec 2018 13:34:01 +0100 Subject: [PATCH 3/3] Don't ignore parse error when constructing report --- src/formatting.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/formatting.rs b/src/formatting.rs index 326059c7357..f909086f0e0 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -73,7 +73,12 @@ fn format_project( let source_map = Rc::new(SourceMap::new(FilePathMapping::empty())); let mut parse_session = make_parse_sess(source_map.clone(), config); let mut report = FormatReport::new(); - let krate = parse_crate(input, &parse_session, config, &mut report)?; + let krate = match parse_crate(input, &parse_session, config, &mut report) { + Ok(krate) => krate, + // Surface parse error via Session (errors are merged there from report) + Err(ErrorKind::ParseError) => return Ok(report), + Err(e) => return Err(e), + }; timer = timer.done_parsing(); // Suppress error output if we have to do any further parsing.