From 1cbe9772cd294580c64535766be7449dbae8c6f8 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 25 Oct 2026 17:09:17 +0100 Subject: [PATCH 1/3] Keep each file's line endings when formatting On Windows, bsc -format wrote its output through text-mode stdout, so every LF became CRLF: rescript format rewrote LF files to CRLF and format --check reported every LF file. Formatting a CRLF file was not stable either, on any platform: the scanner keeps the CRs inside comments, template literals and raw JS, so those lines came out as CR LF beside the printer's plain LF, and text-mode output or an editor turned them into CR CR LF, adding a blank line on each run. Formatting now reads the source with LF line endings and gives the result CRLF ones when the source's first line break is CRLF. bsc -format and res_parser -print res write stdout in binary mode, also inside a captured compiler request, and the editor format command keeps a CRLF document's line endings. With that, Windows CI runs the syntax roundtrip tests and the OCaml and ReScript format checks, whose file lists now go through xargs to stay under the Windows command-line length limit. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Christoph Knittel --- .github/workflows/ci.yml | 2 +- analysis/src/commands.ml | 16 ++++-- changes/format-line-endings.fix.md | 1 + changes/windows-format-tests.internal.md | 1 + compiler/bsc/rescript_compiler_driver.ml | 2 +- compiler/ext/compiler_request_output.ml | 52 +++++++++++++++++-- compiler/ext/compiler_request_output.mli | 5 ++ compiler/syntax/cli/res_cli.ml | 26 ++++++---- compiler/syntax/src/res_driver.ml | 4 +- compiler/syntax/src/res_line_endings.ml | 31 +++++++++++ compiler/syntax/src/res_line_endings.mli | 17 ++++++ compiler/syntax/src/res_multi_printer.ml | 12 +++-- .../tests/format/05-format-line-endings.sh | 45 ++++++++++++++++ rewatch/tests/suite.sh | 1 + scripts/format.sh | 6 ++- scripts/format_check.sh | 39 ++++++-------- .../ounit_compiler_request_tests.ml | 32 ++++++++++++ tests/syntax_tests/res_test.ml | 31 +++++++++++ 18 files changed, 273 insertions(+), 50 deletions(-) create mode 100644 changes/format-line-endings.fix.md create mode 100644 changes/windows-format-tests.internal.md create mode 100644 compiler/syntax/src/res_line_endings.ml create mode 100644 compiler/syntax/src/res_line_endings.mli create mode 100755 rewatch/tests/format/05-format-line-endings.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 59a7cb695b1..b331db6dd89 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -167,7 +167,7 @@ jobs: - name: "Syntax: Run tests" env: - ROUNDTRIP_TEST: ${{ runner.os == 'Windows' && '0' || '1' }} + ROUNDTRIP_TEST: 1 run: ./scripts/test_syntax.sh shell: bash diff --git a/analysis/src/commands.ml b/analysis/src/commands.ml index 746c9d9ea50..70ef05b26f9 100644 --- a/analysis/src/commands.ml +++ b/analysis/src/commands.ml @@ -300,6 +300,13 @@ let format ~source ~kind_file = Lsp.Types.TextEdit.create ~newText:text ~range in + (* Keep a CRLF document's line endings, rather than return mixed ones *) + let restore_line_endings text = + if Res_line_endings.uses_crlf source then Res_line_endings.to_crlf text + else text + in + let source = Res_line_endings.to_lf source in + let result = match kind_file with | Files.Res -> ( @@ -309,8 +316,9 @@ let format ~source ~kind_file = match List.length diagnostics > 0 with | true -> Error "Document has syntax errors" | false -> - Ok (Res_printer.print_implementation ~comments structure |> create_range) - ) + Ok + (Res_printer.print_implementation ~comments structure + |> restore_line_endings |> create_range)) | Resi -> ( let {Res_driver.parsetree = signature; comments; diagnostics} = Res_driver.parsing_engine.parse_interface_from_source ~source @@ -318,7 +326,9 @@ let format ~source ~kind_file = match List.length diagnostics > 0 with | true -> Error "Document has syntax errors" | false -> - Ok (Res_printer.print_interface ~comments signature |> create_range)) + Ok + (Res_printer.print_interface ~comments signature + |> restore_line_endings |> create_range)) | Other -> Error "Failed to format, file not supported" in diff --git a/changes/format-line-endings.fix.md b/changes/format-line-endings.fix.md new file mode 100644 index 00000000000..4da23a3b539 --- /dev/null +++ b/changes/format-line-endings.fix.md @@ -0,0 +1 @@ +- Make the formatter keep each file's line endings. On Windows it no longer rewrites LF files with CRLF or reports them in `rescript format --check`, and formatting a CRLF file no longer adds a carriage return inside multi-line comments, template literals and raw JS on every run. diff --git a/changes/windows-format-tests.internal.md b/changes/windows-format-tests.internal.md new file mode 100644 index 00000000000..99ac025800d --- /dev/null +++ b/changes/windows-format-tests.internal.md @@ -0,0 +1 @@ +- Run the syntax roundtrip tests and the OCaml and ReScript format checks on Windows CI. diff --git a/compiler/bsc/rescript_compiler_driver.ml b/compiler/bsc/rescript_compiler_driver.ml index 95b681469c6..c4a5214b584 100644 --- a/compiler/bsc/rescript_compiler_driver.ml +++ b/compiler/bsc/rescript_compiler_driver.ml @@ -176,7 +176,7 @@ let format_file input = input in match !((Clflags.current ()).output_name) with - | None -> Compiler_request_output.write_stdout formatted + | None -> Compiler_request_output.write_stdout_binary formatted | Some fname -> Ext_io.write_file fname formatted let set_color_option option = diff --git a/compiler/ext/compiler_request_output.ml b/compiler/ext/compiler_request_output.ml index ae1ac7ff39f..06dc8bb538b 100644 --- a/compiler/ext/compiler_request_output.ml +++ b/compiler/ext/compiler_request_output.ml @@ -5,6 +5,9 @@ type sink = { prefix: string; buffer: Buffer.t; mutable file: (string * out_channel) option; + mutable exact: bool; + (* The buffer holds the final bytes: once binary output is written, text + is converted as it is written instead of when it is read. *) } type streams = { @@ -55,8 +58,30 @@ let write_sink sink text position length = | Some (_, channel) -> output_string channel (text_mode_input ~win32:Sys.win32 (String.sub text position length)) + | None when sink.exact -> + Buffer.add_string sink.buffer + (text_mode_line_endings ~win32:Sys.win32 + (String.sub text position length)) | None -> Buffer.add_substring sink.buffer text position length +let write_sink_binary sink text = + match sink.file with + | Some (_, channel) -> + flush channel; + set_binary_mode_out channel true; + output_string channel text; + flush channel; + set_binary_mode_out channel false + | None -> + if not sink.exact then ( + let converted = + text_mode_line_endings ~win32:Sys.win32 (Buffer.contents sink.buffer) + in + Buffer.clear sink.buffer; + Buffer.add_string sink.buffer converted; + sink.exact <- true); + Buffer.add_string sink.buffer text + let sink_channel sink = match sink.file with | Some (_, channel) -> channel @@ -65,15 +90,22 @@ let sink_channel sink = Filename.open_temp_file ~mode:[Open_text] sink.prefix ".log" in (* Output written so far goes first. The channel is in text mode, so it - already converts line endings. *) - output_string channel - (text_mode_input ~win32:Sys.win32 (Buffer.contents sink.buffer)); + already converts line endings, unless the buffer holds final bytes. *) + if sink.exact then ( + set_binary_mode_out channel true; + output_string channel (Buffer.contents sink.buffer); + flush channel; + set_binary_mode_out channel false) + else + output_string channel + (text_mode_input ~win32:Sys.win32 (Buffer.contents sink.buffer)); Buffer.clear sink.buffer; sink.file <- Some (path, channel); channel let sink_contents sink = match sink.file with + | None when sink.exact -> Buffer.contents sink.buffer | None -> text_mode_line_endings ~win32:Sys.win32 (Buffer.contents sink.buffer) | Some (path, channel) -> @@ -115,6 +147,16 @@ let write_stdout text = | Some streams -> write_sink streams.stdout text 0 (String.length text) | None -> output_string Stdlib.stdout text +let write_stdout_binary text = + match current () with + | Some streams -> write_sink_binary streams.stdout text + | None -> + flush Stdlib.stdout; + set_binary_mode_out Stdlib.stdout true; + output_string Stdlib.stdout text; + flush Stdlib.stdout; + set_binary_mode_out Stdlib.stdout false + let write_stderr text = match current () with | Some streams -> write_sink streams.stderr text 0 (String.length text) @@ -134,7 +176,9 @@ let print_stderr text = exception Failed of {error: exn; stdout: string; stderr: string} let with_capture action = - let sink prefix = {prefix; buffer = Buffer.create 256; file = None} in + let sink prefix = + {prefix; buffer = Buffer.create 256; file = None; exact = false} + in let stdout = sink "rescript-compiler-stdout-" in let stderr = sink "rescript-compiler-stderr-" in let streams = diff --git a/compiler/ext/compiler_request_output.mli b/compiler/ext/compiler_request_output.mli index 14a88a017f5..57448cb84ac 100644 --- a/compiler/ext/compiler_request_output.mli +++ b/compiler/ext/compiler_request_output.mli @@ -6,6 +6,11 @@ val is_active : unit -> bool val stdout_formatter : unit -> Format.formatter val stderr_formatter : unit -> Format.formatter val write_stdout : string -> unit + +val write_stdout_binary : string -> unit +(** Write [text] as is, without the CRLF line endings that a text-mode channel + writes on Windows. *) + val write_stderr : string -> unit val print_stdout : string -> unit val print_stderr : string -> unit diff --git a/compiler/syntax/cli/res_cli.ml b/compiler/syntax/cli/res_cli.ml index da701416e81..6c95fa91139 100644 --- a/compiler/syntax/cli/res_cli.ml +++ b/compiler/syntax/cli/res_cli.ml @@ -61,9 +61,6 @@ end = struct end module Cli_arg_processor = struct - type backend = Parser : 'diagnostics Res_driver.parsing_engine -> backend - [@@unboxed] - let process_file ~is_interface ~width ~recover ~target ~jsx_version ~jsx_module ~test_ast_conversion filename = let len = String.length filename in @@ -71,7 +68,6 @@ module Cli_arg_processor = struct is_interface || (len > 0 && (String.get [@doesNotRaise]) filename (len - 1) = 'i') in - let parsing_engine = Parser Res_driver.parsing_engine in let print_engine = match target with | "ml" -> Res_driver_ml_printer.print_engine @@ -87,7 +83,9 @@ module Cli_arg_processor = struct exit 1 in - let (Parser backend) = parsing_engine in + (* Read sources with LF line endings, so a CRLF file prints like an LF one, + also inside comments, template literals and raw JS. *) + let load_source () = Res_line_endings.to_lf (Ext_io.load_file filename) in (* Color the Format tags (e.g. @{...@}) of printed diagnostics *) Misc.Color.setup None; @@ -95,10 +93,13 @@ module Cli_arg_processor = struct if target = "tokens" then print_engine.print_implementation ~width ~filename ~comments:[] [] else if process_interface then - let parse_result = backend.parse_interface ~filename in + let parse_result = + Res_driver.parse_interface_from_source ~display_filename:filename + ~source:(load_source ()) + in if parse_result.invalid then ( - backend.string_of_diagnostics ~source:parse_result.source - ~filename:parse_result.filename parse_result.diagnostics; + Res_diagnostics.print_report parse_result.diagnostics + parse_result.source; if recover then print_engine.print_interface ~width ~filename ~comments:parse_result.comments parse_result.parsetree @@ -120,10 +121,13 @@ module Cli_arg_processor = struct print_engine.print_interface ~width ~filename ~comments:parse_result.comments parsetree else - let parse_result = backend.parse_implementation ~filename in + let parse_result = + Res_driver.parse_implementation_from_source ~display_filename:filename + ~source:(load_source ()) + in if parse_result.invalid then ( - backend.string_of_diagnostics ~source:parse_result.source - ~filename:parse_result.filename parse_result.diagnostics; + Res_diagnostics.print_report parse_result.diagnostics + parse_result.source; if recover then print_engine.print_implementation ~width ~filename ~comments:parse_result.comments parse_result.parsetree diff --git a/compiler/syntax/src/res_driver.ml b/compiler/syntax/src/res_driver.ml index 13f34613d45..8e98f0beddf 100644 --- a/compiler/syntax/src/res_driver.ml +++ b/compiler/syntax/src/res_driver.ml @@ -155,11 +155,11 @@ let print_engine = { print_implementation = (fun ~width ~filename:_ ~comments structure -> - Compiler_request_output.write_stdout + Compiler_request_output.write_stdout_binary (Res_printer.print_implementation ~width structure ~comments)); print_interface = (fun ~width ~filename:_ ~comments signature -> - Compiler_request_output.write_stdout + Compiler_request_output.write_stdout_binary (Res_printer.print_interface ~width signature ~comments)); } diff --git a/compiler/syntax/src/res_line_endings.ml b/compiler/syntax/src/res_line_endings.ml new file mode 100644 index 00000000000..3c8b771079f --- /dev/null +++ b/compiler/syntax/src/res_line_endings.ml @@ -0,0 +1,31 @@ +let uses_crlf source = + match String.index_opt source '\n' with + | Some index -> index > 0 && source.[index - 1] = '\r' + | None -> false + +let to_lf source = + if not (String.contains source '\r') then source + else + let length = String.length source in + let converted = Buffer.create length in + String.iteri + (fun index character -> + if + not + (character = '\r' && index + 1 < length && source.[index + 1] = '\n') + then Buffer.add_char converted character) + source; + Buffer.contents converted + +let to_crlf text = + let converted = Buffer.create (String.length text) in + String.iter + (fun character -> + if character = '\n' then Buffer.add_char converted '\r'; + Buffer.add_char converted character) + text; + Buffer.contents converted + +let preserve source print = + let printed = print (to_lf source) in + if uses_crlf source then to_crlf printed else printed diff --git a/compiler/syntax/src/res_line_endings.mli b/compiler/syntax/src/res_line_endings.mli new file mode 100644 index 00000000000..cce096a94e6 --- /dev/null +++ b/compiler/syntax/src/res_line_endings.mli @@ -0,0 +1,17 @@ +(* The printer emits LF line endings, but the scanner keeps a CRLF source's CRs + inside comments, template literals and raw JS. Printed as is, those lines + end in CR LF next to plain LF lines, and an editor or a text-mode channel + that turns each LF into CRLF makes them CR CR LF. *) + +(* Whether the first line break of [source] is CRLF *) +val uses_crlf : string -> bool + +(* Replaces each CRLF with LF. A lone CR stays. *) +val to_lf : string -> string + +(* Replaces each LF with CRLF. *) +val to_crlf : string -> string + +(* [preserve source print] prints [source] with LF line endings and gives the + result CRLF line endings when [source] uses them. *) +val preserve : string -> (string -> string) -> string diff --git a/compiler/syntax/src/res_multi_printer.ml b/compiler/syntax/src/res_multi_printer.ml index f126eb7b7af..88eee70ee42 100644 --- a/compiler/syntax/src/res_multi_printer.ml +++ b/compiler/syntax/src/res_multi_printer.ml @@ -1,7 +1,9 @@ (* print res files to res syntax *) -let print_res ~ignore_parse_errors ~is_interface ~filename = +let print_res ~ignore_parse_errors ~is_interface ~filename source = if is_interface then ( - let parse_result = Res_driver.parsing_engine.parse_interface ~filename in + let parse_result = + Res_driver.parse_interface_from_source ~display_filename:filename ~source + in if parse_result.invalid then ( Res_diagnostics.print_report parse_result.diagnostics parse_result.source; if not ignore_parse_errors then raise_notrace Res_driver.Already_reported); @@ -9,7 +11,8 @@ let print_res ~ignore_parse_errors ~is_interface ~filename = ~comments:parse_result.comments parse_result.parsetree) else let parse_result = - Res_driver.parsing_engine.parse_implementation ~filename + Res_driver.parse_implementation_from_source ~display_filename:filename + ~source in if parse_result.invalid then ( Res_diagnostics.print_report parse_result.diagnostics parse_result.source; @@ -24,7 +27,8 @@ let print ?(ignore_parse_errors = false) input = let len = String.length input in len > 0 && String.unsafe_get input (len - 1) = 'i' in - print_res ~ignore_parse_errors ~is_interface ~filename:input + Res_line_endings.preserve (Ext_io.load_file input) + (print_res ~ignore_parse_errors ~is_interface ~filename:input) [@@raises Res_driver.Already_reported] (* suppress unused optional arg *) diff --git a/rewatch/tests/format/05-format-line-endings.sh b/rewatch/tests/format/05-format-line-endings.sh new file mode 100755 index 00000000000..ea1a2173e80 --- /dev/null +++ b/rewatch/tests/format/05-format-line-endings.sh @@ -0,0 +1,45 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: It should keep each file's line endings" + +dir=format-line-endings +mkdir -p $dir +trap 'rm -rf "$dir"' EXIT + +# Formatted source with lines inside a comment and a template literal, which +# used to get CR CR LF line endings when formatted with CRLF ones +formatted='/* a\n multi-line comment */\nlet s = `line 1\nline 2`\n' +printf "$formatted" > $dir/Lf.res +printf "$formatted" | sed 's/$/\r/' > $dir/Crlf.res +printf 'let x = 1\n' > $dir/UnformattedLf.res +printf 'let x = 1\r\n' > $dir/UnformattedCrlf.res + +if ! check_output=$(rewatch format --check $dir/Lf.res $dir/Crlf.res 2>&1); then + error "Formatted LF and CRLF files failed the format check" + echo "$check_output" + exit 1 +fi + +cp $dir/Lf.res $dir/Lf.expected +cp $dir/Crlf.res $dir/Crlf.expected +printf 'let x = 1\n' > $dir/UnformattedLf.expected +printf 'let x = 1\r\n' > $dir/UnformattedCrlf.expected + +if ! format_output=$(rewatch format $dir/*.res 2>&1); then + error "Error formatting LF and CRLF files" + echo "$format_output" + exit 1 +fi + +for file in Lf Crlf UnformattedLf UnformattedCrlf; do + if ! cmp -s $dir/$file.res $dir/$file.expected; then + error "Formatting $file.res changed its line endings" + od -c $dir/$file.res | head -10 + exit 1 + fi +done + +success "LF and CRLF files keep their line endings" diff --git a/rewatch/tests/suite.sh b/rewatch/tests/suite.sh index a1551ffcc97..757d4407596 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-line-endings.sh && # Clean tests ./clean/01-clean-single-project.sh && diff --git a/scripts/format.sh b/scripts/format.sh index b162e3784fb..48694989afa 100755 --- a/scripts/format.sh +++ b/scripts/format.sh @@ -8,8 +8,10 @@ echo Formatting OCaml code... opam exec -- dune build @fmt --auto-promote echo Formatting ReScript code... -files=$(find packages tests -type f \( -name "*.res" -o -name "*.resi" \) ! -name "syntaxErrors*" ! -name "generated_mocha_test.res" ! -path "tests/build_tests/super_errors/fixtures/break_keyword_binding.res" ! -path "tests/tools_tests/*/NoMigration.res*" ! -path "tests/build_tests/super_errors_multi/fixtures/Iface_not_compiled/*" ! -path "tests/syntax_tests*" ! -path "tests/analysis_tests/tests*" ! -path "*/node_modules/*") -./cli/rescript.js format $files +# xargs passes the files in batches that fit the platform's command-line +# length limit, which is only 32K characters on Windows. +find packages tests -type f \( -name "*.res" -o -name "*.resi" \) ! -name "syntaxErrors*" ! -name "generated_mocha_test.res" ! -path "tests/build_tests/super_errors/fixtures/break_keyword_binding.res" ! -path "tests/tools_tests/*/NoMigration.res*" ! -path "tests/build_tests/super_errors_multi/fixtures/Iface_not_compiled/*" ! -path "tests/syntax_tests*" ! -path "tests/analysis_tests/tests*" ! -path "*/node_modules/*" -print0 \ + | xargs -0 node ./cli/rescript.js format echo Formatting JS code... yarn format diff --git a/scripts/format_check.sh b/scripts/format_check.sh index ca6bf50aa2a..acb4d5d855a 100755 --- a/scripts/format_check.sh +++ b/scripts/format_check.sh @@ -6,29 +6,24 @@ warningYellow='\033[0;33m' successGreen='\033[0;32m' reset='\033[0m' -case "$(uname -s)" in - Darwin|Linux) - echo "Checking OCaml code formatting..." - if opam exec -- dune build @fmt; then - printf "${successGreen}✅ OCaml code formatting ok.${reset}\n" - else - printf "${warningYellow}⚠️ OCaml code formatting issues found.${reset}\n" - exit 1 - fi +echo "Checking OCaml code formatting..." +if opam exec -- dune build @fmt; then + printf "${successGreen}✅ OCaml code formatting ok.${reset}\n" +else + printf "${warningYellow}⚠️ OCaml code formatting issues found.${reset}\n" + exit 1 +fi - echo "Checking ReScript code formatting..." - files=$(find packages tests -type f \( -name "*.res" -o -name "*.resi" \) ! -name "syntaxErrors*" ! -name "generated_mocha_test.res" ! -path "tests/build_tests/super_errors/fixtures/break_keyword_binding.res" ! -path "tests/tools_tests/*/NoMigration.res*" ! -path "tests/build_tests/super_errors_multi/fixtures/Iface_not_compiled/*" ! -path "tests/syntax_tests*" ! -path "tests/analysis_tests/tests*" ! -path "*/node_modules/*") - if ./cli/rescript.js format --check $files; then - printf "${successGreen}✅ ReScript code formatting ok.${reset}\n" - else - printf "${warningYellow}⚠️ ReScript code formatting issues found. Run 'make format' to fix.${reset}\n" - exit 1 - fi - ;; - *) - # Does not work on Windows - echo "Code formatting checks skipped for this platform." -esac +echo "Checking ReScript code formatting..." +# xargs passes the files in batches that fit the platform's command-line +# length limit, which is only 32K characters on Windows. +if find packages tests -type f \( -name "*.res" -o -name "*.resi" \) ! -name "syntaxErrors*" ! -name "generated_mocha_test.res" ! -path "tests/build_tests/super_errors/fixtures/break_keyword_binding.res" ! -path "tests/tools_tests/*/NoMigration.res*" ! -path "tests/build_tests/super_errors_multi/fixtures/Iface_not_compiled/*" ! -path "tests/syntax_tests*" ! -path "tests/analysis_tests/tests*" ! -path "*/node_modules/*" -print0 \ + | xargs -0 node ./cli/rescript.js format --check; then + printf "${successGreen}✅ ReScript code formatting ok.${reset}\n" +else + printf "${warningYellow}⚠️ ReScript code formatting issues found. Run 'make format' to fix.${reset}\n" + exit 1 +fi echo "Checking JS code formatting..." if yarn check; then diff --git a/tests/ounit_tests/ounit_compiler_request_tests.ml b/tests/ounit_tests/ounit_compiler_request_tests.ml index e797dd7404d..cac6ab4ff79 100644 --- a/tests/ounit_tests/ounit_compiler_request_tests.ml +++ b/tests/ounit_tests/ounit_compiler_request_tests.ml @@ -259,6 +259,37 @@ let captured_line_endings_tests _context = ("before\r\nplain" ^ newline ^ "after\r\n") stdout +(* Binary output keeps its bytes, as formatted source written by bsc -format + does, while text around it still gets a bsc process's line endings. *) +let captured_binary_output_tests _context = + let newline = if Sys.win32 then "\r\n" else "\n" in + let expected = "text" ^ newline ^ "lf\ncrlf\r\n" ^ "more" ^ newline in + let (), stdout, _ = + Compiler_request_output.with_capture (fun () -> + Compiler_request_output.write_stdout "text\n"; + Compiler_request_output.write_stdout_binary "lf\ncrlf\r\n"; + Compiler_request_output.write_stdout "more\n") + in + assert_equal ~printer:String.escaped expected stdout; + (* The same holds once stdout is backed by a file *) + let (), stdout, _ = + Compiler_request_output.with_capture (fun () -> + Compiler_request_output.write_stdout "text\n"; + ignore (Compiler_request_output.stdout_channel () : out_channel); + Compiler_request_output.write_stdout_binary "lf\ncrlf\r\n"; + Compiler_request_output.write_stdout "more\n") + in + assert_equal ~printer:String.escaped expected stdout; + (* and when stdout gets a file only after the binary output *) + let (), stdout, _ = + Compiler_request_output.with_capture (fun () -> + Compiler_request_output.write_stdout "text\n"; + Compiler_request_output.write_stdout_binary "lf\ncrlf\r\n"; + ignore (Compiler_request_output.stdout_channel () : out_channel); + Compiler_request_output.write_stdout "more\n") + in + assert_equal ~printer:String.escaped expected stdout + (* Readers of compiler artifacts close their file when decoding fails, so a long-lived build process does not keep it open. *) let failed_decoding_closes_files_tests _context = @@ -1632,6 +1663,7 @@ let suites = "request_local_order" >:: request_local_order_tests; "failed_capture" >:: failed_capture_tests; "captured_line_endings" >:: captured_line_endings_tests; + "captured_binary_output" >:: captured_binary_output_tests; "failed_decoding_closes_files" >:: failed_decoding_closes_files_tests; "gentype_source_lookup" >:: gentype_source_lookup_tests; "standalone_output" >:: standalone_output_tests; diff --git a/tests/syntax_tests/res_test.ml b/tests/syntax_tests/res_test.ml index 461ff2866c4..8d32728467f 100644 --- a/tests/syntax_tests/res_test.ml +++ b/tests/syntax_tests/res_test.ml @@ -24,6 +24,37 @@ let () = let x: int |}) +(* Formatting keeps a file's line endings, also inside comments, template + literals and raw JS, and formatting the result again changes nothing *) +let () = + let lf = + "/* a\n\ + \ multi-line comment */\n\ + let s = `line 1\n\ + line 2`\n\ + let r = %raw(`\n\ + \ function () {\n\ + \ return 1\n\ + \ }\n\ + `)\n" + in + let format source = + let filename = Filename.temp_file "line_endings" ".res" in + Fun.protect + (fun () -> + let channel = open_out_bin filename in + output_string channel source; + close_out channel; + Res_multi_printer.print filename) + ~finally:(fun () -> Sys.remove filename) + in + List.iter + (fun source -> + let formatted = format source in + assert (formatted = source); + assert (format formatted = formatted)) + [lf; Res_line_endings.to_crlf lf] + let () = print_endline "✅ multi printer api tests" let () = From 88297dc8d6d848ac62bc4b076f6cdaa8cbaabe31 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 25 Oct 2026 18:45:10 +0100 Subject: [PATCH 2/3] Expect formatted files to keep their line endings in rewatch tests The rewatch format unit tests expected formatted output to have CRLF line endings on Windows, which was the text-mode output bug. They now expect each file to keep its own line endings, and check that a formatted CRLF file passes format --check and is left unchanged by format, through the in-process formatter. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Christoph Knittel --- tests/rewatch_ounit_tests/format_tests.ml | 32 +++++++++++++---------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/tests/rewatch_ounit_tests/format_tests.ml b/tests/rewatch_ounit_tests/format_tests.ml index 364226a516e..9830a26312a 100644 --- a/tests/rewatch_ounit_tests/format_tests.ml +++ b/tests/rewatch_ounit_tests/format_tests.ml @@ -16,11 +16,6 @@ let write_file = Test_support.write_file let with_temp_dir = Test_support.with_temp_dir "rewatch-ocaml-format-" -(* The formatter writes text-mode output, so lines end in CRLF on Windows. *) -let native text = - if Sys.win32 then String.concat "\r\n" (String.split_on_char '\n' text) - else text - let tests = "format_tests" >:: fun _context -> check @@ -70,8 +65,16 @@ let tests = with_temp_dir (fun root -> let unformatted = Filename.concat root "Unformatted.res" in let formatted = Filename.concat root "Formatted.res" in + let formatted_crlf = Filename.concat root "FormattedCrlf.res" in write_file unformatted "let first=1\n"; - write_file formatted (native "let second = 2\n"); + write_file formatted "let second = 2\n"; + write_file formatted_crlf "/* a\r\n b */\r\nlet third = 3\r\n"; + check + (try + Format.format_files ~check:true [formatted; formatted_crlf]; + true + with Format.Error _ -> false) + "format --check accepts formatted LF and CRLF files"; check (try Format.format_files ~check:true [unformatted; formatted]; @@ -81,11 +84,14 @@ let tests = check (File_util.read_file unformatted = "let first=1\n") "format --check leaves files unchanged"; - Format.format_files ~check:false [unformatted; formatted]; + Format.format_files ~check:false [unformatted; formatted; formatted_crlf]; check - (File_util.read_file unformatted = native "let first = 1\n" - && File_util.read_file formatted = native "let second = 2\n") - "the embedded formatter rewrites only files that need formatting"; + (File_util.read_file unformatted = "let first = 1\n" + && File_util.read_file formatted = "let second = 2\n" + && File_util.read_file formatted_crlf + = "/* a\r\n b */\r\nlet third = 3\r\n") + "the embedded formatter rewrites only files that need formatting, \ + keeping their line endings"; check (try write_file unformatted "let =\n"; @@ -119,13 +125,11 @@ let tests = (fun () -> Unix.chdir root; Format.run_files ~verbosity:0 ~check:false []); - let formatted path = - File_util.read_file path = native "let value = 1\n" - in + let formatted path = File_util.read_file path = "let value = 1\n" in check (formatted root_source) "implicit format includes the current package"; check - (File_util.read_file orphan_interface = native "let value: int\n") + (File_util.read_file orphan_interface = "let value: int\n") "implicit format includes an orphan interface"; check (formatted duplicate_one && formatted duplicate_two) From 25243f2ed85ad54b3d0799b392cb706aa66558da Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 25 Oct 2026 18:56:23 +0100 Subject: [PATCH 3/3] Read syntax test file lists before running tests in the background The syntax test script read each file list in a while loop from a file and started a background job per file. Under MSYS on Windows that occasionally corrupted the names the loop read, so a test file was skipped, or, in the roundtrip tests now running on Windows, reported as a failure. The script now reads each list into an array before it starts any job. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Christoph Knittel --- scripts/test_syntax.sh | 47 ++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/scripts/test_syntax.sh b/scripts/test_syntax.sh index 7f132253943..d4f1c922ab6 100755 --- a/scripts/test_syntax.sh +++ b/scripts/test_syntax.sh @@ -14,6 +14,17 @@ function exp { echo "$(dirname $1)/expected/$(basename $1).txt" } +# Lists the .res and .resi files under the given directories in the array +# files. The list is read before any test runs in the background: under MSYS +# on Windows, starting background jobs while a loop reads the list from a file +# corrupts what the loop reads. +function listFiles { + files=() + while IFS= read -r file; do + files+=("$file") + done < <(find "$@" -name "*.res" -o -name "*.resi") +} + taskCount=0 function maybeWait { let taskCount+=1 @@ -39,32 +50,32 @@ rm -rf temp mkdir temp # parsing -find syntax_tests/data/parsing/{errors,infiniteLoops,recovery} -name "*.res" -o -name "*.resi" >temp/files.txt -while read file; do +listFiles syntax_tests/data/parsing/{errors,infiniteLoops,recovery} +for file in "${files[@]}"; do $DUNE_BIN_DIR/res_parser -recover -print ml $file &> $(exp $file) & maybeWait -done temp/files.txt -while read file; do +done +listFiles syntax_tests/data/parsing/{grammar,other} +for file in "${files[@]}"; do $DUNE_BIN_DIR/res_parser -print ml $file &> $(exp $file) & maybeWait -done temp/files.txt -while read file; do +listFiles syntax_tests/data/{printer,conversion} +for file in "${files[@]}"; do $DUNE_BIN_DIR/res_parser $file &> $(exp $file) & maybeWait -done temp/files.txt -while read file; do +listFiles syntax_tests/data/ast-mapping +for file in "${files[@]}"; do $DUNE_BIN_DIR/res_parser -test-ast-conversion -jsx-version 4 $file &> $(exp $file) & maybeWait -done temp/files.txt -while read file; do +listFiles syntax_tests/data/ppx/react +for file in "${files[@]}"; do $DUNE_BIN_DIR/res_parser -jsx-version 4 $file &> $(exp $file) & maybeWait -done temp/files.txt - while read file; do { + listFiles syntax_tests/data/printer + for file in "${files[@]}"; do { mkdir -p temp/$(dirname $file) rescript1=temp/$file.res rescript2=temp/$file.2.res @@ -107,7 +118,7 @@ if [[ $ROUNDTRIP_TEST = 1 ]]; then diff --unified $rescript1 $rescript2 [[ "$?" = 1 ]] && echo 1 > $roundtripTestsResult } & maybeWait - done