Even better best effort comment preservation in JSX comments

microsoft · Dec 8, 2020 · d1db247 · d1db247
1 parent b35a401
commit d1db247
Show file tree

Hide file tree

Showing 36 changed files with 520 additions and 272 deletions.
diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts
@@ -2824,8 +2824,8 @@ namespace ts {
             }
             pos = writeTokenText(token, writer, pos);
             if (isSimilarNode && contextNode.end !== pos) {
-                const isJsxExprContex = contextNode.kind === SyntaxKind.JsxExpression;
-                emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ !isJsxExprContex, /*forceNoNewline*/ isJsxExprContex);
+                const isJsxExprContext = contextNode.kind === SyntaxKind.JsxExpression;
+                emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ !isJsxExprContext, /*forceNoNewline*/ isJsxExprContext);
             }
             return pos;
         }
@@ -3378,12 +3378,35 @@ namespace ts {
             writePunctuation("}");
         }
 
+        function hasTrailingCommentsAtPosition(pos: number) {
+            let result = false;
+            forEachTrailingCommentRange(currentSourceFile?.text || "", pos + 1, () => result = true);
+            return result;
+        }
+
+        function hasLeadingCommentsAtPosition(pos: number) {
+            let result = false;
+            forEachLeadingCommentRange(currentSourceFile?.text || "", pos + 1, () => result = true);
+            return result;
+        }
+
+        function hasCommentsAtPosition(pos: number) {
+            return hasTrailingCommentsAtPosition(pos) || hasLeadingCommentsAtPosition(pos);
+        }
+
         function emitJsxExpression(node: JsxExpression) {
-            if (node.expression || (!commentsDisabled && !nodeIsSynthesized(node) && getTextOfNode(node).length > 2 && getTextOfNode(node).indexOf("/*") > -1)) { // preserve empty expressions if they contain comments!
-                emitTokenWithComment(SyntaxKind.OpenBraceToken, node.pos, writePunctuation, node);
+            if (node.expression || (!commentsDisabled && !nodeIsSynthesized(node) && hasCommentsAtPosition(node.pos))) { // preserve empty expressions if they contain comments!
+                const isMultiline = currentSourceFile && !nodeIsSynthesized(node) && getLineAndCharacterOfPosition(currentSourceFile, node.pos).line !== getLineAndCharacterOfPosition(currentSourceFile, node.end).line;
+                if (isMultiline) {
+                    writer.increaseIndent();
+                }
+                const end = emitTokenWithComment(SyntaxKind.OpenBraceToken, node.pos, writePunctuation, node);
                 emit(node.dotDotDotToken);
                 emitExpression(node.expression);
-                writePunctuation("}");
+                emitTokenWithComment(SyntaxKind.CloseBraceToken, node.expression?.end || end, writePunctuation, node);
+                if (isMultiline) {
+                    writer.decreaseIndent();
+                }
             }
         }
 
@@ -5240,16 +5263,24 @@ namespace ts {
             exitComment();
         }
 
-        function emitTrailingCommentOfPositionNoNewline(commentPos: number, commentEnd: number) {
+        function emitTrailingCommentOfPositionNoNewline(commentPos: number, commentEnd: number, kind: SyntaxKind) {
             // trailing comments of a position are emitted at /*trailing comment1 */space/*trailing comment*/space
 
             emitPos(commentPos);
             writeCommentRange(currentSourceFile!.text, getCurrentLineMap(), writer, commentPos, commentEnd, newLine);
             emitPos(commentEnd);
+
+            if (kind === SyntaxKind.SingleLineCommentTrivia) {
+                writer.writeLine(); // still write a newline for single-line comments, so closing tokens aren't written on the same line
+            }
         }
 
         function emitTrailingCommentOfPosition(commentPos: number, commentEnd: number, _kind: SyntaxKind, hasTrailingNewLine: boolean) {
-            emitTrailingCommentOfPositionNoNewline(commentPos, commentEnd);
+            // trailing comments of a position are emitted at /*trailing comment1 */space/*trailing comment*/space
+
+            emitPos(commentPos);
+            writeCommentRange(currentSourceFile!.text, getCurrentLineMap(), writer, commentPos, commentEnd, newLine);
+            emitPos(commentEnd);
 
             if (hasTrailingNewLine) {
                 writer.writeLine();

diff --git a/tests/baselines/reference/commentEmittingInPreserveJsx1.js b/tests/baselines/reference/commentEmittingInPreserveJsx1.js
@@ -39,19 +39,21 @@ var React = require("react");
 </div>;
 <div>
     // Not Comment
-
+    {
+    //Comment just Fine
+    }
     // Another not Comment
 </div>;
 <div>
     // Not Comment
     {
-//Comment just Fine
-"Hi"}
+    //Comment just Fine
+    "Hi"}
     // Another not Comment
 </div>;
 <div>
     /* Not Comment */
     {
-//Comment just Fine
-"Hi"}
+    //Comment just Fine
+    "Hi"}
 </div>;
diff --git a/...baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).js b/...baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).js
@@ -1,14 +1,21 @@
 //// [commentsOnJSXExpressionsArePreserved.tsx]
-// file is interntionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
-namespace JSX {
-    interface IntrsinsicElements { [index: string]: any }
-    type JSXElement = any;
-}
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
 class Component {
     render() {
         return <div>
             {/* missing */}
             {null/* preserved */}
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
         </div>;
     }
 }
@@ -21,6 +28,16 @@ var Component = /** @class */ (function () {
         return <div>
             {/* missing */}
             {null /* preserved */}
+            {
+            // ??? 1
+            }
+            {// ??? 2
+            }
+            {// ??? 3
+            }
+            {
+            // ??? 4
+            /* ??? 5 */ }
         </div>;
     };
     return Component;

diff --git a/...ines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).symbols b/...ines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).symbols
@@ -1,24 +1,27 @@
 === tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
-// file is interntionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
-namespace JSX {
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
 >JSX : Symbol(JSX, Decl(commentsOnJSXExpressionsArePreserved.tsx, 0, 0))
 
-    interface IntrsinsicElements { [index: string]: any }
->IntrsinsicElements : Symbol(IntrsinsicElements, Decl(commentsOnJSXExpressionsArePreserved.tsx, 1, 15))
->index : Symbol(index, Decl(commentsOnJSXExpressionsArePreserved.tsx, 2, 36))
-
-    type JSXElement = any;
->JSXElement : Symbol(JSXElement, Decl(commentsOnJSXExpressionsArePreserved.tsx, 2, 57))
-}
 class Component {
->Component : Symbol(Component, Decl(commentsOnJSXExpressionsArePreserved.tsx, 4, 1))
+>Component : Symbol(Component, Decl(commentsOnJSXExpressionsArePreserved.tsx, 1, 16))
 
     render() {
->render : Symbol(Component.render, Decl(commentsOnJSXExpressionsArePreserved.tsx, 5, 17))
+>render : Symbol(Component.render, Decl(commentsOnJSXExpressionsArePreserved.tsx, 2, 17))
 
         return <div>
             {/* missing */}
             {null/* preserved */}
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
         </div>;
     }
 }
diff --git a/...elines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).types b/...elines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=commonjs).types
@@ -1,26 +1,29 @@
 === tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
-// file is interntionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
-namespace JSX {
-    interface IntrsinsicElements { [index: string]: any }
->index : string
-
-    type JSXElement = any;
->JSXElement : any
-}
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
 class Component {
 >Component : Component
 
     render() {
 >render : () => any
 
         return <div>
-><div>            {/* missing */}            {null/* preserved */}        </div> : error
+><div>            {/* missing */}            {null/* preserved */}            {                // ??? 1            }            { // ??? 2            }            {// ??? 3            }            {                // ??? 4            /* ??? 5 */}        </div> : error
 >div : any
 
             {/* missing */}
             {null/* preserved */}
 >null : null
-
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
         </div>;
 >div : any
     }

diff --git a/...s/baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).js b/...s/baselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).js
@@ -1,14 +1,21 @@
 //// [commentsOnJSXExpressionsArePreserved.tsx]
-// file is interntionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
-namespace JSX {
-    interface IntrsinsicElements { [index: string]: any }
-    type JSXElement = any;
-}
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
 class Component {
     render() {
         return <div>
             {/* missing */}
             {null/* preserved */}
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
         </div>;
     }
 }
@@ -21,6 +28,16 @@ var Component = /** @class */ (function () {
         return <div>
             {/* missing */}
             {null /* preserved */}
+            {
+            // ??? 1
+            }
+            {// ??? 2
+            }
+            {// ??? 3
+            }
+            {
+            // ??? 4
+            /* ??? 5 */ }
         </div>;
     };
     return Component;

diff --git a/...elines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).symbols b/...elines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).symbols
@@ -1,24 +1,27 @@
 === tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
-// file is interntionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
-namespace JSX {
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
 >JSX : Symbol(JSX, Decl(commentsOnJSXExpressionsArePreserved.tsx, 0, 0))
 
-    interface IntrsinsicElements { [index: string]: any }
->IntrsinsicElements : Symbol(IntrsinsicElements, Decl(commentsOnJSXExpressionsArePreserved.tsx, 1, 15))
->index : Symbol(index, Decl(commentsOnJSXExpressionsArePreserved.tsx, 2, 36))
-
-    type JSXElement = any;
->JSXElement : Symbol(JSXElement, Decl(commentsOnJSXExpressionsArePreserved.tsx, 2, 57))
-}
 class Component {
->Component : Symbol(Component, Decl(commentsOnJSXExpressionsArePreserved.tsx, 4, 1))
+>Component : Symbol(Component, Decl(commentsOnJSXExpressionsArePreserved.tsx, 1, 16))
 
     render() {
->render : Symbol(Component.render, Decl(commentsOnJSXExpressionsArePreserved.tsx, 5, 17))
+>render : Symbol(Component.render, Decl(commentsOnJSXExpressionsArePreserved.tsx, 2, 17))
 
         return <div>
             {/* missing */}
             {null/* preserved */}
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
         </div>;
     }
 }
diff --git a/...aselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).types b/...aselines/reference/commentsOnJSXExpressionsArePreserved(jsx=preserve,module=system).types
@@ -1,26 +1,29 @@
 === tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx ===
-// file is interntionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
-namespace JSX {
-    interface IntrsinsicElements { [index: string]: any }
->index : string
-
-    type JSXElement = any;
->JSXElement : any
-}
+// file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+namespace JSX {}
 class Component {
 >Component : Component
 
     render() {
 >render : () => any
 
         return <div>
-><div>            {/* missing */}            {null/* preserved */}        </div> : error
+><div>            {/* missing */}            {null/* preserved */}            {                // ??? 1            }            { // ??? 2            }            {// ??? 3            }            {                // ??? 4            /* ??? 5 */}        </div> : error
 >div : any
 
             {/* missing */}
             {null/* preserved */}
 >null : null
-
+            {
+                // ??? 1
+            }
+            { // ??? 2
+            }
+            {// ??? 3
+            }
+            {
+                // ??? 4
+            /* ??? 5 */}
         </div>;
 >div : any
     }

diff --git a/...ines/reference/commentsOnJSXExpressionsArePreserved(jsx=react,module=commonjs).errors.txt b/...ines/reference/commentsOnJSXExpressionsArePreserved(jsx=react,module=commonjs).errors.txt
@@ -1,19 +1,26 @@
-tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx(8,17): error TS2304: Cannot find name 'React'.
+tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx(5,17): error TS2304: Cannot find name 'React'.
 
 
 ==== tests/cases/compiler/commentsOnJSXExpressionsArePreserved.tsx (1 errors) ====
-    // file is interntionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
-    namespace JSX {
-        interface IntrsinsicElements { [index: string]: any }
-        type JSXElement = any;
-    }
+    // file is intentionally not a module - this tests for a crash in the module/system transforms alongside the `react-jsx` and `react-jsxdev` outputs
+    namespace JSX {}
     class Component {
         render() {
             return <div>
                     ~~~
 !!! error TS2304: Cannot find name 'React'.
                 {/* missing */}
                 {null/* preserved */}
+                {
+                    // ??? 1
+                }
+                { // ??? 2
+                }
+                {// ??? 3
+                }
+                {
+                    // ??? 4
+                /* ??? 5 */}
             </div>;
         }
     }