diff --git a/changes/operand-line-comment.fix.md b/changes/operand-line-comment.fix.md new file mode 100644 index 0000000000..28f4f60ed3 --- /dev/null +++ b/changes/operand-line-comment.fix.md @@ -0,0 +1 @@ +- Keep a line comment in the operand of `switch`, `if`, `try` or a loop in the operand instead of moving it into the block when formatting. diff --git a/compiler/syntax/src/res_doc.ml b/compiler/syntax/src/res_doc.ml index e2222c2288..5ba1a50e8c 100644 --- a/compiler/syntax/src/res_doc.ml +++ b/compiler/syntax/src/res_doc.ml @@ -122,6 +122,25 @@ let rec will_break doc = | IfBreaks {yes; no} -> will_break yes || will_break no | _ -> false +(* See documentation in interface file *) +let has_pending_line_suffix doc = + (* [pending]: a line suffix is queued; [broken]: the innermost enclosing + group is printed in break mode, so its line breaks flush the queue. *) + let rec walk ~broken pending doc = + match doc with + | LineSuffix _ -> true + | LineBreak _ -> pending && not broken + | Nil | Text _ | BreakParent -> pending + | Indent doc -> walk ~broken pending doc + | Concat docs -> List.fold_left (walk ~broken) pending docs + | Group {should_break; doc} -> + walk ~broken:(should_break || will_break doc) pending doc + | IfBreaks {yes; no; broken = if_broken} -> + walk ~broken pending (if broken || if_broken then yes else no) + | CustomLayout docs -> List.exists (walk ~broken:false pending) docs + in + walk ~broken:false false doc + let join ~sep docs = let rec loop acc sep docs = match docs with diff --git a/compiler/syntax/src/res_doc.mli b/compiler/syntax/src/res_doc.mli index d3423658c0..13caa1a0c2 100644 --- a/compiler/syntax/src/res_doc.mli +++ b/compiler/syntax/src/res_doc.mli @@ -50,6 +50,13 @@ val will_break : t -> bool [will_break] to detect a forced break in a custom layout and explicitly add [break_parent] to propagate it to the parent document. *) +val has_pending_line_suffix : t -> bool +(** [has_pending_line_suffix doc] checks whether a line suffix (a trailing line + comment) in [doc] is still queued at its end, because no line break of a + group that breaks follows it. Groups that only break for lack of width + count as flat. Text after [doc] on the same line would precede the + comment. *) + val to_string : width:int -> t -> string val debug : t -> unit diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index ed4327dc01..2df53ca625 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -3165,6 +3165,38 @@ and print_expression_args ~state (args : Parsetree.expression list) cmt_tbl = Doc.rparen; ] +(* The operand of [switch], [try], [if], [while] or a loop, which is followed + by more code on the same line. A line comment still pending at its end + would move after that code, into the block, so the operand goes into + parentheses with the comment before the [)]. *) +and print_operand_before_brace doc = + if Doc.has_pending_line_suffix doc then add_operand_parens doc else doc + +(* An operand followed by a block, see [print_operand_before_brace], with the + parentheses or braces it needs *) +and print_block_operand ~state expr cmt_tbl = + let doc = print_expression_with_comments ~state expr cmt_tbl in + print_operand_before_brace + (match Parens.expr expr with + | Parens.Parenthesized -> add_operand_parens doc + | Braced braces -> print_braces doc expr braces + | Nothing -> doc) + +(* Parentheses around such an operand, or around one that needs them, such as + a coercion. They break when a line comment is still pending at the end of + the operand, so that the comment stays before the [)]. *) +and add_operand_parens doc = + if Doc.has_pending_line_suffix doc then + Doc.breakable_group ~force_break:true + (Doc.concat + [ + Doc.lparen; + Doc.indent (Doc.concat [Doc.soft_line; doc]); + Doc.soft_line; + Doc.rparen; + ]) + else add_parens doc + and print_if_chain ~state pexp_attributes ifs else_expr cmt_tbl = let if_docs = Doc.join ~sep:Doc.space @@ -3180,14 +3212,14 @@ and print_if_chain ~state pexp_attributes ifs else_expr cmt_tbl = print_expression_with_comments ~state if_expr cmt_tbl in match Parens.expr if_expr with - | Parens.Parenthesized -> add_parens doc + | Parens.Parenthesized -> add_operand_parens doc | Braced braces -> print_braces doc if_expr braces | Nothing -> Doc.if_breaks (add_parens doc) doc in Doc.concat [ if_txt; - Doc.group condition; + print_operand_before_brace (Doc.group condition); Doc.space; (let then_expr = match Parsetree_viewer.process_braces then_expr with @@ -3659,7 +3691,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = let condition = let doc = print_expression_with_comments ~state expr1 cmt_tbl in match Parens.expr expr1 with - | Parens.Parenthesized -> add_parens doc + | Parens.Parenthesized -> add_operand_parens doc | Braced braces -> print_braces doc expr1 braces | Nothing -> doc in @@ -3667,8 +3699,11 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = (Doc.concat [ Doc.text "while "; - (if Parsetree_viewer.is_block_expr expr1 then condition - else Doc.group (Doc.if_breaks (add_parens condition) condition)); + print_operand_before_brace + (match Parens.expr expr1 with + | Parens.Parenthesized -> condition + | _ when Parsetree_viewer.is_block_expr expr1 -> condition + | _ -> Doc.group (Doc.if_breaks (add_parens condition) condition)); Doc.space; print_expression_block ~state ~braces:true expr2 cmt_tbl; ]) @@ -3679,19 +3714,9 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = Doc.text "for "; print_pattern ~state pattern cmt_tbl; Doc.text " in "; - (let doc = - print_expression_with_comments ~state from_expr cmt_tbl - in - match Parens.expr from_expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc from_expr braces - | Nothing -> doc); + print_block_operand ~state from_expr cmt_tbl; print_direction_flag direction_flag; - (let doc = print_expression_with_comments ~state to_expr cmt_tbl in - match Parens.expr to_expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc to_expr braces - | Nothing -> doc); + print_block_operand ~state to_expr cmt_tbl; Doc.space; print_expression_block ~state ~braces:true body cmt_tbl; ]) @@ -3702,13 +3727,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = Doc.text "for "; print_pattern ~state pattern cmt_tbl; Doc.text " of "; - (let doc = - print_expression_with_comments ~state array_expr cmt_tbl - in - match Parens.expr array_expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc array_expr braces - | Nothing -> doc); + print_block_operand ~state array_expr cmt_tbl; Doc.space; print_expression_block ~state ~braces:true body cmt_tbl; ]) @@ -3719,13 +3738,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = Doc.text "for await "; print_pattern ~state pattern cmt_tbl; Doc.text " of "; - (let doc = - print_expression_with_comments ~state iterable_expr cmt_tbl - in - match Parens.expr iterable_expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc iterable_expr braces - | Nothing -> doc); + print_block_operand ~state iterable_expr cmt_tbl; Doc.space; print_expression_block ~state ~braces:true body cmt_tbl; ]) @@ -3805,13 +3818,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = | Pexp_sequence _ -> print_expression_block ~state ~braces:true e cmt_tbl | Pexp_let _ -> print_expression_block ~state ~braces:true e cmt_tbl | Pexp_try (expr, cases, finally) -> - let expr_doc = - let doc = print_expression_with_comments ~state expr cmt_tbl in - match Parens.expr expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc expr braces - | Nothing -> doc - in + let expr_doc = print_block_operand ~state expr cmt_tbl in let catch_doc = match cases with | [] -> Doc.nil @@ -3830,13 +3837,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = in Doc.concat [Doc.text "try "; expr_doc; catch_doc; finally_doc] | Pexp_match (expr, cases) -> - let expr_doc = - let doc = print_expression_with_comments ~state expr cmt_tbl in - match Parens.expr expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc expr braces - | Nothing -> doc - in + let expr_doc = print_block_operand ~state expr cmt_tbl in Doc.concat [ Doc.text "switch "; diff --git a/tests/syntax_tests/data/printer/comments/expected/operandLineComment.res.txt b/tests/syntax_tests/data/printer/comments/expected/operandLineComment.res.txt new file mode 100644 index 0000000000..f00f09f6b5 --- /dev/null +++ b/tests/syntax_tests/data/printer/comments/expected/operandLineComment.res.txt @@ -0,0 +1,227 @@ +// A line comment in the operand of a block construct stays in the operand + +switch ( + obj.x // c +) { +| _ => () +} + +switch ( + obj.x.y // c +) { +| _ => () +} + +switch ( + obj.x.y // c // d +) { +| _ => () +} + +switch ( + arr[0] // c +) { +| _ => () +} + +switch ( + obj["member"] // c +) { +| _ => () +} + +switch ( + obj["member"] // c +) { +| _ => () +} + +switch ( + obj->f // c +) { +| _ => () +} + +switch ( + a + b // c +) { +| _ => () +} + +switch f( + // c + x, +) { +| _ => () +} + +switch x { +// c +| _ => () +} + +if ( + obj.x // c +) { + () +} else if ( + obj.y // d +) { + () +} + +while ( + obj.x // c +) { + () +} + +for x of ( + obj.x // c +) { + () +} + +let f = async () => { + for await x of ( + obj.x // c + ) { + () + } +} + +for i in ( + 0 // c +) to n { + () +} + +for i in 0 to ( + n // c +) { + () +} + +try ( + obj.x // c +) catch { +| _ => () +} + +// An operand printed in its own parentheses keeps the comment inside them +switch ( + f(a, b) :> int // c +) { +| _ => () +} + +switch ( + f(a, b): int // c +) { +| _ => () +} + +if ( + f(a, b) :> bool // c +) { + () +} + +while ( + f(a, b) :> bool // c +) { + () +} + +for x of ( + f(a, b) :> array // c +) { + () +} + +try ( + f(a, b) :> int // c +) catch { +| _ => () +} + +// A comment after an item of the operand +switch ( + [a, b] // c // d +) { +| _ => () +} + +switch ( + list{a, ...b, ...c} // c +) { +| _ => () +} + +switch ( + (a, b) // c +) { +| _ => () +} + +switch { + x: a, // c + y: b, +} { +| _ => () +} + +switch ( + a->f(x, y) // c +) { +| _ => () +} + +switch ( + a->f(x, y) // c // d +) { +| _ => () +} + +if ( + f(a, b) // c +) { + () +} + +while ( + f(a, b) + 1 > 0 // c +) { + () +} + +for x of ( + [a, b] // c +) { + () +} + +try ( + [...a, b] // c +) catch { +| _ => () +} + +// A binary expression or pipe in an item of the operand +switch ( + [a + b, d] // c +) { +| _ => () +} + +if ( + f(a && b) // c +) { + () +} + +for x of ( + [a->f(y)] // c +) { + () +} diff --git a/tests/syntax_tests/data/printer/comments/operandLineComment.res b/tests/syntax_tests/data/printer/comments/operandLineComment.res new file mode 100644 index 0000000000..b9d09a6bd3 --- /dev/null +++ b/tests/syntax_tests/data/printer/comments/operandLineComment.res @@ -0,0 +1,223 @@ +// A line comment in the operand of a block construct stays in the operand + +switch obj // c +.x { +| _ => () +} + +switch obj // c +.x.y { +| _ => () +} + +switch obj // c +.x // d +.y { +| _ => () +} + +switch arr[// c +0] { +| _ => () +} + +switch obj[// c +"member"] { +| _ => () +} + +switch obj["member"] // c +{ +| _ => () +} + +switch obj // c +->f { +| _ => () +} + +switch a // c ++ b { +| _ => () +} + +switch f(// c +x) { +| _ => () +} + +switch x { // c +| _ => () +} + +if obj // c +.x { + () +} else if obj // d +.y { + () +} + +while obj // c +.x { + () +} + +for x of obj // c +.x { + () +} + +let f = async () => { + for await x of obj // c + .x { + () + } +} + +for i in 0 // c +to n { + () +} + +for i in 0 to n // c +{ + () +} + +try obj // c +.x catch { +| _ => () +} + +// An operand printed in its own parentheses keeps the comment inside them +switch (f( + a, // c + b, +) :> int) { +| _ => () +} + +switch (f( + a, // c + b, +): int) { +| _ => () +} + +if (f( + a, // c + b, +) :> bool) { + () +} + +while (f( + a, // c + b, +) :> bool) { + () +} + +for x of (f( + a, // c + b, +) :> array) { + () +} + +try (f( + a, // c + b, +) :> int) catch { +| _ => () +} + +// A comment after an item of the operand +switch [ + a, // c + b, // d +] { +| _ => () +} + +switch list{ + a, // c + ...b, + ...c, +} { +| _ => () +} + +switch ( + a, // c + b, +) { +| _ => () +} + +switch { + x: a, // c + y: b, +} { +| _ => () +} + +switch a->f( + x, // c + y, +) { +| _ => () +} + +switch a // c +->f( + x, // d + y, +) { +| _ => () +} + +if f( + a, // c + b, +) { + () +} + +while f( + a, // c + b, +) + 1 > 0 { + () +} + +for x of [ + a, // c + b, +] { + () +} + +try [ + ...a, // c + b, +] catch { +| _ => () +} + +// A binary expression or pipe in an item of the operand +switch [a // c ++ b, d] { +| _ => () +} + +if f(a // c +&& b) { + () +} + +for x of [a // c +->f(y)] { + () +}