Merge pull request microsoft#509 from Microsoft/emitLeadingCommentsForCurly

sheetalkamat · sheetalkamat · commit e6cd3e15b027 · 2014-08-22T16:01:49.000-07:00
Emit leading comments for '}' of function/constructor block
diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts
@@ -330,6 +330,8 @@ module ts {
             /** Emit Trailing comments of the node */
             var emitTrailingComments = compilerOptions.removeComments ? (node: Node) => { } : emitTrailingDeclarationComments;
 
+            var emitLeadingCommentsOfPosition = compilerOptions.removeComments ? (pos: number) => { } : emitLeadingCommentsOfLocalPosition;
+
             var detachedCommentsInfo: { nodePos: number; detachedCommentEndPos: number }[];
             /** Emit detached comments of the node */
             var emitDetachedComments = compilerOptions.removeComments ? (node: TextRange) => { } : emitDetachedCommentsAtPosition;
@@ -1390,12 +1392,14 @@ module ts {
                         write(";");
                         emitTrailingComments(node.body);
                     }
-                    decreaseIndent();
                     writeLine();
                     if (node.body.kind === SyntaxKind.FunctionBlock) {
+                        emitLeadingCommentsOfPosition((<Block>node.body).statements.end);
+                        decreaseIndent();
                         emitToken(SyntaxKind.CloseBraceToken, (<Block>node.body).statements.end);
                     }
                     else {
+                        decreaseIndent();
                         emitStart(node.body);
                         write("}");
                         emitEnd(node.body);
@@ -1648,8 +1652,11 @@ module ts {
                         if (superCall) statements = statements.slice(1);
                         emitLines(statements);
                     }
-                    decreaseIndent();
                     writeLine();
+                    if (ctor) {
+                        emitLeadingCommentsOfPosition((<Block>ctor.body).statements.end);
+                    }
+                    decreaseIndent();
                     emitToken(SyntaxKind.CloseBraceToken, ctor ? (<Block>ctor.body).statements.end : node.members.end);
                     scopeEmitEnd();
                     emitEnd(<Node>ctor || node);
@@ -2077,23 +2084,34 @@ module ts {
                 }
             }
 
+            function hasDetachedComments(pos: number) {
+                return detachedCommentsInfo !== undefined && detachedCommentsInfo[detachedCommentsInfo.length - 1].nodePos === pos;
+            }
+
+            function getLeadingCommentsWithoutDetachedComments() {
+                // get the leading comments from detachedPos
+                var leadingComments = getLeadingComments(currentSourceFile.text, detachedCommentsInfo[detachedCommentsInfo.length - 1].detachedCommentEndPos);
+                if (detachedCommentsInfo.length - 1) {
+                    detachedCommentsInfo.pop();
+                }
+                else {
+                    detachedCommentsInfo = undefined;
+                }
+
+                return leadingComments;
+            }
+
             function emitLeadingDeclarationComments(node: Node) {
                 // Emit the leading comments only if the parent's pos doesnt match because parent should take care of emitting these comments
                 if (node.parent.kind === SyntaxKind.SourceFile || node.pos !== node.parent.pos) {
                     var leadingComments: Comment[];
-                    if (detachedCommentsInfo === undefined || detachedCommentsInfo[detachedCommentsInfo.length - 1].nodePos !== node.pos) {
-                        // get the leading comments from the node
-                        leadingComments = getLeadingCommentsOfNode(node, currentSourceFile);
+                    if (hasDetachedComments(node.pos)) {
+                        // get comments without detached comments
+                        leadingComments = getLeadingCommentsWithoutDetachedComments();
                     }
                     else {
-                        // get the leading comments from detachedPos
-                        leadingComments = getLeadingComments(currentSourceFile.text, detachedCommentsInfo[detachedCommentsInfo.length - 1].detachedCommentEndPos);
-                        if (detachedCommentsInfo.length - 1) {
-                            detachedCommentsInfo.pop();
-                        }
-                        else {
-                            detachedCommentsInfo = undefined;
-                        }
+                        // get the leading comments from the node
+                        leadingComments = getLeadingCommentsOfNode(node, currentSourceFile);
                     }
                     emitNewLineBeforeLeadingComments(node, leadingComments, writer);
                     // Leading comments are emitted at /*leading comment1 */space/*leading comment*/space
@@ -2110,6 +2128,21 @@ module ts {
                 }
             }
 
+            function emitLeadingCommentsOfLocalPosition(pos: number) {
+                var leadingComments: Comment[];
+                if (hasDetachedComments(pos)) {
+                    // get comments without detached comments
+                    leadingComments = getLeadingCommentsWithoutDetachedComments();
+                }
+                else {
+                    // get the leading comments from the node
+                    leadingComments = getLeadingComments(currentSourceFile.text, pos);
+                }
+                emitNewLineBeforeLeadingComments({ pos: pos, end: pos }, leadingComments, writer);
+                // Leading comments are emitted at /*leading comment1 */space/*leading comment*/space
+                emitComments(leadingComments, /*trailingSeparator*/ true, writer, writeComment);
+            }
+
             function emitDetachedCommentsAtPosition(node: TextRange) {
                 var leadingComments = getLeadingComments(currentSourceFile.text, node.pos);
                 if (leadingComments) {
diff --git a/tests/baselines/reference/callOverloads1.js b/tests/baselines/reference/callOverloads1.js
@@ -20,6 +20,7 @@ Foo();
 //// [callOverloads1.js]
 var Foo = (function () {
     function Foo(x) {
+        // WScript.Echo("Constructor function has executed");
     }
     Foo.prototype.bar1 = function () {
     };
diff --git a/tests/baselines/reference/callOverloads2.js b/tests/baselines/reference/callOverloads2.js
@@ -28,6 +28,7 @@ Foo();
 //// [callOverloads2.js]
 var Foo = (function () {
     function Foo(x) {
+        // WScript.Echo("Constructor function has executed");
     }
     Foo.prototype.bar1 = function () {
     };
diff --git a/tests/baselines/reference/callOverloads3.js b/tests/baselines/reference/callOverloads3.js
@@ -21,6 +21,7 @@ Foo("s");
 //// [callOverloads3.js]
 var Foo = (function () {
     function Foo(x) {
+        // WScript.Echo("Constructor function has executed");
     }
     Foo.prototype.bar1 = function () {
     };
diff --git a/tests/baselines/reference/callOverloads4.js b/tests/baselines/reference/callOverloads4.js
@@ -21,6 +21,7 @@ Foo("s");
 //// [callOverloads4.js]
 var Foo = (function () {
     function Foo(x) {
+        // WScript.Echo("Constructor function has executed");
     }
     Foo.prototype.bar1 = function () {
     };
diff --git a/tests/baselines/reference/callOverloads5.js b/tests/baselines/reference/callOverloads5.js
@@ -22,6 +22,7 @@ Foo("s");
 //// [callOverloads5.js]
 var Foo = (function () {
     function Foo(x) {
+        // WScript.Echo("Constructor function has executed");
     }
     Foo.prototype.bar1 = function (a) {
     };
diff --git a/tests/baselines/reference/classMemberInitializerWithLamdaScoping.js b/tests/baselines/reference/classMemberInitializerWithLamdaScoping.js
@@ -52,6 +52,8 @@ var Test1 = (function () {
         this.field1 = field1;
         this.messageHandler = function () {
             console.log(field1); // But this should be error as the field1 will resolve to var field1 
+            // but since this code would be generated inside constructor, in generated js
+            // it would resolve to private field1 and thats not what user intended here. 
         };
     }
     Test1.staticMessageHandler = function () {
diff --git a/tests/baselines/reference/classMemberInitializerWithLamdaScoping2.js b/tests/baselines/reference/classMemberInitializerWithLamdaScoping2.js
@@ -25,6 +25,8 @@ var Test1 = (function () {
         this.field1 = field1;
         this.messageHandler = function () {
             console.log(field1); // But this should be error as the field1 will resolve to var field1 
+            // but since this code would be generated inside constructor, in generated js
+            // it would resolve to private field1 and thats not what user intended here. 
         };
     }
     return Test1;
diff --git a/tests/baselines/reference/classOrder1.js b/tests/baselines/reference/classOrder1.js
@@ -16,6 +16,7 @@ var A = (function () {
     function A() {
     }
     A.prototype.foo = function () {
+        /*WScript.Echo("Here!");*/
     };
     return A;
 })();
diff --git a/tests/baselines/reference/commentsClass.js b/tests/baselines/reference/commentsClass.js
@@ -63,6 +63,14 @@ class c8 {
 }
 var i8 = new c8();
 var i8_c = c8;
+
+class c9 {
+    constructor() {
+        /// This is some detached comment
+
+        // should emit this leading comment of } too
+    }
+}
 
 
 //// [commentsClass.js]
@@ -123,11 +131,20 @@ var c8 = (function () {
     /** constructor comment
     */
     function c8() {
+        /** constructor comment2
+        */
     }
     return c8;
 })();
 var i8 = new c8();
 var i8_c = c8;
+var c9 = (function () {
+    function c9() {
+        /// This is some detached comment
+        // should emit this leading comment of } too
+    }
+    return c9;
+})();
 
 
 //// [commentsClass.d.ts]
@@ -178,3 +195,6 @@ declare class c8 {
 }
 declare var i8: c8;
 declare var i8_c: typeof c8;
+declare class c9 {
+    constructor();
+}
diff --git a/tests/baselines/reference/commentsClass.types b/tests/baselines/reference/commentsClass.types
@@ -130,3 +130,13 @@ var i8_c = c8;
 >i8_c : typeof c8
 >c8 : typeof c8
 
+class c9 {
+>c9 : c9
+
+    constructor() {
+        /// This is some detached comment
+
+        // should emit this leading comment of } too
+    }
+}
+
diff --git a/tests/baselines/reference/commentsFunction.js b/tests/baselines/reference/commentsFunction.js
@@ -41,7 +41,19 @@ lambdaFoo = (a, b) => a * b; // This is trailing comment
 /*leading comment*/(() => 0); //trailing comment
 
 function blah4(/*1*/a: string/*2*/,/*3*/b: string/*4*/) {
-}
+}
+
+function foo1() {
+
+    // should emit this
+}
+
+function foo2() {
+    /// This is some detached comment
+
+    // should emit this leading comment of } too
+}
+
 
 //// [commentsFunction.js]
 /** This comment should appear for foo*/
@@ -79,6 +91,13 @@ lambdaFoo = function (a, b) { return a * b; }; // This is trailing comment
 /*leading comment*/ (function () { return 0; }); //trailing comment
 function blah4(/*1*/ a /*2*/, /*3*/ b /*4*/) {
 }
+function foo1() {
+    // should emit this
+}
+function foo2() {
+    /// This is some detached comment
+    // should emit this leading comment of } too
+}
 
 
 //// [commentsFunction.d.ts]
@@ -98,3 +117,5 @@ declare function blah(a: string): void;
 declare function blah2(a: string): void;
 declare function blah3(a: string): void;
 declare function blah4(a: string, b: string): void;
+declare function foo1(): void;
+declare function foo2(): void;
diff --git a/tests/baselines/reference/commentsFunction.types b/tests/baselines/reference/commentsFunction.types
@@ -110,3 +110,18 @@ function blah4(/*1*/a: string/*2*/,/*3*/b: string/*4*/) {
 >a : string
 >b : string
 }
+
+function foo1() {
+>foo1 : () => void
+
+    // should emit this
+}
+
+function foo2() {
+>foo2 : () => void
+
+    /// This is some detached comment
+
+    // should emit this leading comment of } too
+}
+
diff --git a/tests/baselines/reference/genericCallWithGenericSignatureArguments.js b/tests/baselines/reference/genericCallWithGenericSignatureArguments.js
@@ -66,6 +66,8 @@ function other2(x) {
     var r7 = foo(function (a) { return a; }, function (b) { return b; }); // T => T
     var r7b = foo(function (a) { return a; }, function (b) { return b; }); // {} => {}
     var r8 = r7(null);
+    // BUG 835518
+    //var r9 = r7(new Date());
 }
 function foo2(a, b) {
     var r;
diff --git a/tests/baselines/reference/genericCallWithObjectTypeArgsAndNumericIndexer.js b/tests/baselines/reference/genericCallWithObjectTypeArgsAndNumericIndexer.js
@@ -54,4 +54,6 @@ function other3(arg) {
     var b;
     var r2 = foo(b);
     var d = r2[1];
+    // BUG 821629
+    //var u: U = r2[1]; // ok
 }
diff --git a/tests/baselines/reference/genericCallWithObjectTypeArgsAndStringIndexer.js b/tests/baselines/reference/genericCallWithObjectTypeArgsAndStringIndexer.js
@@ -55,4 +55,6 @@ function other3(arg) {
     var b;
     var r2 = foo(b);
     var d = r2['hm']; // ok
+    // BUG 821629
+    //var u: U = r2['hm']; // ok
 }
diff --git a/tests/baselines/reference/logicalOrOperatorWithTypeParameters.js b/tests/baselines/reference/logicalOrOperatorWithTypeParameters.js
@@ -37,6 +37,7 @@ function fn2(t, u, v) {
     var r4 = u || u;
     var r5 = u || v;
     var r6 = u || v;
+    //var r7: T = u || v;
 }
 function fn3(t, u) {
     var r1 = t || u;
diff --git a/tests/baselines/reference/missingReturnStatement1.js b/tests/baselines/reference/missingReturnStatement1.js
@@ -11,6 +11,7 @@ var Foo = (function () {
     function Foo() {
     }
     Foo.prototype.foo = function () {
+        //return 4;
     };
     return Foo;
 })();
diff --git a/tests/baselines/reference/out-flag.js b/tests/baselines/reference/out-flag.js
@@ -27,6 +27,7 @@ var MyClass = (function () {
         return 42;
     };
     MyClass.prototype.SetCount = function (value) {
+        //
     };
     return MyClass;
 })();
diff --git a/tests/baselines/reference/out-flag.js.map b/tests/baselines/reference/out-flag.js.map
diff --git a/tests/baselines/reference/out-flag.sourcemap.txt b/tests/baselines/reference/out-flag.sourcemap.txt
@@ -143,26 +143,34 @@ sourceFile:out-flag.ts
 4 >Emitted(10, 44) Source(12, 21) + SourceIndex(0) name (MyClass)
 5 >Emitted(10, 49) Source(12, 34) + SourceIndex(0) name (MyClass)
 ---
+>>>        //
+1 >^^^^^^^^
+2 >        ^^
+1 >)
+  >    {
+  >        
+2 >        //
+1 >Emitted(11, 9) Source(14, 9) + SourceIndex(0) name (MyClass.SetCount)
+2 >Emitted(11, 11) Source(14, 11) + SourceIndex(0) name (MyClass.SetCount)
+---
 >>>    };
 1 >^^^^
 2 >    ^
 3 >     ^^^^^^^^^^^^^^^->
-1 >)
-  >    {
-  >        //
+1 >
   >    
 2 >    }
-1 >Emitted(11, 5) Source(15, 5) + SourceIndex(0) name (MyClass.SetCount)
-2 >Emitted(11, 6) Source(15, 6) + SourceIndex(0) name (MyClass.SetCount)
+1 >Emitted(12, 5) Source(15, 5) + SourceIndex(0) name (MyClass.SetCount)
+2 >Emitted(12, 6) Source(15, 6) + SourceIndex(0) name (MyClass.SetCount)
 ---
 >>>    return MyClass;
 1->^^^^
 2 >    ^^^^^^^^^^^^^^
 1->
   >
 2 >    }
-1->Emitted(12, 5) Source(16, 1) + SourceIndex(0) name (MyClass)
-2 >Emitted(12, 19) Source(16, 2) + SourceIndex(0) name (MyClass)
+1->Emitted(13, 5) Source(16, 1) + SourceIndex(0) name (MyClass)
+2 >Emitted(13, 19) Source(16, 2) + SourceIndex(0) name (MyClass)
 ---
 >>>})();
 1 >
@@ -186,9 +194,9 @@ sourceFile:out-flag.ts
   >         //
   >     }
   > }
-1 >Emitted(13, 1) Source(16, 1) + SourceIndex(0) name (MyClass)
-2 >Emitted(13, 2) Source(16, 2) + SourceIndex(0) name (MyClass)
-3 >Emitted(13, 2) Source(4, 1) + SourceIndex(0)
-4 >Emitted(13, 6) Source(16, 2) + SourceIndex(0)
+1 >Emitted(14, 1) Source(16, 1) + SourceIndex(0) name (MyClass)
+2 >Emitted(14, 2) Source(16, 2) + SourceIndex(0) name (MyClass)
+3 >Emitted(14, 2) Source(4, 1) + SourceIndex(0)
+4 >Emitted(14, 6) Source(16, 2) + SourceIndex(0)
 ---
 >>>//# sourceMappingURL=out-flag.js.map
diff --git a/tests/baselines/reference/properties.js b/tests/baselines/reference/properties.js
diff --git a/tests/baselines/reference/properties.js.map b/tests/baselines/reference/properties.js.map
diff --git a/tests/baselines/reference/properties.sourcemap.txt b/tests/baselines/reference/properties.sourcemap.txt
diff --git a/tests/cases/compiler/commentsClass.ts b/tests/cases/compiler/commentsClass.ts
diff --git a/tests/cases/compiler/commentsFunction.ts b/tests/cases/compiler/commentsFunction.ts

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,7 @@ Foo();`
`20`	`20`	`//// [callOverloads1.js]`
`21`	`21`	`var Foo = (function () {`
`22`	`22`	`function Foo(x) {`
	`23`	`+ // WScript.Echo("Constructor function has executed");`
`23`	`24`	`}`
`24`	`25`	`Foo.prototype.bar1 = function () {`
`25`	`26`	`};`
Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,7 @@ Foo();`
`28`	`28`	`//// [callOverloads2.js]`
`29`	`29`	`var Foo = (function () {`
`30`	`30`	`function Foo(x) {`
	`31`	`+ // WScript.Echo("Constructor function has executed");`
`31`	`32`	`}`
`32`	`33`	`Foo.prototype.bar1 = function () {`
`33`	`34`	`};`
Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,7 @@ Foo("s");`
`21`	`21`	`//// [callOverloads3.js]`
`22`	`22`	`var Foo = (function () {`
`23`	`23`	`function Foo(x) {`
	`24`	`+ // WScript.Echo("Constructor function has executed");`
`24`	`25`	`}`
`25`	`26`	`Foo.prototype.bar1 = function () {`
`26`	`27`	`};`
Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,7 @@ Foo("s");`
`22`	`22`	`//// [callOverloads5.js]`
`23`	`23`	`var Foo = (function () {`
`24`	`24`	`function Foo(x) {`
	`25`	`+ // WScript.Echo("Constructor function has executed");`
`25`	`26`	`}`
`26`	`27`	`Foo.prototype.bar1 = function (a) {`
`27`	`28`	`};`
Original file line number	Diff line number	Diff line change
`@@ -52,6 +52,8 @@ var Test1 = (function () {`
`52`	`52`	`this.field1 = field1;`
`53`	`53`	`this.messageHandler = function () {`
`54`	`54`	`console.log(field1); // But this should be error as the field1 will resolve to var field1`
	`55`	`+ // but since this code would be generated inside constructor, in generated js`
	`56`	`+ // it would resolve to private field1 and thats not what user intended here.`
`55`	`57`	`};`
`56`	`58`	`}`
`57`	`59`	`Test1.staticMessageHandler = function () {`
Original file line number	Diff line number	Diff line change
`@@ -25,6 +25,8 @@ var Test1 = (function () {`
`25`	`25`	`this.field1 = field1;`
`26`	`26`	`this.messageHandler = function () {`
`27`	`27`	`console.log(field1); // But this should be error as the field1 will resolve to var field1`
	`28`	`+ // but since this code would be generated inside constructor, in generated js`
	`29`	`+ // it would resolve to private field1 and thats not what user intended here.`
`28`	`30`	`};`
`29`	`31`	`}`
`30`	`32`	`return Test1;`
Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@ var A = (function () {`
`16`	`16`	`function A() {`
`17`	`17`	`}`
`18`	`18`	`A.prototype.foo = function () {`
	`19`	`+ /WScript.Echo("Here!");/`
`19`	`20`	`};`
`20`	`21`	`return A;`
`21`	`22`	`})();`