Add fmt test to make sure formatting works for all parser test cases

crates/compiler/fmt/src/collection.rs

@@ -8,6 +8,7 @@ use crate::{
 
 #[derive(PartialEq, Eq, Clone, Copy, Debug)]
 pub enum Braces {
+    Round,
     Square,
     Curly,
 }
@@ -22,11 +23,13 @@ pub fn fmt_collection<'a, 'buf, T: ExtractSpaces<'a> + Formattable>(
     <T as ExtractSpaces<'a>>::Item: Formattable,
 {
     let start = match braces {
+        Braces::Round => '(',
         Braces::Curly => '{',
         Braces::Square => '[',
     };
 
     let end = match braces {
+        Braces::Round => ')',
         Braces::Curly => '}',
         Braces::Square => ']',
     };

crates/compiler/fmt/src/def.rs

@@ -134,7 +134,7 @@ impl<'a> Formattable for TypeDef<'a> {
 
                 if !self.is_multiline() {
                     debug_assert_eq!(members.len(), 1);
-                    buf.push_str(" ");
+                    buf.spaces(1);
                     members[0].format_with_options(
                         buf,
                         Parens::NotNeeded,

crates/compiler/fmt/src/expr.rs

@@ -326,9 +326,6 @@ impl<'a> Formattable for Expr<'a> {
             Record(fields) => {
                 fmt_record(buf, None, *fields, indent);
             }
-            Tuple(_fields) => {
-                todo!("format tuple");
-            }
             RecordUpdate { update, fields } => {
                 fmt_record(buf, Some(*update), *fields, indent);
             }
@@ -386,6 +383,7 @@ impl<'a> Formattable for Expr<'a> {
                 fmt_if(buf, branches, final_else, self.is_multiline(), indent);
             }
             When(loc_condition, branches) => fmt_when(buf, loc_condition, branches, indent),
+            Tuple(items) => fmt_collection(buf, indent, Braces::Round, *items, Newlines::No),
             List(items) => fmt_collection(buf, indent, Braces::Square, *items, Newlines::No),
             BinOps(lefts, right) => fmt_binops(buf, lefts, right, false, parens, indent),
             UnaryOp(sub_expr, unary_op) => {
@@ -421,7 +419,10 @@ impl<'a> Formattable for Expr<'a> {
                 buf.push('.');
                 buf.push_str(key);
             }
-            MalformedIdent(_, _) => {}
+            MalformedIdent(str, _) => {
+                buf.indent(indent);
+                buf.push_str(str)
+            }
             MalformedClosure => {}
             PrecedenceConflict { .. } => {}
         }
@@ -505,10 +506,10 @@ fn push_op(buf: &mut Buf, op: BinOp) {
 pub fn fmt_str_literal<'buf>(buf: &mut Buf<'buf>, literal: StrLiteral, indent: u16) {
     use roc_parse::ast::StrLiteral::*;
 
-    buf.indent(indent);
-    buf.push('"');
     match literal {
         PlainLine(string) => {
+            buf.indent(indent);
+            buf.push('"');
             // When a PlainLine contains '\n' or '"', format as a block string
             if string.contains('"') || string.contains('\n') {
                 buf.push_str("\"\"");
@@ -523,15 +524,21 @@ pub fn fmt_str_literal<'buf>(buf: &mut Buf<'buf>, literal: StrLiteral, indent: u
             } else {
                 buf.push_str_allow_spaces(string);
             };
+            buf.push('"');
         }
         Line(segments) => {
+            buf.indent(indent);
+            buf.push('"');
             for seg in segments.iter() {
                 format_str_segment(seg, buf, 0)
             }
+            buf.push('"');
         }
         Block(lines) => {
             // Block strings will always be formatted with """ on new lines
-            buf.push_str("\"\"");
+            buf.ensure_ends_with_newline();
+            buf.indent(indent);
+            buf.push_str("\"\"\"");
             buf.newline();
 
             for segments in lines.iter() {
@@ -543,10 +550,9 @@ pub fn fmt_str_literal<'buf>(buf: &mut Buf<'buf>, literal: StrLiteral, indent: u
                 buf.newline();
             }
             buf.indent(indent);
-            buf.push_str("\"\"");
+            buf.push_str("\"\"\"");
         }
     }
-    buf.push('"');
 }
 
 fn fmt_binops<'a, 'buf>(

crates/compiler/fmt/src/lib.rs

@@ -109,7 +109,7 @@ impl<'a> Buf<'a> {
         if self.spaces_to_flush > 0 {
             self.flush_spaces();
             self.newline();
-        } else if !self.text.ends_with('\n') {
+        } else if !self.text.ends_with('\n') && !self.text.is_empty() {
             self.newline()
         }
     }

crates/compiler/fmt/tests/test_fmt.rs

@@ -16,11 +16,15 @@ mod test_fmt {
     use roc_parse::state::State;
     use roc_test_utils::{assert_multiline_str_eq, workspace_root};
 
-    // Not intended to be used directly in tests; please use expr_formats_to or expr_formats_same
-    fn expr_formats_to(input: &str, expected: &str) {
+    /// Check that the given input is formatted to the given output.
+    /// If the expected output is `None`, then this only checks that the input
+    /// parses without error, formats without error, and
+    /// (optionally, based on the value of `check_stability`) re-parses to
+    /// the same AST as the original.
+    fn expr_formats(input: &str, expected: Option<&str>, check_stability: bool) {
         let arena = Bump::new();
         let input = input.trim();
-        let expected = expected.trim();
+        let expected = expected.map(|e| e.trim());
 
         match roc_parse::test_helpers::parse_expr_with(&arena, input) {
             Ok(actual) => {
@@ -32,12 +36,17 @@ mod test_fmt {
 
                 let output = buf.as_str();
 
-                assert_multiline_str_eq!(expected, output);
+                if let Some(expected) = expected {
+                    assert_multiline_str_eq!(expected, output);
+                }
 
                 let reparsed_ast = roc_parse::test_helpers::parse_expr_with(&arena, output).unwrap_or_else(|err| {
                     panic!(
-                        "After formatting, the source code no longer parsed!\n\nParse error was: {:?}\n\nThe code that failed to parse:\n\n{}\n\n",
-                        err, output
+                        "After formatting, the source code no longer parsed!\n\n\
+                        Parse error was: {:?}\n\n\
+                        The code that failed to parse:\n\n{}\n\n\
+                        The original ast was:\n\n{:#?}\n\n",
+                        err, output, actual
                     );
                 });
 
@@ -60,21 +69,27 @@ mod test_fmt {
                 }
 
                 // Now verify that the resultant formatting is _stable_ - i.e. that it doesn't change again if re-formatted
-                let mut reformatted_buf = Buf::new_in(&arena);
-                reparsed_ast.format_with_options(&mut reformatted_buf, Parens::NotNeeded, Newlines::Yes, 0);
+                if check_stability {
+                    let mut reformatted_buf = Buf::new_in(&arena);
+                    reparsed_ast.format_with_options(&mut reformatted_buf, Parens::NotNeeded, Newlines::Yes, 0);
 
-                if output != reformatted_buf.as_str() {
-                    eprintln!("Formatting bug; formatting is not stable. Reformatting the formatted code changed it again, as follows:\n\n");
+                    if output != reformatted_buf.as_str() {
+                        eprintln!("Formatting bug; formatting is not stable. Reformatting the formatted code changed it again, as follows:\n\n");
 
-                    assert_multiline_str_eq!(output, reformatted_buf.as_str());
+                        assert_multiline_str_eq!(output, reformatted_buf.as_str());
+                    }
                 }
             }
             Err(error) => panic!("Unexpected parse failure when parsing this for formatting:\n\n{}\n\nParse error was:\n\n{:?}\n\n", input, error)
         };
     }
 
+    fn expr_formats_to(input: &str, expected: &str) {
+        expr_formats(input, Some(expected), true);
+    }
+
     fn expr_formats_same(input: &str) {
-        expr_formats_to(input, input);
+        expr_formats(input, Some(input), true);
     }
 
     fn fmt_module_and_defs<'a>(
@@ -5825,4 +5840,27 @@ mod test_fmt {
     //            "#
     //        ));
     //    }
+
+    #[test]
+    fn parse_test_snapshots_format_without_error() {
+        fn list(dir: &std::path::Path) -> std::vec::Vec<String> {
+            std::fs::read_dir(dir)
+                .unwrap()
+                .map(|f| f.unwrap().file_name().to_str().unwrap().to_string())
+                .collect::<std::vec::Vec<_>>()
+        }
+
+        let base = std::path::PathBuf::from("../parse/tests/snapshots/pass");
+        for file in list(&base) {
+            if file.ends_with(".expr.roc") {
+                println!("formatting {}", file);
+                let contents = std::fs::read_to_string(base.join(file)).unwrap();
+                expr_formats(&contents, None, false);
+            } else if file.ends_with(".module.roc") {
+                // TODO: re-format module defs and ensure they're correct.
+                // Note that these tests don't have an actual module header,
+                // so we'll have to pre-pend that for this test.
+            }
+        }
+    }
 }