Format a blank line around module items that span several lines

Why:
    Dense code, often written by AI agents, runs multi-line modules and
    functions together, which makes a file hard to scan (#8960). The
    formatter already collapses several blank lines into one; it now also
    separates an item that prints on more than one line from its
    neighbours in structures and signatures. Blocks inside expressions
    keep their layout.

Format modified (structure items):
Example:
    module Details = {
      let make = () => React.string("details")
    }
    module Output = Tool.TextResult.Output
    let failed = Tool.TextResult.failed
Before (rescript):
    module Details = {
      let make = () => React.string("details")
    }
    module Output = Tool.TextResult.Output
    let failed = Tool.TextResult.failed
After (rescript):
    module Details = {
      let make = () => React.string("details")
    }

    module Output = Tool.TextResult.Output
    let failed = Tool.TextResult.failed

Format unchanged (one-line items with comments):
Example:
    let a = 1
    // about b
    let b = 2 // trailing
    let c = 3
After (rescript):
    let a = 1
    // about b
    let b = 2 // trailing
    let c = 3

Review: Is "multi-line" decided on the source or on the output? On the output: the new Res_doc.item_separator measures the items on each side with the printer's own layout decisions, stopping at the first line break, so an item that only breaks at the line width counts and formatting twice gives the same text. All 1025 printer and idempotency fixtures are stable across two passes.
Review: Why a Res_doc.comment marker? Leading comments print inside the item's document (a value binding carries them), so a `//` line above a one-line item would make it multi-line. The marker keeps comments out of the measurement; doc comments are attributes and still count.
Review: Why move own-line trailing comments below the blank line? A comment inside the previous item's source, such as `f(a // c => 1)`, prints on its own line after the item. Formatting again makes it a leading comment of the next item, after the blank line, so the first pass puts it there too.
Review: Does this reformat existing code? Yes: 346 of the 1870 .res/.resi files `make format` covers gain blank lines. They are not reformatted in this commit, so `make checkformat` reports them.

Signed-off-by: Roland Peelen <roland@magic.dev>

Author: rolandpeelen

changes/formatter-multiline-item-spacing.polish.md

@@ -0,0 +1 @@
+- Format a blank line before and after a top-level or module-level item that spans several lines.

compiler/syntax/src/res_doc.ml

@@ -23,6 +23,8 @@ type t =
   | Group of {mutable should_break: bool; doc: t}
   | CustomLayout of t list
   | BreakParent
+  | ItemSeparator of {prev: t; next: t} (* see [item_separator] *)
+  | Comment of t (* see [comment] *)
 
 let nil = Nil
 let line = LineBreak Classic
@@ -52,6 +54,8 @@ let group d = Group {should_break = false; doc = d}
 let breakable_group ~force_break d = Group {should_break = force_break; doc = d}
 let custom_layout gs = CustomLayout gs
 let break_parent = BreakParent
+let item_separator ~prev ~next = ItemSeparator {prev; next}
+let comment d = Comment d
 
 let space = Text " "
 let comma = Text ","
@@ -75,9 +79,9 @@ let propagate_forced_breaks doc =
     match doc with
     | Text _ | Nil | LineSuffix _ -> false
     | BreakParent -> true
-    | LineBreak (Hard | Literal) -> true
+    | LineBreak (Hard | Literal) | ItemSeparator _ -> true
     | LineBreak (Classic | Soft) -> false
-    | Indent children ->
+    | Indent children | Comment children ->
       let child_forces_break = walk children in
       child_forces_break
     | IfBreaks ({yes = true_doc; no = false_doc} as ib) ->
@@ -115,9 +119,12 @@ let propagate_forced_breaks doc =
 (* See documentation in interface file *)
 let rec will_break doc =
   match doc with
-  | LineBreak (Hard | Literal) | BreakParent | Group {should_break = true} ->
+  | LineBreak (Hard | Literal)
+  | BreakParent | ItemSeparator _
+  | Group {should_break = true} ->
     true
-  | Group {doc} | Indent doc | CustomLayout (doc :: _) -> will_break doc
+  | Group {doc} | Indent doc | Comment doc | CustomLayout (doc :: _) ->
+    will_break doc
   | Concat docs -> List.exists will_break docs
   | IfBreaks {yes; no} -> will_break yes || will_break no
   | _ -> false
@@ -151,10 +158,12 @@ let fits w stack =
     | _, Nil | _, LineSuffix _ | _, BreakParent -> ()
     | _, Text txt -> width := width.contents - String.length txt
     | _, Indent doc -> calculate (indent + 2) mode doc
+    | _, Comment doc -> calculate indent mode doc
     | Flat, LineBreak Hard | Flat, LineBreak Literal -> result := Some true
     | Flat, LineBreak Classic -> width := width.contents - 1
     | Flat, LineBreak Soft -> ()
     | Break, LineBreak _ -> result := Some true
+    | _, ItemSeparator _ -> result := Some true
     | _, Group {should_break = true; doc} -> calculate indent Break doc
     | _, Group {doc} -> calculate indent mode doc
     | _, IfBreaks {yes = break_doc; broken = true} ->
@@ -184,6 +193,61 @@ let fits w stack =
   in
   calculate_all stack
 
+let select_custom_layout ~width ~pos ind rest docs =
+  let rec find_group_that_fits groups =
+    match groups with
+    | [] -> Nil
+    | [last_group] -> last_group
+    | doc :: docs ->
+      if fits (width - pos) ((ind, Flat, doc) :: rest) then doc
+      else find_group_that_fits docs
+  in
+  find_group_that_fits docs
+
+(* Whether [doc], printed from the start of a line indented by [ind], spans more
+   than one line. Takes the layout decisions of [to_string] up to the first line
+   break. Comments and line suffixes do not count. *)
+let spans_lines ~width ind mode doc =
+  let rec loop ~pos stack =
+    match stack with
+    | [] -> false
+    | (ind, mode, doc) :: rest -> (
+      match doc with
+      | Nil | BreakParent | LineSuffix _ | Comment _ -> loop ~pos rest
+      | Text txt ->
+        String.contains txt '\n' || loop ~pos:(pos + String.length txt) rest
+      | Concat docs ->
+        loop ~pos (List.map (fun doc -> (ind, mode, doc)) docs @ rest)
+      | Indent doc -> loop ~pos ((ind + 2, mode, doc) :: rest)
+      | IfBreaks {yes; no; broken} ->
+        let doc = if broken || mode = Break then yes else no in
+        loop ~pos ((ind, mode, doc) :: rest)
+      | LineBreak (Hard | Literal) | ItemSeparator _ -> true
+      | LineBreak Classic -> mode = Break || loop ~pos:(pos + 1) rest
+      | LineBreak Soft -> mode = Break || loop ~pos rest
+      | Group {should_break; doc} ->
+        let flat = (ind, Flat, doc) :: rest in
+        if should_break || not (fits (width - pos) flat) then
+          loop ~pos ((ind, Break, doc) :: rest)
+        else loop ~pos flat
+      | CustomLayout docs ->
+        let doc = select_custom_layout ~width ~pos ind rest docs in
+        loop ~pos ((ind, Flat, doc) :: rest))
+  in
+  loop ~pos:ind [(ind, mode, doc)]
+
+(* Own-line trailing comments are line suffixes that start with a hard line. *)
+let starts_with_hard_line doc =
+  match doc with
+  | LineBreak Hard | Concat (LineBreak Hard :: _) -> true
+  | _ -> false
+
+let rec drop_leading_hard_lines doc =
+  match doc with
+  | LineBreak Hard -> Nil
+  | Concat (LineBreak Hard :: docs) -> drop_leading_hard_lines (Concat docs)
+  | doc -> doc
+
 let to_string ~width doc =
   propagate_forced_breaks doc;
   let buffer = Mini_buffer.create 1000 in
@@ -201,6 +265,7 @@ let to_string ~width doc =
         let ops = List.map (fun doc -> (ind, mode, doc)) docs in
         process ~pos line_suffices (List.append ops rest)
       | Indent doc -> process ~pos line_suffices ((ind + 2, mode, doc) :: rest)
+      | Comment doc -> process ~pos line_suffices ((ind, mode, doc) :: rest)
       | IfBreaks {yes = break_doc; broken = true} ->
         process ~pos line_suffices ((ind, mode, break_doc) :: rest)
       | IfBreaks {yes = break_doc; no = flat_doc} ->
@@ -243,16 +308,28 @@ let to_string ~width doc =
         then process ~pos line_suffices ((ind, Break, doc) :: rest)
         else process ~pos line_suffices ((ind, Flat, doc) :: rest)
       | CustomLayout docs ->
-        let rec find_group_that_fits groups =
-          match groups with
-          | [] -> Nil
-          | [last_group] -> last_group
-          | doc :: docs ->
-            if fits (width - pos) ((ind, Flat, doc) :: rest) then doc
-            else find_group_that_fits docs
-        in
-        let doc = find_group_that_fits docs in
-        process ~pos line_suffices ((ind, Flat, doc) :: rest))
+        let doc = select_custom_layout ~width ~pos ind rest docs in
+        process ~pos line_suffices ((ind, Flat, doc) :: rest)
+      | ItemSeparator {prev; next} ->
+        if spans_lines ~width ind mode prev || spans_lines ~width ind mode next
+        then
+          (* Own-line trailing comments of [prev] go below the blank line, where
+             they print once they are leading comments of [next]. *)
+          let separator = (ind, mode, hard_line) in
+          let same_line, below =
+            List.partition
+              (fun (_, _, doc) -> not (starts_with_hard_line doc))
+              (List.rev line_suffices)
+          in
+          let rest =
+            match below with
+            | [] -> rest
+            | (ind, mode, doc) :: below ->
+              ((ind, mode, drop_leading_hard_lines doc) :: below)
+              @ (separator :: rest)
+          in
+          process ~pos (List.rev same_line) (separator :: separator :: rest)
+        else process ~pos line_suffices ((ind, mode, hard_line) :: rest))
     | [] -> (
       match line_suffices with
       | [] -> ()
@@ -332,6 +409,8 @@ let debug t =
         | Literal -> "Literal"
       in
       text ("LineBreak(" ^ break_txt ^ ")")
+    | ItemSeparator _ -> text "ItemSeparator"
+    | Comment doc -> concat [text "comment("; to_doc doc; text ")"]
     | Group {should_break; doc} ->
       group
         (concat
@@ -356,5 +435,5 @@ let is_nil doc =
   match doc with
   | Nil -> true
   | Text _ | LineSuffix _ | Concat _ | Indent _ | IfBreaks _ | LineBreak _
-  | Group _ | CustomLayout _ | BreakParent ->
+  | Group _ | CustomLayout _ | BreakParent | ItemSeparator _ | Comment _ ->
     false

compiler/syntax/src/res_doc.mli

@@ -18,6 +18,15 @@ val breakable_group : force_break:bool -> t -> t
  * will be checked until one fits. *)
 val custom_layout : t list -> t
 val break_parent : t
+
+val item_separator : prev:t -> next:t -> t
+(** A hard line, and a blank line when [prev] or [next], the documents printed
+    on each side of it, span several lines. It measures them but does not print
+    them. *)
+
+val comment : t -> t
+(** Prints like its argument, which [item_separator] does not measure. *)
+
 val join : sep:t -> t list -> t
 
 (* [(doc1, sep1); (doc2,sep2)] joins as doc1 sep1 doc2 *)

compiler/syntax/src/res_printer.ml

@@ -326,10 +326,8 @@ let print_leading_comments node tbl loc =
         else if diff > 1 then Doc.concat [Doc.hard_line; Doc.hard_line]
         else Doc.hard_line
       in
-      let doc =
-        Doc.group
-          (Doc.concat [Doc.concat (List.rev (cmt_doc :: acc)); separator; node])
-      in
+      let comments = Doc.concat (List.rev (separator :: cmt_doc :: acc)) in
+      let doc = Doc.group (Doc.concat [Doc.comment comments; node]) in
       doc
     | comment :: (next_comment :: _comments as rest) ->
       let cmt_doc = print_leading_comment ~next_comment comment in
@@ -387,7 +385,9 @@ let print_comments_before_marker doc (tbl : Comment_table.t) loc marker =
     Hashtbl.replace tbl.trailing loc after_marker;
     print_trailing_comments doc tbl.trailing loc
 
-let print_list ~get_loc ~nodes ~print ?(force_break = false) t =
+(* [~space_multiline] adds a blank line next to nodes printed on several lines *)
+let print_list ~get_loc ~nodes ~print ?(force_break = false)
+    ?(space_multiline = false) t =
   let rec loop first_loc_opt prev_loc_opt acc nodes =
     match nodes with
     | [] -> (first_loc_opt, prev_loc_opt, Doc.concat (List.rev acc))
@@ -403,12 +403,14 @@ let print_list ~get_loc ~nodes ~print ?(force_break = false) t =
       else
         let doc = print_comments raw_doc t loc in
         let acc =
-          match prev_loc_opt with
-          | None -> [doc]
-          | Some prev_loc ->
+          match (prev_loc_opt, acc) with
+          | None, _ | _, [] -> [doc]
+          | Some prev_loc, prev_doc :: _ ->
             let sep =
               if start_pos.pos_lnum - prev_loc.Location.loc_end.pos_lnum > 1
               then Doc.concat [Doc.hard_line; Doc.hard_line]
+              else if space_multiline then
+                Doc.item_separator ~prev:prev_doc ~next:doc
               else Doc.hard_line
             in
             doc :: sep :: acc
@@ -690,7 +692,7 @@ let rec print_structure ~state (s : Parsetree.structure) t =
   | [] -> print_comments_inside_file t
   | structure ->
     let previous_can_continue = ref false in
-    print_list
+    print_list ~space_multiline:true
       ~get_loc:(fun s -> s.Parsetree.pstr_loc)
       ~nodes:structure
       ~print:(fun item cmt_tbl ->
@@ -1108,7 +1110,7 @@ and print_signature ~state signature cmt_tbl =
   match signature with
   | [] -> print_comments_inside_file cmt_tbl
   | signature ->
-    print_list
+    print_list ~space_multiline:true
       ~get_loc:(fun s -> s.Parsetree.psig_loc)
       ~nodes:signature
       ~print:(print_signature_item ~state)

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

@@ -1,9 +1,11 @@
 let value = {42}
+
 let branch = if true {
   1
 } else {
   2
 }
+
 let case = switch value {
 | 42 => value
 | _ => 0

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

@@ -7,6 +7,7 @@ let id = x => x
 
 // labeled, optional, and default parameters
 let labeled = (~x, ~y) => x - y
+
 let optional = (~x=?, ~y=1, z) => {
   switch x {
   | Some(x) => x + y + z

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

@@ -39,7 +39,9 @@ let elementWithConditionalChildren = ReactDOM.jsx(
     }),
   },
 )
+
 let commentOnly = ReactDOM.jsx("div", {/* no child */})
+
 let commentsBetweenElements = ReactDOM.jsx(
   "div",
   {

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

@@ -22,7 +22,9 @@ let nestedFragments = React.jsxs(
     ]),
   },
 )
+
 let commentOnly = React.jsx(React.jsxFragment, {}) // no child
+
 let commentsBetweenElements = React.jsxs(
   React.jsxFragment /* between */,
   {children: React.array([ReactDOM.jsx("span", {}), ReactDOM.jsx("span", {})])},

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

@@ -3,7 +3,9 @@ module A = {
   type props<'x> = {
     x: 'x,
   }
+
   let make = ({x, _}: props<_>): React.element => React.string(x)
+
   let make = React.component({
     let \"JsxComponents$A" = (props: props<_>) => make(props)
     \"JsxComponents$A"
@@ -15,6 +17,7 @@ module B = {
 
   /** Doc */
   @deprecated @genType let make = ({x, _}: props): React.element => React.string(x)
+
   /** Doc */
   @deprecated @genType
   let make = React.component({
@@ -26,6 +29,7 @@ module B = {
 
 module C = {
   let make = (props: props): React.element => React.string(props.x)
+
   let make = React.component({
     let \"JsxComponents$C" = (props: props): React.element => make(props)
     \"JsxComponents$C"
@@ -34,6 +38,7 @@ module C = {
 
 module D = {
   let make = (props: props): React.element => React.string(props.x)
+
   let make = React.component({
     let \"JsxComponents$D" = (props: props): React.element => make(props)
     \"JsxComponents$D"
@@ -57,6 +62,7 @@ module F: {
   }
 
   let make: React.component<props<string>>
+
   @jsx.componentWithProps
   let other: props => React.element
 } = {
@@ -66,6 +72,7 @@ module F: {
   }
 
   let make = ({x, _}: props<_>): React.element => React.string(x)
+
   let make = React.component({
     let \"JsxComponents$F" = (props: props<_>) => make(props)
 

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

@@ -1,7 +1,9 @@
 let add = %raw("(a, b) => a + b")
+
 let template = %raw(`function (a) {
   return "x" + a
 }`)
+
 let escaped = %raw(`"a\`b\${c}"`)
 let fn = %ffi("(a) => a + 1")
 let applied = %ffi(`(a, b) => a * b`)(2, 3)