diff --git a/CHANGELOG.md b/CHANGELOG.md index d444142eb8..08cdfd876c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ #### :nail_care: Polish - Avoid running `rescript-schema-ppx` and `sury-ppx` on source files without an `@schema` annotation. https://github.com/rescript-lang/rescript/pull/8662 +- Format JSX expression children and standalone comments with braces to prepare for future literal text support. https://github.com/rescript-lang/rescript/pull/8666 #### :house: Internal diff --git a/compiler/syntax/JSX.md b/compiler/syntax/JSX.md index fc6f5ecf27..e929282d7a 100644 --- a/compiler/syntax/JSX.md +++ b/compiler/syntax/JSX.md @@ -42,6 +42,8 @@ rewrites those nodes as calls to the configured JSX module: directly, which supports external components; - a lowercase tag is emitted through the configured host-element module; - fragments use the configured `jsxFragment` value; +- comment-only child containers are retained as comments by the parser and do + not contribute a child; bare `{}` still denotes an empty record expression; - one child becomes a `children` prop and multiple children become an array; - keyed elements select the keyed runtime entry point; - at most one props spread is accepted, and it must precede explicit props. diff --git a/compiler/syntax/src/res_comments_table.ml b/compiler/syntax/src/res_comments_table.ml index c60c3a7602..72917f32fa 100644 --- a/compiler/syntax/src/res_comments_table.ml +++ b/compiler/syntax/src/res_comments_table.ml @@ -417,6 +417,7 @@ type node = | CoreType of Parsetree.core_type | ExprArgument of {expr: Parsetree.expression; loc: Location.t} | Expression of Parsetree.expression + | JsxChild of Parsetree.expression | ExprRecordRow of Longident.t Asttypes.loc * Parsetree.expression | ExtensionConstructor of Parsetree.extension_constructor | LabelDeclaration of Parsetree.label_declaration @@ -448,7 +449,7 @@ let get_loc node = } | CoreType ct -> ct.ptyp_loc | ExprArgument {loc} -> loc - | Expression e -> ( + | Expression e | JsxChild e -> ( match e.pexp_attributes with | ({txt = "res.braces" | "ns.braces"; loc}, _) :: _ -> loc | _ -> e.pexp_loc) @@ -640,6 +641,13 @@ and walk_node node tbl comments = | CoreType ct -> walk_core_type ct tbl comments | ExprArgument ea -> walk_expr_argument ea.expr ea.loc tbl comments | Expression e -> walk_expression e tbl comments + | JsxChild e -> + (* Braces may contain comments outside the expression itself. Keep those + on the child, rather than moving them into record fields or call args. *) + let leading, inside, trailing = partition_by_loc comments e.pexp_loc in + attach tbl.leading e.pexp_loc leading; + walk_expression e tbl inside; + attach tbl.trailing e.pexp_loc trailing | ExprRecordRow (ri, e) -> walk_expr_record_row (ri, e) tbl comments | ExtensionConstructor ec -> walk_extension_constructor ec tbl comments | LabelDeclaration ld -> walk_label_declaration ld tbl comments @@ -658,23 +666,29 @@ and walk_node node tbl comments = | ValueBinding vb -> walk_value_binding vb tbl comments | JsxProp prop -> walk_jsx_prop prop tbl comments -and walk_list : ?prev_loc:Location.t -> node list -> t -> Comment.t list -> unit - = - fun ?prev_loc l t comments -> +and walk_list : ?prev_node:node -> node list -> t -> Comment.t list -> unit = + fun ?prev_node l t comments -> match l with | _ when comments = [] -> () | [] -> ( - match prev_loc with - | Some loc -> attach t.trailing loc comments + match prev_node with + | Some node -> attach t.trailing (get_loc node) comments | None -> ()) | node :: rest -> let curr_loc = get_loc node in let leading, inside, trailing = partition_by_loc comments curr_loc in - (match prev_loc with + (match prev_node with | None -> (* first node, all leading comments attach here *) attach t.leading curr_loc leading - | Some prev_loc -> + | Some (JsxChild ({pexp_desc = Pexp_jsx_element _} as child)) + when Res_parens.jsx_child_expr child = Nothing -> + (* Standalone containers put their comments on separate lines. Always + attach comments between bare JSX elements to the next child so the + first format and subsequent parses agree. *) + attach t.leading curr_loc leading + | Some prev_node -> + let prev_loc = get_loc prev_node in (* Same line *) if prev_loc.loc_end.pos_lnum == curr_loc.loc_start.pos_lnum then ( let after_prev, before_curr = @@ -692,7 +706,7 @@ and walk_list : ?prev_loc:Location.t -> node list -> t -> Comment.t list -> unit in attach t.leading curr_loc leading); walk_node node t inside; - walk_list ~prev_loc:curr_loc rest t trailing + walk_list ~prev_node:node rest t trailing (* The parsetree doesn't always contain location info about the opening or * closing token of a "list-of-things". This routine visits the whole list, @@ -1599,15 +1613,14 @@ and walk_expression expr t comments = | Pexp_jsx_element (Jsx_fragment { - jsx_fragment_opening = opening_greater_than; + jsx_fragment_opening = _opening_greater_than; jsx_fragment_children = children; jsx_fragment_closing = _closing_lesser_than; - }) -> - let opening_token = {expr.pexp_loc with loc_end = opening_greater_than} in - let on_same_line, rest = partition_by_on_same_line opening_token comments in - attach t.trailing opening_token on_same_line; - let xs = children |> List.map (fun e -> Expression e) in - walk_list xs t rest + }) -> ( + match children with + | [] -> attach t.inside expr.pexp_loc comments + | children -> walk_list (List.map (fun e -> JsxChild e) children) t comments + ) | Pexp_jsx_element (Jsx_unary_element { @@ -1696,12 +1709,6 @@ and walk_expression expr t comments = rest in - (* comments after '>' on the same line should be attached to '>' *) - let after_opening_greater_than, rest = - partition_by_on_same_line opening_greater_than_loc rest - in - attach t.trailing opening_greater_than_loc after_opening_greater_than; - let comments_for_children, _rest = match closing_tag with | None -> (rest, []) @@ -1712,39 +1719,9 @@ and walk_expression expr t comments = partition_leading_trailing rest closing_tag_loc in match children with - | [] -> ( - (* attach all comments to the closing tag if there are no children *) - match closing_tag with - | None -> - (* if there is no closing tag, the comments will attached after the expression *) - () - | Some closing_tag -> - let closing_tag_loc = - Parsetree_viewer.container_element_closing_tag_loc closing_tag - in - if - opening_greater_than_loc.loc_end.pos_lnum - < closing_tag_loc.loc_start.pos_lnum + 1 - then ( - (* In this case, there are no children but there are comments between the opening and closing tag, - We can attach these the inside table, to easily print them later as indented comments - For example: -
- // comment 1 - // comment 2 -
- *) - let inside_comments, leading_for_closing_tag = - partition_between_lines opening_greater_than_loc.loc_end.pos_lnum - closing_tag_loc.loc_start.pos_lnum comments_for_children - in - attach t.inside expr.pexp_loc inside_comments; - attach t.leading closing_tag_loc leading_for_closing_tag) - else - (* if the closing tag is on the same line, attach comments to the opening tag *) - attach t.leading closing_tag_loc comments_for_children) + | [] -> attach t.inside expr.pexp_loc comments_for_children | children -> - let children_nodes = List.map (fun e -> Expression e) children in + let children_nodes = List.map (fun e -> JsxChild e) children in walk_list children_nodes t comments_for_children (* It is less likely that there are comments inside the closing tag, diff --git a/compiler/syntax/src/res_core.ml b/compiler/syntax/src/res_core.ml index c1a905c33c..fd294d0b9f 100644 --- a/compiler/syntax/src/res_core.ml +++ b/compiler/syntax/src/res_core.ml @@ -2964,6 +2964,22 @@ and parse_jsx_children p : Parsetree.jsx_children = parse_primary_expr ~operand:(parse_atomic_expr p) ~no_call:true p in loop p (child :: children) + | Lbrace when Parser.peek2 p = Rbrace -> + let start_pos = Parser.start_pos p in + Parser.next p; + let comments_before = p.comments in + Parser.next p; + (* Only comment-containing containers are trivia. Keep bare {} as an + empty record, including when nested inside an expression container. *) + if p.comments != comments_before then loop p children + else + let loc = mk_loc start_pos (Parser.position p) in + let child = + parse_primary_expr + ~operand:(Ast_helper.Exp.record ~loc [] None) + ~no_call:true p + in + loop p (child :: children) | token when Grammar.is_jsx_child_start token -> let child = parse_primary_expr ~operand:(parse_atomic_expr p) ~no_call:true p diff --git a/compiler/syntax/src/res_parens.ml b/compiler/syntax/src/res_parens.ml index b24c5fbede..42324d7035 100644 --- a/compiler/syntax/src/res_parens.ml +++ b/compiler/syntax/src/res_parens.ml @@ -348,39 +348,17 @@ let jsx_child_expr expr = match expr.Parsetree.pexp_desc with | Parsetree.Pexp_let _ | Pexp_sequence _ | Pexp_letexception _ | Pexp_letmodule _ | Pexp_open _ -> + (* These expressions already print as braced blocks. *) Nothing | _ -> ( let opt_braces, _ = Parsetree_viewer.process_braces_attr expr in match opt_braces with | Some ({Location.loc = braces_loc}, _) -> Braced braces_loc | _ -> ( - match expr with - | { - Parsetree.pexp_desc = - Pexp_constant (Pconst_integer (x, _) | Pconst_float (x, _)); - pexp_attributes = []; - } - when starts_with_minus x -> - Parenthesized - | _ when Parsetree_viewer.expr_is_await expr -> Parenthesized - | { - Parsetree.pexp_desc = - ( Pexp_ident _ | Pexp_constant _ | Pexp_regexp _ | Pexp_field _ - | Pexp_construct _ | Pexp_variant _ | Pexp_array _ | Pexp_pack _ - | Pexp_record _ | Pexp_object_literal _ | Pexp_extension _ - | Pexp_letmodule _ | Pexp_letexception _ | Pexp_open _ - | Pexp_sequence _ | Pexp_let _ | Pexp_jsx_element _ ); - pexp_attributes = []; - } -> - Nothing - | { - Parsetree.pexp_desc = - Pexp_constraint - ({pexp_desc = Pexp_pack _}, {ptyp_desc = Ptyp_package _}); - pexp_attributes = []; - } -> - Nothing - | {pexp_desc = Pexp_jsx_element _} -> Nothing + match expr.pexp_desc with + | Pexp_jsx_element _ -> Nothing + (* JSX child expressions use braces even when the legacy grammar accepts + them bare. Records need an outer pair around their own braces. *) | _ -> Parenthesized)) let binary_expr expr = diff --git a/compiler/syntax/src/res_parens.mli b/compiler/syntax/src/res_parens.mli index 8d304823f4..c036093484 100644 --- a/compiler/syntax/src/res_parens.mli +++ b/compiler/syntax/src/res_parens.mli @@ -23,6 +23,9 @@ val field_expr : Parsetree.expression -> kind val ternary_operand : Parsetree.expression -> kind val jsx_prop_expr : Parsetree.expression -> kind + +(* JSX children use braces for [Parenthesized]; [Nothing] is reserved for JSX + elements and expressions that already print as braced blocks. *) val jsx_child_expr : Parsetree.expression -> kind val binary_expr : Parsetree.expression -> kind diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index 7854dbe709..6ddf6ce76f 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -59,12 +59,6 @@ let has_leading_line_comment tbl loc = | Some comment -> Comment.is_single_line_comment comment | None -> false -let get_leading_line_comment_count tbl loc = - match Hashtbl.find_opt tbl.Comment_table.leading loc with - | Some comments -> - List.filter Comment.is_single_line_comment comments |> List.length - | None -> 0 - let has_trailing_single_line_comment tbl loc = match Hashtbl.find_opt tbl.Comment_table.trailing loc with | Some (comment :: _) -> Comment.is_single_line_comment comment @@ -4629,6 +4623,23 @@ and print_jsx_unary_tag ~state tag_name props expr_loc cmt_tbl = closing_tag_doc; ]) +(* Standalone JSX comments are trivia, not expression children. Keep their + delimiters inside braces so future literal text cannot absorb them. *) +and print_jsx_comment_container comments_tbl loc = + match Hashtbl.find_opt comments_tbl loc with + | None | Some [] -> Doc.nil + | Some comments -> + Hashtbl.remove comments_tbl loc; + let docs = + List.map + (fun comment -> + if Comment.is_single_line_comment comment then + Doc.concat [Doc.text ("//" ^ Comment.txt comment); Doc.break_parent] + else print_multiline_comment_content (Comment.txt comment)) + comments + in + add_braces (Doc.join docs ~sep:Doc.hard_line) + and print_jsx_container_tag ~state tag_name (opening_greater_than : Lexing.position) props (children : Parsetree.jsx_children) @@ -4658,12 +4669,9 @@ and print_jsx_container_tag ~state tag_name in let line_sep = get_line_sep_for_jsx_children children in let print_children children = - Doc.concat - [ - Doc.indent - (Doc.concat [Doc.line; print_jsx_children ~state children cmt_tbl]); - line_sep; - ] + let doc = print_jsx_children ~state children cmt_tbl in + if line_sep = Doc.nil then doc + else Doc.concat [Doc.indent (Doc.concat [line_sep; doc]); line_sep] in (* comments between the opening and closing tag *) @@ -4723,7 +4731,7 @@ and print_jsx_container_tag ~state tag_name [ (if has_children then print_children children else if not has_comments_inside then Doc.soft_line - else print_comments_inside cmt_tbl pexp_loc); + else print_jsx_comment_container cmt_tbl.inside pexp_loc); closing_element_doc; ]; ]) @@ -4751,22 +4759,30 @@ and print_jsx_fragment ~state (opening_greater_than : Lexing.position) (Doc.concat [ opening; - Doc.indent - (Doc.concat [Doc.line; print_jsx_children ~state children cmt_tbl]); + (let doc = + if has_children then print_jsx_children ~state children cmt_tbl + else print_jsx_comment_container cmt_tbl.inside fragment_loc + in + if line_sep = Doc.nil then doc + else Doc.indent (Doc.concat [line_sep; doc])); (if has_children then line_sep else Doc.nil); closing; ]) and get_line_sep_for_jsx_children (children : Parsetree.jsx_children) = + (* Keep wrapping a single expression inside its braces, never between it and + its tag. Layout between sibling children still uses the legacy whitespace + rules until literal JSX text is introduced. *) if List.length children > 1 || List.exists (function - | {Parsetree.pexp_desc = Pexp_jsx_element _} -> true + | {Parsetree.pexp_desc = Pexp_jsx_element _} as child -> + Parens.jsx_child_expr child = Nothing | _ -> false) children then Doc.hard_line - else Doc.line + else Doc.nil and print_jsx_children ~state (children : Parsetree.jsx_children) cmt_tbl = let open Parsetree in @@ -4784,23 +4800,59 @@ and print_jsx_children ~state (children : Parsetree.jsx_children) cmt_tbl = in let sep = get_line_sep_for_jsx_children children in let print_expr (expr : Parsetree.expression) = - let leading_line_comment_present = - has_leading_line_comment cmt_tbl expr.pexp_loc - in - let expr_doc = print_expression_with_comments ~state expr cmt_tbl in - let add_parens_or_braces expr_doc = - (* {(20: int)} make sure that we also protect the expression inside *) - let inner_doc = - if Parens.braced_expr expr then add_parens expr_doc else expr_doc + let loc = get_loc expr in + match (expr.pexp_desc, Parens.jsx_child_expr expr) with + | Pexp_jsx_element _, Nothing -> + let leading = print_jsx_comment_container cmt_tbl.leading loc in + let trailing = print_jsx_comment_container cmt_tbl.trailing loc in + Doc.concat + [ + leading; + (if leading = Doc.nil then Doc.nil else Doc.hard_line); + print_expression_with_comments ~state expr cmt_tbl; + (if trailing = Doc.nil then Doc.nil else Doc.hard_line); + trailing; + ] + | _ -> + let has_line_comment = + has_leading_line_comment cmt_tbl loc + || has_any_trailing_line_comment cmt_tbl loc + || has_leading_line_comment cmt_tbl expr.pexp_loc + || has_any_trailing_line_comment cmt_tbl expr.pexp_loc in - if leading_line_comment_present then add_braces inner_doc - else Doc.concat [Doc.lbrace; inner_doc; Doc.rbrace] - in - match Parens.jsx_child_expr expr with - | Nothing -> print_comments expr_doc cmt_tbl (get_loc expr) - | Parenthesized -> add_parens_or_braces expr_doc - | Braced braces_loc -> - print_comments (add_parens_or_braces expr_doc) cmt_tbl braces_loc + (* Consume comments before printing subnodes that may share these + locations. Both outer and inner comments belong inside the child + expression's braces, outside the future JSX text region. *) + let leading = print_leading_comments Doc.nil cmt_tbl.leading loc in + let inner_leading = + print_leading_comments Doc.nil cmt_tbl.leading expr.pexp_loc + in + let inner_trailing = + print_trailing_comments Doc.nil cmt_tbl.trailing expr.pexp_loc + in + let trailing = print_trailing_comments Doc.nil cmt_tbl.trailing loc in + let expr_doc = + match expr.pexp_desc with + | Pexp_let _ | Pexp_sequence _ | Pexp_letexception _ | Pexp_letmodule _ + | Pexp_open _ + when not + (Parsetree_viewer.has_printable_attributes expr.pexp_attributes) + -> + print_expression_block ~state ~braces:false expr cmt_tbl + | _ -> + let doc = print_expression_with_comments ~state expr cmt_tbl in + if Parens.braced_expr expr then add_parens doc else doc + in + add_braces + (Doc.concat + [ + leading; + inner_leading; + expr_doc; + inner_trailing; + trailing; + (if has_line_comment then Doc.break_parent else Doc.nil); + ]) in match children with | [] -> Doc.nil @@ -4819,15 +4871,13 @@ and print_jsx_children ~state (children : Parsetree.jsx_children) cmt_tbl = loc.loc_start.pos_lnum in let lines_between = start_line_y - end_line_x - 1 in - let leading_single_line_comments = - get_leading_line_comment_count cmt_tbl (get_loc y) + (* Comment containers add lines around their comments. Do not mistake + those generated lines for blank lines between children on reparse. *) + let has_between_comments = + has_leading_comments cmt_tbl (get_loc y) + || Hashtbl.mem cmt_tbl.trailing (get_loc x) in - (* If there are lines between the jsx elements, we preserve at least one line *) - if - (* Unless they are all comments *) - (* The edge case of comment followed by blank line is not caught here *) - lines_between > 0 && not (lines_between = leading_single_line_comments) - then + if lines_between > 0 && not has_between_comments then let doc = Doc.concat [print_expr x; sep; Doc.hard_line] in visit (Doc.concat [acc; doc]) rest else diff --git a/packages/dev-playground/src/Main.res b/packages/dev-playground/src/Main.res index 9a37f0ad1b..57670095bc 100644 --- a/packages/dev-playground/src/Main.res +++ b/packages/dev-playground/src/Main.res @@ -273,9 +273,7 @@ let outputNode = (output, activeTab, onSourceMapSelect): View.node => { event->Event.preventDefault onSourceMapSelect() }} - > - {View.text(sourceMapDirective)} - , + >{View.text(sourceMapDirective)}, View.text(output->String.slice(~start=directiveEnd)), ]) } @@ -297,9 +295,7 @@ let pushOutputText = (nodes: array, text, onSourceMapSelect) => { event->Event.preventDefault onSourceMapSelect() }} - > - {View.text(sourceMapDirective)} - , + >{View.text(sourceMapDirective)}, ) nodes->Array.push(View.text(text->String.slice(~start=directiveEnd))) } @@ -366,9 +362,7 @@ let mappedJavaScriptNode = ( onMappingSelect(mapping) } }} - > - {View.text(text)} - , + >{View.text(text)}, ) } | None => pushOutputText(nodes, text, onSourceMapSelect) @@ -429,9 +423,7 @@ module TabButton = { + >{View.text(tabLabel(tab))} } } @@ -439,9 +431,9 @@ module Problems = { @jsx.component let make = (~compileResult: Signal.t>) => {
-
{View.text("Problems")}
- 
-        {View.signalText(() =>
+      
{View.text("Problems")}

+      
{
+        View.signalText(() =>
           switch Signal.get(compileResult) {
           | Some({result: Ok({warnings})}) if warnings->Array.length > 0 =>
             warnings->Array.join("\n")
@@ -451,8 +443,8 @@ module Problems = {
           | Some({result: Error({message})}) => message
           | _ => "No problems reported."
           }
-        )}
-      

+        )
+      }
} } @@ -479,7 +471,7 @@ module SettingsPanel = { Computed.make(() => CompilerApi.selectableCompilerVersions( Signal.get(config).compilerVersion, - )->Array.map(version => ) + )->Array.map(version => ) ), ) @@ -488,9 +480,7 @@ module SettingsPanel = { Signal.get(activeTab) === Settings ? "settings-panel" : "settings-panel hidden-panel"} >
- + + >{View.signalFragment(compilerVersionOptions)}
- -
- {View.signalText(() => + +
{ + View.signalText(() => switch Signal.get(compilerInfo) { | Some(info) => `${info.version} / API ${info.apiVersion} / ${info.bundleId}` | None => "loading" } - )} -
+ ) + }
- + + ) + }
- + Signal.get(config).warnFlags} @@ -556,9 +544,7 @@ module SettingsPanel = { scheduleUrlSync() compileNow() }} - > - {View.text("Reset")} - + >{View.text("Reset")}
- +
- +
-
{View.text("Source Map")}
+
{View.text("Source Map")}
- + + ) + }
@@ -636,7 +622,7 @@ module SettingsPanel = { {View.text("Include sources content")}
- + Signal.get(config).sourceMapMode === Disabled} @@ -666,18 +652,18 @@ module SettingsPanel = { compileNow() }} /> - +
- -
- {View.signalText(() => + +
{ + View.signalText(() => switch Signal.get(compilerInfo) { | Some(info) => info.libraries->Array.join(", ") | None => "loading" } - )} -
+ ) + }
} @@ -693,14 +679,14 @@ module StatusBadge = { | Compiling | Loading => "status status-busy" | Ready => "status" }} - > - {View.signalText(() => + >{ + View.signalText(() => switch Signal.get(status) { | Failed(message) => message | other => statusLabel(other) } - )} - + ) + } } } @@ -1257,7 +1243,7 @@ module App = {
-

{View.text("ReScript Developer Playground")}

+

{View.text("ReScript Developer Playground")}

@@ -1268,11 +1254,11 @@ module App = { >
-

{View.text("Source")}

+

{View.text("Source")}

- + - + >{View.text("Reset")} +
editorShellStyle(Signal.get(editorScrollTop))} >
- 
-              {View.signalFragment(highlightedSource)}
-
+ 
{View.signalFragment(highlightedSource)}