From 4707528caa894ddea7026a9e99534c72f898f27c Mon Sep 17 00:00:00 2001 From: Sean Hagstrom Date: Sun, 1 May 2022 18:28:49 +0100 Subject: [PATCH] feature(formatter): allow single blank line around comments in lists, records, type annotations --- compiler/fmt/src/collection.rs | 24 +- compiler/fmt/src/expr.rs | 29 +- compiler/fmt/src/spaces.rs | 25 ++ compiler/fmt/tests/test_fmt.rs | 709 +++++++++++++++++++++++++++++++++ 4 files changed, 781 insertions(+), 6 deletions(-) diff --git a/compiler/fmt/src/collection.rs b/compiler/fmt/src/collection.rs index 9e54b327cf..836ad04210 100644 --- a/compiler/fmt/src/collection.rs +++ b/compiler/fmt/src/collection.rs @@ -2,7 +2,7 @@ use roc_parse::ast::{Collection, ExtractSpaces}; use crate::{ annotation::{Formattable, Newlines}, - spaces::{fmt_comments_only, NewlineAt, INDENT}, + spaces::{count_leading_newlines, fmt_comments_only, NewlineAt, INDENT}, Buf, }; @@ -25,12 +25,27 @@ pub fn fmt_collection<'a, 'buf, T: ExtractSpaces<'a> + Formattable>( buf.indent(braces_indent); buf.push(start); - for item in items.iter() { + for (index, item) in items.iter().enumerate() { let item = item.extract_spaces(); + let is_first_item = index == 0; buf.newline(); + if !item.before.is_empty() { + let is_only_newlines = item.before.iter().all(|s| s.is_newline()); + + if !is_first_item + && !is_only_newlines + && count_leading_newlines(item.before.iter()) > 1 + { + buf.newline(); + } + fmt_comments_only(buf, item.before.iter(), NewlineAt::Bottom, item_indent); + + if !is_only_newlines && count_leading_newlines(item.before.iter().rev()) > 0 { + buf.newline(); + } } item.item.format(buf, item_indent); @@ -41,6 +56,11 @@ pub fn fmt_collection<'a, 'buf, T: ExtractSpaces<'a> + Formattable>( fmt_comments_only(buf, item.after.iter(), NewlineAt::Top, item_indent); } } + + if count_leading_newlines(items.final_comments().iter()) > 1 { + buf.newline(); + } + fmt_comments_only( buf, items.final_comments().iter(), diff --git a/compiler/fmt/src/expr.rs b/compiler/fmt/src/expr.rs index fecb7fee37..7e038bc148 100644 --- a/compiler/fmt/src/expr.rs +++ b/compiler/fmt/src/expr.rs @@ -2,7 +2,7 @@ use crate::annotation::{Formattable, Newlines, Parens}; use crate::collection::fmt_collection; use crate::def::fmt_def; use crate::pattern::fmt_pattern; -use crate::spaces::{fmt_comments_only, fmt_spaces, NewlineAt, INDENT}; +use crate::spaces::{count_leading_newlines, fmt_comments_only, fmt_spaces, NewlineAt, INDENT}; use crate::Buf; use roc_module::called_via::{self, BinOp}; use roc_parse::ast::{ @@ -1105,16 +1105,38 @@ fn fmt_record<'a, 'buf>( if is_multiline { let field_indent = indent + INDENT; - for field in loc_fields.iter() { + for (index, field) in loc_fields.iter().enumerate() { // comma addition is handled by the `format_field_multiline` function // since we can have stuff like: // { x # comment // , y // } // In this case, we have to move the comma before the comment. + + let is_first_item = index == 0; + if let AssignedField::SpaceBefore(_sub_field, spaces) = &field.value { + let is_only_newlines = spaces.iter().all(|s| s.is_newline()); + if !is_first_item + && !is_only_newlines + && count_leading_newlines(spaces.iter()) > 1 + { + buf.newline(); + } + + fmt_comments_only(buf, spaces.iter(), NewlineAt::Top, field_indent); + + if !is_only_newlines && count_leading_newlines(spaces.iter().rev()) > 0 { + buf.newline(); + } + } + format_field_multiline(buf, &field.value, field_indent, ""); } + if count_leading_newlines(final_comments.iter()) > 1 { + buf.newline(); + } + fmt_comments_only(buf, final_comments.iter(), NewlineAt::Top, field_indent); buf.newline(); @@ -1189,7 +1211,7 @@ fn format_field_multiline<'a, 'buf, T>( buf.push_str(name.value); buf.push(','); } - AssignedField::SpaceBefore(sub_field, spaces) => { + AssignedField::SpaceBefore(sub_field, _spaces) => { // We have something like that: // ``` // # comment @@ -1197,7 +1219,6 @@ fn format_field_multiline<'a, 'buf, T>( // ``` // we'd like to preserve this - fmt_comments_only(buf, spaces.iter(), NewlineAt::Top, indent); format_field_multiline(buf, sub_field, indent, separator_prefix); } AssignedField::SpaceAfter(sub_field, spaces) => { diff --git a/compiler/fmt/src/spaces.rs b/compiler/fmt/src/spaces.rs index 4cb4f6fb8a..5ce9a64121 100644 --- a/compiler/fmt/src/spaces.rs +++ b/compiler/fmt/src/spaces.rs @@ -117,6 +117,31 @@ fn fmt_comment<'buf>(buf: &mut Buf<'buf>, comment: &str) { buf.push_str(comment.trim_end()); } +pub fn count_leading_newlines<'a, I>(data: I) -> u16 +where + I: Iterator>, +{ + let mut count = 0; + let mut allow_counting = false; + + for (index, val) in data.enumerate() { + let is_first = index == 0; + let is_newline = matches!(val, CommentOrNewline::Newline); + + if is_first && is_newline { + allow_counting = true + } + + if is_newline && allow_counting { + count += 1; + } else { + break; + } + } + + count +} + fn fmt_docs<'buf>(buf: &mut Buf<'buf>, docs: &str) { buf.push_str("##"); if !docs.starts_with(' ') { diff --git a/compiler/fmt/tests/test_fmt.rs b/compiler/fmt/tests/test_fmt.rs index 51a87c5797..3aae1b480e 100644 --- a/compiler/fmt/tests/test_fmt.rs +++ b/compiler/fmt/tests/test_fmt.rs @@ -192,6 +192,714 @@ mod test_fmt { ); } + #[test] + fn type_annotation_allow_blank_line_before_and_after_comment() { + expr_formats_same(indoc!( + r#" + person : + { + firstName : Str, + # comment + lastName : Str, + } + + person + "# + )); + + expr_formats_same(indoc!( + r#" + person : + { + firstName : Str, + + # comment + lastName : Str, + } + + person + "# + )); + + expr_formats_same(indoc!( + r#" + person : + { + firstName : Str, + # comment + + lastName : Str, + } + + person + "# + )); + + expr_formats_same(indoc!( + r#" + person : + { + firstName : Str, + + # comment + + lastName : Str, + } + + person + "# + )); + + expr_formats_same(indoc!( + r#" + person : + { + firstName : Str, + + # comment 1 + + lastName : Str, + + # comment 2 + # comment 3 + } + + person + "# + )); + + expr_formats_same(indoc!( + r#" + person : + { + firstName : Str, + + # comment 1 + + lastName : Str, + # comment 2 + # comment 3 + } + + person + "# + )); + + expr_formats_to( + indoc!( + r#" + person : + { + + # comment + + firstName : Str, + lastName : Str, + } + + person + "# + ), + indoc!( + r#" + person : + { + # comment + + firstName : Str, + lastName : Str, + } + + person + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + person : + { + firstName : Str, + lastName : Str, + + # comment + + } + + person + "# + ), + indoc!( + r#" + person : + { + firstName : Str, + lastName : Str, + + # comment + } + + person + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + person : + { + firstName : Str, + + + # comment + lastName : Str, + } + + person + "# + ), + indoc!( + r#" + person : + { + firstName : Str, + + # comment + lastName : Str, + } + + person + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + person : + { + firstName : Str, + # comment + + + lastName : Str, + } + + person + "# + ), + indoc!( + r#" + person : + { + firstName : Str, + # comment + + lastName : Str, + } + + person + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + person : + { + firstName : Str, + + + # comment + + + lastName : Str, + } + + person + "# + ), + indoc!( + r#" + person : + { + firstName : Str, + + # comment + + lastName : Str, + } + + person + "# + ), + ); + } + + #[test] + fn record_allow_blank_line_before_and_after_comment() { + expr_formats_same(indoc!( + r#" + person = { + firstName: "first", + # comment 1 + lastName: "last", + } + + person + "# + )); + + expr_formats_same(indoc!( + r#" + person = { + firstName: "first", + # comment 1 + + lastName: "last", + } + + person + "# + )); + + expr_formats_same(indoc!( + r#" + person = { + firstName: "first", + + # comment 1 + lastName: "last", + } + + person + "# + )); + + expr_formats_same(indoc!( + r#" + person = { + firstName: "first", + + # comment 1 + + lastName: "last", + } + + person + "# + )); + + expr_formats_same(indoc!( + r#" + person = { + firstName: "first", + + # comment 1 + + lastName: "last", + + # comment 2 + # comment 3 + } + + person + "# + )); + + expr_formats_same(indoc!( + r#" + person = { + firstName: "first", + + # comment 1 + + lastName: "last", + # comment 2 + # comment 3 + } + + person + "# + )); + + expr_formats_to( + indoc!( + r#" + person = { + + # comment + + firstName: "first", + lastName: "last", + } + + person + "# + ), + indoc!( + r#" + person = { + # comment + + firstName: "first", + lastName: "last", + } + + person + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + person = { + firstName: "first", + lastName: "last", + + # comment + + } + + person + "# + ), + indoc!( + r#" + person = { + firstName: "first", + lastName: "last", + + # comment + } + + person + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + person = { + firstName: "first", + + + # comment 1 + lastName: "last", + } + + person + "# + ), + indoc!( + r#" + person = { + firstName: "first", + + # comment 1 + lastName: "last", + } + + person + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + person = { + firstName: "first", + # comment 1 + + + lastName: "last", + } + + person + "# + ), + indoc!( + r#" + person = { + firstName: "first", + # comment 1 + + lastName: "last", + } + + person + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + person = { + firstName: "first", + + + # comment 1 + + + lastName: "last", + } + + person + "# + ), + indoc!( + r#" + person = { + firstName: "first", + + # comment 1 + + lastName: "last", + } + + person + "# + ), + ); + } + + #[test] + fn list_allow_blank_line_before_and_after_comment() { + expr_formats_same(indoc!( + r#" + list = [ + 0, + # comment + 1, + ] + + list + "# + )); + + expr_formats_same(indoc!( + r#" + list = [ + 0, + + # comment + 1, + ] + + list + "# + )); + + expr_formats_same(indoc!( + r#" + list = [ + 0, + # comment + + 1, + ] + + list + "# + )); + + expr_formats_same(indoc!( + r#" + list = [ + 0, + + # comment + + 1, + ] + + list + "# + )); + + expr_formats_same(indoc!( + r#" + list = [ + 0, + + # comment 1 + + 1, + + # comment 2 + # comment 3 + ] + + list + "# + )); + + expr_formats_same(indoc!( + r#" + list = [ + 0, + + # comment 1 + + 1, + # comment 2 + # comment 3 + ] + + list + "# + )); + expr_formats_to( + indoc!( + r#" + list = [ + + # comment + + 0, + 1, + ] + + list + "# + ), + indoc!( + r#" + list = [ + # comment + + 0, + 1, + ] + + list + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + list = [ + 0, + 1, + + # comment + + ] + + list + "# + ), + indoc!( + r#" + list = [ + 0, + 1, + + # comment + ] + + list + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + list = [ + 0, + + + # comment + 1, + ] + + list + "# + ), + indoc!( + r#" + list = [ + 0, + + # comment + 1, + ] + + list + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + list = [ + 0, + # comment + + + 1, + ] + + list + "# + ), + indoc!( + r#" + list = [ + 0, + # comment + + 1, + ] + + list + "# + ), + ); + + expr_formats_to( + indoc!( + r#" + list = [ + 0, + + + # comment + + + 1, + ] + + list + "# + ), + indoc!( + r#" + list = [ + 0, + + # comment + + 1, + ] + + list + "# + ), + ); + } + #[test] fn force_space_at_beginning_of_comment() { expr_formats_to( @@ -1648,6 +2356,7 @@ mod test_fmt { r#" [ # Thirty Seven + 37, # Thirty Eight 38,