Merge pull request #2069 from rtfeldman/joshuawarner32/impl-app-header-formatting

Implement [more] app header formatting

compiler/fmt/src/annotation.rs

@@ -55,6 +55,30 @@ pub trait Formattable<'a> {
     }
 }
 
+/// A reference to a formattable value is also formattable
+impl<'a, T> Formattable<'a> for &'a T
+where
+    T: Formattable<'a>,
+{
+    fn is_multiline(&self) -> bool {
+        (*self).is_multiline()
+    }
+
+    fn format_with_options(
+        &self,
+        buf: &mut String<'a>,
+        parens: Parens,
+        newlines: Newlines,
+        indent: u16,
+    ) {
+        (*self).format_with_options(buf, parens, newlines, indent)
+    }
+
+    fn format(&self, buf: &mut String<'a>, indent: u16) {
+        (*self).format(buf, indent)
+    }
+}
+
 /// A Located formattable value is also formattable
 impl<'a, T> Formattable<'a> for Located<T>
 where

compiler/fmt/src/collection.rs

@@ -0,0 +1,49 @@
+use bumpalo::collections::String;
+use roc_parse::ast::Collection;
+
+use crate::{
+    annotation::{Formattable, Newlines, Parens},
+    spaces::{fmt_comments_only, newline, NewlineAt, INDENT},
+};
+
+pub struct CollectionConfig {
+    pub begin: char,
+    pub end: char,
+    pub delimiter: char,
+}
+
+pub fn fmt_collection<'a, F: Formattable<'a>>(
+    buf: &mut String<'a>,
+    items: Collection<'a, F>,
+    indent: u16,
+    config: CollectionConfig,
+) {
+    let loc_items = items.items;
+    let final_comments = items.final_comments();
+    buf.push(config.begin);
+    if !loc_items.is_empty() || !final_comments.iter().all(|c| c.is_newline()) {
+        let is_multiline = loc_items.iter().any(|item| item.is_multiline());
+        if is_multiline {
+            let item_indent = indent + INDENT;
+            for item in loc_items.iter() {
+                newline(buf, item_indent);
+                item.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, item_indent);
+                buf.push(config.delimiter);
+            }
+            fmt_comments_only(buf, final_comments.iter(), NewlineAt::Top, item_indent);
+            newline(buf, indent);
+        } else {
+            // is_multiline == false
+            let mut iter = loc_items.iter().peekable();
+            while let Some(item) = iter.next() {
+                buf.push(' ');
+                item.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, indent);
+                if iter.peek().is_some() {
+                    buf.push(config.delimiter);
+                }
+            }
+            buf.push(' ');
+        }
+    }
+    buf.push(config.end);
+}

compiler/fmt/src/expr.rs

@@ -4,10 +4,10 @@ use crate::pattern::fmt_pattern;
 use crate::spaces::{add_spaces, fmt_comments_only, fmt_spaces, newline, NewlineAt, INDENT};
 use bumpalo::collections::String;
 use roc_module::called_via::{self, BinOp};
-use roc_parse::ast::StrSegment;
 use roc_parse::ast::{
     AssignedField, Base, Collection, CommentOrNewline, Expr, Pattern, WhenBranch,
 };
+use roc_parse::ast::{StrLiteral, StrSegment};
 use roc_region::all::Located;
 
 impl<'a> Formattable<'a> for Expr<'a> {
@@ -144,53 +144,7 @@ impl<'a> Formattable<'a> for Expr<'a> {
                 }
             }
             Str(literal) => {
-                use roc_parse::ast::StrLiteral::*;
+                fmt_str_literal(buf, *literal, indent);
-
-                buf.push('"');
-                match literal {
-                    PlainLine(string) => {
-                        buf.push_str(string);
-                    }
-                    Line(segments) => {
-                        for seg in segments.iter() {
-                            format_str_segment(seg, buf, 0)
-                        }
-                    }
-                    Block(lines) => {
-                        buf.push_str("\"\"");
-
-                        if lines.len() > 1 {
-                            // Since we have multiple lines, format this with
-                            // the `"""` symbols on their own lines, and the
-                            newline(buf, indent);
-
-                            for segments in lines.iter() {
-                                for seg in segments.iter() {
-                                    format_str_segment(seg, buf, indent);
-                                }
-
-                                newline(buf, indent);
-                            }
-                        } else {
-                            // This is a single-line block string, for example:
-                            //
-                            //     """Whee, "quotes" inside quotes!"""
-
-                            // This loop will run either 0 or 1 times.
-                            for segments in lines.iter() {
-                                for seg in segments.iter() {
-                                    format_str_segment(seg, buf, indent);
-                                }
-
-                                // Don't print a newline here, because we either
-                                // just printed 1 or 0 lines.
-                            }
-                        }
-
-                        buf.push_str("\"\"");
-                    }
-                }
-                buf.push('"');
             }
             Var { module_name, ident } => {
                 if !module_name.is_empty() {
@@ -377,6 +331,56 @@ fn push_op(buf: &mut String, op: BinOp) {
     }
 }
 
+pub fn fmt_str_literal<'a>(buf: &mut String<'a>, literal: StrLiteral<'a>, indent: u16) {
+    use roc_parse::ast::StrLiteral::*;
+
+    buf.push('"');
+    match literal {
+        PlainLine(string) => {
+            buf.push_str(string);
+        }
+        Line(segments) => {
+            for seg in segments.iter() {
+                format_str_segment(seg, buf, 0)
+            }
+        }
+        Block(lines) => {
+            buf.push_str("\"\"");
+
+            if lines.len() > 1 {
+                // Since we have multiple lines, format this with
+                // the `"""` symbols on their own lines, and the
+                newline(buf, indent);
+
+                for segments in lines.iter() {
+                    for seg in segments.iter() {
+                        format_str_segment(seg, buf, indent);
+                    }
+
+                    newline(buf, indent);
+                }
+            } else {
+                // This is a single-line block string, for example:
+                //
+                //     """Whee, "quotes" inside quotes!"""
+
+                // This loop will run either 0 or 1 times.
+                for segments in lines.iter() {
+                    for seg in segments.iter() {
+                        format_str_segment(seg, buf, indent);
+                    }
+
+                    // Don't print a newline here, because we either
+                    // just printed 1 or 0 lines.
+                }
+            }
+
+            buf.push_str("\"\"");
+        }
+    }
+    buf.push('"');
+}
+
 fn fmt_bin_ops<'a>(
     buf: &mut String<'a>,
     lefts: &'a [(Located<Expr<'a>>, Located<BinOp>)],

compiler/fmt/src/lib.rs

@@ -2,6 +2,7 @@
 // See github.com/rtfeldman/roc/issues/800 for discussion of the large_enum_variant check.
 #![allow(clippy::large_enum_variant)]
 pub mod annotation;
+pub mod collection;
 pub mod def;
 pub mod expr;
 pub mod module;

compiler/fmt/src/module.rs

@@ -1,7 +1,13 @@
-use crate::spaces::{fmt_spaces, INDENT};
+use crate::annotation::Formattable;
+use crate::collection::{fmt_collection, CollectionConfig};
+use crate::expr::fmt_str_literal;
+use crate::spaces::{fmt_default_spaces, fmt_spaces, INDENT};
 use bumpalo::collections::String;
 use roc_parse::ast::{Collection, Module};
-use roc_parse::header::{AppHeader, ExposesEntry, ImportsEntry, InterfaceHeader, PlatformHeader};
+use roc_parse::header::{
+    AppHeader, ExposesEntry, ImportsEntry, InterfaceHeader, PackageEntry, PackageOrPath,
+    PlatformHeader, To,
+};
 use roc_region::all::Located;
 
 pub fn fmt_module<'a>(buf: &mut String<'a>, module: &'a Module<'a>) {
@@ -24,46 +30,19 @@ pub fn fmt_interface_header<'a>(buf: &mut String<'a>, header: &'a InterfaceHeade
     buf.push_str("interface");
 
     // module name
-    if header.after_interface_keyword.is_empty() {
+    fmt_default_spaces(buf, header.after_interface_keyword, " ", indent);
-        buf.push(' ');
-    } else {
-        fmt_spaces(buf, header.after_interface_keyword.iter(), indent);
-    }
-
     buf.push_str(header.name.value.as_str());
 
     // exposes
-    if header.before_exposes.is_empty() {
+    fmt_default_spaces(buf, header.before_exposes, " ", indent);
-        buf.push(' ');
-    } else {
-        fmt_spaces(buf, header.before_exposes.iter(), indent);
-    }
-
     buf.push_str("exposes");
-
+    fmt_default_spaces(buf, header.after_exposes, " ", indent);
-    if header.after_exposes.is_empty() {
-        buf.push(' ');
-    } else {
-        fmt_spaces(buf, header.after_exposes.iter(), indent);
-    }
-
     fmt_exposes(buf, &header.exposes, indent);
 
     // imports
-    if header.before_imports.is_empty() {
+    fmt_default_spaces(buf, header.before_imports, " ", indent);
-        buf.push(' ');
-    } else {
-        fmt_spaces(buf, header.before_imports.iter(), indent);
-    }
-
     buf.push_str("imports");
-
+    fmt_default_spaces(buf, header.after_imports, " ", indent);
-    if header.after_imports.is_empty() {
-        buf.push(' ');
-    } else {
-        fmt_spaces(buf, header.after_imports.iter(), indent);
-    }
-
     fmt_imports(buf, header.imports, indent);
 }
 
@@ -72,12 +51,30 @@ pub fn fmt_app_header<'a>(buf: &mut String<'a>, header: &'a AppHeader<'a>) {
 
     buf.push_str("app");
 
-    // imports
+    fmt_default_spaces(buf, header.after_app_keyword, " ", indent);
-    buf.push_str("imports");
+    fmt_str_literal(buf, header.name.value, indent);
 
-    fmt_spaces(buf, header.before_imports.iter(), indent);
+    // packages
+    fmt_default_spaces(buf, header.before_packages, " ", indent);
+    buf.push_str("packages");
+    fmt_default_spaces(buf, header.after_packages, " ", indent);
+    fmt_packages(buf, header.packages, indent);
+
+    // imports
+    fmt_default_spaces(buf, header.before_imports, " ", indent);
+    buf.push_str("imports");
+    fmt_default_spaces(buf, header.after_imports, " ", indent);
     fmt_imports(buf, header.imports, indent);
-    fmt_spaces(buf, header.after_imports.iter(), indent);
+
+    // provides
+    fmt_default_spaces(buf, header.before_provides, " ", indent);
+    buf.push_str("provides");
+    fmt_default_spaces(buf, header.after_provides, " ", indent);
+    fmt_provides(buf, header.provides, indent);
+    fmt_default_spaces(buf, header.before_to, " ", indent);
+    buf.push_str("to");
+    fmt_default_spaces(buf, header.after_to, " ", indent);
+    fmt_to(buf, header.to.value, indent);
 }
 
 pub fn fmt_platform_header<'a>(_buf: &mut String<'a>, _header: &'a PlatformHeader<'a>) {
@@ -89,25 +86,42 @@ fn fmt_imports<'a>(
     loc_entries: Collection<'a, Located<ImportsEntry<'a>>>,
     indent: u16,
 ) {
-    buf.push('[');
+    fmt_collection(
+        buf,
+        loc_entries,
+        indent,
+        CollectionConfig {
+            begin: '[',
+            end: ']',
+            delimiter: ',',
+        },
+    );
+}
 
-    if !loc_entries.is_empty() {
+fn fmt_provides<'a>(
-        buf.push(' ');
+    buf: &mut String<'a>,
-    }
+    loc_entries: Collection<'a, Located<ExposesEntry<'a, &'a str>>>,
+    indent: u16,
+) {
+    fmt_collection(
+        buf,
+        loc_entries,
+        indent,
+        CollectionConfig {
+            begin: '[',
+            end: ']',
+            delimiter: ',',
+        },
+    );
+}
 
-    for (index, loc_entry) in loc_entries.iter().enumerate() {
+fn fmt_to<'a>(buf: &mut String<'a>, to: To<'a>, indent: u16) {
-        if index > 0 {
+    match to {
-            buf.push_str(", ");
+        To::ExistingPackage(name) => {
+            buf.push_str(name);
         }
-
+        To::NewPackage(package_or_path) => fmt_package_or_path(buf, &package_or_path, indent),
-        fmt_imports_entry(buf, &loc_entry.value, indent);
     }
-
-    if !loc_entries.is_empty() {
-        buf.push(' ');
-    }
-
-    buf.push(']');
 }
 
 fn fmt_exposes<'a>(
@@ -115,28 +129,29 @@ fn fmt_exposes<'a>(
     loc_entries: &'a Collection<'a, Located<ExposesEntry<'a, &'a str>>>,
     indent: u16,
 ) {
-    buf.push('[');
+    fmt_collection(
-
+        buf,
-    if !loc_entries.is_empty() {
+        *loc_entries,
-        buf.push(' ');
+        indent,
-    }
+        CollectionConfig {
-
+            begin: '[',
-    for (index, loc_entry) in loc_entries.iter().enumerate() {
+            end: ']',
-        if index > 0 {
+            delimiter: ',',
-            buf.push_str(", ");
+        },
-        }
+    );
-
-        fmt_exposes_entry(buf, &loc_entry.value, indent);
-    }
-
-    if !loc_entries.is_empty() {
-        buf.push(' ');
-    }
-
-    buf.push(']');
 }
 
-fn fmt_exposes_entry<'a>(buf: &mut String<'a>, entry: &'a ExposesEntry<'a, &'a str>, indent: u16) {
+impl<'a> Formattable<'a> for ExposesEntry<'a, &'a str> {
+    fn is_multiline(&self) -> bool {
+        false
+    }
+
+    fn format(&self, buf: &mut String<'a>, indent: u16) {
+        fmt_exposes_entry(buf, self, indent);
+    }
+}
+
+fn fmt_exposes_entry<'a>(buf: &mut String<'a>, entry: &ExposesEntry<'a, &'a str>, indent: u16) {
     use roc_parse::header::ExposesEntry::*;
 
     match entry {
@@ -153,7 +168,76 @@ fn fmt_exposes_entry<'a>(buf: &mut String<'a>, entry: &'a ExposesEntry<'a, &'a s
     }
 }
 
-fn fmt_imports_entry<'a>(buf: &mut String<'a>, entry: &'a ImportsEntry<'a>, indent: u16) {
+fn fmt_packages<'a>(
+    buf: &mut String<'a>,
+    loc_entries: Collection<'a, Located<PackageEntry<'a>>>,
+    indent: u16,
+) {
+    fmt_collection(
+        buf,
+        loc_entries,
+        indent,
+        CollectionConfig {
+            begin: '{',
+            end: '}',
+            delimiter: ',',
+        },
+    );
+}
+
+impl<'a> Formattable<'a> for PackageEntry<'a> {
+    fn is_multiline(&self) -> bool {
+        false
+    }
+
+    fn format(&self, buf: &mut String<'a>, indent: u16) {
+        fmt_packages_entry(buf, self, indent);
+    }
+}
+
+impl<'a> Formattable<'a> for ImportsEntry<'a> {
+    fn is_multiline(&self) -> bool {
+        false
+    }
+
+    fn format(&self, buf: &mut String<'a>, indent: u16) {
+        fmt_imports_entry(buf, self, indent);
+    }
+}
+fn fmt_packages_entry<'a>(buf: &mut String<'a>, entry: &PackageEntry<'a>, indent: u16) {
+    use PackageEntry::*;
+    match entry {
+        Entry {
+            shorthand,
+            spaces_after_shorthand,
+            package_or_path,
+        } => {
+            buf.push_str(shorthand);
+            buf.push(':');
+            fmt_default_spaces(buf, spaces_after_shorthand, " ", indent);
+            fmt_package_or_path(buf, &package_or_path.value, indent);
+        }
+        SpaceBefore(sub_entry, spaces) => {
+            fmt_spaces(buf, spaces.iter(), indent);
+            fmt_packages_entry(buf, sub_entry, indent);
+        }
+        SpaceAfter(sub_entry, spaces) => {
+            fmt_packages_entry(buf, sub_entry, indent);
+            fmt_spaces(buf, spaces.iter(), indent);
+        }
+    }
+}
+
+fn fmt_package_or_path<'a>(buf: &mut String<'a>, package_or_path: &PackageOrPath<'a>, indent: u16) {
+    match package_or_path {
+        PackageOrPath::Package(_name, _version) => {
+            todo!("format package");
+        }
+        PackageOrPath::Path(str_literal) => fmt_str_literal(buf, *str_literal, indent),
+    }
+}
+
+fn fmt_imports_entry<'a>(buf: &mut String<'a>, entry: &ImportsEntry<'a>, indent: u16) {
     use roc_parse::header::ImportsEntry::*;
 
     match entry {
@@ -161,17 +245,18 @@ fn fmt_imports_entry<'a>(buf: &mut String<'a>, entry: &'a ImportsEntry<'a>, inde
             buf.push_str(module.as_str());
 
             if !loc_exposes_entries.is_empty() {
-                buf.push_str(".{ ");
+                buf.push('.');
 
-                for (index, loc_entry) in loc_exposes_entries.iter().enumerate() {
+                fmt_collection(
-                    if index > 0 {
+                    buf,
-                        buf.push_str(", ");
+                    *loc_exposes_entries,
-                    }
+                    indent,
-
+                    CollectionConfig {
-                    fmt_exposes_entry(buf, &loc_entry.value, indent);
+                        begin: '{',
-                }
+                        end: '}',
-
+                        delimiter: ',',
-                buf.push_str(" }");
+                    },
+                );
             }
         }
 

compiler/fmt/src/spaces.rs

@@ -16,9 +16,22 @@ pub fn add_spaces(buf: &mut String<'_>, spaces: u16) {
     }
 }
 
-pub fn fmt_spaces<'a, I>(buf: &mut String<'a>, spaces: I, indent: u16)
+pub fn fmt_default_spaces<'a>(
+    buf: &mut String<'a>,
+    spaces: &[CommentOrNewline<'a>],
+    default: &str,
+    indent: u16,
+) {
+    if spaces.is_empty() {
+        buf.push_str(default);
+    } else {
+        fmt_spaces(buf, spaces.iter(), indent);
+    }
+}
+
+pub fn fmt_spaces<'b, 'a: 'b, I>(buf: &mut String<'a>, spaces: I, indent: u16)
 where
-    I: Iterator<Item = &'a CommentOrNewline<'a>>,
+    I: Iterator<Item = &'b CommentOrNewline<'a>>,
 {
     use self::CommentOrNewline::*;
 

compiler/fmt/tests/test_fmt.rs

@@ -2480,6 +2480,15 @@ mod test_fmt {
         ));
     }
 
+    #[test]
+    fn single_line_app() {
+        module_formats_same(indoc!(
+            r#"
+                app "Foo" packages { base: "platform" } imports [] provides [ main ] to base
+            "#
+        ));
+    }
+
     /// Annotations and aliases
 
     #[test]

compiler/load/src/file.rs

@@ -1747,7 +1747,7 @@ fn update<'a>(
             match header_extra {
                 App { to_platform } => {
                     debug_assert!(matches!(state.platform_path, PlatformPath::NotSpecified));
-                    state.platform_path = PlatformPath::Valid(to_platform.clone());
+                    state.platform_path = PlatformPath::Valid(to_platform);
                 }
                 PkgConfig { main_for_host, .. } => {
                     debug_assert!(matches!(state.platform_data, None));
@@ -2632,7 +2632,7 @@ fn parse_header<'a>(
                 packages,
                 exposes: header.provides.items,
                 imports: header.imports.items,
-                to_platform: Some(header.to.value.clone()),
+                to_platform: Some(header.to.value),
             };
 
             let (module_id, app_module_header_msg) = send_header(

compiler/parse/src/header.rs

@@ -72,7 +72,7 @@ pub struct InterfaceHeader<'a> {
     pub after_imports: &'a [CommentOrNewline<'a>],
 }
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq)]
 pub enum To<'a> {
     ExistingPackage(&'a str),
     NewPackage(PackageOrPath<'a>),