Emit leading comment before brace token

Kanchalai Tanglertsampan · Kanchalai Tanglertsampan · commit 80eae169ee2d · 2017-02-07T11:56:44.000-08:00
diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts
@@ -1347,6 +1347,8 @@ namespace ts {
             else {
                 writeToken(SyntaxKind.OpenBraceToken, node.pos, /*contextNode*/ node);
                 emitBlockStatements(node);
+                // We have to call emitLeadingComments explicitly here because otherwise leading comments of the close brace token will not be emitted
+                emitLeadingComments(node.statements.end, /*isEmittedNode*/true);
                 writeToken(SyntaxKind.CloseBraceToken, node.statements.end, /*contextNode*/ node);
             }
         }
diff --git a/tests/baselines/reference/amdImportAsPrimaryExpression.js b/tests/baselines/reference/amdImportAsPrimaryExpression.js
@@ -26,5 +26,6 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "./foo_0"], function (require, exports, foo) {
     "use strict";
     if (foo.E1.A === 0) {
+    // Should cause runtime import - interesting optimization possibility, as gets inlined to 0.
     }
 });
diff --git a/tests/baselines/reference/argsInScope.js b/tests/baselines/reference/argsInScope.js
@@ -17,6 +17,7 @@ var C = (function () {
     }
     C.prototype.P = function (ii, j, k) {
         for (var i = 0; i < arguments.length; i++) {
+        // WScript.Echo("param: " + arguments[i]);
         }
     };
     return C;
diff --git a/tests/baselines/reference/commentLeadingCloseBrace.js b/tests/baselines/reference/commentLeadingCloseBrace.js
@@ -0,0 +1,24 @@
+//// [commentLeadingCloseBrace.ts]
+declare function commentedParameters(...args): any;
+
+function ifelse() {
+    if (commentedParameters(1, 2)) {
+        /*comment1*/
+        commentedParameters(3, 4);
+        /*comment2*/
+    } else {
+        commentedParameters(5, 6);
+    }
+}
+
+//// [commentLeadingCloseBrace.js]
+function ifelse() {
+    if (commentedParameters(1, 2)) {
+        /*comment1*/
+        commentedParameters(3, 4);
+    /*comment2*/
+    }
+    else {
+        commentedParameters(5, 6);
+    }
+}
diff --git a/tests/baselines/reference/commentLeadingCloseBrace.symbols b/tests/baselines/reference/commentLeadingCloseBrace.symbols
@@ -0,0 +1,21 @@
+=== tests/cases/compiler/commentLeadingCloseBrace.ts ===
+declare function commentedParameters(...args): any;
+>commentedParameters : Symbol(commentedParameters, Decl(commentLeadingCloseBrace.ts, 0, 0))
+>args : Symbol(args, Decl(commentLeadingCloseBrace.ts, 0, 37))
+
+function ifelse() {
+>ifelse : Symbol(ifelse, Decl(commentLeadingCloseBrace.ts, 0, 51))
+
+    if (commentedParameters(1, 2)) {
+>commentedParameters : Symbol(commentedParameters, Decl(commentLeadingCloseBrace.ts, 0, 0))
+
+        /*comment1*/
+        commentedParameters(3, 4);
+>commentedParameters : Symbol(commentedParameters, Decl(commentLeadingCloseBrace.ts, 0, 0))
+
+        /*comment2*/
+    } else {
+        commentedParameters(5, 6);
+>commentedParameters : Symbol(commentedParameters, Decl(commentLeadingCloseBrace.ts, 0, 0))
+    }
+}
diff --git a/tests/baselines/reference/commentLeadingCloseBrace.types b/tests/baselines/reference/commentLeadingCloseBrace.types
@@ -0,0 +1,30 @@
+=== tests/cases/compiler/commentLeadingCloseBrace.ts ===
+declare function commentedParameters(...args): any;
+>commentedParameters : (...args: any[]) => any
+>args : any[]
+
+function ifelse() {
+>ifelse : () => void
+
+    if (commentedParameters(1, 2)) {
+>commentedParameters(1, 2) : any
+>commentedParameters : (...args: any[]) => any
+>1 : 1
+>2 : 2
+
+        /*comment1*/
+        commentedParameters(3, 4);
+>commentedParameters(3, 4) : any
+>commentedParameters : (...args: any[]) => any
+>3 : 3
+>4 : 4
+
+        /*comment2*/
+    } else {
+        commentedParameters(5, 6);
+>commentedParameters(5, 6) : any
+>commentedParameters : (...args: any[]) => any
+>5 : 5
+>6 : 6
+    }
+}
diff --git a/tests/baselines/reference/commonJSImportAsPrimaryExpression.js b/tests/baselines/reference/commonJSImportAsPrimaryExpression.js
@@ -27,4 +27,5 @@ exports.C1 = C1;
 "use strict";
 var foo = require("./foo_0");
 if (foo.C1.s1) {
+// Should cause runtime import
 }
diff --git a/tests/baselines/reference/duplicateLocalVariable1.js b/tests/baselines/reference/duplicateLocalVariable1.js
@@ -391,6 +391,7 @@ var TestRunner = (function () {
                     }
                 }
                 if (testResult === false) {
+                //console.log(e.message);
                 }
             }
             if ((testcase.errorMessageRegEx !== undefined) && !exception) {
diff --git a/tests/baselines/reference/jsFileCompilationLetBeingRenamed.js b/tests/baselines/reference/jsFileCompilationLetBeingRenamed.js
@@ -9,5 +9,6 @@ function foo(a) {
 //// [out.js]
 function foo(a) {
     for (var a_1 = 0; a_1 < 10; a_1++) {
+    // do something
     }
 }
diff --git a/tests/baselines/reference/narrowExceptionVariableInCatchClause.js b/tests/baselines/reference/narrowExceptionVariableInCatchClause.js
@@ -27,6 +27,7 @@ function tryCatch() {
 //// [narrowExceptionVariableInCatchClause.js]
 function tryCatch() {
     try {
+    // do stuff...
     }
     catch (err) {
         if (isFooError(err)) {
diff --git a/tests/baselines/reference/noCatchBlock.js b/tests/baselines/reference/noCatchBlock.js
diff --git a/tests/baselines/reference/noCatchBlock.js.map b/tests/baselines/reference/noCatchBlock.js.map
diff --git a/tests/baselines/reference/noCatchBlock.sourcemap.txt b/tests/baselines/reference/noCatchBlock.sourcemap.txt
@@ -12,6 +12,7 @@ sourceFile:noCatchBlock.ts
 1 >
 2 >^^^^
 3 >    ^
+4 >     ^^->
 1 >
   >
 2 >try 
@@ -20,34 +21,51 @@ sourceFile:noCatchBlock.ts
 2 >Emitted(1, 5) Source(2, 5) + SourceIndex(0)
 3 >Emitted(1, 6) Source(2, 6) + SourceIndex(0)
 ---
+>>>// ...
+1->
+2 >^^^^^^
+1->
+  > 
+2 >// ...
+1->Emitted(2, 1) Source(3, 2) + SourceIndex(0)
+2 >Emitted(2, 7) Source(3, 8) + SourceIndex(0)
+---
 >>>}
 1 >
 2 >^
 3 > ^^^^^^^^^->
 1 >
-  > // ...
   >
 2 >}
-1 >Emitted(2, 1) Source(4, 1) + SourceIndex(0)
-2 >Emitted(2, 2) Source(4, 2) + SourceIndex(0)
+1 >Emitted(3, 1) Source(4, 1) + SourceIndex(0)
+2 >Emitted(3, 2) Source(4, 2) + SourceIndex(0)
 ---
 >>>finally {
 1->^^^^^^^^
 2 >        ^
+3 >         ^^^^^^^^^^^^^^^^->
 1-> finally 
 2 >        {
-1->Emitted(3, 9) Source(4, 11) + SourceIndex(0)
-2 >Emitted(3, 10) Source(4, 12) + SourceIndex(0)
+1->Emitted(4, 9) Source(4, 11) + SourceIndex(0)
+2 >Emitted(4, 10) Source(4, 12) + SourceIndex(0)
+---
+>>>// N.B. No 'catch' block
+1->
+2 >^^^^^^^^^^^^^^^^^^^^^^^^
+1->
+  > 
+2 >// N.B. No 'catch' block
+1->Emitted(5, 1) Source(5, 2) + SourceIndex(0)
+2 >Emitted(5, 25) Source(5, 26) + SourceIndex(0)
 ---
 >>>}
 1 >
 2 >^
 3 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^->
 1 >
-  > // N.B. No 'catch' block
   >
 2 >}
-1 >Emitted(4, 1) Source(6, 1) + SourceIndex(0)
-2 >Emitted(4, 2) Source(6, 2) + SourceIndex(0)
+1 >Emitted(6, 1) Source(6, 1) + SourceIndex(0)
+2 >Emitted(6, 2) Source(6, 2) + SourceIndex(0)
 ---
 >>>//# sourceMappingURL=noCatchBlock.js.map
diff --git a/tests/baselines/reference/objectRestForOf.js b/tests/baselines/reference/objectRestForOf.js
@@ -37,4 +37,5 @@ for (let _b of array) {
 }
 for (const norest of array.map(a => (Object.assign({}, a, { x: 'a string' })))) {
     [norest.x, norest.y];
+// x is now a string. who knows why.
 }
diff --git a/tests/baselines/reference/parserRealSource14.js b/tests/baselines/reference/parserRealSource14.js
@@ -1011,6 +1011,7 @@ var TypeScript;
                         ctx.path.push(cur);
                     }
                     else {
+                    //logger.log("TODO: Ignoring node because minChar, limChar not better than previous node in stack");
                     }
                 }
                 // The AST walker skips comments, but we might be in one, so check the pre/post comments for this node manually
diff --git a/tests/baselines/reference/recursiveReturns.js b/tests/baselines/reference/recursiveReturns.js
@@ -23,6 +23,7 @@ function R1() {
 function R2() { R2(); }
 function R3(n) {
     if (n == 0) {
+    //return;
     }
     else {
         R3(n--);
diff --git a/tests/baselines/reference/sourceMap-SkippedNode.js b/tests/baselines/reference/sourceMap-SkippedNode.js
diff --git a/tests/baselines/reference/sourceMap-SkippedNode.js.map b/tests/baselines/reference/sourceMap-SkippedNode.js.map
diff --git a/tests/baselines/reference/sourceMap-SkippedNode.sourcemap.txt b/tests/baselines/reference/sourceMap-SkippedNode.sourcemap.txt
@@ -12,41 +12,59 @@ sourceFile:sourceMap-SkippedNode.ts
 1 >
 2 >^^^^
 3 >    ^
+4 >     ^^->
 1 >
 2 >try 
 3 >    {
 1 >Emitted(1, 1) Source(1, 1) + SourceIndex(0)
 2 >Emitted(1, 5) Source(1, 5) + SourceIndex(0)
 3 >Emitted(1, 6) Source(1, 6) + SourceIndex(0)
 ---
+>>>// ...
+1->
+2 >^^^^^^
+1->
+  >
+2 >// ...
+1->Emitted(2, 1) Source(2, 1) + SourceIndex(0)
+2 >Emitted(2, 7) Source(2, 7) + SourceIndex(0)
+---
 >>>}
 1 >
 2 >^
 3 > ^^^^^^^^^->
 1 >
-  >// ...
   >
 2 >}
-1 >Emitted(2, 1) Source(3, 1) + SourceIndex(0)
-2 >Emitted(2, 2) Source(3, 2) + SourceIndex(0)
+1 >Emitted(3, 1) Source(3, 1) + SourceIndex(0)
+2 >Emitted(3, 2) Source(3, 2) + SourceIndex(0)
 ---
 >>>finally {
 1->^^^^^^^^
 2 >        ^
+3 >         ^^^^^^^^^^^^^^^^->
 1-> finally 
 2 >        {
-1->Emitted(3, 9) Source(3, 11) + SourceIndex(0)
-2 >Emitted(3, 10) Source(3, 12) + SourceIndex(0)
+1->Emitted(4, 9) Source(3, 11) + SourceIndex(0)
+2 >Emitted(4, 10) Source(3, 12) + SourceIndex(0)
+---
+>>>// N.B. No 'catch' block
+1->
+2 >^^^^^^^^^^^^^^^^^^^^^^^^
+1->
+  >
+2 >// N.B. No 'catch' block
+1->Emitted(5, 1) Source(4, 1) + SourceIndex(0)
+2 >Emitted(5, 25) Source(4, 25) + SourceIndex(0)
 ---
 >>>}
 1 >
 2 >^
 3 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^->
 1 >
-  >// N.B. No 'catch' block
   >
 2 >}
-1 >Emitted(4, 1) Source(5, 1) + SourceIndex(0)
-2 >Emitted(4, 2) Source(5, 2) + SourceIndex(0)
+1 >Emitted(6, 1) Source(5, 1) + SourceIndex(0)
+2 >Emitted(6, 2) Source(5, 2) + SourceIndex(0)
 ---
 >>>//# sourceMappingURL=sourceMap-SkippedNode.js.map
diff --git a/tests/cases/compiler/commentLeadingCloseBrace.ts b/tests/cases/compiler/commentLeadingCloseBrace.ts
@@ -0,0 +1,11 @@
+﻿declare function commentedParameters(...args): any;
+
+function ifelse() {
+    if (commentedParameters(1, 2)) {
+        /*comment1*/
+        commentedParameters(3, 4);
+        /*comment2*/
+    } else {
+        commentedParameters(5, 6);
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -1347,6 +1347,8 @@ namespace ts {`
`1347`	`1347`	`else {`
`1348`	`1348`	`writeToken(SyntaxKind.OpenBraceToken, node.pos, /contextNode/ node);`
`1349`	`1349`	`emitBlockStatements(node);`
	`1350`	`+ // We have to call emitLeadingComments explicitly here because otherwise leading comments of the close brace token will not be emitted`
	`1351`	`+ emitLeadingComments(node.statements.end, /isEmittedNode/true);`
`1350`	`1352`	`writeToken(SyntaxKind.CloseBraceToken, node.statements.end, /contextNode/ node);`
`1351`	`1353`	`}`
`1352`	`1354`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,5 +26,6 @@ define(["require", "exports"], function (require, exports) {`
`26`	`26`	`define(["require", "exports", "./foo_0"], function (require, exports, foo) {`
`27`	`27`	`"use strict";`
`28`	`28`	`if (foo.E1.A === 0) {`
	`29`	`+ // Should cause runtime import - interesting optimization possibility, as gets inlined to 0.`
`29`	`30`	`}`
`30`	`31`	`});`
Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@ var C = (function () {`
`17`	`17`	`}`
`18`	`18`	`C.prototype.P = function (ii, j, k) {`
`19`	`19`	`for (var i = 0; i < arguments.length; i++) {`
	`20`	`+ // WScript.Echo("param: " + arguments[i]);`
`20`	`21`	`}`
`21`	`22`	`};`
`22`	`23`	`return C;`
Original file line number	Diff line number	Diff line change
`@@ -27,4 +27,5 @@ exports.C1 = C1;`
`27`	`27`	`"use strict";`
`28`	`28`	`var foo = require("./foo_0");`
`29`	`29`	`if (foo.C1.s1) {`
	`30`	`+// Should cause runtime import`
`30`	`31`	`}`
Original file line number	Diff line number	Diff line change
`@@ -391,6 +391,7 @@ var TestRunner = (function () {`
`391`	`391`	`}`
`392`	`392`	`}`
`393`	`393`	`if (testResult === false) {`
	`394`	`+ //console.log(e.message);`
`394`	`395`	`}`
`395`	`396`	`}`
`396`	`397`	`if ((testcase.errorMessageRegEx !== undefined) && !exception) {`
Original file line number	Diff line number	Diff line change
`@@ -9,5 +9,6 @@ function foo(a) {`
`9`	`9`	`//// [out.js]`
`10`	`10`	`function foo(a) {`
`11`	`11`	`for (var a_1 = 0; a_1 < 10; a_1++) {`
	`12`	`+ // do something`
`12`	`13`	`}`
`13`	`14`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,7 @@ function tryCatch() {`
`27`	`27`	`//// [narrowExceptionVariableInCatchClause.js]`
`28`	`28`	`function tryCatch() {`
`29`	`29`	`try {`
	`30`	`+ // do stuff...`
`30`	`31`	`}`
`31`	`32`	`catch (err) {`
`32`	`33`	`if (isFooError(err)) {`