From 9de0dae9539c4a943e7411cd807dcffe05201cb4 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Wed, 28 Oct 2026 15:42:38 +0000 Subject: [PATCH] Write the formatter's own line breaks with the file's newline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: Since #9042, formatting converted the whole source to LF and the output back to CRLF, string literals included, so `bsc -format` changed the program when a string held a raw line break of the other kind. Each conversion also dropped only one CR before an LF, so a comment with `\r\r\n` changed on every run (#9100). The source is now parsed as is, and the printer writes the line breaks it emits with the newline of the source (CRLF when the first line break is CRLF), while literals keep their bytes. bsc -format, res_parser -print res and the editor's format command do the same. In the examples below, `\r` and `\n` stand for CR and LF, the heading holds the input, and each version is what `bsc -format` prints for it. Format modified (let x = 1\r\nlet s = "a\nb"\r\n): Before (text): let x = 1\r\nlet s = "a\r\nb"\r\n After (text): let x = 1\r\nlet s = "a\nb"\r\n Format modified (let x = 1\nlet s = "a\r\nb"\n): Before (text): let x = 1\nlet s = "a\nb"\n After (text): let x = 1\nlet s = "a\r\nb"\n Format modified (let x = 1\n/* a\r\r\nb */\n): Before (text): first run, then second run let x = 1\n/* a\r\n b */\n let x = 1\n/* a\n b */\n After (text): first and second run let x = 1\n/* a\n b */\n Review: Why does the comment lose its CRs? The printer reindents a multi-line `/* … */` comment and writes that comment's line breaks itself, as it already did for the lines after the first. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Signed-off-by: Christoph Knittel --- analysis/src/commands.ml | 14 ++++------ changes/format-literal-line-breaks.fix.md | 1 + compiler/syntax/cli/res_cli.ml | 10 ++++--- compiler/syntax/src/res_doc.ml | 6 ++--- compiler/syntax/src/res_doc.mli | 6 ++++- compiler/syntax/src/res_driver.ml | 6 ++--- compiler/syntax/src/res_driver.mli | 3 ++- compiler/syntax/src/res_line_endings.ml | 33 +++-------------------- compiler/syntax/src/res_line_endings.mli | 25 ++++++----------- compiler/syntax/src/res_minibuffer.ml | 4 +-- compiler/syntax/src/res_minibuffer.mli | 2 +- compiler/syntax/src/res_multi_printer.ml | 9 ++++--- compiler/syntax/src/res_printer.ml | 18 +++++++++---- compiler/syntax/src/res_printer.mli | 12 +++++++-- tests/syntax_tests/res_test.ml | 23 ++++++++++++++-- 15 files changed, 88 insertions(+), 84 deletions(-) create mode 100644 changes/format-literal-line-breaks.fix.md diff --git a/analysis/src/commands.ml b/analysis/src/commands.ml index 70ef05b26f9..a5f42acb2a3 100644 --- a/analysis/src/commands.ml +++ b/analysis/src/commands.ml @@ -301,11 +301,7 @@ let format ~source ~kind_file = 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 newline = Res_line_endings.newline source in let result = match kind_file with @@ -317,8 +313,8 @@ let format ~source ~kind_file = | true -> Error "Document has syntax errors" | false -> Ok - (Res_printer.print_implementation ~comments structure - |> restore_line_endings |> create_range)) + (Res_printer.print_implementation ~newline ~comments structure + |> create_range)) | Resi -> ( let {Res_driver.parsetree = signature; comments; diagnostics} = Res_driver.parsing_engine.parse_interface_from_source ~source @@ -327,8 +323,8 @@ let format ~source ~kind_file = | true -> Error "Document has syntax errors" | false -> Ok - (Res_printer.print_interface ~comments signature - |> restore_line_endings |> create_range)) + (Res_printer.print_interface ~newline ~comments signature + |> create_range)) | Other -> Error "Failed to format, file not supported" in diff --git a/changes/format-literal-line-breaks.fix.md b/changes/format-literal-line-breaks.fix.md new file mode 100644 index 00000000000..c7109e5dedd --- /dev/null +++ b/changes/format-literal-line-breaks.fix.md @@ -0,0 +1 @@ +- Make `bsc -format` keep the line breaks inside string, template and character literals and polymorphic variant names as written, and format comments with `\r\r\n` stably. diff --git a/compiler/syntax/cli/res_cli.ml b/compiler/syntax/cli/res_cli.ml index 6c95fa91139..8aed6b66978 100644 --- a/compiler/syntax/cli/res_cli.ml +++ b/compiler/syntax/cli/res_cli.ml @@ -68,13 +68,18 @@ module Cli_arg_processor = struct is_interface || (len > 0 && (String.get [@doesNotRaise]) filename (len - 1) = 'i') in + let source = lazy (Ext_io.load_file filename) in + let load_source () = Lazy.force source in let print_engine = match target with | "ml" -> Res_driver_ml_printer.print_engine | "ast" -> Res_ast_debugger.print_engine | "comments" -> Res_ast_debugger.comments_print_engine | "tokens" -> Res_token_debugger.token_print_engine - | "res" -> Res_driver.print_engine + | "res" -> + (* Keep the line endings of a CRLF file, like bsc -format *) + Res_driver.print_engine + ~newline:(Res_line_endings.newline (load_source ())) | "doc" -> Res_ast_debugger.doc_print_engine | target -> print_endline @@ -83,9 +88,6 @@ module Cli_arg_processor = struct exit 1 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; diff --git a/compiler/syntax/src/res_doc.ml b/compiler/syntax/src/res_doc.ml index 5ba1a50e8cf..e483ad4752e 100644 --- a/compiler/syntax/src/res_doc.ml +++ b/compiler/syntax/src/res_doc.ml @@ -203,7 +203,7 @@ let fits w stack = in calculate_all stack -let to_string ~width doc = +let to_string ?(newline = "\n") ~width doc = propagate_forced_breaks doc; let buffer = Mini_buffer.create 1000 in @@ -234,7 +234,7 @@ let to_string ~width doc = Mini_buffer.add_char buffer '\n'; process ~pos:0 [] rest) else ( - Mini_buffer.flush_newline buffer; + Mini_buffer.flush_newline buffer ~newline; Mini_buffer.add_string buffer (String.make ind ' ' [@doesNotRaise]); process ~pos:ind [] rest) @@ -249,7 +249,7 @@ let to_string ~width doc = Mini_buffer.add_string buffer " "; pos + 1 | Hard -> - Mini_buffer.flush_newline buffer; + Mini_buffer.flush_newline buffer ~newline; 0 | Literal -> Mini_buffer.add_char buffer '\n'; diff --git a/compiler/syntax/src/res_doc.mli b/compiler/syntax/src/res_doc.mli index 13caa1a0c25..93fd93e35bc 100644 --- a/compiler/syntax/src/res_doc.mli +++ b/compiler/syntax/src/res_doc.mli @@ -57,7 +57,11 @@ val has_pending_line_suffix : t -> bool count as flat. Text after [doc] on the same line would precede the comment. *) -val to_string : width:int -> t -> string +val to_string : ?newline:string -> width:int -> t -> string +(** [to_string ~newline ~width doc] writes the line breaks of [doc] as + [newline] (default LF). A [literal_line] is part of a literal's contents + and stays LF. *) + val debug : t -> unit val is_nil : t -> bool diff --git a/compiler/syntax/src/res_driver.ml b/compiler/syntax/src/res_driver.ml index 8e98f0beddf..681d3127411 100644 --- a/compiler/syntax/src/res_driver.ml +++ b/compiler/syntax/src/res_driver.ml @@ -151,16 +151,16 @@ let parse_interface_from_source ~display_filename ~source = comments = List.rev engine.comments; } -let print_engine = +let print_engine ~newline = { print_implementation = (fun ~width ~filename:_ ~comments structure -> Compiler_request_output.write_stdout_binary - (Res_printer.print_implementation ~width structure ~comments)); + (Res_printer.print_implementation ~width ~newline structure ~comments)); print_interface = (fun ~width ~filename:_ ~comments signature -> Compiler_request_output.write_stdout_binary - (Res_printer.print_interface ~width signature ~comments)); + (Res_printer.print_interface ~width ~newline signature ~comments)); } exception Already_reported diff --git a/compiler/syntax/src/res_driver.mli b/compiler/syntax/src/res_driver.mli index f941d92419c..0eb1ebdda2d 100644 --- a/compiler/syntax/src/res_driver.mli +++ b/compiler/syntax/src/res_driver.mli @@ -49,7 +49,8 @@ type print_engine = { val parsing_engine : Res_diagnostics.t list parsing_engine -val print_engine : print_engine +(* Prints ReScript with line breaks written as [newline] *) +val print_engine : newline:string -> print_engine exception Already_reported diff --git a/compiler/syntax/src/res_line_endings.ml b/compiler/syntax/src/res_line_endings.ml index 3c8b771079f..65a694b40d6 100644 --- a/compiler/syntax/src/res_line_endings.ml +++ b/compiler/syntax/src/res_line_endings.ml @@ -1,31 +1,4 @@ -let uses_crlf source = +let newline 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 + | Some index when index > 0 && source.[index - 1] = '\r' -> "\r\n" + | _ -> "\n" diff --git a/compiler/syntax/src/res_line_endings.mli b/compiler/syntax/src/res_line_endings.mli index cce096a94e6..87cd067618c 100644 --- a/compiler/syntax/src/res_line_endings.mli +++ b/compiler/syntax/src/res_line_endings.mli @@ -1,17 +1,8 @@ -(* 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 +(* The printer writes its own line breaks with the newline it is given, and + copies comments and literals with their bytes unchanged. Formatting a file + with the newline of its source keeps a CRLF file CRLF without touching the + line breaks inside its string literals, which are part of the program. *) + +(* The newline of [source]: CRLF when its first line break is CRLF, LF + otherwise *) +val newline : string -> string diff --git a/compiler/syntax/src/res_minibuffer.ml b/compiler/syntax/src/res_minibuffer.ml index d36a1679ad5..8354bae0144 100644 --- a/compiler/syntax/src/res_minibuffer.ml +++ b/compiler/syntax/src/res_minibuffer.ml @@ -38,10 +38,10 @@ let add_string b s = b.position <- new_position (* adds newline and trims all preceding whitespace *) -let flush_newline b = +let flush_newline b ~newline = let position = ref b.position in while Bytes.unsafe_get b.buffer (!position - 1) = ' ' && !position >= 0 do position := !position - 1 done; b.position <- !position; - add_char b '\n' + add_string b newline diff --git a/compiler/syntax/src/res_minibuffer.mli b/compiler/syntax/src/res_minibuffer.mli index 0a2bffa538d..8a387520f4d 100644 --- a/compiler/syntax/src/res_minibuffer.mli +++ b/compiler/syntax/src/res_minibuffer.mli @@ -3,4 +3,4 @@ val add_char : t -> char -> unit val add_string : t -> string -> unit val contents : t -> string val create : int -> t -val flush_newline : t -> unit +val flush_newline : t -> newline:string -> unit diff --git a/compiler/syntax/src/res_multi_printer.ml b/compiler/syntax/src/res_multi_printer.ml index 88eee70ee42..a32261c4f49 100644 --- a/compiler/syntax/src/res_multi_printer.ml +++ b/compiler/syntax/src/res_multi_printer.ml @@ -1,5 +1,6 @@ (* print res files to res syntax *) let print_res ~ignore_parse_errors ~is_interface ~filename source = + let newline = Res_line_endings.newline source in if is_interface then ( let parse_result = Res_driver.parse_interface_from_source ~display_filename:filename ~source @@ -7,7 +8,7 @@ let print_res ~ignore_parse_errors ~is_interface ~filename source = 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); - Res_printer.print_interface ~width:Res_printer.default_print_width + Res_printer.print_interface ~width:Res_printer.default_print_width ~newline ~comments:parse_result.comments parse_result.parsetree) else let parse_result = @@ -18,7 +19,7 @@ let print_res ~ignore_parse_errors ~is_interface ~filename source = Res_diagnostics.print_report parse_result.diagnostics parse_result.source; if not ignore_parse_errors then raise_notrace Res_driver.Already_reported); Res_printer.print_implementation ~width:Res_printer.default_print_width - ~comments:parse_result.comments parse_result.parsetree + ~newline ~comments:parse_result.comments parse_result.parsetree [@@raises Res_driver.Already_reported] (* print the given file named input to from "language" to res, general interface exposed by the compiler *) @@ -27,8 +28,8 @@ let print ?(ignore_parse_errors = false) input = let len = String.length input in len > 0 && String.unsafe_get input (len - 1) = 'i' in - Res_line_endings.preserve (Ext_io.load_file input) - (print_res ~ignore_parse_errors ~is_interface ~filename:input) + print_res ~ignore_parse_errors ~is_interface ~filename:input + (Ext_io.load_file input) [@@raises Res_driver.Already_reported] (* suppress unused optional arg *) diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index a319ad34b36..fe7dc0a9aaa 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -181,6 +181,14 @@ let print_multiline_comment_content txt = Doc.concat [Doc.text "/* "; Doc.text (Comment.trim_spaces line); Doc.text " */"] | first :: rest -> + (* The printer writes the line break, so drop the CRs of a CRLF one *) + let first = + let len = ref (String.length first) in + while !len > 0 && String.unsafe_get first (!len - 1) = '\r' do + decr len + done; + String.sub first 0 !len [@doesNotRaise] + in let first_line = Comment.trim_spaces first in Doc.concat [ @@ -6448,12 +6456,12 @@ let interface_doc (s : Parsetree.signature) ~comments = Comment_table.walk_signature s cmt_tbl comments; print_signature ~state:(State.init ()) s cmt_tbl -let print_implementation ?(width = default_print_width) +let print_implementation ?(width = default_print_width) ?(newline = "\n") (s : Parsetree.structure) ~comments = - Doc.to_string ~width (implementation_doc s ~comments) ^ "\n" + Doc.to_string ~newline ~width (implementation_doc s ~comments) ^ newline -let print_interface ?(width = default_print_width) (s : Parsetree.signature) - ~comments = - Doc.to_string ~width (interface_doc s ~comments) ^ "\n" +let print_interface ?(width = default_print_width) ?(newline = "\n") + (s : Parsetree.signature) ~comments = + Doc.to_string ~newline ~width (interface_doc s ~comments) ^ newline let print_structure s t = print_structure ~state:(State.init ()) s t diff --git a/compiler/syntax/src/res_printer.mli b/compiler/syntax/src/res_printer.mli index d4e74bf91e1..75bb6b01168 100644 --- a/compiler/syntax/src/res_printer.mli +++ b/compiler/syntax/src/res_printer.mli @@ -25,9 +25,17 @@ val interface_doc : Parsetree.signature -> comments:Res_comment.t list -> Res_doc.t val print_implementation : - ?width:int -> Parsetree.structure -> comments:Res_comment.t list -> string + ?width:int -> + ?newline:string -> + Parsetree.structure -> + comments:Res_comment.t list -> + string val print_interface : - ?width:int -> Parsetree.signature -> comments:Res_comment.t list -> string + ?width:int -> + ?newline:string -> + Parsetree.signature -> + comments:Res_comment.t list -> + string val print_ident_like : ?allow_uident:bool -> ?allow_hyphen:bool -> string -> Res_doc.t diff --git a/tests/syntax_tests/res_test.ml b/tests/syntax_tests/res_test.ml index 8d32728467f..7160862958e 100644 --- a/tests/syntax_tests/res_test.ml +++ b/tests/syntax_tests/res_test.ml @@ -25,11 +25,17 @@ 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 *) + literals and raw JS, and formatting the result again changes nothing. The + line breaks inside literals are part of the program and keep their bytes, + whatever the file's line endings, and so do the ones inside comments. *) let () = + let to_crlf text = String.concat "\r\n" (String.split_on_char '\n' text) in let lf = "/* a\n\ \ multi-line comment */\n\ + /* a comment\n\ + \ * with stars\n\ + \ */\n\ let s = `line 1\n\ line 2`\n\ let r = %raw(`\n\ @@ -53,7 +59,20 @@ let () = let formatted = format source in assert (formatted = source); assert (format formatted = formatted)) - [lf; Res_line_endings.to_crlf lf] + [ + lf; + to_crlf lf; + "let x = 1\r\nlet s = \"a\nb\"\r\n"; + "let x = 1\nlet s = \"a\r\nb\"\n"; + "let x = 1\r\nlet t = `a\nb`\r\nlet c = '\n'\r\nlet p = #\"a\nb\"\r\n"; + "let x = 1\nlet t = `a\r\nb`\nlet c = '\r'\nlet p = #\"a\r\nb\"\n"; + "let x = 1\n/* a\r\r\nb\nc */\nlet y = 2\n"; + "let x = 1\r\n/* a\nb\r\nc */\r\nlet y = 2\r\n"; + ]; + (* The printer writes the line breaks of a comment it reindents *) + let formatted = format "let x = 1\n/* a\r\r\n b */\nlet y = 2\n" in + assert (formatted = "let x = 1\n/* a\n b */\nlet y = 2\n"); + assert (format (to_crlf formatted) = to_crlf formatted) let () = print_endline "✅ multi printer api tests"