Merge pull request #2438 from rtfeldman/fix-module-formatting

Fix some module formatting bugs

compiler/fmt/src/annotation.rs

@@ -88,7 +88,12 @@ where
     T: Formattable,
 {
     fn is_multiline(&self) -> bool {
-        self.items.iter().any(|item| item.is_multiline()) || !self.final_comments().is_empty()
+        // if there are any comments, they must go on their own line
+        // because otherwise they'd comment out the closing delimiter
+        !self.final_comments().is_empty() ||
+        // if any of the items in the collection are multiline,
+        // then the whole collection must be multiline
+        self.items.iter().any(Formattable::is_multiline)
     }
 }
 

compiler/fmt/src/collection.rs

@@ -16,8 +16,6 @@ pub fn fmt_collection<'a, 'buf, T: ExtractSpaces<'a> + Formattable>(
 ) where
     <T as ExtractSpaces<'a>>::Item: Formattable,
 {
-    buf.indent(indent);
-
     if items.is_multiline() {
         let braces_indent = indent;
         let item_indent = braces_indent + INDENT;
@@ -50,9 +48,12 @@ pub fn fmt_collection<'a, 'buf, T: ExtractSpaces<'a> + Formattable>(
             item_indent,
         );
         buf.newline();
+        buf.indent(braces_indent);
+        buf.push(end);
     } else {
         // is_multiline == false
         // there is no comment to add
+        buf.indent(indent);
         buf.push(start);
         let mut iter = items.iter().peekable();
         while let Some(item) = iter.next() {
@@ -66,7 +67,7 @@ pub fn fmt_collection<'a, 'buf, T: ExtractSpaces<'a> + Formattable>(
         if !items.is_empty() {
             buf.spaces(1);
         }
+
+        buf.push(end);
     }
-    buf.indent(indent);
-    buf.push(end);
 }

compiler/fmt/src/module.rs

@@ -204,8 +204,14 @@ fn fmt_package_name<'buf>(buf: &mut Buf<'buf>, name: PackageName, _indent: u16)
 
 impl<'a, T: Formattable> Formattable for Spaced<'a, T> {
     fn is_multiline(&self) -> bool {
-        // TODO
-        false
+        use Spaced::*;
+
+        match self {
+            Item(formattable) => formattable.is_multiline(),
+            SpaceBefore(formattable, spaces) | SpaceAfter(formattable, spaces) => {
+                !spaces.is_empty() || formattable.is_multiline()
+            }
+        }
     }
 
     fn format_with_options<'buf>(
@@ -236,7 +242,7 @@ fn fmt_imports<'a, 'buf>(
     loc_entries: Collection<'a, Loc<Spaced<'a, ImportsEntry<'a>>>>,
     indent: u16,
 ) {
-    fmt_collection(buf, indent, '[', ']', loc_entries, Newlines::No)
+    fmt_collection(buf, indent + INDENT, '[', ']', loc_entries, Newlines::No)
 }
 
 fn fmt_provides<'a, 'buf>(
@@ -246,9 +252,9 @@ fn fmt_provides<'a, 'buf>(
     indent: u16,
 ) {
     fmt_collection(buf, indent, '[', ']', loc_exposed_names, Newlines::No);
-    if let Some(loc_provided_types) = loc_provided_types {
+    if let Some(loc_provided) = loc_provided_types {
         fmt_default_spaces(buf, &[], indent);
-        fmt_collection(buf, indent, '{', '}', loc_provided_types, Newlines::No);
+        fmt_collection(buf, indent + INDENT, '{', '}', loc_provided, Newlines::No);
     }
 }
 
@@ -266,7 +272,7 @@ fn fmt_exposes<'buf, N: Formattable + Copy>(
     loc_entries: Collection<'_, Loc<Spaced<'_, N>>>,
     indent: u16,
 ) {
-    fmt_collection(buf, indent, '[', ']', loc_entries, Newlines::No)
+    fmt_collection(buf, indent + INDENT, '[', ']', loc_entries, Newlines::No)
 }
 
 pub trait FormatName {
@@ -300,7 +306,8 @@ impl<'a> Formattable for ExposedName<'a> {
         false
     }
 
-    fn format<'buf>(&self, buf: &mut Buf<'buf>, _indent: u16) {
+    fn format<'buf>(&self, buf: &mut Buf<'buf>, indent: u16) {
+        buf.indent(indent);
         buf.push_str(self.as_str());
     }
 }
@@ -348,6 +355,8 @@ fn fmt_packages_entry<'a, 'buf>(buf: &mut Buf<'buf>, entry: &PackageEntry<'a>, i
 fn fmt_imports_entry<'a, 'buf>(buf: &mut Buf<'buf>, entry: &ImportsEntry<'a>, indent: u16) {
     use roc_parse::header::ImportsEntry::*;
 
+    buf.indent(indent);
+
     match entry {
         Module(module, loc_exposes_entries) => {
             buf.push_str(module.as_str());

compiler/fmt/tests/test_fmt.rs

@@ -2640,6 +2640,25 @@ mod test_fmt {
         ));
     }
 
+    #[test]
+    fn multi_line_interface() {
+        module_formats_same(indoc!(
+            r#"
+                interface Foo
+                    exposes
+                        [
+                            Stuff,
+                            Things,
+                            somethingElse,
+                        ]
+                    imports
+                        [
+                            Blah,
+                            Baz.{ stuff, things },
+                        ]"#
+        ));
+    }
+
     #[test]
     fn single_line_app() {
         module_formats_same(indoc!(
@@ -2668,6 +2687,31 @@ mod test_fmt {
         ));
     }
 
+    #[test]
+    fn multi_line_hosted() {
+        module_formats_same(indoc!(
+            r#"
+                hosted Foo
+                    exposes
+                        [
+                            Stuff,
+                            Things,
+                            somethingElse,
+                        ]
+                    imports
+                        [
+                            Blah,
+                            Baz.{ stuff, things },
+                        ]
+                    generates Bar with
+                        [
+                            map,
+                            after,
+                            loop,
+                        ]"#
+        ));
+    }
+
     /// Annotations and aliases
 
     #[test]

examples/effect/thing/platform-dir/Package-Config.roc

@@ -1,4 +1,4 @@
-platform "folkertdev/foo"
+platform "roc-examples/cli"
     requires {} { main : Effect {} }
     exposes []
     packages {}

examples/false-interpreter/platform/Package-Config.roc

@@ -1,5 +1,5 @@
 platform "examples/cli"
-    requires {} { main : Str -> Task {} [] }# TODO FIXME
+    requires {} { main : Str -> Task {} [] }
     exposes []
     packages {}
     imports [ Task.{ Task } ]