Improve multiline printing of record types and values

CHANGELOG.md

@@ -38,6 +38,7 @@
 - Rewatch: plain output when not running in tty. https://github.com/rescript-lang/rescript/pull/7970
 - Streamline rewatch help texts. https://github.com/rescript-lang/rescript/pull/7973
 - Rewatch: Reduced build progress output from 7 steps to 3 for cleaner, less verbose logging. https://github.com/rescript-lang/rescript/pull/7971
+- Formatter: Improve multiline printing of record types and values. https://github.com/rescript-lang/rescript/pull/7993
 
 #### :house: Internal
 

compiler/syntax/src/res_printer.ml

@@ -1282,7 +1282,7 @@ and print_type_declaration ~state ~name ~equal_sign ~rec_flag i
           manifest;
           Doc.concat [Doc.space; Doc.text equal_sign; Doc.space];
           print_private_flag td.ptype_private;
-          print_record_declaration ~state lds cmt_tbl;
+          print_record_declaration ~record_loc:td.ptype_loc ~state lds cmt_tbl;
         ]
     | Ptype_variant cds ->
       let manifest =
@@ -1370,8 +1370,8 @@ and print_type_declaration2 ?inline_record_definitions ~state ~rec_flag
             manifest;
             Doc.concat [Doc.space; Doc.text equal_sign; Doc.space];
             print_private_flag td.ptype_private;
-            print_record_declaration ?inline_record_definitions ~state lds
-              cmt_tbl;
+            print_record_declaration ?inline_record_definitions
+              ~record_loc:td.ptype_loc ~state lds cmt_tbl;
           ]
     | Ptype_variant cds ->
       let manifest =
@@ -1465,12 +1465,22 @@ and print_type_param ~state (param : Parsetree.core_type * Asttypes.variance)
   Doc.concat [printed_variance; print_typ_expr ~state typ cmt_tbl]
 
 and print_record_declaration ?check_break_from_loc ?inline_record_definitions
-    ~state (lds : Parsetree.label_declaration list) cmt_tbl =
+    ?record_loc ~state (lds : Parsetree.label_declaration list) cmt_tbl =
+  let get_field_start_line (ld : Parsetree.label_declaration) =
+    (* For spread fields (...), use the type location instead of pld_loc
+       because pld_loc may incorrectly include preceding whitespace *)
+    if ld.pld_name.txt = "..." then ld.pld_type.ptyp_loc.loc_start.pos_lnum
+    else ld.pld_loc.loc_start.pos_lnum
+  in
   let force_break =
-    match (check_break_from_loc, lds, List.rev lds) with
+    match (check_break_from_loc, record_loc, lds) with
     | Some loc, _, _ -> loc.Location.loc_start.pos_lnum < loc.loc_end.pos_lnum
-    | _, first :: _, last :: _ ->
-      first.pld_loc.loc_start.pos_lnum < last.pld_loc.loc_end.pos_lnum
+    | None, Some loc, first :: _ ->
+      (* Check if first field is on a different line than the opening brace *)
+      loc.loc_start.pos_lnum < get_field_start_line first
+    | None, None, first :: _ ->
+      let last = List.hd (List.rev lds) in
+      get_field_start_line first < last.pld_loc.loc_end.pos_lnum
     | _, _, _ -> false
   in
   Doc.breakable_group ~force_break
@@ -3223,7 +3233,14 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl =
          *   b: 2,
          *  }` -> record is written on multiple lines, break the group *)
         let force_break =
-          e.pexp_loc.loc_start.pos_lnum < e.pexp_loc.loc_end.pos_lnum
+          match (spread_expr, rows) with
+          | Some expr, _ ->
+            (* If there's a spread, compare with spread expression's location *)
+            e.pexp_loc.loc_start.pos_lnum < expr.pexp_loc.loc_start.pos_lnum
+          | None, first_row :: _ ->
+            (* Otherwise, compare with the first row's location *)
+            e.pexp_loc.loc_start.pos_lnum < first_row.lid.loc.loc_start.pos_lnum
+          | None, [] -> false
         in
         let punning_allowed =
           match (spread_expr, rows) with

tests/syntax_tests/data/ast-mapping/expected/JSXElements.res.txt

@@ -2,19 +2,9 @@ let emptyUnary = ReactDOM.jsx("input", {})
 
 let emptyNonunary = ReactDOM.jsx("div", {})
 
-let emptyUnaryWithAttributes = ReactDOM.jsx(
-  "input",
-  {
-    type_: "text",
-  },
-)
+let emptyUnaryWithAttributes = ReactDOM.jsx("input", {type_: "text"})
 
-let emptyNonunaryWithAttributes = ReactDOM.jsx(
-  "div",
-  {
-    className: "container",
-  },
-)
+let emptyNonunaryWithAttributes = ReactDOM.jsx("div", {className: "container"})
 
 let elementWithChildren = ReactDOM.jsxs(
   "div",

tests/syntax_tests/data/ppx/react/expected/aliasProps.res.txt

@@ -2,7 +2,10 @@
 
 module C0 = {
   @res.jsxComponentProps
-  type props<'priority, 'text> = {priority: 'priority, text?: 'text}
+  type props<'priority, 'text> = {
+    priority: 'priority,
+    text?: 'text,
+  }
 
   let make = ({priority: _, text: ?__text, _}: props<_, _>) => {
     let text = switch __text {
@@ -20,7 +23,10 @@ module C0 = {
 
 module C1 = {
   @res.jsxComponentProps
-  type props<'priority, 'text> = {priority: 'priority, text?: 'text}
+  type props<'priority, 'text> = {
+    priority: 'priority,
+    text?: 'text,
+  }
 
   let make = ({priority: p, text: ?__text, _}: props<_, _>) => {
     let text = switch __text {
@@ -38,7 +44,9 @@ module C1 = {
 
 module C2 = {
   @res.jsxComponentProps
-  type props<'foo> = {foo?: 'foo}
+  type props<'foo> = {
+    foo?: 'foo,
+  }
 
   let make = ({foo: ?__bar, _}: props<_>) => {
     let bar = switch __bar {
@@ -56,7 +64,11 @@ module C2 = {
 
 module C3 = {
   @res.jsxComponentProps
-  type props<'foo, 'a, 'b> = {foo?: 'foo, a?: 'a, b: 'b}
+  type props<'foo, 'a, 'b> = {
+    foo?: 'foo,
+    a?: 'a,
+    b: 'b,
+  }
 
   let make = ({foo: ?__bar, a: ?__a, b, _}: props<_, _, _>) => {
     let bar = switch __bar {
@@ -82,7 +94,10 @@ module C3 = {
 
 module C4 = {
   @res.jsxComponentProps
-  type props<'a, 'x> = {a: 'a, x?: 'x}
+  type props<'a, 'x> = {
+    a: 'a,
+    x?: 'x,
+  }
 
   let make = ({a: b, x: ?__x, _}: props<_, _>) => {
     let x = switch __x {
@@ -100,7 +115,10 @@ module C4 = {
 
 module C5 = {
   @res.jsxComponentProps
-  type props<'a, 'z> = {a: 'a, z?: 'z}
+  type props<'a, 'z> = {
+    a: 'a,
+    z?: 'z,
+  }
 
   let make = ({a: (x, y), z: ?__z, _}: props<_, _>) => {
     let z = switch __z {
@@ -124,7 +142,10 @@ module C6 = {
     let make: React.component<props>
   }
   @res.jsxComponentProps
-  type props<'comp, 'x> = {comp: 'comp, x: 'x}
+  type props<'comp, 'x> = {
+    comp: 'comp,
+    x: 'x,
+  }
 
   let make = ({comp: module(Comp: Comp), x: (a, b), _}: props<_, _>): React.element =>
     React.jsx(Comp.make, {})

tests/syntax_tests/data/ppx/react/expected/asyncAwait.res.txt

@@ -2,7 +2,9 @@ let f = a => Js.Promise.resolve(a + a)
 
 module C0 = {
   @res.jsxComponentProps
-  type props<'a> = {a: 'a}
+  type props<'a> = {
+    a: 'a,
+  }
 
   let make = async ({a, _}: props<_>) => {
     let a = await f(a)
@@ -17,7 +19,9 @@ module C0 = {
 
 module C1 = {
   @res.jsxComponentProps
-  type props<'status> = {status: 'status}
+  type props<'status> = {
+    status: 'status,
+  }
 
   let make = async ({status, _}: props<_>): React.element => {
     switch status {

tests/syntax_tests/data/ppx/react/expected/commentAtTop.res.txt

@@ -1,5 +1,7 @@
 @res.jsxComponentProps
-type props<'msg> = {msg: 'msg} // test React JSX file
+type props<'msg> = {
+  msg: 'msg, // test React JSX file
+}
 
 let make = ({msg, _}: props<_>): React.element => {
   ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})

tests/syntax_tests/data/ppx/react/expected/defaultValueProp.res.txt

@@ -1,6 +1,9 @@
 module C0 = {
   @res.jsxComponentProps
-  type props<'a, 'b> = {a?: 'a, b?: 'b}
+  type props<'a, 'b> = {
+    a?: 'a,
+    b?: 'b,
+  }
   let make = ({a: ?__a, b: ?__b, _}: props<_, _>) => {
     let a = switch __a {
     | Some(a) => a
@@ -20,7 +23,10 @@ module C0 = {
 
 module C1 = {
   @res.jsxComponentProps
-  type props<'a, 'b> = {a?: 'a, b: 'b}
+  type props<'a, 'b> = {
+    a?: 'a,
+    b: 'b,
+  }
 
   let make = ({a: ?__a, b, _}: props<_, _>) => {
     let a = switch __a {
@@ -39,7 +45,9 @@ module C1 = {
 module C2 = {
   let a = "foo"
   @res.jsxComponentProps
-  type props<'a> = {a?: 'a}
+  type props<'a> = {
+    a?: 'a,
+  }
 
   let make = ({a: ?__a, _}: props<_>) => {
     let a = switch __a {
@@ -57,7 +65,9 @@ module C2 = {
 
 module C3 = {
   @res.jsxComponentProps
-  type props<'disabled> = {disabled?: 'disabled}
+  type props<'disabled> = {
+    disabled?: 'disabled,
+  }
 
   let make = ({disabled: ?__everythingDisabled, _}: props<bool>) => {
     let everythingDisabled = switch __everythingDisabled {

tests/syntax_tests/data/ppx/react/expected/externalWithCustomName.res.txt

@@ -2,7 +2,10 @@
 
 module Foo = {
   @res.jsxComponentProps @live
-  type props<'a, 'b> = {a: 'a, b: 'b}
+  type props<'a, 'b> = {
+    a: 'a,
+    b: 'b,
+  }
 
   @module("Foo")
   external component: React.component<props<int, string>> = "component"

tests/syntax_tests/data/ppx/react/expected/fileLevelConfig.res.txt

@@ -2,7 +2,9 @@
 
 module V4A = {
   @res.jsxComponentProps
-  type props<'msg> = {msg: 'msg}
+  type props<'msg> = {
+    msg: 'msg,
+  }
 
   let make = ({msg, _}: props<_>): React.element => {
     ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})

tests/syntax_tests/data/ppx/react/expected/forwardRef.resi.txt

@@ -14,7 +14,9 @@ module V4C: {
 
   module ForwardRef: {
     @res.jsxComponentProps
-    type props<'ref> = {ref?: 'ref}
+    type props<'ref> = {
+      ref?: 'ref,
+    }
 
     let make: React.component<props<React.ref<Js.Nullable.t<inputRef>>>>
   }
@@ -34,7 +36,9 @@ module V4CUncurried: {
 
   module ForwardRef: {
     @res.jsxComponentProps
-    type props<'ref> = {ref?: 'ref}
+    type props<'ref> = {
+      ref?: 'ref,
+    }
 
     let make: React.component<props<React.ref<Js.Nullable.t<inputRef>>>>
   }
@@ -56,7 +60,9 @@ module V4A: {
 
   module ForwardRef: {
     @res.jsxComponentProps
-    type props<'ref> = {ref?: 'ref}
+    type props<'ref> = {
+      ref?: 'ref,
+    }
 
     let make: React.component<props<React.ref<Js.Nullable.t<inputRef>>>>
   }
@@ -76,7 +82,9 @@ module V4AUncurried: {
 
   module ForwardRef: {
     @res.jsxComponentProps
-    type props<'ref> = {ref?: 'ref}
+    type props<'ref> = {
+      ref?: 'ref,
+    }
 
     let make: React.component<props<React.ref<Js.Nullable.t<inputRef>>>>
   }

tests/syntax_tests/data/ppx/react/expected/interface.res.txt

@@ -1,6 +1,8 @@
 module A = {
   @res.jsxComponentProps
-  type props<'x> = {x: 'x}
+  type props<'x> = {
+    x: 'x,
+  }
   let make = ({x, _}: props<_>): React.element => React.string(x)
   let make = {
     let \"Interface$A" = (props: props<_>) => make(props)

tests/syntax_tests/data/ppx/react/expected/interface.resi.txt

@@ -1,6 +1,8 @@
 module A: {
   @res.jsxComponentProps
-  type props<'x> = {x: 'x}
+  type props<'x> = {
+    x: 'x,
+  }
   let make: React.component<props<string>>
 }
 

tests/syntax_tests/data/ppx/react/expected/interfaceWithRef.res.txt

@@ -1,4 +1,8 @@
-@res.jsxComponentProps type props<'x, 'ref> = {x: 'x, ref?: 'ref}
+@res.jsxComponentProps
+type props<'x, 'ref> = {
+  x: 'x,
+  ref?: 'ref,
+}
 let make = (
   {x, _}: props<string, ReactDOM.Ref.currentDomRef>,
   ref: Js.Nullable.t<ReactDOM.Ref.currentDomRef>,

tests/syntax_tests/data/ppx/react/expected/interfaceWithRef.resi.txt

@@ -1,2 +1,6 @@
-@res.jsxComponentProps type props<'x, 'ref> = {x: 'x, ref?: 'ref}
+@res.jsxComponentProps
+type props<'x, 'ref> = {
+  x: 'x,
+  ref?: 'ref,
+}
 let make: React.component<props<string, ReactDOM.Ref.currentDomRef>>

tests/syntax_tests/data/ppx/react/expected/mangleKeyword.res.txt

@@ -2,7 +2,10 @@
 
 module C4A0 = {
   @res.jsxComponentProps
-  type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}
+  type props<'T_open, 'T_type> = {
+    @as("open") _open: 'T_open,
+    @as("type") _type: 'T_type,
+  }
 
   let make = ({@as("open") _open, @as("type") _type, _}: props<_, string>): React.element =>
     React.string(_open)
@@ -14,7 +17,10 @@ module C4A0 = {
 }
 module C4A1 = {
   @res.jsxComponentProps @live
-  type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}
+  type props<'T_open, 'T_type> = {
+    @as("open") _open: 'T_open,
+    @as("type") _type: 'T_type,
+  }
 
   external make: React.component<props<string, string>> = "default"
 }