fix: Avoid tail comments inside tag from being eaten (#664)

Author: ah-yu

CHANGELOG.md

@@ -31,6 +31,8 @@
 - Fix location issue in error messages with JSX V4 where the multiple props types are defined https://github.com/rescript-lang/syntax/pull/655
 - Fix location issue in make function in JSX V4 that breaks dead code elimination https://github.com/rescript-lang/syntax/pull/660
 - Fix parsing (hence pretty printing) of expressions with underscore `_` and comments.
+- Fix printing of comments inside JSX tag https://github.com/rescript-lang/syntax/pull/664
+- Fix issue where formatter erases tail comments inside JSX tag https://github.com/rescript-lang/syntax/issues/663
 
 ## ReScript 10.0
 

src/res_comments_table.ml

@@ -1,5 +1,6 @@
 module Comment = Res_comment
 module Doc = Res_doc
+module ParsetreeViewer = Res_parsetree_viewer
 
 type t = {
   leading: (Location.t, Comment.t list) Hashtbl.t;
@@ -1310,9 +1311,43 @@ and walkExpression expr t comments =
         walkExpression callExpr t inside;
         after)
     in
-    let afterExpr, rest = partitionAdjacentTrailing callExpr.pexp_loc after in
-    attach t.trailing callExpr.pexp_loc afterExpr;
-    walkList (arguments |> List.map (fun (_, e) -> ExprArgument e)) t rest
+    if ParsetreeViewer.isJsxExpression expr then (
+      let props =
+        arguments
+        |> List.filter (fun (label, _) ->
+               match label with
+               | Asttypes.Labelled "children" -> false
+               | Asttypes.Nolabel -> false
+               | _ -> true)
+      in
+      let maybeChildren =
+        arguments
+        |> List.find_opt (fun (label, _) ->
+               label = Asttypes.Labelled "children")
+      in
+      match maybeChildren with
+      (* There is no need to deal with this situation as the children cannot be NONE *)
+      | None -> ()
+      | Some (_, children) ->
+        let leading, inside, _ = partitionByLoc after children.pexp_loc in
+        if props = [] then
+          (* All comments inside a tag are trailing comments of the tag if there are no props
+             <A
+             // comment
+             // comment
+             />
+          *)
+          let afterExpr, _ =
+            partitionAdjacentTrailing callExpr.pexp_loc after
+          in
+          attach t.trailing callExpr.pexp_loc afterExpr
+        else
+          walkList (props |> List.map (fun (_, e) -> ExprArgument e)) t leading;
+        walkExpression children t inside)
+    else
+      let afterExpr, rest = partitionAdjacentTrailing callExpr.pexp_loc after in
+      attach t.trailing callExpr.pexp_loc afterExpr;
+      walkList (arguments |> List.map (fun (_, e) -> ExprArgument e)) t rest
   | Pexp_fun (_, _, _, _) | Pexp_newtype _ -> (
     let _, parameters, returnExpr = funExpr expr in
     let comments =

src/res_printer.ml

@@ -4039,20 +4039,15 @@ and printJsxExpression ~customLayout lident args cmtTbl =
                     {
                       Parsetree.pexp_desc =
                         Pexp_construct ({txt = Longident.Lident "[]"}, None);
-                      pexp_loc = loc;
-                    } ->
-                  if isSelfClosing then
-                    Doc.concat
-                      [Doc.line; printComments (Doc.text "/>") cmtTbl loc]
-                  else
-                    Doc.concat [Doc.softLine; printComments Doc.nil cmtTbl loc]
-                | _ -> Doc.nil);
+                    }
+                  when isSelfClosing ->
+                  Doc.concat [Doc.line; Doc.text "/>"]
+                | _ -> Doc.concat [Doc.softLine; Doc.greaterThan]);
               ]);
          (if isSelfClosing then Doc.nil
          else
            Doc.concat
              [
-               Doc.greaterThan;
                (if hasChildren then printChildren children
                else
                  match children with

tests/conversion/reason/expected/bracedJsx.res.txt

@@ -111,7 +111,8 @@ let make = () => {
     <div
       className=Styles.terminal
       onClick={event => (event->ReactEvent.Mouse.target)["querySelector"]("input")["focus"]()}
-      ref={containerRef->ReactDOMRe.Ref.domRef}>
+      ref={containerRef->ReactDOMRe.Ref.domRef}
+    >
       {state.history
       ->Array.mapWithIndex((index, item) =>
         <div key={j`$index`} className=Styles.line>

tests/printer/comments/expected/jsx.res.txt

@@ -9,7 +9,7 @@ module Cite = {
 
 <A
   value=""
-// Comment
+  // Comment
 />
 
 <A /* comment */ />
@@ -18,6 +18,19 @@ module Cite = {
   // Comment
 </A>
 
+<A
+// comment1
+// comment 2
+/>
+
+<A
+  // comment1
+  value=""
+  // comment2
+>
+  <B /* comment3 */ />
+</A>
+
 <div>
   // Must not jump inside braces
   {React.string("Hello, World!")}

tests/printer/comments/jsx.res

@@ -20,6 +20,19 @@ module Cite = {
   // Comment
 </A>
 
+<A
+// comment1
+// comment 2
+/>
+
+<A
+// comment1
+value=""
+// comment2
+>
+  <B /* comment3 */ />
+</A>
+
 <div>
   // Must not jump inside braces
   {React.string("Hello, World!")}

tests/printer/expr/expected/jsx.res.txt

@@ -104,7 +104,8 @@ let avatarSection =
           onMouseLeave={_ => setHoveringAdmin(false)}
           onClick={_e => {
             stopImpersonating(csrfToken)
-          }}>
+          }}
+        >
           <Avatar user={viewer} size={45} />
         </div>
       : React.nullElement}

tests/printer/other/expected/signaturePicker.res.txt

@@ -28,7 +28,8 @@ let make = () => {
     <div className="pr-2 font-bold text-gray-400 text-lg"> {"Signature"->string} </div>
     <select
       id="country"
-      className="transition duration-150 ease-in-out sm:text-sm sm:leading-5 border-none font-bold text-2xl text-gray-600 bg-transparent">
+      className="transition duration-150 ease-in-out sm:text-sm sm:leading-5 border-none font-bold text-2xl text-gray-600 bg-transparent"
+    >
       {options
       ->Belt.List.map(option =>
         <option key={option->TimeSignature.toString}>
@@ -47,7 +48,8 @@ let make = () => {
       fill="none"
       strokeLinecap="round"
       strokeLinejoin="round"
-      className="text-gray-400">
+      className="text-gray-400"
+    >
       <polyline points="6 9 12 15 18 9" />
     </svg>
   </label>