Make multi-line item spacing the experimental feature MultilineItemSpacing

Why:
    Spacing multi-line module items changes the output of `rescript
    format` for most existing projects (341 files in this repository
    alone), so it is opt-in, like `let?` (LetUnwrap). Without the feature
    the formatter prints exactly as before, and the snapshots of the
    previous commit return to their master state.

Format modified (multi-line item spacing, opt-in):
Example:
    module A = {
      let x = 1
    }
    let y = 2
Before (rescript): always, since the previous commit
    module A = {
      let x = 1
    }

    let y = 2
After (rescript): "experimental-features": {"MultilineItemSpacing": true}
    module A = {
      let x = 1
    }

    let y = 2

Format unchanged (default):
Example:
    module A = {
      let x = 1
    }
    let y = 2
After (rescript): without the feature
    module A = {
      let x = 1
    }
    let y = 2

Diagnostic modified (unknown experimental feature):
Example:
    "experimental-features": {"FooBar": true}
Before (diagnostic): Unknown experimental feature 'FooBar'. Available features: LetUnwrap
After (diagnostic): Unknown experimental feature 'FooBar'. Available features: LetUnwrap, MultilineItemSpacing

Review: How does `rescript format` learn about the feature? Both build systems pass the root project's experimental flags, the same ones a build passes, to `bsc` before `-format`; `-format` runs as soon as bsc reads it, so the order matters. Outside a project, format runs without them, as before. Covered by rewatch/tests/format/06-format-experimental-multiline-item-spacing.sh, which passes for both rescript.exe and rescript-rust.exe.
Review: How do the syntax tests enable it? `res_parser` accepts `-enable-experimental`, and scripts/test_syntax.sh prints and round-trips each file under tests/syntax_tests/data/experimental/<Feature>/ with that feature enabled.
Review: What about tools that call `bsc -format` directly instead of `rescript format`? They format without the feature unless they pass `-enable-experimental MultilineItemSpacing` before `-format`.

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

Author: rolandpeelen

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

@@ -1 +1 @@
-- Format a blank line before and after a top-level or module-level item that spans several lines.
+- Add the experimental feature `MultilineItemSpacing`: with `"experimental-features": {"MultilineItemSpacing": true}`, `rescript format` puts a blank line before and after a module item that spans several lines.

compiler/bsc/rescript_compiler_driver.ml

@@ -492,7 +492,8 @@ let command_line_flags () : (string * Bsc_args.spec * string) array =
     ( "-enable-experimental",
       string_call Experimental_features.enable_from_string,
       "Enable experimental features: repeatable, e.g. -enable-experimental \
-       LetUnwrap" );
+       LetUnwrap. Pass it before -format to format with a feature such as \
+       MultilineItemSpacing" );
     (* Not used, the build system did the expansion *)
     ( "-bs-no-bin-annot",
       clear (Clflags.current ()).binary_annotations,

compiler/ml/experimental_features.ml

@@ -1,12 +1,14 @@
-type feature = LetUnwrap
+type feature = LetUnwrap | MultilineItemSpacing
 
 let to_string (f : feature) : string =
   match f with
   | LetUnwrap -> "LetUnwrap"
+  | MultilineItemSpacing -> "MultilineItemSpacing"
 
 let from_string (s : string) : feature option =
   match s with
   | "LetUnwrap" -> Some LetUnwrap
+  | "MultilineItemSpacing" -> Some MultilineItemSpacing
   | _ -> None
 
 module Feature_set = Set.Make (struct

compiler/ml/experimental_features.mli

@@ -1,4 +1,8 @@
-type feature = LetUnwrap
+type feature =
+  | LetUnwrap
+  | MultilineItemSpacing
+      (** The formatter separates module items that span several lines with a
+          blank line. *)
 
 val enable_from_string : string -> unit
 

compiler/syntax/cli/res_cli.ml

@@ -55,6 +55,9 @@ end = struct
       ( "-test-ast-conversion",
         Arg.Unit (fun () -> test_ast_conversion := true),
         "Test the ast conversion" );
+      ( "-enable-experimental",
+        Arg.String Experimental_features.enable_from_string,
+        "Enable an experimental feature, e.g. MultilineItemSpacing" );
     ]
 
   let parse () = Arg.parse spec (fun f -> file := f) usage

compiler/syntax/src/res_printer.ml

@@ -687,12 +687,15 @@ module State = struct
   let should_break_callback t = t.custom_layout > custom_layout_threshold
 end
 
+let space_multiline () =
+  Experimental_features.is_enabled Experimental_features.MultilineItemSpacing
+
 let rec print_structure ~state (s : Parsetree.structure) t =
   match s with
   | [] -> print_comments_inside_file t
   | structure ->
     let previous_can_continue = ref false in
-    print_list ~space_multiline:true
+    print_list ~space_multiline:(space_multiline ())
       ~get_loc:(fun s -> s.Parsetree.pstr_loc)
       ~nodes:structure
       ~print:(fun item cmt_tbl ->
@@ -1110,7 +1113,7 @@ and print_signature ~state signature cmt_tbl =
   match signature with
   | [] -> print_comments_inside_file cmt_tbl
   | signature ->
-    print_list ~space_multiline:true
+    print_list ~space_multiline:(space_multiline ())
       ~get_loc:(fun s -> s.Parsetree.psig_loc)
       ~nodes:signature
       ~print:(print_signature_item ~state)

docs/docson/build-schema.json

@@ -556,6 +556,10 @@
         "LetUnwrap": {
           "type": "boolean",
           "description": "Enable `let?` syntax."
+        },
+        "MultilineItemSpacing": {
+          "type": "boolean",
+          "description": "Format a blank line before and after a module item that spans several lines."
         }
       },
       "additionalProperties": false

rewatch-ocaml/config.ml

@@ -246,11 +246,11 @@ let decode_experimental_features path fields =
     reject_duplicates path "experimental-features" features;
     features
     |> List.concat_map (fun (name, value) ->
-        if name <> "LetUnwrap" then
+        if not (List.mem name ["LetUnwrap"; "MultilineItemSpacing"]) then
           fail path
             (Printf.sprintf
                "Unknown experimental feature '%s'. Available features: \
-                LetUnwrap"
+                LetUnwrap, MultilineItemSpacing"
                name);
         match value with
         | `Bool true -> ["-enable-experimental"; name]

rewatch-ocaml/format.ml

@@ -115,11 +115,22 @@ let files_in_scope ~verbosity =
 let formatting_error target stderr =
   Printf.sprintf "Error formatting %s: %s" target stderr
 
-let format_request path =
-  Compiler_process.{args = ["-format"; path]; cwd = Sys.getcwd ()}
+(* The experimental features of the current project, such as
+   MultilineItemSpacing, as bsc flags. They must precede -format. Outside a
+   project, format runs without them, like Rust rewatch. *)
+let experimental_args () =
+  try
+    let current = Config.load_root (Sys.getcwd ()) in
+    (Project_context.workspace_config_for current).experimental_args
+  with Config.Error _ -> []
+
+let format_request ~experimental_args path =
+  Compiler_process.
+    {args = experimental_args @ ["-format"; path]; cwd = Sys.getcwd ()}
 
 let formatted ?poll ~target path =
-  let result = Compiler_process.run ?poll (format_request path) in
+  let request = format_request ~experimental_args:(experimental_args ()) path in
+  let result = Compiler_process.run ?poll request in
   if not (Process.succeeded result) then
     raise (Error (formatting_error target result.stderr));
   result.stdout
@@ -129,6 +140,7 @@ let format_check_summary = function
   | count -> Printf.sprintf "The %d files listed above need formatting" count
 
 let format_files ?poll ~check files =
+  let experimental_args = experimental_args () in
   let incorrect = ref 0 in
   let works =
     files
@@ -137,7 +149,8 @@ let format_files ?poll ~check files =
           {key = Printf.sprintf "%08d" index; dependencies = []; value = path})
   in
   let next path = function
-    | None -> Some (Compiler_process.task (format_request path))
+    | None ->
+      Some (Compiler_process.task (format_request ~experimental_args path))
     | Some result ->
       if not (Process.succeeded result) then
         raise (Error (formatting_error path result.stderr));

rewatch/CompilerConfigurationSpec.md

@@ -122,6 +122,7 @@ An object of feature flags to enable experimental compiler behavior. Only suppor
 Currently supported features:
 
 - LetUnwrap: Enable `let?` syntax.
+- MultilineItemSpacing: `rescript format` puts a blank line before and after a module item that spans several lines.
 
 ### Warnings