fix(parser): better unclosed delims handling (#4472)

Author: calebcartwright

src/formatting/syntux/parser.rs

@@ -23,7 +23,6 @@ pub(crate) struct Directory {
 /// A parser for Rust source code.
 pub(crate) struct Parser<'a> {
     parser: RawParser<'a>,
-    sess: &'a ParseSess,
 }
 
 /// A builder for the `Parser`.
@@ -74,7 +73,7 @@ impl<'a> ParserBuilder<'a> {
             }
         };
 
-        Ok(Parser { parser, sess })
+        Ok(Parser { parser })
     }
 
     fn parser(
@@ -159,6 +158,25 @@ impl<'a> Parser<'a> {
         input: Input,
         directory_ownership: Option<DirectoryOwnership>,
         sess: &'a ParseSess,
+    ) -> Result<ast::Crate, ParserError> {
+        let krate = Parser::parse_crate_inner(config, input, directory_ownership, sess)?;
+        if !sess.has_errors() {
+            return Ok(krate);
+        }
+
+        if sess.can_reset_errors() {
+            sess.reset_errors();
+            return Ok(krate);
+        }
+
+        Err(ParserError::ParseError)
+    }
+
+    fn parse_crate_inner(
+        config: &'a Config,
+        input: Input,
+        directory_ownership: Option<DirectoryOwnership>,
+        sess: &'a ParseSess,
     ) -> Result<ast::Crate, ParserError> {
         let mut parser = ParserBuilder::default()
             .config(config)
@@ -167,25 +185,14 @@ impl<'a> Parser<'a> {
             .sess(sess)
             .build()?;
 
-        parser.parse_crate_inner()
+        parser.parse_crate_mod()
     }
 
-    fn parse_crate_inner(&mut self) -> Result<ast::Crate, ParserError> {
+    fn parse_crate_mod(&mut self) -> Result<ast::Crate, ParserError> {
         let mut parser = AssertUnwindSafe(&mut self.parser);
 
         match catch_unwind(move || parser.parse_crate_mod()) {
-            Ok(Ok(krate)) => {
-                if !self.sess.has_errors() {
-                    return Ok(krate);
-                }
-
-                if self.sess.can_reset_errors() {
-                    self.sess.reset_errors();
-                    return Ok(krate);
-                }
-
-                Err(ParserError::ParseError)
-            }
+            Ok(Ok(k)) => Ok(k),
             Ok(Err(mut db)) => {
                 db.emit();
                 Err(ParserError::ParseError)

src/test/mod.rs

@@ -11,13 +11,12 @@ 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,
-    formatting::modules::{ModuleResolutionError, ModuleResolutionErrorKind},
-    is_nightly_channel, FormatReport, FormatReportFormatterBuilder, Input, OperationError,
+    format, is_nightly_channel, FormatReport, FormatReportFormatterBuilder, Input, OperationError,
     OperationSetting,
 };
 
 mod configuration_snippet;
+mod parser;
 
 const DIFF_CONTEXT_SIZE: usize = 3;
 
@@ -523,44 +522,6 @@ 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");
-    }
-}
-
-#[test]
-fn parser_creation_errors_on_entry_new_parser_from_file_panic() {
-    // See also https://github.com/rust-lang/rustfmt/issues/4418
-    let filename = "tests/parser/issue_4418.rs";
-    let file = PathBuf::from(filename);
-    let (config, operation, _) = read_config(&file);
-    if let Err(OperationError::ParseError { input, is_panic }) =
-        format_file(&file, operation, config)
-    {
-        assert_eq!(input.as_path().unwrap(), file);
-        assert!(is_panic);
-    } else {
-        panic!("Expected ParseError 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<PathBuf>, opt_config: &Option<PathBuf>) -> (Vec<FormatReport>, u32, u32) {

src/test/parser.rs

@@ -0,0 +1,58 @@
+use std::path::PathBuf;
+
+use super::{format_file, read_config};
+use crate::{
+    formatting::modules::{ModuleResolutionError, ModuleResolutionErrorKind},
+    OperationError,
+};
+
+#[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");
+    }
+}
+
+fn assert_parser_error(filename: &str, exp_panic: bool) {
+    let file = PathBuf::from(filename);
+    let (config, operation, _) = read_config(&file);
+    if let Err(OperationError::ParseError { input, is_panic }) =
+        format_file(&file, operation, config)
+    {
+        assert_eq!(input.as_path().unwrap(), file);
+        assert_eq!(is_panic, exp_panic);
+    } else {
+        panic!("Expected ParseError operation error");
+    }
+}
+
+#[test]
+fn parser_creation_errors_on_entry_new_parser_from_file_panic() {
+    // See also https://github.com/rust-lang/rustfmt/issues/4418
+    let filename = "tests/parser/issue_4418.rs";
+    let should_panic = true;
+    assert_parser_error(filename, should_panic);
+}
+
+#[test]
+fn crate_parsing_errors_on_unclosed_delims() {
+    // See also https://github.com/rust-lang/rustfmt/issues/4466
+    let filename = "tests/parser/unclosed-delims/issue_4466.rs";
+    let should_panic = false;
+    assert_parser_error(filename, should_panic);
+}

tests/parser/unclosed-delims/issue_4466.rs

@@ -0,0 +1,11 @@
+fn main() {
+    if true {
+        println!("answer: {}", a_func();
+    } else {
+        println!("don't think so.");
+    }
+}
+
+fn a_func() -> i32 {
+    42
+}