diff --git a/changes/formatter-multiline-item-spacing.polish.md b/changes/formatter-multiline-item-spacing.polish.md new file mode 100644 index 00000000000..b5d71338686 --- /dev/null +++ b/changes/formatter-multiline-item-spacing.polish.md @@ -0,0 +1 @@ +- 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. diff --git a/compiler/bsc/rescript_compiler_driver.ml b/compiler/bsc/rescript_compiler_driver.ml index 95b681469c6..c8c8dbff3e0 100644 --- a/compiler/bsc/rescript_compiler_driver.ml +++ b/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, diff --git a/compiler/ml/experimental_features.ml b/compiler/ml/experimental_features.ml index 5ec591c740f..67de5b9ce1f 100644 --- a/compiler/ml/experimental_features.ml +++ b/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 diff --git a/compiler/ml/experimental_features.mli b/compiler/ml/experimental_features.mli index ad3fdd925c2..e5900d1dfe4 100644 --- a/compiler/ml/experimental_features.mli +++ b/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 diff --git a/compiler/syntax/cli/res_cli.ml b/compiler/syntax/cli/res_cli.ml index da701416e81..01317f092b9 100644 --- a/compiler/syntax/cli/res_cli.ml +++ b/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 diff --git a/compiler/syntax/src/res_doc.ml b/compiler/syntax/src/res_doc.ml index e2222c22882..14369d3c0e9 100644 --- a/compiler/syntax/src/res_doc.ml +++ b/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 diff --git a/compiler/syntax/src/res_doc.mli b/compiler/syntax/src/res_doc.mli index d3423658c04..805b7748aa2 100644 --- a/compiler/syntax/src/res_doc.mli +++ b/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 *) diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index 8c0424b1ba3..01b63b7d303 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/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,15 +403,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 @@ -685,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 + print_list ~space_multiline:(space_multiline ()) ~get_loc:(fun s -> s.Parsetree.pstr_loc) ~nodes:structure ~print:(fun item cmt_tbl -> @@ -1108,7 +1113,7 @@ and print_signature ~state signature cmt_tbl = match signature with | [] -> print_comments_inside_file cmt_tbl | signature -> - print_list + print_list ~space_multiline:(space_multiline ()) ~get_loc:(fun s -> s.Parsetree.psig_loc) ~nodes:signature ~print:(print_signature_item ~state) diff --git a/docs/docson/build-schema.json b/docs/docson/build-schema.json index 21f48b1f223..4be54e2af44 100644 --- a/docs/docson/build-schema.json +++ b/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 diff --git a/rewatch-ocaml/config.ml b/rewatch-ocaml/config.ml index eb5fd35d5d7..e386b4bbeca 100644 --- a/rewatch-ocaml/config.ml +++ b/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] diff --git a/rewatch-ocaml/format.ml b/rewatch-ocaml/format.ml index a79bfb5740c..be27926f944 100644 --- a/rewatch-ocaml/format.ml +++ b/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)); diff --git a/rewatch/CompilerConfigurationSpec.md b/rewatch/CompilerConfigurationSpec.md index ca172caaaee..3e156c93457 100644 --- a/rewatch/CompilerConfigurationSpec.md +++ b/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 diff --git a/rewatch/src/config.rs b/rewatch/src/config.rs index 20f772b33b0..256ea7c0832 100644 --- a/rewatch/src/config.rs +++ b/rewatch/src/config.rs @@ -505,6 +505,7 @@ pub enum DeprecationWarning { #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub enum ExperimentalFeature { LetUnwrap, + MultilineItemSpacing, } impl<'de> serde::Deserialize<'de> for ExperimentalFeature { @@ -524,8 +525,9 @@ impl<'de> serde::Deserialize<'de> for ExperimentalFeature { { match v { "LetUnwrap" => Ok(ExperimentalFeature::LetUnwrap), + "MultilineItemSpacing" => Ok(ExperimentalFeature::MultilineItemSpacing), other => { - let available = ["LetUnwrap"].join(", "); + let available = ["LetUnwrap", "MultilineItemSpacing"].join(", "); Err(DeError::custom(format!( "Unknown experimental feature '{other}'. Available features: {available}", ))) @@ -963,6 +965,7 @@ impl Config { "-enable-experimental".to_string(), match feature { ExperimentalFeature::LetUnwrap => "LetUnwrap", + ExperimentalFeature::MultilineItemSpacing => "MultilineItemSpacing", } .to_string(), ] diff --git a/rewatch/src/format.rs b/rewatch/src/format.rs index eed70f0812f..44a50cdd837 100644 --- a/rewatch/src/format.rs +++ b/rewatch/src/format.rs @@ -16,10 +16,11 @@ use clap::ValueEnum; #[instrument(name = "format.format", skip_all)] pub fn format(stdin_extension: Option, check: bool, files: Vec) -> Result<()> { let bsc_path = helpers::get_bsc(); + let experimental_args = experimental_args(); match stdin_extension { Some(extension) => { - format_stdin(&bsc_path, extension)?; + format_stdin(&bsc_path, &experimental_args, extension)?; } None => { let files = if files.is_empty() { @@ -27,13 +28,24 @@ pub fn format(stdin_extension: Option, check: bool, files: Vec Vec { + std::env::current_dir() + .ok() + .and_then(|dir| project_context::ProjectContext::new(&dir).ok()) + .map(|context| context.get_root_config().get_experimental_features_args()) + .unwrap_or_default() +} + fn get_files_in_scope() -> Result> { let current_dir = std::env::current_dir()?; let project_context = project_context::ProjectContext::new(¤t_dir)?; @@ -59,7 +71,7 @@ fn get_files_in_scope() -> Result> { Ok(files) } -fn format_stdin(bsc_exe: &Path, extension: FileExtension) -> Result<()> { +fn format_stdin(bsc_exe: &Path, experimental_args: &[String], extension: FileExtension) -> Result<()> { let extension_value = extension .to_possible_value() .ok_or(anyhow::anyhow!("Could not get extension arg value"))?; @@ -71,7 +83,7 @@ fn format_stdin(bsc_exe: &Path, extension: FileExtension) -> Result<()> { let temp_path = temp_file.path(); let mut cmd = Command::new(bsc_exe); - cmd.arg("-format").arg(temp_path); + cmd.args(experimental_args).arg("-format").arg(temp_path); let output = cmd.output()?; @@ -85,7 +97,7 @@ fn format_stdin(bsc_exe: &Path, extension: FileExtension) -> Result<()> { Ok(()) } -fn format_files(bsc_exe: &Path, files: Vec, check: bool) -> Result<()> { +fn format_files(bsc_exe: &Path, experimental_args: &[String], files: Vec, check: bool) -> Result<()> { let batch_size = 4 * num_cpus::get(); let incorrectly_formatted_files = AtomicUsize::new(0); @@ -93,7 +105,7 @@ fn format_files(bsc_exe: &Path, files: Vec, check: bool) -> Result<()> { batch.iter().try_for_each(|file| { let mut cmd = Command::new(bsc_exe); // Always get formatted output to stdout for comparison - cmd.arg("-format").arg(file); + cmd.args(experimental_args).arg("-format").arg(file); let output = cmd.output()?; diff --git a/rewatch/tests/experimental-invalid/01-invalid-experimental-key.sh b/rewatch/tests/experimental-invalid/01-invalid-experimental-key.sh index ae52f70ae7d..0df1238a304 100755 --- a/rewatch/tests/experimental-invalid/01-invalid-experimental-key.sh +++ b/rewatch/tests/experimental-invalid/01-invalid-experimental-key.sh @@ -25,7 +25,7 @@ if [ $status -eq 0 ]; then exit 1 fi -echo "$out" | grep -q "Unknown experimental feature 'FooBar'. Available features: LetUnwrap" +echo "$out" | grep -q "Unknown experimental feature 'FooBar'. Available features: LetUnwrap, MultilineItemSpacing" if [ $? -ne 0 ]; then error "Missing helpful message for unknown experimental feature" echo "$out" diff --git a/rewatch/tests/format/05-format-experimental-multiline-item-spacing.sh b/rewatch/tests/format/05-format-experimental-multiline-item-spacing.sh new file mode 100755 index 00000000000..ecd254259af --- /dev/null +++ b/rewatch/tests/format/05-format-experimental-multiline-item-spacing.sh @@ -0,0 +1,51 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: format spaces multi-line items only with MultilineItemSpacing" + +input='module A = { + let x = 1 +} +let y = 2 +let z = 3 +' +expected='module A = { + let x = 1 +} + +let y = 2 +let z = 3' + +# The formatter writes CRLF on Windows. $(...) drops the trailing newline, so +# compare against the input without it. +without=$(printf '%s' "$input" | "$REWATCH_EXECUTABLE" format --stdin .res) +status=$? +without=$(printf '%s' "$without" | tr -d '\r') +if [ $status -ne 0 ] || [ "$without" != "${input%$'\n'}" ]; then + error "Format changed spacing without the experimental feature" + echo "$without" + exit 1 +fi + +cp rescript.json rescript.json.bak +node -e ' +const fs=require("fs"); +const j=JSON.parse(fs.readFileSync("rescript.json","utf8")); +j["experimental-features"]={MultilineItemSpacing:true}; +fs.writeFileSync("rescript.json", JSON.stringify(j,null,2)); +' + +with=$(printf '%s' "$input" | "$REWATCH_EXECUTABLE" format --stdin .res) +status=$? +mv rescript.json.bak rescript.json +with=$(printf '%s' "$with" | tr -d '\r') + +if [ $status -ne 0 ] || [ "$with" != "$expected" ]; then + error "Format did not space multi-line items with MultilineItemSpacing" + echo "$with" + exit 1 +fi + +success "MultilineItemSpacing is opt-in for format" diff --git a/rewatch/tests/suite.sh b/rewatch/tests/suite.sh index a1551ffcc97..418724c8bbc 100755 --- a/rewatch/tests/suite.sh +++ b/rewatch/tests/suite.sh @@ -120,6 +120,7 @@ fi ./format/02-format-single-file.sh && ./format/03-format-stdin.sh && ./format/04-format-current-project.sh && +./format/05-format-experimental-multiline-item-spacing.sh && # Clean tests ./clean/01-clean-single-project.sh && diff --git a/scripts/test_syntax.sh b/scripts/test_syntax.sh index c615a3b9f0a..2bac2862b06 100755 --- a/scripts/test_syntax.sh +++ b/scripts/test_syntax.sh @@ -60,6 +60,17 @@ while read file; do $DUNE_BIN_DIR/res_parser -test-ast-conversion -jsx-version 4 $file &> $(exp $file) & maybeWait done / +function experimentalFlags { + case $1 in + syntax_tests/data/experimental/*) echo "-enable-experimental $(echo $1 | cut -d/ -f4)" ;; + esac +} +find syntax_tests/data/experimental -name "*.res" -o -name "*.resi" >temp/files.txt +while read file; do + $DUNE_BIN_DIR/res_parser $(experimentalFlags $file) $file &> $(exp $file) & maybeWait +done temp/files.txt while read file; do @@ -88,7 +99,7 @@ if [[ $ROUNDTRIP_TEST = 1 ]]; then roundtripTestsResult="temp/result.txt" touch $roundtripTestsResult - find syntax_tests/data/{idempotency,printer} -name "*.res" -o -name "*.resi" >temp/files.txt + find syntax_tests/data/{idempotency,printer,experimental} -name "*.res" -o -name "*.resi" >temp/files.txt while read file; do { mkdir -p temp/$(dirname $file) rescript1=temp/$file.res @@ -100,8 +111,9 @@ if [[ $ROUNDTRIP_TEST = 1 ]]; then esac # Format the original file, then format the result again - $DUNE_BIN_DIR/res_parser $resIntf -print res $file > $rescript1 - $DUNE_BIN_DIR/res_parser $resIntf -print res $rescript1 > $rescript2 + flags="$resIntf $(experimentalFlags $file)" + $DUNE_BIN_DIR/res_parser $flags -print res $file > $rescript1 + $DUNE_BIN_DIR/res_parser $flags -print res $rescript1 > $rescript2 # Check idempotency: formatting the formatted text must not change it diff --unified $rescript1 $rescript2 diff --git a/tests/syntax_tests/data/experimental/MultilineItemSpacing/expected/multilineSpacing.res.txt b/tests/syntax_tests/data/experimental/MultilineItemSpacing/expected/multilineSpacing.res.txt new file mode 100644 index 00000000000..083f6ae7239 --- /dev/null +++ b/tests/syntax_tests/data/experimental/MultilineItemSpacing/expected/multilineSpacing.res.txt @@ -0,0 +1,71 @@ +module Details = { + @react.component + let make = (~input as {filePath}: input) => React.string(filePath) +} + +module Body = { + @react.component + let make = (~input as {filePath, oldString}: input) => <> + + + +} + +module Output = Tool.TextResult.Output +let failed = Tool.TextResult.failed + +// one-line items stay together, a comment above one does not count +let a = 1 +// about b +let b = 2 + +/** a doc comment prints on its own line */ +let c = 3 + +let d = 4 // trailing + +// an item that only breaks because of the line width is multi-line +let long = someFunctionWithAVeryLongName( + argumentNumberOne, + argumentNumberTwo, + argumentNumber3, + argument4, +) + +let e = 5 + +type r = { + x: int, +} + +let f = 6 + +// an own-line trailing comment of the previous item goes below the blank line +f(a => 1) + +// relocated +let g = { + 1 +} + +// blocks are not affected +let h = () => { + let x = 1 + let y = { + 2 + } + x + y +} + +// a template literal with a line break is multi-line +let s = `multi +line` + +let t = 5 + +// a trailing comment stays on the last line of a multi-line item +let g = { + 1 +} // trailing + +let h = 6 diff --git a/tests/syntax_tests/data/experimental/MultilineItemSpacing/expected/multilineSpacing.resi.txt b/tests/syntax_tests/data/experimental/MultilineItemSpacing/expected/multilineSpacing.resi.txt new file mode 100644 index 00000000000..28194c311d2 --- /dev/null +++ b/tests/syntax_tests/data/experimental/MultilineItemSpacing/expected/multilineSpacing.resi.txt @@ -0,0 +1,19 @@ +type t +let a: int + +type r = { + x: int, +} + +let b: int +// a comment does not count +let c: int + +/** Documented */ +let d: int + +module M: { + let x: int +} + +let e: int diff --git a/tests/syntax_tests/data/experimental/MultilineItemSpacing/multilineSpacing.res b/tests/syntax_tests/data/experimental/MultilineItemSpacing/multilineSpacing.res new file mode 100644 index 00000000000..7eb0267f80d --- /dev/null +++ b/tests/syntax_tests/data/experimental/MultilineItemSpacing/multilineSpacing.res @@ -0,0 +1,57 @@ +module Details = { + @react.component + let make = (~input as {filePath}: input) => React.string(filePath) +} +module Body = { + @react.component + let make = (~input as {filePath, oldString}: input) => <> + + + +} +module Output = Tool.TextResult.Output +let failed = Tool.TextResult.failed + +// one-line items stay together, a comment above one does not count +let a = 1 +// about b +let b = 2 +/** a doc comment prints on its own line */ +let c = 3 +let d = 4 // trailing + +// an item that only breaks because of the line width is multi-line +let long = someFunctionWithAVeryLongName(argumentNumberOne, argumentNumberTwo, argumentNumber3, argument4) +let e = 5 +type r = { + x: int, +} +let f = 6 + +// an own-line trailing comment of the previous item goes below the blank line +f(a + // relocated + => 1) +let g = { + 1 +} + +// blocks are not affected +let h = () => { + let x = 1 + let y = { + 2 + } + x + y +} + +// a template literal with a line break is multi-line +let s = `multi +line` +let t = 5 + +// a trailing comment stays on the last line of a multi-line item +let g = { + 1 +} // trailing +let h = 6 diff --git a/tests/syntax_tests/data/experimental/MultilineItemSpacing/multilineSpacing.resi b/tests/syntax_tests/data/experimental/MultilineItemSpacing/multilineSpacing.resi new file mode 100644 index 00000000000..820ef4a6c11 --- /dev/null +++ b/tests/syntax_tests/data/experimental/MultilineItemSpacing/multilineSpacing.resi @@ -0,0 +1,14 @@ +type t +let a: int +type r = { + x: int, +} +let b: int +// a comment does not count +let c: int +/** Documented */ +let d: int +module M: { + let x: int +} +let e: int diff --git a/tests/syntax_tests/res_test.ml b/tests/syntax_tests/res_test.ml index 461ff2866c4..56f40f837c4 100644 --- a/tests/syntax_tests/res_test.ml +++ b/tests/syntax_tests/res_test.ml @@ -525,6 +525,179 @@ module Parser_api_test = struct windows_crlf () end +(* Res_doc.item_separator puts a blank line next to an item that prints on + several lines, measuring the documents on each side as to_string lays them + out. Res_doc.comment hides comments from the measurement. *) +module Multiline_item_spacing_test = struct + module Doc = Res_doc + + let check name ~expected actual = + if actual <> expected then + failwith + (Printf.sprintf "%s:\nexpected:\n%S\nactual:\n%S" name expected actual) + + (* Module items are printed inside a broken group, like print_structure. *) + let print ?(width = 80) doc = + Doc.to_string ~width (Doc.breakable_group ~force_break:true doc) + let items prev next = Doc.concat [prev; Doc.item_separator ~prev ~next; next] + let a = Doc.text "let a = 1" + let b = Doc.text "let b = 2" + + (* A function call that fits on one line at width 80 but not at width 10. *) + let call = + Doc.group + (Doc.concat + [ + Doc.text "f("; + Doc.indent + (Doc.concat + [ + Doc.soft_line; + Doc.text "x,"; + Doc.line; + Doc.text "y"; + Doc.trailing_comma; + ]); + Doc.soft_line; + Doc.text ")"; + ]) + + let measuring () = + check "one-line items stay together" ~expected:"let a = 1\nlet b = 2" + (print (items a b)); + check "a hard line spans lines" ~expected:"x\ny\n\nlet b = 2" + (print (items (Doc.concat [Doc.text "x"; Doc.hard_line; Doc.text "y"]) b)); + check "a literal line spans lines" ~expected:"`x\ny`\n\nlet b = 2" + (print + (items (Doc.concat [Doc.text "`x"; Doc.literal_line; Doc.text "y`"]) b)); + check "a newline inside text spans lines" ~expected:"`x\ny`\n\nlet b = 2" + (print (items (Doc.text "`x\ny`") b)); + check "an item list spans lines" + ~expected:"let a = 1\nlet b = 2\n\nlet c = 3" + (print (items (items a b) (Doc.text "let c = 3"))); + check "a group that fits is one line" ~expected:"f(x, y)\nlet b = 2" + (print (items call b)); + check "a group that breaks spans lines" + ~expected:"f(\n x,\n y,\n)\n\nlet b = 2" + (print ~width:5 (items call b)); + check "a chosen custom layout is measured" ~expected:"let a = 1\nlet b = 2" + (print (items (Doc.custom_layout [a]) b)); + check "break parents and line suffixes do not span lines" + ~expected:"let a = 1 // c\nlet b = 2" + (print + (items + (Doc.concat + [a; Doc.break_parent; Doc.line_suffix (Doc.text " // c")]) + b)); + check "a comment above an item does not count" + ~expected:"let a = 1\n// c\nlet b = 2" + (print + (items a + (Doc.concat + [Doc.comment (Doc.concat [Doc.text "// c"; Doc.hard_line]); b]))); + let forced_if_breaks = + Doc.if_breaks (Doc.text "a") (Doc.concat [Doc.text "b"; Doc.hard_line]) + in + check "an if-breaks broken by its flat branch takes the break branch" + ~expected:"xa\nlet b = 2" + (print (items (Doc.concat [Doc.text "x"; forced_if_breaks]) b)); + check "an if-breaks in a broken group takes the break branch" + ~expected:"{\n}\n\nlet b = 2" + (print + (items + (Doc.breakable_group ~force_break:true + (Doc.concat + [ + Doc.if_breaks (Doc.text "{") (Doc.text "("); + Doc.line; + Doc.text "}"; + ])) + b)) + + let trailing_comments () = + let block = + Doc.concat [Doc.text "let a = {"; Doc.hard_line; Doc.text "}"] + in + let after_block suffix = + items (Doc.concat [block; Doc.line_suffix suffix]) b + in + check "a same-line trailing comment stays on its line" + ~expected:"let a = {\n} // c\n\nlet b = 2" + (print (after_block (Doc.text " // c"))); + check "an own-line trailing comment moves below the blank line" + ~expected:"let a = {\n}\n\n// c\nlet b = 2" + (print (after_block (Doc.concat [Doc.hard_line; Doc.text "// c"]))); + check "an empty own-line trailing comment prints an empty line" + ~expected:"let a = {\n}\n\n\nlet b = 2" + (print (after_block Doc.hard_line)) + + let predicates () = + let separator = Doc.item_separator ~prev:a ~next:b in + assert (Doc.will_break separator); + assert (Doc.will_break Doc.break_parent); + assert (Doc.will_break (Doc.comment Doc.hard_line)); + assert (not (Doc.will_break (Doc.comment a))); + assert (Doc.is_nil Doc.nil); + [ + a; + Doc.line_suffix a; + Doc.concat [a; b]; + Doc.indent a; + Doc.trailing_comma; + Doc.line; + Doc.group a; + Doc.custom_layout [a]; + Doc.break_parent; + separator; + Doc.comment a; + ] + |> List.iter (fun doc -> assert (not (Doc.is_nil doc))); + check "an empty custom layout prints nothing" ~expected:"" + (print (Doc.custom_layout [])) + + (* debug prints to stdout; send it to the null device. *) + let debug () = + flush stdout; + let saved = Unix.dup Unix.stdout in + let null = Unix.openfile Filename.null [Unix.O_WRONLY] 0 in + Unix.dup2 null Unix.stdout; + Fun.protect + ~finally:(fun () -> + flush stdout; + Unix.dup2 saved Unix.stdout; + Unix.close saved; + Unix.close null) + (fun () -> Doc.debug (Doc.concat [items a b; Doc.comment a])) + + let printer () = + let source = "module A = {\n let x = 1\n}\nlet y = 2\n" in + let format () = + let result = + Res_driver.parse_implementation_from_source ~display_filename:"A.res" + ~source + in + Res_printer.print_implementation ~width:80 result.parsetree + ~comments:result.comments + in + check "the printer keeps items together by default" ~expected:source + (format ()); + Experimental_features.enable_from_string + (Experimental_features.to_string + Experimental_features.MultilineItemSpacing); + Fun.protect ~finally:Experimental_features.reset (fun () -> + check "the printer spaces items with MultilineItemSpacing" + ~expected:"module A = {\n let x = 1\n}\n\nlet y = 2\n" (format ())) + + let run () = + measuring (); + trailing_comments (); + predicates (); + debug (); + printer (); + print_endline "✅ multi-line item spacing" +end + let () = Outcome_printer_tests.run () let () = Parser_api_test.run () let () = Res_utf8_test.run () +let () = Multiline_item_spacing_test.run ()