From 6d2afacd98e637ec195aa887623a8aeb8fd3d0fa Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Wed, 28 Oct 2026 20:24:11 +0000 Subject: [PATCH] Keep doc comments and comments in place in four formatter cases Why: The formatter put doc comments and comments in the wrong place in four of the cases reported in #9107, and in one of them it reordered the attributes in the AST. Doc comments after an attribute were moved in front of all attributes. A single parameter with a doc comment was hugged, so its pattern started at column 0. A comment before a list's tail spread was printed after the dots, in expressions and in patterns. A file with only a line comment got an extra newline. Only the doc comments in front of every other attribute are now printed first; later ones stay in their place, inline. The fifth case, a comment after a parenthesized `switch` or `if` operand that moves into the block, keeps its stable output. AST modified (doc comment after an attribute, after formatting): Example: type u = /** d1 */ @a /** d2 */ {x: int} Before (parse): type_declaration "u" attribute "res.doc" attribute "res.doc" attribute "a" After (parse): type_declaration "u" attribute "res.doc" attribute "a" attribute "res.doc" Format modified (doc comment after an attribute): Example: type u = /** d1 */ @a /** d2 */ {x: int} Before (rescript): /** d1 */ /** d2 */ @a type u = {x: int} After (rescript): /** d1 */ @a /** d2 */ type u = {x: int} Format modified (doc comment on a hugged parameter): Example: let f = (/** r */ {x, y}) => x Before (rescript): let f = (/** r */ {x, y}) => x After (rescript): let f = ( /** r */ {x, y}, ) => x Format modified (comment before a list's tail spread): Example: let l = list{1, /* c */ ...xs} let f = (list{a, /* c */ ...rest}) => a Before (rescript): let l = list{1, .../* c */ xs} let f = (list{a, .../* c */ rest}) => a After (rescript): let l = list{1, /* c */ ...xs} let f = (list{a, /* c */ ...rest}) => a Format modified (file with only a line comment): Example: // c Before (text): "// c\n\n" After (text): "// c\n" Review: Why not fix the comment after a parenthesized operand? It formats stably with the same AST; fixing it needs a token flag on comments or operand locations that include the parentheses. Fixes: #9107 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Signed-off-by: Christoph Knittel --- changes/formatter-comment-placement.fix.md | 1 + compiler/syntax/src/res_parsetree_viewer.ml | 26 +++-- compiler/syntax/src/res_parsetree_viewer.mli | 3 + compiler/syntax/src/res_printer.ml | 95 ++++++++++--------- .../data/printer/comments/array.res | 12 +++ .../printer/comments/docCommentAttributes.res | 12 +++ .../printer/comments/docCommentParameters.res | 8 ++ .../printer/comments/expected/array.res.txt | 12 +++ .../expected/docCommentAttributes.res.txt | 13 +++ .../expected/docCommentParameters.res.txt | 19 ++++ .../fileWithOneSingleLineComment.res.txt | 1 - .../expected/fileWithOnlyComments.res.txt | 1 - .../interfaceWithOneSingleComment.resi.txt | 1 - .../interfaceWithOnlyComments.resi.txt | 1 - .../printer/comments/expected/pattern.res.txt | 2 +- .../modExpr/expected/attributes.res.txt | 2 +- .../data/printer/typeDef/attributes.res | 1 + .../typeDef/expected/attributes.res.txt | 12 +-- 18 files changed, 154 insertions(+), 68 deletions(-) create mode 100644 changes/formatter-comment-placement.fix.md diff --git a/changes/formatter-comment-placement.fix.md b/changes/formatter-comment-placement.fix.md new file mode 100644 index 00000000000..e6c30b52d08 --- /dev/null +++ b/changes/formatter-comment-placement.fix.md @@ -0,0 +1 @@ +- Fix the formatter's placement of a doc comment on a single hugged parameter, of doc comments after an attribute, and of a comment before a list's tail spread, and the extra newline after a file with only a line comment. diff --git a/compiler/syntax/src/res_parsetree_viewer.ml b/compiler/syntax/src/res_parsetree_viewer.ml index 9b772741bc1..edebd338f0d 100644 --- a/compiler/syntax/src/res_parsetree_viewer.ml +++ b/compiler/syntax/src/res_parsetree_viewer.ml @@ -366,13 +366,6 @@ let collect_ternary_parts expr = in collect [] expr -let parameters_should_hug parameters = - match parameters with - | [Parameter {attrs = []; lbl = Nolabel; default_expr = None; pat}] - when is_huggable_pattern pat -> - true - | _ -> false - let should_indent_binary_expr expr = let same_precedence_sub_expression operator sub_expression = match sub_expression with @@ -430,8 +423,25 @@ let is_doc_comment_attribute ((id, payload) : Parsetree.attribute) = true | _ -> false +(* Only the doc comments before every other attribute: one after an attribute + stays in its place, so that printing keeps the order of the attributes. *) let partition_doc_comment_attributes attrs = - List.partition is_doc_comment_attribute attrs + let rec split doc_comments = function + | attr :: attrs when is_doc_comment_attribute attr -> + split (attr :: doc_comments) attrs + | attrs -> (List.rev doc_comments, attrs) + in + split [] attrs + +(* A doc comment can be printed on its own line, which a hugged parameter + cannot start. *) +let parameters_should_hug parameters = + match parameters with + | [Parameter {attrs = []; lbl = Nolabel; default_expr = None; pat}] + when is_huggable_pattern pat + && not (List.exists is_doc_comment_attribute pat.ppat_attributes) -> + true + | _ -> false let rec is_fun_expr expr = match expr.pexp_desc with diff --git a/compiler/syntax/src/res_parsetree_viewer.mli b/compiler/syntax/src/res_parsetree_viewer.mli index 8b77a0ae2c4..f717faa7c7a 100644 --- a/compiler/syntax/src/res_parsetree_viewer.mli +++ b/compiler/syntax/src/res_parsetree_viewer.mli @@ -80,6 +80,9 @@ val should_indent_binary_expr : Parsetree.expression -> bool val should_inline_rhs_binary_expr : Parsetree.expression -> bool val mod_expr_has_attributes : Parsetree.module_expr -> bool val is_doc_comment_attribute : Parsetree.attribute -> bool + +(* The doc comments before every other attribute, and the attributes after + them, which may include later doc comments *) val partition_doc_comment_attributes : Parsetree.attributes -> Parsetree.attributes * Parsetree.attributes diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index a319ad34b36..98f8519d050 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -222,13 +222,14 @@ let print_trailing_comment (prev_loc : Location.t) (node_loc : Location.t) else if not single_line then Doc.concat [Doc.space; content] else Doc.line_suffix (Doc.concat [Doc.space; content]) +let print_comment_content comment = + let txt = Comment.txt comment in + if Comment.is_single_line_comment comment then Doc.text ("//" ^ txt) + else print_multiline_comment_content txt + let print_leading_comment ?next_comment comment = let single_line = Comment.is_single_line_comment comment in - let content = - let txt = Comment.txt comment in - if single_line then Doc.text ("//" ^ txt) - else print_multiline_comment_content txt - in + let content = print_comment_content comment in let separator = Doc.concat [ @@ -294,7 +295,9 @@ let print_comments_inside_file cmt_tbl = match comments with | [] -> Doc.nil | [comment] -> - let cmt_doc = print_leading_comment comment in + (* Nothing follows the last comment: the line break after a line comment + would add an empty line to the final newline of the file. *) + let cmt_doc = print_comment_content comment in let doc = Doc.group (Doc.concat [Doc.concat (List.rev (cmt_doc :: acc))]) in @@ -1754,25 +1757,31 @@ and print_literal_dict_expr ~state (e : Parsetree.expression) cmt_tbl = Doc.soft_line; ]) +(* [...expr] in an array, list or dict. The leading comments of [expr] go + before the dots, which are part of the spread and not of [expr]. *) +and print_spread_with_comments ~state expr cmt_tbl = + let leading_comments_doc = + print_leading_comments Doc.nil cmt_tbl.Comment_table.leading + expr.Parsetree.pexp_loc + in + let doc = + let doc = print_expression ~state expr cmt_tbl in + match Parens.expr_allowing_coercion expr with + | Parens.Parenthesized -> add_parens doc + | Braced braces -> print_braces doc expr braces + | Nothing -> doc + in + Doc.concat + [ + leading_comments_doc; + Doc.dotdotdot; + print_trailing_comments doc cmt_tbl.Comment_table.trailing + expr.Parsetree.pexp_loc; + ] + and print_spread_dict_expr ~state parts (expr : Parsetree.expression) cmt_tbl = let print_spread_part spread_expr = - let leading_comments_doc = - print_leading_comments Doc.nil cmt_tbl.Comment_table.leading - spread_expr.Parsetree.pexp_loc - in - let spread_doc = - let doc = print_expression ~state spread_expr cmt_tbl in - match Parens.expr_allowing_coercion spread_expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc spread_expr braces - | Nothing -> doc - in - let spread_with_trailing_comments = - print_trailing_comments spread_doc cmt_tbl.Comment_table.trailing - spread_expr.Parsetree.pexp_loc - in - Doc.concat - [leading_comments_doc; Doc.dotdotdot; spread_with_trailing_comments] + print_spread_with_comments ~state spread_expr cmt_tbl in let parts_doc = Doc.join @@ -2835,11 +2844,18 @@ and print_pattern ~state (p : Parsetree.pattern) cmt_tbl = (match tail.Parsetree.ppat_desc with | Ppat_construct ({txt = Longident.Lident "[]"}, _) -> Doc.nil | _ -> - let doc = - Doc.concat [Doc.text "..."; print_pattern ~state tail cmt_tbl] + (* Taken before [print_pattern] prints the other comments *) + let leading = + print_leading_comments Doc.nil cmt_tbl.leading tail.ppat_loc in - let tail = print_comments doc cmt_tbl tail.ppat_loc in - Doc.concat [Doc.text ","; Doc.line; tail]); + Doc.concat + [ + Doc.text ","; + Doc.line; + leading; + Doc.text "..."; + print_pattern ~state tail cmt_tbl; + ]); ] in Doc.group @@ -3372,12 +3388,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = [ Doc.text ","; Doc.line; - Doc.dotdotdot; - (let doc = print_expression_with_comments ~state expr cmt_tbl in - match Parens.expr_allowing_coercion expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc expr braces - | Nothing -> doc); + print_spread_with_comments ~state expr cmt_tbl; ] | None -> Doc.nil in @@ -4337,18 +4348,7 @@ and print_spread_expr ~state kind elements cmt_tbl = let element_doc = function | Parsetree.Pspread_item expr -> print_comments (expr_doc expr) cmt_tbl expr.pexp_loc - | Pspread expr -> - (* The leading comments go before the dots, so they're taken before the - expression is printed *) - let leading_comments_doc = - print_leading_comments Doc.nil cmt_tbl.Comment_table.leading - expr.pexp_loc - in - let expr_doc = - print_trailing_comments (expr_doc expr) cmt_tbl.Comment_table.trailing - expr.pexp_loc - in - Doc.concat [leading_comments_doc; Doc.dotdotdot; expr_doc] + | Pspread expr -> print_spread_with_comments ~state expr cmt_tbl in Doc.group (Doc.concat @@ -5960,10 +5960,11 @@ and print_attributes ?loc ?(inline = false) ~state | _ -> Doc.concat [ + (* A doc comment here follows an attribute and stays on its line *) Doc.group - (Doc.join_with_sep + (Doc.join ~sep:Doc.line (List.map - (fun attr -> print_attribute ~state attr cmt_tbl) + (fun attr -> fst (print_attribute ~state attr cmt_tbl)) attrs)); (if inline then Doc.space else line_break); ] diff --git a/tests/syntax_tests/data/printer/comments/array.res b/tests/syntax_tests/data/printer/comments/array.res index 675e9fee965..32779058dc8 100644 --- a/tests/syntax_tests/data/printer/comments/array.res +++ b/tests/syntax_tests/data/printer/comments/array.res @@ -61,3 +61,15 @@ let _ = list{ /* comment 2 */ ...ys /* comment 3 */, b, } + +// A comment before a list's tail spread +let _ = list{1, /* c0 */ ...xs} +let _ = list{1, /* c0 */ ...xs /* c1 */} +let _ = list{1, /* c0 */ ...(xs :> list) /* c1 */} +let _ = list{ + 1, + // c0 + ...xs, +} +let f = (list{a, /* c0 */ ...rest}) => a +let f = (list{a, /* c0 */ ...rest /* c1 */}) => a diff --git a/tests/syntax_tests/data/printer/comments/docCommentAttributes.res b/tests/syntax_tests/data/printer/comments/docCommentAttributes.res index 247d422aa05..59f5bbda116 100644 --- a/tests/syntax_tests/data/printer/comments/docCommentAttributes.res +++ b/tests/syntax_tests/data/printer/comments/docCommentAttributes.res @@ -19,3 +19,15 @@ Normal comment Normal comment Normal comment Normal comment Normal comment */ @a @b external f: unit => unit = "f" + +// A doc comment after an attribute stays after it +@a /** Doc comment */ @b +external f: unit => unit = "f" + +/** Doc comment 1 */ @a /** Doc comment 2 */ +let x = 1 + +type t = | @a /** Doc comment */ A | B +type r = {@a /** Doc comment */ x: int} +type p = [@a /** Doc comment */ #A | #B] +type q = @a /** Doc comment */ int diff --git a/tests/syntax_tests/data/printer/comments/docCommentParameters.res b/tests/syntax_tests/data/printer/comments/docCommentParameters.res index a05695773f5..7994b1cd5f9 100644 --- a/tests/syntax_tests/data/printer/comments/docCommentParameters.res +++ b/tests/syntax_tests/data/printer/comments/docCommentParameters.res @@ -3,3 +3,11 @@ let f = (/** The runner */ run: string) => run let f = (/** Positional */ x, /** Labelled */ ~y, /** Typed */ ~z: int) => x + y + z let f = (/** Optional */ ~a=?, /** Default */ ~b=1, /** Aliased */ ~c as d) => d let f = (/** Locally abstract */ type a, x: a) => x + +// A doc comment on a single parameter that would be hugged +let f = (/** Record */ {x, y}) => x +let f = (/** Array */ [a]) => a +let f = (/** Tuple */ (a, b)) => a +let f = (/** Unit */ ()) => 1 +let f = (@a /** After an attribute */ {x, y}) => x + diff --git a/tests/syntax_tests/data/printer/comments/expected/array.res.txt b/tests/syntax_tests/data/printer/comments/expected/array.res.txt index f333107201a..127caadcc42 100644 --- a/tests/syntax_tests/data/printer/comments/expected/array.res.txt +++ b/tests/syntax_tests/data/printer/comments/expected/array.res.txt @@ -58,3 +58,15 @@ let _ = list{ /* comment 2 */ ...ys /* comment 3 */, b, } + +// A comment before a list's tail spread +let _ = list{1, /* c0 */ ...xs} +let _ = list{1, /* c0 */ ...xs /* c1 */} +let _ = list{1, /* c0 */ ...xs :> list /* c1 */} +let _ = list{ + 1, + // c0 + ...xs, +} +let f = (list{a, /* c0 */ ...rest}) => a +let f = (list{a, /* c0 */ ...rest /* c1 */}) => a diff --git a/tests/syntax_tests/data/printer/comments/expected/docCommentAttributes.res.txt b/tests/syntax_tests/data/printer/comments/expected/docCommentAttributes.res.txt index 247d422aa05..2dee92c6517 100644 --- a/tests/syntax_tests/data/printer/comments/expected/docCommentAttributes.res.txt +++ b/tests/syntax_tests/data/printer/comments/expected/docCommentAttributes.res.txt @@ -19,3 +19,16 @@ Normal comment Normal comment Normal comment Normal comment Normal comment */ @a @b external f: unit => unit = "f" + +// A doc comment after an attribute stays after it +@a /** Doc comment */ @b +external f: unit => unit = "f" + +/** Doc comment 1 */ +@a /** Doc comment 2 */ +let x = 1 + +type t = | @a /** Doc comment */ A | B +type r = {@a /** Doc comment */ x: int} +type p = [@a /** Doc comment */ #A | #B] +type q = @a /** Doc comment */ int diff --git a/tests/syntax_tests/data/printer/comments/expected/docCommentParameters.res.txt b/tests/syntax_tests/data/printer/comments/expected/docCommentParameters.res.txt index 6be363ea313..ab6f0b007fb 100644 --- a/tests/syntax_tests/data/printer/comments/expected/docCommentParameters.res.txt +++ b/tests/syntax_tests/data/printer/comments/expected/docCommentParameters.res.txt @@ -27,3 +27,22 @@ let f = ( type a, x: a, ) => x + +// A doc comment on a single parameter that would be hugged +let f = ( + /** Record */ + {x, y}, +) => x +let f = ( + /** Array */ + [a], +) => a +let f = ( + /** Tuple */ + (a, b), +) => a +let f = ( + /** Unit */ + (), +) => 1 +let f = (@a /** After an attribute */ {x, y}) => x diff --git a/tests/syntax_tests/data/printer/comments/expected/fileWithOneSingleLineComment.res.txt b/tests/syntax_tests/data/printer/comments/expected/fileWithOneSingleLineComment.res.txt index 0f99a48a30b..40e1389f97e 100644 --- a/tests/syntax_tests/data/printer/comments/expected/fileWithOneSingleLineComment.res.txt +++ b/tests/syntax_tests/data/printer/comments/expected/fileWithOneSingleLineComment.res.txt @@ -1,2 +1 @@ // retain this comment - diff --git a/tests/syntax_tests/data/printer/comments/expected/fileWithOnlyComments.res.txt b/tests/syntax_tests/data/printer/comments/expected/fileWithOnlyComments.res.txt index d0df583cc0f..7ebfb940846 100644 --- a/tests/syntax_tests/data/printer/comments/expected/fileWithOnlyComments.res.txt +++ b/tests/syntax_tests/data/printer/comments/expected/fileWithOnlyComments.res.txt @@ -6,4 +6,3 @@ comment */ // more comments - diff --git a/tests/syntax_tests/data/printer/comments/expected/interfaceWithOneSingleComment.resi.txt b/tests/syntax_tests/data/printer/comments/expected/interfaceWithOneSingleComment.resi.txt index 0f99a48a30b..40e1389f97e 100644 --- a/tests/syntax_tests/data/printer/comments/expected/interfaceWithOneSingleComment.resi.txt +++ b/tests/syntax_tests/data/printer/comments/expected/interfaceWithOneSingleComment.resi.txt @@ -1,2 +1 @@ // retain this comment - diff --git a/tests/syntax_tests/data/printer/comments/expected/interfaceWithOnlyComments.resi.txt b/tests/syntax_tests/data/printer/comments/expected/interfaceWithOnlyComments.resi.txt index de095c021bd..adb8c1558cf 100644 --- a/tests/syntax_tests/data/printer/comments/expected/interfaceWithOnlyComments.resi.txt +++ b/tests/syntax_tests/data/printer/comments/expected/interfaceWithOnlyComments.resi.txt @@ -3,4 +3,3 @@ /* test */ // ok - diff --git a/tests/syntax_tests/data/printer/comments/expected/pattern.res.txt b/tests/syntax_tests/data/printer/comments/expected/pattern.res.txt index 3b4a79fb8e0..2bd71d1da15 100644 --- a/tests/syntax_tests/data/printer/comments/expected/pattern.res.txt +++ b/tests/syntax_tests/data/printer/comments/expected/pattern.res.txt @@ -45,7 +45,7 @@ let [/* inside */] = [] let list{/* inside */} = list{} let /* before */ list{/* a1 */ a /* a2 */, /* b1 */ b /* b2 */} /* after */ = list{1, 2} -let /* before */ list{/* a1 */ a /* a2 */, .../* b1 */ b /* b2 */} /* after */ = list{1, 2} +let /* before */ list{/* a1 */ a /* a2 */, /* b1 */ ...b /* b2 */} /* after */ = list{1, 2} // Ppat_record let /* before */ {/* c0 */ name /* c1 */, /* c2 */ age /* c3 */} /* after */ = { diff --git a/tests/syntax_tests/data/printer/modExpr/expected/attributes.res.txt b/tests/syntax_tests/data/printer/modExpr/expected/attributes.res.txt index 1179ae4849c..f1fe4b80e0e 100644 --- a/tests/syntax_tests/data/printer/modExpr/expected/attributes.res.txt +++ b/tests/syntax_tests/data/printer/modExpr/expected/attributes.res.txt @@ -42,7 +42,7 @@ module U = (@JSX F(A))(B) module V = @JSX (X: S) // a functor argument can't start with a doc comment -module W = H((/** doc */ @w X)) +module W = H((@w /** doc */ X)) module Y = H((/** doc */ X: S)) module AA = H(await /** doc */ X) module Z = /** doc */ X diff --git a/tests/syntax_tests/data/printer/typeDef/attributes.res b/tests/syntax_tests/data/printer/typeDef/attributes.res index 7c2662e0f34..aedca77eee5 100644 --- a/tests/syntax_tests/data/printer/typeDef/attributes.res +++ b/tests/syntax_tests/data/printer/typeDef/attributes.res @@ -22,6 +22,7 @@ type t = /** doc */ {x: int} type t = /** doc */ .. type t = /** doc */ @attr {x: int} type t = @attr /** doc */ @attr2 .. +type t = /** d1 */ @attr /** d2 */ {x: int} @outer type t = /* c1 */ /** doc */ /* c2 */ {x: int} // trail type t = /** doc */ A | B diff --git a/tests/syntax_tests/data/printer/typeDef/expected/attributes.res.txt b/tests/syntax_tests/data/printer/typeDef/expected/attributes.res.txt index 600f0dd2429..28a83e2e497 100644 --- a/tests/syntax_tests/data/printer/typeDef/expected/attributes.res.txt +++ b/tests/syntax_tests/data/printer/typeDef/expected/attributes.res.txt @@ -23,8 +23,7 @@ type t = { email: string, name: string, } -/** doc */ -@attr type t = {x: int} +@attr /** doc */ type t = {x: int} @attr type t = .. /** doc */ type t = {x: int} @@ -32,11 +31,10 @@ type t = {x: int} type t = .. /** doc */ @attr type t = {x: int} -/** doc */ -@attr @attr2 type t = .. -/** doc */ -@outer -type t = {/* c1 */ /* c2 */ x: int} // trail +@attr /** doc */ @attr2 type t = .. +/** d1 */ +@attr /** d2 */ type t = {x: int} +@outer /** doc */ type t = {/* c1 */ /* c2 */ x: int} // trail type t = /** doc */ | A