Fix handling of spaces in record types

crates/compiler/fmt/src/annotation.rs

@@ -1,12 +1,15 @@
 use crate::{
     collection::{fmt_collection, Braces},
     expr::{format_spaces, merge_spaces_conservative},
-    node::{Node, Nodify, Sp},
+    node::{Node, NodeSequenceBuilder, Nodify, Sp},
     pattern::pattern_lift_spaces_after,
     spaces::{fmt_comments_only, fmt_spaces, NewlineAt, INDENT},
     Buf,
 };
-use bumpalo::{collections::Vec, Bump};
+use bumpalo::{
+    collections::{String, Vec},
+    Bump,
+};
 use roc_parse::ast::{
     AbilityImpls, AssignedField, Collection, CommentOrNewline, Expr, ExtractSpaces, FunctionArrow,
     ImplementsAbilities, ImplementsAbility, ImplementsClause, Spaceable, Spaces, SpacesAfter,
@@ -354,7 +357,7 @@ fn fmt_ty_ann(
         }
 
         TypeAnnotation::Record { fields, ext } => {
-            fmt_collection(buf, indent, Braces::Curly, *fields, newlines);
+            fmt_ty_field_collection(buf, indent, *fields, newlines);
             fmt_ext(ext, buf, indent);
         }
 
@@ -421,6 +424,37 @@ fn fmt_ty_ann(
     }
 }
 
+fn fmt_ty_field_collection(
+    buf: &mut Buf<'_>,
+    indent: u16,
+    fields: Collection<'_, Loc<AssignedField<'_, TypeAnnotation<'_>>>>,
+    newlines: Newlines,
+) {
+    let arena = buf.text.bump();
+    let mut new_items: Vec<'_, NodeSpaces<'_, Node<'_>>> =
+        Vec::with_capacity_in(fields.len(), arena);
+
+    let mut last_after: &[CommentOrNewline<'_>] = &[];
+
+    for item in fields.items.iter() {
+        let lifted = item.value.to_node(arena, Parens::NotNeeded);
+        let before = merge_spaces_conservative(arena, last_after, lifted.before);
+        last_after = lifted.after;
+        new_items.push(NodeSpaces {
+            before,
+            item: lifted.item,
+            after: &[],
+        });
+    }
+
+    let final_comments = merge_spaces_conservative(arena, last_after, fields.final_comments());
+
+    let new_items =
+        Collection::with_items_and_comments(arena, new_items.into_bump_slice(), final_comments);
+
+    fmt_collection(buf, indent, Braces::Curly, new_items, newlines);
+}
+
 fn fmt_tag_collection<'a>(
     buf: &mut Buf<'_>,
     indent: u16,
@@ -474,7 +508,7 @@ impl<'a> Nodify<'a> for Tag<'a> {
                         let lifted = arg.value.to_node(arena, Parens::InApply);
                         let before = merge_spaces_conservative(arena, last_after, lifted.before);
                         last_after = lifted.after;
-                        new_args.push((before, lifted.item));
+                        new_args.push((Sp::with_space(before), lifted.item));
                     }
 
                     Spaces {
@@ -642,6 +676,70 @@ impl<'a> Formattable for AssignedField<'a, Expr<'a>> {
     }
 }
 
+impl<'a> Nodify<'a> for AssignedField<'a, TypeAnnotation<'a>> {
+    fn to_node<'b>(&'a self, arena: &'b Bump, parens: Parens) -> Spaces<'b, Node<'b>>
+    where
+        'a: 'b,
+    {
+        match self {
+            AssignedField::RequiredValue(name, sp, value) => {
+                assigned_field_value_to_node(name.value, arena, sp, &value.value, ":")
+            }
+            AssignedField::IgnoredValue(name, sp, value) => {
+                let mut n = String::with_capacity_in(name.value.len() + 1, arena);
+                n.push('_');
+                n.push_str(name.value);
+                assigned_field_value_to_node(n.into_bump_str(), arena, sp, &value.value, ":")
+            }
+            AssignedField::OptionalValue(name, sp, value) => {
+                assigned_field_value_to_node(name.value, arena, sp, &value.value, "?")
+            }
+            AssignedField::LabelOnly(name) => Spaces {
+                before: &[],
+                item: Node::Literal(name.value),
+                after: &[],
+            },
+            AssignedField::SpaceBefore(inner, sp) => {
+                let mut inner = inner.to_node(arena, parens);
+                inner.before = merge_spaces_conservative(arena, sp, inner.before);
+                inner
+            }
+            AssignedField::SpaceAfter(inner, sp) => {
+                let mut inner = inner.to_node(arena, parens);
+                inner.after = merge_spaces_conservative(arena, inner.after, sp);
+                inner
+            }
+        }
+    }
+}
+
+fn assigned_field_value_to_node<'a, 'b>(
+    name: &'b str,
+    arena: &'b Bump,
+    sp: &'a [CommentOrNewline<'a>],
+    value: &'a TypeAnnotation<'a>,
+    sep: &'static str,
+) -> Spaces<'b, Node<'b>>
+where
+    'a: 'b,
+{
+    let first = Node::Literal(name);
+
+    let mut b = NodeSequenceBuilder::new(arena, first, 2);
+
+    b.push(Sp::with_space(sp), Node::Literal(sep));
+
+    let value_lifted = value.to_node(arena, Parens::NotNeeded);
+
+    b.push(Sp::with_space(value_lifted.before), value_lifted.item);
+
+    Spaces {
+        before: &[],
+        item: b.build(),
+        after: value_lifted.after,
+    }
+}
+
 fn is_multiline_assigned_field_help<T: Formattable>(afield: &AssignedField<'_, T>) -> bool {
     use self::AssignedField::*;
 
@@ -1274,8 +1372,8 @@ fn parens_around_node<'a, 'b: 'a>(
         before: &[],
         item: Node::DelimitedSequence(
             Braces::Round,
-            arena.alloc_slice_copy(&[(item.before, item.item)]),
-            &[],
+            arena.alloc_slice_copy(&[(item.before.into(), item.item)]),
+            Sp::empty(),
         ),
         // We move the comments/newlines to the outer scope, since they tend to migrate there when re-parsed
         after: item.after,

crates/compiler/fmt/src/node.rs

@@ -1,4 +1,4 @@
-use bumpalo::Bump;
+use bumpalo::{collections::Vec, Bump};
 use roc_parse::ast::{CommentOrNewline, Spaces, TypeAnnotation};
 
 use crate::{
@@ -8,7 +8,42 @@ use crate::{
     Buf,
 };
 
-pub type Sp<'a> = &'a [CommentOrNewline<'a>];
+#[derive(Copy, Clone, Debug)]
+pub struct Sp<'a> {
+    default_space: bool, // if true and comments is empty, use a space (' ')
+    comments: &'a [CommentOrNewline<'a>],
+}
+
+impl<'a> Sp<'a> {
+    pub fn empty() -> Sp<'a> {
+        Sp {
+            default_space: false,
+            comments: &[],
+        }
+    }
+    pub fn space() -> Sp<'a> {
+        Sp {
+            default_space: true,
+            comments: &[],
+        }
+    }
+
+    pub fn with_space(sp: &'a [CommentOrNewline<'a>]) -> Self {
+        Sp {
+            default_space: true,
+            comments: sp,
+        }
+    }
+}
+
+impl<'a> From<&'a [CommentOrNewline<'a>]> for Sp<'a> {
+    fn from(comments: &'a [CommentOrNewline<'a>]) -> Self {
+        Sp {
+            default_space: false,
+            comments,
+        }
+    }
+}
 
 #[derive(Copy, Clone, Debug)]
 pub enum Node<'a> {
@@ -24,20 +59,28 @@ pub trait Nodify<'a> {
         'a: 'b;
 }
 
+fn fmt_sp(buf: &mut Buf, sp: Sp<'_>, indent: u16) {
+    if !sp.comments.is_empty() {
+        fmt_spaces(buf, sp.comments.iter(), indent);
+    } else if sp.default_space {
+        buf.spaces(1);
+    }
+}
+
 impl<'a> Formattable for Node<'a> {
     fn is_multiline(&self) -> bool {
         match self {
             Node::DelimitedSequence(_braces, lefts, right) => {
-                right.is_empty()
+                right.comments.is_empty()
                     && lefts
                         .iter()
-                        .any(|(sp, l)| l.is_multiline() || !sp.is_empty())
+                        .any(|(sp, l)| l.is_multiline() || !sp.comments.is_empty())
             }
             Node::Sequence(first, rest) => {
                 first.is_multiline()
                     || rest
                         .iter()
-                        .any(|(sp, l)| l.is_multiline() || !sp.is_empty())
+                        .any(|(sp, l)| l.is_multiline() || !sp.comments.is_empty())
             }
             Node::TypeAnnotation(type_annotation) => type_annotation.is_multiline(),
             Node::Literal(_) => false,
@@ -51,16 +94,10 @@ impl<'a> Formattable for Node<'a> {
                 buf.push(braces.start());
 
                 for (sp, l) in *lefts {
-                    if !sp.is_empty() {
-                        fmt_spaces(buf, sp.iter(), indent);
-                    }
-
+                    fmt_sp(buf, *sp, indent);
                     l.format_with_options(buf, parens, newlines, indent);
                 }
-
-                if !right.is_empty() {
-                    fmt_spaces(buf, right.iter(), indent);
-                }
+                fmt_sp(buf, *right, indent);
 
                 buf.indent(indent);
                 buf.push(braces.end());
@@ -69,12 +106,7 @@ impl<'a> Formattable for Node<'a> {
                 first.format_with_options(buf, parens, newlines, indent);
 
                 for (sp, l) in *rest {
-                    if !sp.is_empty() {
-                        fmt_spaces(buf, sp.iter(), indent);
-                    } else {
-                        buf.spaces(1);
-                    }
-
+                    fmt_sp(buf, *sp, indent);
                     l.format_with_options(buf, parens, newlines, indent);
                 }
             }
@@ -88,3 +120,28 @@ impl<'a> Formattable for Node<'a> {
         }
     }
 }
+
+pub struct NodeSequenceBuilder<'a> {
+    first: Node<'a>,
+    rest: Vec<'a, (Sp<'a>, Node<'a>)>,
+}
+
+impl<'a> NodeSequenceBuilder<'a> {
+    pub fn new(arena: &'a Bump, first: Node<'a>, capacity: usize) -> Self {
+        Self {
+            first,
+            rest: Vec::with_capacity_in(capacity, arena),
+        }
+    }
+
+    pub fn push(&mut self, sp: Sp<'a>, literal: Node<'a>) {
+        self.rest.push((sp, literal));
+    }
+
+    pub fn build(self) -> Node<'a> {
+        Node::Sequence(
+            self.rest.bump().alloc(self.first),
+            self.rest.into_bump_slice(),
+        )
+    }
+}

crates/compiler/test_syntax/tests/snapshots/pass/comment_parens_in_typ_annotation_record_field.expr.formatted.roc

@@ -0,0 +1,4 @@
+i : {
+    t : J, #
+}
+A

crates/compiler/test_syntax/tests/snapshots/pass/comment_parens_in_typ_annotation_record_field.expr.result-ast

@@ -0,0 +1,59 @@
+@0-13 SpaceAfter(
+    Defs(
+        Defs {
+            tags: [
+                EitherIndex(2147483648),
+            ],
+            regions: [
+                @0-11,
+            ],
+            space_before: [
+                Slice<roc_parse::ast::CommentOrNewline> { start: 0, length: 0 },
+            ],
+            space_after: [
+                Slice<roc_parse::ast::CommentOrNewline> { start: 0, length: 0 },
+            ],
+            spaces: [],
+            type_defs: [],
+            value_defs: [
+                Annotation(
+                    @0-1 Identifier {
+                        ident: "i",
+                    },
+                    @2-11 Record {
+                        fields: [
+                            @3-10 RequiredValue(
+                                @3-4 "t",
+                                [],
+                                @6-7 SpaceAfter(
+                                    Apply(
+                                        "",
+                                        "J",
+                                        [],
+                                    ),
+                                    [
+                                        LineComment(
+                                            "",
+                                        ),
+                                    ],
+                                ),
+                            ),
+                        ],
+                        ext: None,
+                    },
+                ),
+            ],
+        },
+        @12-13 SpaceBefore(
+            Tag(
+                "A",
+            ),
+            [
+                Newline,
+            ],
+        ),
+    ),
+    [
+        Newline,
+    ],
+)

crates/compiler/test_syntax/tests/snapshots/pass/comment_parens_in_typ_annotation_record_field.expr.roc

@@ -0,0 +1,3 @@
+i:{t:(J#
+)}
+A

crates/compiler/test_syntax/tests/test_snapshots.rs

@@ -367,6 +367,7 @@ mod test_snapshots {
         pass/comment_in_tuple_ext.expr,
         pass/comment_indent_in_parens.expr,
         pass/comment_inside_empty_list.expr,
+        pass/comment_parens_in_typ_annotation_record_field.expr,
         pass/comment_with_non_ascii.expr,
         pass/compare_apply_record.expr,
         pass/control_characters_in_scalar.expr,