From 24f7692a7335a83bb2d8bb70ea1268f2b3dd983b Mon Sep 17 00:00:00 2001 From: Joshua Warner Date: Mon, 22 Nov 2021 16:45:21 -0800 Subject: [PATCH] Implement [more] app header formatting Also, refactor out a utility method to format a Collection. This method can currently replace some of the formatting done in module headers - but the goal is eventually to be able to replace the code in fmt_list as well, such that there is 'one true way' to format collections. --- compiler/fmt/src/annotation.rs | 24 ++++ compiler/fmt/src/collection.rs | 49 +++++++ compiler/fmt/src/expr.rs | 100 ++++++------- compiler/fmt/src/lib.rs | 1 + compiler/fmt/src/module.rs | 253 ++++++++++++++++++++++----------- compiler/fmt/src/spaces.rs | 17 ++- compiler/fmt/tests/test_fmt.rs | 9 ++ compiler/load/src/file.rs | 4 +- compiler/parse/src/header.rs | 2 +- 9 files changed, 322 insertions(+), 137 deletions(-) create mode 100644 compiler/fmt/src/collection.rs diff --git a/compiler/fmt/src/annotation.rs b/compiler/fmt/src/annotation.rs index 98d156415d..5f6887965c 100644 --- a/compiler/fmt/src/annotation.rs +++ b/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 where diff --git a/compiler/fmt/src/collection.rs b/compiler/fmt/src/collection.rs new file mode 100644 index 0000000000..3128c4dcf3 --- /dev/null +++ b/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); +} diff --git a/compiler/fmt/src/expr.rs b/compiler/fmt/src/expr.rs index cb356ff53a..7b5984706e 100644 --- a/compiler/fmt/src/expr.rs +++ b/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::*; - - 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('"'); + fmt_str_literal(buf, *literal, indent); } 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>, Located)], diff --git a/compiler/fmt/src/lib.rs b/compiler/fmt/src/lib.rs index 23b40379d5..eb7377d6d8 100644 --- a/compiler/fmt/src/lib.rs +++ b/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; diff --git a/compiler/fmt/src/module.rs b/compiler/fmt/src/module.rs index 17522326c2..3a9a8a495b 100644 --- a/compiler/fmt/src/module.rs +++ b/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() { - buf.push(' '); - } else { - fmt_spaces(buf, header.after_interface_keyword.iter(), indent); - } - + fmt_default_spaces(buf, header.after_interface_keyword, " ", indent); buf.push_str(header.name.value.as_str()); // exposes - if header.before_exposes.is_empty() { - buf.push(' '); - } else { - fmt_spaces(buf, header.before_exposes.iter(), indent); - } - + fmt_default_spaces(buf, header.before_exposes, " ", indent); buf.push_str("exposes"); - - if header.after_exposes.is_empty() { - buf.push(' '); - } else { - fmt_spaces(buf, header.after_exposes.iter(), indent); - } - + fmt_default_spaces(buf, header.after_exposes, " ", indent); fmt_exposes(buf, &header.exposes, indent); // imports - if header.before_imports.is_empty() { - buf.push(' '); - } else { - fmt_spaces(buf, header.before_imports.iter(), indent); - } - + fmt_default_spaces(buf, header.before_imports, " ", indent); buf.push_str("imports"); - - if header.after_imports.is_empty() { - buf.push(' '); - } else { - fmt_spaces(buf, header.after_imports.iter(), indent); - } - + fmt_default_spaces(buf, header.after_imports, " ", 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 - buf.push_str("imports"); + fmt_default_spaces(buf, header.after_app_keyword, " ", indent); + 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>>, indent: u16, ) { - buf.push('['); + fmt_collection( + buf, + loc_entries, + indent, + CollectionConfig { + begin: '[', + end: ']', + delimiter: ',', + }, + ); +} - if !loc_entries.is_empty() { - buf.push(' '); - } +fn fmt_provides<'a>( + buf: &mut String<'a>, + loc_entries: Collection<'a, Located>>, + indent: u16, +) { + fmt_collection( + buf, + loc_entries, + indent, + CollectionConfig { + begin: '[', + end: ']', + delimiter: ',', + }, + ); +} - for (index, loc_entry) in loc_entries.iter().enumerate() { - if index > 0 { - buf.push_str(", "); +fn fmt_to<'a>(buf: &mut String<'a>, to: To<'a>, indent: u16) { + match to { + To::ExistingPackage(name) => { + buf.push_str(name); } - - fmt_imports_entry(buf, &loc_entry.value, indent); + To::NewPackage(package_or_path) => fmt_package_or_path(buf, &package_or_path, 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>>, indent: u16, ) { - buf.push('['); - - if !loc_entries.is_empty() { - buf.push(' '); - } - - for (index, loc_entry) in loc_entries.iter().enumerate() { - if index > 0 { - buf.push_str(", "); - } - - fmt_exposes_entry(buf, &loc_entry.value, indent); - } - - if !loc_entries.is_empty() { - buf.push(' '); - } - - buf.push(']'); + fmt_collection( + buf, + *loc_entries, + indent, + CollectionConfig { + begin: '[', + end: ']', + delimiter: ',', + }, + ); } -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>>, + 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() { - if index > 0 { - buf.push_str(", "); - } - - fmt_exposes_entry(buf, &loc_entry.value, indent); - } - - buf.push_str(" }"); + fmt_collection( + buf, + *loc_exposes_entries, + indent, + CollectionConfig { + begin: '{', + end: '}', + delimiter: ',', + }, + ); } } diff --git a/compiler/fmt/src/spaces.rs b/compiler/fmt/src/spaces.rs index db7d083493..afb8bc52db 100644 --- a/compiler/fmt/src/spaces.rs +++ b/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>, + I: Iterator>, { use self::CommentOrNewline::*; diff --git a/compiler/fmt/tests/test_fmt.rs b/compiler/fmt/tests/test_fmt.rs index db0a632508..689784559b 100644 --- a/compiler/fmt/tests/test_fmt.rs +++ b/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] diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 06e66705c9..b8b54b9536 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -1743,7 +1743,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)); @@ -2628,7 +2628,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( diff --git a/compiler/parse/src/header.rs b/compiler/parse/src/header.rs index b1fbcc23eb..404e0cc601 100644 --- a/compiler/parse/src/header.rs +++ b/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>),