From 1fcc9bad6f164e0e5cc5dede51af02f0e59b70d1 Mon Sep 17 00:00:00 2001 From: calebcartwright Date: Thu, 5 Sep 2019 19:14:40 -0500 Subject: [PATCH 1/6] refactor: gracefully handle parse error in ignored file --- src/formatting.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/formatting.rs b/src/formatting.rs index b58911705c0..e23572c33d0 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -88,7 +88,13 @@ fn format_project( ) { Ok(krate) => krate, // Surface parse error via Session (errors are merged there from report) - Err(ErrorKind::ParseError) => return Ok(report), + Err(ErrorKind::ParseError) => { + // https://github.com/rust-lang/rustfmt/issues/3779 + if ignore_path_set.is_match(&main_file) { + return Ok(FormatReport::new()); + } + return Ok(report); + } Err(e) => return Err(e), }; timer = timer.done_parsing(); From d3a8a97978af134bdc10fc4f5c6d0500bf0ec439 Mon Sep 17 00:00:00 2001 From: calebcartwright Date: Thu, 5 Sep 2019 23:14:00 -0500 Subject: [PATCH 2/6] refactor: don't emit parse errors on main_file ignore matches --- src/formatting.rs | 63 ++++++++++++++++++++++++++++++++++++++++------ src/ignore_path.rs | 1 + 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index e23572c33d0..d45ef762e87 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -7,8 +7,8 @@ use std::rc::Rc; use std::time::{Duration, Instant}; use syntax::ast; -use syntax::errors::emitter::{ColorConfig, Emitter}; -use syntax::errors::{DiagnosticBuilder, Handler}; +use syntax::errors::emitter::{ColorConfig, Emitter, EmitterWriter}; +use syntax::errors::{DiagnosticBuilder, Handler, HandlerFlags}; use syntax::parse::{self, ParseSess}; use syntax::source_map::{FilePathMapping, SourceMap, Span, DUMMY_SP}; @@ -70,13 +70,17 @@ fn format_project( Ok(set) => set, Err(e) => return Err(ErrorKind::InvalidGlobPattern(e)), }; - if config.skip_children() && ignore_path_set.is_match(&main_file) { + let is_ignore_match = ignore_path_set.is_match(&main_file); + if config.skip_children() && is_ignore_match { return Ok(FormatReport::new()); } // Parse the crate. let source_map = Rc::new(SourceMap::new(FilePathMapping::empty())); - let mut parse_session = make_parse_sess(source_map.clone(), config); + let mut parse_session = make_parse_sess(source_map.clone(), config, &ignore_path_set); + if is_ignore_match { + parse_session.span_diagnostic = Handler::with_emitter(true, None, silent_emitter()); + } let mut report = FormatReport::new(); let directory_ownership = input.to_directory_ownership(); let krate = match parse_crate( @@ -90,7 +94,7 @@ fn format_project( // Surface parse error via Session (errors are merged there from report) Err(ErrorKind::ParseError) => { // https://github.com/rust-lang/rustfmt/issues/3779 - if ignore_path_set.is_match(&main_file) { + if is_ignore_match { return Ok(FormatReport::new()); } return Ok(report); @@ -689,6 +693,33 @@ fn parse_crate( Err(ErrorKind::ParseError) } +struct SilentOnIgnoredFilesEmitter { + ignore_path_set: IgnorePathSet, + source_map: Rc, + emitter: EmitterWriter, +} + +impl Emitter for SilentOnIgnoredFilesEmitter { + fn emit_diagnostic(&mut self, db: &DiagnosticBuilder<'_>) { + if let Some(primary_span) = &db.span.primary_span() { + let file_name = self.source_map.span_to_filename(*primary_span); + match file_name { + syntax_pos::FileName::Real(ref path) => { + if self + .ignore_path_set + .is_match(&FileName::Real(path.to_path_buf())) + { + db.handler.reset_err_count(); + return; + } + } + _ => (), + }; + } + self.emitter.emit_diagnostic(db); + } +} + /// Emitter which discards every error. struct SilentEmitter; @@ -700,7 +731,11 @@ fn silent_emitter() -> Box { Box::new(SilentEmitter {}) } -fn make_parse_sess(source_map: Rc, config: &Config) -> ParseSess { +fn make_parse_sess( + source_map: Rc, + config: &Config, + ignore_path_set: &IgnorePathSet, +) -> ParseSess { let tty_handler = if config.hide_parse_errors() { let silent_emitter = silent_emitter(); Handler::with_emitter(true, None, silent_emitter) @@ -711,7 +746,21 @@ fn make_parse_sess(source_map: Rc, config: &Config) -> ParseSess { } else { ColorConfig::Never }; - Handler::with_tty_emitter(color_cfg, true, None, Some(source_map.clone())) + let emitter_writer = + EmitterWriter::stderr(color_cfg, Some(source_map.clone()), false, false); + let emitter = Box::new(SilentOnIgnoredFilesEmitter { + ignore_path_set: ignore_path_set.clone(), + source_map: source_map.clone(), + emitter: emitter_writer, + }); + Handler::with_emitter_and_flags( + emitter, + HandlerFlags { + can_emit_warnings: false, + treat_err_as_bug: None, + ..Default::default() + }, + ) }; ParseSess::with_span_handler(tty_handler, source_map) diff --git a/src/ignore_path.rs b/src/ignore_path.rs index d8974e12b8f..a72cacbeef9 100644 --- a/src/ignore_path.rs +++ b/src/ignore_path.rs @@ -2,6 +2,7 @@ use ignore::{self, gitignore}; use crate::config::{FileName, IgnoreList}; +#[derive(Clone)] pub(crate) struct IgnorePathSet { ignore_set: gitignore::Gitignore, } From a99b99bfe3ab593d4d212fcede5faf6229db2592 Mon Sep 17 00:00:00 2001 From: calebcartwright Date: Sun, 8 Sep 2019 15:44:12 -0500 Subject: [PATCH 3/6] feat: continue on recovered parse errors in ignore files --- src/formatting.rs | 55 ++++++++++++++++++++++++---------- src/test/mod.rs | 2 ++ tests/config/issue-3779.toml | 4 +++ tests/source/issue-3779/ice.rs | 3 ++ tests/source/issue-3779/lib.rs | 8 +++++ tests/target/issue-3779/ice.rs | 3 ++ tests/target/issue-3779/lib.rs | 8 +++++ 7 files changed, 68 insertions(+), 15 deletions(-) create mode 100644 tests/config/issue-3779.toml create mode 100644 tests/source/issue-3779/ice.rs create mode 100644 tests/source/issue-3779/lib.rs create mode 100644 tests/target/issue-3779/ice.rs create mode 100644 tests/target/issue-3779/lib.rs diff --git a/src/formatting.rs b/src/formatting.rs index d45ef762e87..7d9dd6f8c8a 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -1,5 +1,6 @@ // High level formatting functions. +use std::cell::RefCell; use std::collections::HashMap; use std::io::{self, Write}; use std::panic::{catch_unwind, AssertUnwindSafe}; @@ -70,17 +71,19 @@ fn format_project( Ok(set) => set, Err(e) => return Err(ErrorKind::InvalidGlobPattern(e)), }; - let is_ignore_match = ignore_path_set.is_match(&main_file); - if config.skip_children() && is_ignore_match { + if config.skip_children() && ignore_path_set.is_match(&main_file) { return Ok(FormatReport::new()); } // Parse the crate. + let can_reset = Rc::new(RefCell::new(false)); let source_map = Rc::new(SourceMap::new(FilePathMapping::empty())); - let mut parse_session = make_parse_sess(source_map.clone(), config, &ignore_path_set); - if is_ignore_match { - parse_session.span_diagnostic = Handler::with_emitter(true, None, silent_emitter()); - } + let mut parse_session = make_parse_sess( + source_map.clone(), + config, + &ignore_path_set, + can_reset.clone(), + ); let mut report = FormatReport::new(); let directory_ownership = input.to_directory_ownership(); let krate = match parse_crate( @@ -89,16 +92,11 @@ fn format_project( config, &mut report, directory_ownership, + can_reset.clone(), ) { Ok(krate) => krate, // Surface parse error via Session (errors are merged there from report) - Err(ErrorKind::ParseError) => { - // https://github.com/rust-lang/rustfmt/issues/3779 - if is_ignore_match { - return Ok(FormatReport::new()); - } - return Ok(report); - } + Err(ErrorKind::ParseError) => return Ok(report), Err(e) => return Err(e), }; timer = timer.done_parsing(); @@ -630,6 +628,7 @@ fn parse_crate( config: &Config, report: &mut FormatReport, directory_ownership: Option, + parser_error_resetter: Rc>, ) -> Result { let input_is_stdin = input.is_text(); @@ -677,6 +676,15 @@ fn parse_crate( if !parse_session.span_diagnostic.has_errors() { return Ok(c); } + // This scenario occurs when the parser encountered errors + // but was still able to recover. If all of the parser errors + // occurred in files that are ignored, then reset + // the error count and continue. + // https://github.com/rust-lang/rustfmt/issues/3779 + if *parser_error_resetter.borrow() { + parse_session.span_diagnostic.reset_err_count(); + return Ok(c); + } } Ok(Err(mut diagnostics)) => diagnostics.iter_mut().for_each(DiagnosticBuilder::emit), Err(_) => { @@ -697,6 +705,9 @@ struct SilentOnIgnoredFilesEmitter { ignore_path_set: IgnorePathSet, source_map: Rc, emitter: EmitterWriter, + can_reset: bool, + has_non_ignorable_parser_errors: bool, + parser_error_resetter: Rc>, } impl Emitter for SilentOnIgnoredFilesEmitter { @@ -709,13 +720,22 @@ impl Emitter for SilentOnIgnoredFilesEmitter { .ignore_path_set .is_match(&FileName::Real(path.to_path_buf())) { - db.handler.reset_err_count(); + if !self.has_non_ignorable_parser_errors && !self.can_reset { + self.can_reset = true; + *self.parser_error_resetter.borrow_mut() = true; + } return; } } _ => (), }; } + + self.has_non_ignorable_parser_errors = true; + if self.can_reset { + *self.parser_error_resetter.borrow_mut() = false; + } + self.can_reset = false; self.emitter.emit_diagnostic(db); } } @@ -735,6 +755,7 @@ fn make_parse_sess( source_map: Rc, config: &Config, ignore_path_set: &IgnorePathSet, + parser_error_resetter: Rc>, ) -> ParseSess { let tty_handler = if config.hide_parse_errors() { let silent_emitter = silent_emitter(); @@ -746,12 +767,16 @@ fn make_parse_sess( } else { ColorConfig::Never }; + let emitter_writer = - EmitterWriter::stderr(color_cfg, Some(source_map.clone()), false, false); + EmitterWriter::stderr(color_cfg, Some(source_map.clone()), false, false, None); let emitter = Box::new(SilentOnIgnoredFilesEmitter { + has_non_ignorable_parser_errors: false, + can_reset: false, ignore_path_set: ignore_path_set.clone(), source_map: source_map.clone(), emitter: emitter_writer, + parser_error_resetter, }); Handler::with_emitter_and_flags( emitter, diff --git a/src/test/mod.rs b/src/test/mod.rs index 3793a98ab00..47430760bee 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -27,6 +27,8 @@ const SKIP_FILE_WHITE_LIST: &[&str] = &[ "configs/skip_children/foo/mod.rs", "issue-3434/no_entry.rs", "issue-3665/sub_mod.rs", + // Testing for issue-3779 + "issue-3779/ice.rs", // These files and directory are a part of modules defined inside `cfg_if!`. "cfg_if/mod.rs", "cfg_if/detect", diff --git a/tests/config/issue-3779.toml b/tests/config/issue-3779.toml new file mode 100644 index 00000000000..83505f409af --- /dev/null +++ b/tests/config/issue-3779.toml @@ -0,0 +1,4 @@ +unstable_features = true +ignore = [ + "tests/**/issue-3779/ice.rs" +] diff --git a/tests/source/issue-3779/ice.rs b/tests/source/issue-3779/ice.rs new file mode 100644 index 00000000000..cde21412d94 --- /dev/null +++ b/tests/source/issue-3779/ice.rs @@ -0,0 +1,3 @@ +pub fn bar() { + 1x; +} diff --git a/tests/source/issue-3779/lib.rs b/tests/source/issue-3779/lib.rs new file mode 100644 index 00000000000..d7c2ae28975 --- /dev/null +++ b/tests/source/issue-3779/lib.rs @@ -0,0 +1,8 @@ +// rustfmt-config: issue-3779.toml + +#[path = "ice.rs"] +mod ice; + +fn foo() { +println!("abc") ; + } diff --git a/tests/target/issue-3779/ice.rs b/tests/target/issue-3779/ice.rs new file mode 100644 index 00000000000..cde21412d94 --- /dev/null +++ b/tests/target/issue-3779/ice.rs @@ -0,0 +1,3 @@ +pub fn bar() { + 1x; +} diff --git a/tests/target/issue-3779/lib.rs b/tests/target/issue-3779/lib.rs new file mode 100644 index 00000000000..ebebce417d1 --- /dev/null +++ b/tests/target/issue-3779/lib.rs @@ -0,0 +1,8 @@ +// rustfmt-config: issue-3779.toml + +#[path = "ice.rs"] +mod ice; + +fn foo() { + println!("abc"); +} From 1cac774310d0c4d7c84eac370c41f0d61aba0ed0 Mon Sep 17 00:00:00 2001 From: calebcartwright Date: Wed, 11 Sep 2019 10:27:24 -0500 Subject: [PATCH 4/6] refactor: use rc for ignorepath set --- src/formatting.rs | 10 +++++----- src/ignore_path.rs | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index 7d9dd6f8c8a..43416224bf1 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -68,7 +68,7 @@ fn format_project( let input_is_stdin = main_file == FileName::Stdin; let ignore_path_set = match IgnorePathSet::from_ignore_list(&config.ignore()) { - Ok(set) => set, + Ok(set) => Rc::new(set), Err(e) => return Err(ErrorKind::InvalidGlobPattern(e)), }; if config.skip_children() && ignore_path_set.is_match(&main_file) { @@ -81,7 +81,7 @@ fn format_project( let mut parse_session = make_parse_sess( source_map.clone(), config, - &ignore_path_set, + ignore_path_set.clone(), can_reset.clone(), ); let mut report = FormatReport::new(); @@ -702,7 +702,7 @@ fn parse_crate( } struct SilentOnIgnoredFilesEmitter { - ignore_path_set: IgnorePathSet, + ignore_path_set: Rc, source_map: Rc, emitter: EmitterWriter, can_reset: bool, @@ -754,7 +754,7 @@ fn silent_emitter() -> Box { fn make_parse_sess( source_map: Rc, config: &Config, - ignore_path_set: &IgnorePathSet, + ignore_path_set: Rc, parser_error_resetter: Rc>, ) -> ParseSess { let tty_handler = if config.hide_parse_errors() { @@ -773,7 +773,7 @@ fn make_parse_sess( let emitter = Box::new(SilentOnIgnoredFilesEmitter { has_non_ignorable_parser_errors: false, can_reset: false, - ignore_path_set: ignore_path_set.clone(), + ignore_path_set: ignore_path_set, source_map: source_map.clone(), emitter: emitter_writer, parser_error_resetter, diff --git a/src/ignore_path.rs b/src/ignore_path.rs index a72cacbeef9..d8974e12b8f 100644 --- a/src/ignore_path.rs +++ b/src/ignore_path.rs @@ -2,7 +2,6 @@ use ignore::{self, gitignore}; use crate::config::{FileName, IgnoreList}; -#[derive(Clone)] pub(crate) struct IgnorePathSet { ignore_set: gitignore::Gitignore, } From 0531683677eb82dc9f586e9ad51ad1a35bd18e29 Mon Sep 17 00:00:00 2001 From: calebcartwright Date: Fri, 20 Sep 2019 17:48:07 -0500 Subject: [PATCH 5/6] refactor: minor updates on pr feedback --- src/formatting.rs | 43 +++++++++++++++---------------------------- 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index 43416224bf1..8cd2a021f83 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -9,7 +9,7 @@ use std::time::{Duration, Instant}; use syntax::ast; use syntax::errors::emitter::{ColorConfig, Emitter, EmitterWriter}; -use syntax::errors::{DiagnosticBuilder, Handler, HandlerFlags}; +use syntax::errors::{DiagnosticBuilder, Handler}; use syntax::parse::{self, ParseSess}; use syntax::source_map::{FilePathMapping, SourceMap, Span, DUMMY_SP}; @@ -76,13 +76,13 @@ fn format_project( } // Parse the crate. - let can_reset = Rc::new(RefCell::new(false)); + let can_reset_parser_errors = Rc::new(RefCell::new(false)); let source_map = Rc::new(SourceMap::new(FilePathMapping::empty())); let mut parse_session = make_parse_sess( source_map.clone(), config, - ignore_path_set.clone(), - can_reset.clone(), + Rc::clone(&ignore_path_set), + can_reset_parser_errors.clone(), ); let mut report = FormatReport::new(); let directory_ownership = input.to_directory_ownership(); @@ -92,7 +92,7 @@ fn format_project( config, &mut report, directory_ownership, - can_reset.clone(), + can_reset_parser_errors.clone(), ) { Ok(krate) => krate, // Surface parse error via Session (errors are merged there from report) @@ -628,7 +628,7 @@ fn parse_crate( config: &Config, report: &mut FormatReport, directory_ownership: Option, - parser_error_resetter: Rc>, + can_reset_parser_errors: Rc>, ) -> Result { let input_is_stdin = input.is_text(); @@ -681,7 +681,7 @@ fn parse_crate( // occurred in files that are ignored, then reset // the error count and continue. // https://github.com/rust-lang/rustfmt/issues/3779 - if *parser_error_resetter.borrow() { + if *can_reset_parser_errors.borrow() { parse_session.span_diagnostic.reset_err_count(); return Ok(c); } @@ -705,9 +705,8 @@ struct SilentOnIgnoredFilesEmitter { ignore_path_set: Rc, source_map: Rc, emitter: EmitterWriter, - can_reset: bool, has_non_ignorable_parser_errors: bool, - parser_error_resetter: Rc>, + can_reset: Rc>, } impl Emitter for SilentOnIgnoredFilesEmitter { @@ -720,9 +719,8 @@ impl Emitter for SilentOnIgnoredFilesEmitter { .ignore_path_set .is_match(&FileName::Real(path.to_path_buf())) { - if !self.has_non_ignorable_parser_errors && !self.can_reset { - self.can_reset = true; - *self.parser_error_resetter.borrow_mut() = true; + if !self.has_non_ignorable_parser_errors { + *self.can_reset.borrow_mut() = true; } return; } @@ -732,10 +730,7 @@ impl Emitter for SilentOnIgnoredFilesEmitter { } self.has_non_ignorable_parser_errors = true; - if self.can_reset { - *self.parser_error_resetter.borrow_mut() = false; - } - self.can_reset = false; + *self.can_reset.borrow_mut() = false; self.emitter.emit_diagnostic(db); } } @@ -755,7 +750,7 @@ fn make_parse_sess( source_map: Rc, config: &Config, ignore_path_set: Rc, - parser_error_resetter: Rc>, + can_reset: Rc>, ) -> ParseSess { let tty_handler = if config.hide_parse_errors() { let silent_emitter = silent_emitter(); @@ -772,20 +767,12 @@ fn make_parse_sess( EmitterWriter::stderr(color_cfg, Some(source_map.clone()), false, false, None); let emitter = Box::new(SilentOnIgnoredFilesEmitter { has_non_ignorable_parser_errors: false, - can_reset: false, ignore_path_set: ignore_path_set, - source_map: source_map.clone(), + source_map: Rc::clone(&source_map), emitter: emitter_writer, - parser_error_resetter, + can_reset, }); - Handler::with_emitter_and_flags( - emitter, - HandlerFlags { - can_emit_warnings: false, - treat_err_as_bug: None, - ..Default::default() - }, - ) + Handler::with_emitter(true, None, emitter) }; ParseSess::with_span_handler(tty_handler, source_map) From 9fdeef7cd66d9fa5373dbaee21c81868ad575eb5 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Sat, 5 Oct 2019 10:26:37 -0500 Subject: [PATCH 6/6] refactor: apply rustc-ap 606 updates --- src/formatting.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index eb88f604085..eec33e6ba79 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -710,7 +710,7 @@ struct SilentOnIgnoredFilesEmitter { } impl Emitter for SilentOnIgnoredFilesEmitter { - fn emit_diagnostic(&mut self, db: &DiagnosticBuilder<'_>) { + fn emit_diagnostic(&mut self, db: &Diagnostic) { if let Some(primary_span) = &db.span.primary_span() { let file_name = self.source_map.span_to_filename(*primary_span); match file_name { @@ -763,8 +763,14 @@ fn make_parse_sess( ColorConfig::Never }; - let emitter_writer = - EmitterWriter::stderr(color_cfg, Some(source_map.clone()), false, false, None); + let emitter_writer = 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,