Skip to content

Commit e72f307

Browse files
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.
1 parent c60416e commit e72f307

File tree

1 file changed

+215
-36
lines changed

1 file changed

+215
-36
lines changed

src/formatting.rs

+215-36
Original file line numberDiff line numberDiff line change
@@ -715,37 +715,43 @@ fn parse_crate(
715715
struct SilentOnIgnoredFilesEmitter {
716716
ignore_path_set: Rc<IgnorePathSet>,
717717
source_map: Rc<SourceMap>,
718-
emitter: EmitterWriter,
718+
emitter: Box<dyn Emitter + Send>,
719719
has_non_ignorable_parser_errors: bool,
720720
can_reset: Rc<RefCell<bool>>,
721721
}
722722

723+
impl SilentOnIgnoredFilesEmitter {
724+
fn handle_non_ignoreable_error(&mut self, db: &Diagnostic) {
725+
self.has_non_ignorable_parser_errors = true;
726+
*self.can_reset.borrow_mut() = false;
727+
self.emitter.emit_diagnostic(db);
728+
}
729+
}
730+
723731
impl Emitter for SilentOnIgnoredFilesEmitter {
724732
fn source_map(&self) -> Option<&Lrc<SourceMap>> {
725733
None
726734
}
735+
727736
fn emit_diagnostic(&mut self, db: &Diagnostic) {
737+
if db.level == DiagnosticLevel::Fatal {
738+
return self.handle_non_ignoreable_error(db);
739+
}
728740
if let Some(primary_span) = &db.span.primary_span() {
729741
let file_name = self.source_map.span_to_filename(*primary_span);
730-
match file_name {
731-
rustc_span::FileName::Real(ref path) => {
732-
if self
733-
.ignore_path_set
734-
.is_match(&FileName::Real(path.to_path_buf()))
735-
{
736-
if !self.has_non_ignorable_parser_errors {
737-
*self.can_reset.borrow_mut() = true;
738-
}
739-
return;
742+
if let rustc_span::FileName::Real(ref path) = file_name {
743+
if self
744+
.ignore_path_set
745+
.is_match(&FileName::Real(path.to_path_buf()))
746+
{
747+
if !self.has_non_ignorable_parser_errors {
748+
*self.can_reset.borrow_mut() = true;
740749
}
750+
return;
741751
}
742-
_ => (),
743752
};
744753
}
745-
746-
self.has_non_ignorable_parser_errors = true;
747-
*self.can_reset.borrow_mut() = false;
748-
self.emitter.emit_diagnostic(db);
754+
self.handle_non_ignoreable_error(db);
749755
}
750756
}
751757

@@ -759,7 +765,7 @@ impl Emitter for SilentEmitter {
759765
fn emit_diagnostic(&mut self, _db: &Diagnostic) {}
760766
}
761767

762-
fn silent_emitter() -> Box<SilentEmitter> {
768+
fn silent_emitter() -> Box<dyn Emitter + Send> {
763769
Box::new(SilentEmitter {})
764770
}
765771

@@ -769,36 +775,38 @@ fn make_parse_sess(
769775
ignore_path_set: Rc<IgnorePathSet>,
770776
can_reset: Rc<RefCell<bool>>,
771777
) -> ParseSess {
772-
let tty_handler = if config.hide_parse_errors() {
773-
let silent_emitter = silent_emitter();
774-
Handler::with_emitter(true, None, silent_emitter)
778+
let supports_color = term::stderr().map_or(false, |term| term.supports_color());
779+
let color_cfg = if supports_color {
780+
ColorConfig::Auto
775781
} else {
776-
let supports_color = term::stderr().map_or(false, |term| term.supports_color());
777-
let color_cfg = if supports_color {
778-
ColorConfig::Auto
779-
} else {
780-
ColorConfig::Never
781-
};
782+
ColorConfig::Never
783+
};
782784

783-
let emitter_writer = EmitterWriter::stderr(
785+
let emitter = if config.hide_parse_errors() {
786+
silent_emitter()
787+
} else {
788+
Box::new(EmitterWriter::stderr(
784789
color_cfg,
785790
Some(source_map.clone()),
786791
false,
787792
false,
788793
None,
789794
false,
790-
);
791-
let emitter = Box::new(SilentOnIgnoredFilesEmitter {
795+
))
796+
};
797+
let handler = Handler::with_emitter(
798+
true,
799+
None,
800+
Box::new(SilentOnIgnoredFilesEmitter {
792801
has_non_ignorable_parser_errors: false,
793-
ignore_path_set: ignore_path_set,
794-
source_map: Rc::clone(&source_map),
795-
emitter: emitter_writer,
802+
source_map: source_map.clone(),
803+
emitter,
804+
ignore_path_set,
796805
can_reset,
797-
});
798-
Handler::with_emitter(true, None, emitter)
799-
};
806+
}),
807+
);
800808

801-
ParseSess::with_span_handler(tty_handler, source_map)
809+
ParseSess::with_span_handler(handler, source_map)
802810
}
803811

804812
fn should_emit_verbose<F>(is_stdin: bool, config: &Config, f: F)
@@ -809,3 +817,174 @@ where
809817
f();
810818
}
811819
}
820+
821+
#[cfg(test)]
822+
mod tests {
823+
use super::*;
824+
825+
mod emitter {
826+
use super::*;
827+
use crate::config::IgnoreList;
828+
use crate::is_nightly_channel;
829+
use crate::utils::mk_sp;
830+
use rustc_span::{BytePos, FileName as SourceMapFileName, MultiSpan, DUMMY_SP};
831+
use std::path::{Path, PathBuf};
832+
833+
struct TestEmitter {
834+
num_emitted_errors: Rc<RefCell<u32>>,
835+
}
836+
837+
impl Emitter for TestEmitter {
838+
fn source_map(&self) -> Option<&Lrc<SourceMap>> {
839+
None
840+
}
841+
fn emit_diagnostic(&mut self, _db: &Diagnostic) {
842+
*self.num_emitted_errors.borrow_mut() += 1;
843+
}
844+
}
845+
846+
fn build_diagnostic(level: DiagnosticLevel, span: Option<MultiSpan>) -> Diagnostic {
847+
Diagnostic {
848+
level,
849+
code: None,
850+
message: vec![],
851+
children: vec![],
852+
suggestions: vec![],
853+
span: span.unwrap_or_else(MultiSpan::new),
854+
sort_span: DUMMY_SP,
855+
}
856+
}
857+
858+
fn build_emitter(
859+
num_emitted_errors: Rc<RefCell<u32>>,
860+
can_reset: Rc<RefCell<bool>>,
861+
source_map: Option<Rc<SourceMap>>,
862+
ignore_list: Option<IgnoreList>,
863+
) -> SilentOnIgnoredFilesEmitter {
864+
let emitter_writer = TestEmitter { num_emitted_errors };
865+
let source_map =
866+
source_map.unwrap_or_else(|| Rc::new(SourceMap::new(FilePathMapping::empty())));
867+
let ignore_path_set =
868+
Rc::new(IgnorePathSet::from_ignore_list(&ignore_list.unwrap_or_default()).unwrap());
869+
SilentOnIgnoredFilesEmitter {
870+
has_non_ignorable_parser_errors: false,
871+
source_map,
872+
emitter: Box::new(emitter_writer),
873+
ignore_path_set,
874+
can_reset,
875+
}
876+
}
877+
878+
fn get_ignore_list(config: &str) -> IgnoreList {
879+
Config::from_toml(config, Path::new("")).unwrap().ignore()
880+
}
881+
882+
#[test]
883+
fn handles_fatal_parse_error_in_ignored_file() {
884+
let num_emitted_errors = Rc::new(RefCell::new(0));
885+
let can_reset_errors = Rc::new(RefCell::new(false));
886+
let ignore_list = get_ignore_list(r#"ignore = ["foo.rs"]"#);
887+
let source_map = Rc::new(SourceMap::new(FilePathMapping::empty()));
888+
let source =
889+
String::from(r#"extern "system" fn jni_symbol!( funcName ) ( ... ) -> {} "#);
890+
source_map.new_source_file(SourceMapFileName::Real(PathBuf::from("foo.rs")), source);
891+
let mut emitter = build_emitter(
892+
Rc::clone(&num_emitted_errors),
893+
Rc::clone(&can_reset_errors),
894+
Some(Rc::clone(&source_map)),
895+
Some(ignore_list),
896+
);
897+
let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
898+
let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, Some(span));
899+
emitter.emit_diagnostic(&fatal_diagnostic);
900+
assert_eq!(*num_emitted_errors.borrow(), 1);
901+
assert_eq!(*can_reset_errors.borrow(), false);
902+
}
903+
904+
#[test]
905+
fn handles_recoverable_parse_error_in_ignored_file() {
906+
if !is_nightly_channel!() {
907+
return;
908+
}
909+
let num_emitted_errors = Rc::new(RefCell::new(0));
910+
let can_reset_errors = Rc::new(RefCell::new(false));
911+
let ignore_list = get_ignore_list(r#"ignore = ["foo.rs"]"#);
912+
let source_map = Rc::new(SourceMap::new(FilePathMapping::empty()));
913+
let source = String::from(r#"pub fn bar() { 1x; }"#);
914+
source_map.new_source_file(SourceMapFileName::Real(PathBuf::from("foo.rs")), source);
915+
let mut emitter = build_emitter(
916+
Rc::clone(&num_emitted_errors),
917+
Rc::clone(&can_reset_errors),
918+
Some(Rc::clone(&source_map)),
919+
Some(ignore_list),
920+
);
921+
let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
922+
let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span));
923+
emitter.emit_diagnostic(&non_fatal_diagnostic);
924+
assert_eq!(*num_emitted_errors.borrow(), 0);
925+
assert_eq!(*can_reset_errors.borrow(), true);
926+
}
927+
928+
#[test]
929+
fn handles_recoverable_parse_error_in_non_ignored_file() {
930+
if !is_nightly_channel!() {
931+
return;
932+
}
933+
let num_emitted_errors = Rc::new(RefCell::new(0));
934+
let can_reset_errors = Rc::new(RefCell::new(false));
935+
let source_map = Rc::new(SourceMap::new(FilePathMapping::empty()));
936+
let source = String::from(r#"pub fn bar() { 1x; }"#);
937+
source_map.new_source_file(SourceMapFileName::Real(PathBuf::from("foo.rs")), source);
938+
let mut emitter = build_emitter(
939+
Rc::clone(&num_emitted_errors),
940+
Rc::clone(&can_reset_errors),
941+
Some(Rc::clone(&source_map)),
942+
None,
943+
);
944+
let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
945+
let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span));
946+
emitter.emit_diagnostic(&non_fatal_diagnostic);
947+
assert_eq!(*num_emitted_errors.borrow(), 1);
948+
assert_eq!(*can_reset_errors.borrow(), false);
949+
}
950+
951+
#[test]
952+
fn handles_mix_of_recoverable_parse_error() {
953+
if !is_nightly_channel!() {
954+
return;
955+
}
956+
let num_emitted_errors = Rc::new(RefCell::new(0));
957+
let can_reset_errors = Rc::new(RefCell::new(false));
958+
let source_map = Rc::new(SourceMap::new(FilePathMapping::empty()));
959+
let ignore_list = get_ignore_list(r#"ignore = ["foo.rs"]"#);
960+
let bar_source = String::from(r#"pub fn bar() { 1x; }"#);
961+
let foo_source = String::from(r#"pub fn foo() { 1x; }"#);
962+
let fatal_source =
963+
String::from(r#"extern "system" fn jni_symbol!( funcName ) ( ... ) -> {} "#);
964+
source_map
965+
.new_source_file(SourceMapFileName::Real(PathBuf::from("bar.rs")), bar_source);
966+
source_map
967+
.new_source_file(SourceMapFileName::Real(PathBuf::from("foo.rs")), foo_source);
968+
source_map.new_source_file(
969+
SourceMapFileName::Real(PathBuf::from("fatal.rs")),
970+
fatal_source,
971+
);
972+
let mut emitter = build_emitter(
973+
Rc::clone(&num_emitted_errors),
974+
Rc::clone(&can_reset_errors),
975+
Some(Rc::clone(&source_map)),
976+
Some(ignore_list),
977+
);
978+
let bar_span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
979+
let foo_span = MultiSpan::from_span(mk_sp(BytePos(21), BytePos(22)));
980+
let bar_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(bar_span));
981+
let foo_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(foo_span));
982+
let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, None);
983+
emitter.emit_diagnostic(&bar_diagnostic);
984+
emitter.emit_diagnostic(&foo_diagnostic);
985+
emitter.emit_diagnostic(&fatal_diagnostic);
986+
assert_eq!(*num_emitted_errors.borrow(), 2);
987+
assert_eq!(*can_reset_errors.borrow(), false);
988+
}
989+
}
990+
}

0 commit comments

Comments
 (0)