From 3eb864ebc5f162ad0a1d259128e95dcbf05d0244 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Mon, 25 May 2020 18:23:24 -0500 Subject: [PATCH] fix: handle recoverable submod parse errors correctly (#4200) --- rustfmt-core/rustfmt-lib/src/formatting.rs | 9 ++++--- .../rustfmt-lib/src/formatting/modules.rs | 4 +-- .../src/formatting/syntux/parser.rs | 20 +++++++++++--- rustfmt-core/rustfmt-lib/src/test/mod.rs | 26 ++++++++++++++++++- .../tests/parser/issue-4126/invalid.rs | 6 +++++ .../tests/parser/issue-4126/lib.rs | 2 ++ 6 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 rustfmt-core/rustfmt-lib/tests/parser/issue-4126/invalid.rs create mode 100644 rustfmt-core/rustfmt-lib/tests/parser/issue-4126/lib.rs diff --git a/rustfmt-core/rustfmt-lib/src/formatting.rs b/rustfmt-core/rustfmt-lib/src/formatting.rs index 22842c8aef9..e4b22bf889c 100644 --- a/rustfmt-core/rustfmt-lib/src/formatting.rs +++ b/rustfmt-core/rustfmt-lib/src/formatting.rs @@ -106,10 +106,6 @@ fn format_project( }); } }; - timer = timer.done_parsing(); - - // Suppress error output if we have to do any further parsing. - parse_session.set_silent_emitter(); let files = modules::ModResolver::new( &parse_session, @@ -118,6 +114,11 @@ fn format_project( ) .visit_crate(&krate)?; + timer = timer.done_parsing(); + + // Suppress error output if we have to do any further parsing. + parse_session.set_silent_emitter(); + for (path, module) in files { let should_ignore = !input_is_stdin && parse_session.ignore_file(&path); if (!operation_setting.recursive && path != main_file) || should_ignore { diff --git a/rustfmt-core/rustfmt-lib/src/formatting/modules.rs b/rustfmt-core/rustfmt-lib/src/formatting/modules.rs index f5c50529185..cd3f50611ff 100644 --- a/rustfmt-core/rustfmt-lib/src/formatting/modules.rs +++ b/rustfmt-core/rustfmt-lib/src/formatting/modules.rs @@ -36,8 +36,8 @@ pub(crate) struct ModResolver<'ast, 'sess> { #[error("failed to resolve mod `{module}`: {kind}")] #[derive(Debug, Error)] pub struct ModuleResolutionError { - module: String, - kind: ModuleResolutionErrorKind, + pub(crate) module: String, + pub(crate) kind: ModuleResolutionErrorKind, } #[derive(Debug, Error)] diff --git a/rustfmt-core/rustfmt-lib/src/formatting/syntux/parser.rs b/rustfmt-core/rustfmt-lib/src/formatting/syntux/parser.rs index 69bac50b5bd..df7cfb1a377 100644 --- a/rustfmt-core/rustfmt-lib/src/formatting/syntux/parser.rs +++ b/rustfmt-core/rustfmt-lib/src/formatting/syntux/parser.rs @@ -177,7 +177,9 @@ impl<'a> Parser<'a> { Ok(_attrs) => (), Err(mut e) => { e.cancel(); - sess.reset_errors(); + if sess.can_reset_errors() { + sess.reset_errors(); + } return None; } } @@ -186,13 +188,25 @@ impl<'a> Parser<'a> { Ok(m) => Some(m), Err(mut db) => { db.cancel(); - sess.reset_errors(); + if sess.can_reset_errors() { + sess.reset_errors(); + } None } } })); match result { - Ok(Some(m)) => Ok(m), + Ok(Some(m)) => { + if !sess.has_errors() { + return Ok(m); + } + + if sess.can_reset_errors() { + sess.reset_errors(); + return Ok(m); + } + Err(ParserError::ParseError) + } Ok(None) => Err(ParserError::ParseError), Err(..) if path.exists() => Err(ParserError::ParseError), Err(_) => Err(ParserError::ParsePanicError), diff --git a/rustfmt-core/rustfmt-lib/src/test/mod.rs b/rustfmt-core/rustfmt-lib/src/test/mod.rs index b6f9aff7535..4f4a1b08f33 100644 --- a/rustfmt-core/rustfmt-lib/src/test/mod.rs +++ b/rustfmt-core/rustfmt-lib/src/test/mod.rs @@ -11,7 +11,9 @@ use crate::emitter::rustfmt_diff::{make_diff, print_diff, Mismatch, ModifiedChun use crate::config::{Config, FileName, NewlineStyle}; use crate::{ emitter::{emit_format_report, Color, EmitMode, EmitterConfig}, - format, is_nightly_channel, FormatReport, FormatReportFormatterBuilder, Input, OperationError, + format, + formatting::modules::{ModuleResolutionError, ModuleResolutionErrorKind}, + is_nightly_channel, FormatReport, FormatReportFormatterBuilder, Input, OperationError, OperationSetting, }; @@ -519,6 +521,28 @@ fn format_lines_errors_are_reported_with_tabs() { assert!(report.has_errors()); } +#[test] +fn parser_errors_in_submods_are_surfaced() { + // See also https://github.com/rust-lang/rustfmt/issues/4126 + let filename = "tests/parser/issue-4126/lib.rs"; + let file = PathBuf::from(filename); + let exp_mod_name = "invalid"; + let (config, operation, _) = read_config(&file); + if let Err(OperationError::ModuleResolutionError { 0: inner }) = + format_file(&file, operation, config) + { + let ModuleResolutionError { module, kind } = inner; + assert_eq!(&module, exp_mod_name); + if let ModuleResolutionErrorKind::ParseError { file } = kind { + assert_eq!(file, PathBuf::from("tests/parser/issue-4126/invalid.rs")); + } else { + panic!("Expected parser error"); + } + } else { + panic!("Expected ModuleResolution operation error"); + } +} + // For each file, run rustfmt and collect the output. // Returns the number of files checked and the number of failures. fn check_files(files: Vec, opt_config: &Option) -> (Vec, u32, u32) { diff --git a/rustfmt-core/rustfmt-lib/tests/parser/issue-4126/invalid.rs b/rustfmt-core/rustfmt-lib/tests/parser/issue-4126/invalid.rs new file mode 100644 index 00000000000..7709c848464 --- /dev/null +++ b/rustfmt-core/rustfmt-lib/tests/parser/issue-4126/invalid.rs @@ -0,0 +1,6 @@ +fn foo() { + if bar && if !baz { + next_is_none = Some(true); + } + println!("foo"); +} diff --git a/rustfmt-core/rustfmt-lib/tests/parser/issue-4126/lib.rs b/rustfmt-core/rustfmt-lib/tests/parser/issue-4126/lib.rs new file mode 100644 index 00000000000..ab1536000fe --- /dev/null +++ b/rustfmt-core/rustfmt-lib/tests/parser/issue-4126/lib.rs @@ -0,0 +1,2 @@ +// rustfmt-recursive: true +mod invalid;