Format a blank line around multi-line module items (#8961)

* 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. The source line span (`let x = someLongCall(a, b, c, d)` is one source line but prints on several) and Doc.will_break (a record that wraps by width is force-broken only on the second pass) would both add the blank line only on the second format. The measurement costs at most one output line per item. The roundtrip test formats every printer fixture twice and gets the same text.
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, even when the item is measured before print_list adds its own comments. 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: How are the measurement's branches tested? tests/syntax_tests/res_test.ml prints documents built for each case (line breaks, groups that fit and break, if_breaks, comments, trailing comments) and checks the output, as the printer fixtures do not reach every branch first.
Review: What else prints through it? The editor's code actions and `rescript-tools format-codeblocks` for code blocks in docstrings, so the Xform and FormatDocstringsTest2 snapshots gain a blank line.
Review: Do attributes count? Yes, an attribute or doc comment on its own line makes an item print on two lines, so runs of `@genType` or documented one-line items are spaced; an inline `@val external` is not. The fixtures cover both, and nested module bodies.
Review: Does a string literal with a line break count? Yes: it prints on several lines, like a template literal, so the line-ending test in res_test.ml separates such items with a blank line, in the file's own line endings.
Review: Is it opt-in? No: every `rescript format` uses it. The next commit formats this repository with it.

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

* Format the repository with multi-line item spacing

The previous commit makes the formatter put a blank line around module
items that span several lines. `make format` applies it to the
ReScript sources it covers: 353 files gain 1179 blank lines and lose
nothing. rewatch/testrepo/packages/dep02/src/Array.res gains one too:
the rewatch format test expects exactly the testrepo's 8 deliberately
unformatted files to change. The outputs that record source line
numbers follow: the super_errors snapshots, the `assert` locations in
the generated test JavaScript, the doc-extraction snapshots of
tests/tools_tests, and the locations in React.res and JsxEvent.res that
the completion and reanalyze snapshots of tests/analysis_tests point
to.

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

---------

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
@@ -129,9 +136,9 @@ let has_pending_line_suffix doc =
   let rec walk ~broken pending doc =
     match doc with
     | LineSuffix _ -> true
-    | LineBreak _ -> pending && not broken
+    | LineBreak _ | ItemSeparator _ -> pending && not broken
     | Nil | Text _ | BreakParent -> pending
-    | Indent doc -> walk ~broken pending doc
+    | Indent doc | Comment doc -> walk ~broken pending doc
     | Concat docs -> List.fold_left (walk ~broken) pending docs
     | Group {should_break; doc} ->
       walk ~broken:(should_break || will_break doc) pending doc
@@ -170,10 +177,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} ->
@@ -203,6 +212,59 @@ 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)]
+
+(* An own-line trailing comment is a line suffix that starts with a hard line:
+   returns it without its leading hard lines. *)
+let rec own_line_suffix doc =
+  match doc with
+  | LineBreak Hard -> Some Nil
+  | Concat (LineBreak Hard :: docs) ->
+    let rest = Concat docs in
+    Some (Option.value (own_line_suffix rest) ~default:rest)
+  | _ -> None
+
 let to_string ?(newline = "\n") ~width doc =
   propagate_forced_breaks doc;
   let buffer = Mini_buffer.create 1000 in
@@ -220,6 +282,7 @@ let to_string ?(newline = "\n") ~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} ->
@@ -262,16 +325,30 @@ let to_string ?(newline = "\n") ~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_map
+              (fun ((ind, mode, doc) as suffix) ->
+                match own_line_suffix doc with
+                | None -> Either.Left suffix
+                | Some comment -> Either.Right ((ind, mode, comment), suffix))
+              (List.rev line_suffices)
+          in
+          let rest =
+            match below with
+            | [] -> rest
+            | (first, _) :: below ->
+              (first :: List.map snd 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
       | [] -> ()
@@ -351,6 +428,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
@@ -375,5 +454,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

@@ -337,10 +337,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
@@ -398,7 +396,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))
@@ -414,15 +414,17 @@ 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
+          | 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
+          | _ -> [doc]
         in
         let first_loc_opt =
           match first_loc_opt with
@@ -701,7 +703,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 ->
@@ -1119,7 +1121,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)