Address PR comments

Author: Yui T

src/compiler/emitter.ts

@@ -212,7 +212,6 @@ namespace ts {
         } = comments;
 
         let currentSourceFile: SourceFile;
-        let statementOffset: number;  // We cache the index of first non prologue so we don't have to recalculate it multiple times like in emitBodyInDirect
         let nodeIdToGeneratedName: string[]; // Map of generated names for specific nodes.
         let autoGeneratedIdToGeneratedName: string[]; // Map of generated names for temp and loop variables.
         let generatedNames: Map<string>; // Set of names generated by the NameGenerator.
@@ -312,7 +311,6 @@ namespace ts {
 
         function setSourceFile(sourceFile: SourceFile) {
             currentSourceFile = sourceFile;
-            statementOffset = getIndexOfFirstNonPrologueDirectives(sourceFile.statements);
             comments.setSourceFile(sourceFile);
             if (onSetSourceFile) {
                 onSetSourceFile(sourceFile);
@@ -736,19 +734,6 @@ namespace ts {
             return node && substituteNode && substituteNode(hint, node) || node;
         }
 
-        function emitBodyIndirect(node: Node, elements: NodeArray<Node>, emitCallback: (node: Node) => void): void {
-            // If the node is a sourceFile and it has prologueDirective (statmentOffSet is not zero) that is synthesize
-            // We will need to emit detached comment here because emitPrologueDirective will not emit comments of such prologue directive
-            const shouldEmitDetachedComment = node.kind !== SyntaxKind.SourceFile ? true :
-                statementOffset === 0 || nodeIsSynthesized((node as SourceFile).statements[statementOffset - 1]);
-            if (emitBodyWithDetachedComments && shouldEmitDetachedComment) {
-                emitBodyWithDetachedComments(node, elements, emitCallback);
-            }
-            else {
-                emitCallback(node);
-            }
-        }
-
         function emitHelpersIndirect(node: Node) {
             if (onEmitHelpers) {
                 onEmitHelpers(node, writeLines);
@@ -1652,7 +1637,12 @@ namespace ts {
                 ? emitBlockFunctionBodyOnSingleLine
                 : emitBlockFunctionBodyWorker;
 
-            emitBodyIndirect(body, body.statements, emitBlockFunctionBody);
+            if (emitBodyWithDetachedComments) {
+                emitBodyWithDetachedComments(body, body.statements, emitBlockFunctionBody);
+            }
+            else {
+                emitBlockFunctionBody(body);
+            }
 
             decreaseIndent();
             writeToken(SyntaxKind.CloseBraceToken, body.statements.end, body);
@@ -2061,14 +2051,28 @@ namespace ts {
 
         function emitSourceFile(node: SourceFile) {
             writeLine();
-            emitBodyIndirect(node, node.statements, emitSourceFileWorker);
+            // If the node is a sourceFile and it has prologueDirective (statmentOffSet is not zero) that is synthesize
+            // We will need to emit detached comment here because emitPrologueDirective will not emit comments of such prologue directive
+            const statements = node.statements;
+            // Emit detached comment if there iare no prologue directives or this is a synthesized prologue directives.
+            // The synthesized node will have no leading comment so some comments may be missed.
+            const shouldEmitDetachedComment = statements.length === 0 ||
+                !isPrologueDirective(statements[0]) ||
+                (isPrologueDirective(statements[0]) && nodeIsSynthesized(statements[0]));
+            if (emitBodyWithDetachedComments && shouldEmitDetachedComment) {
+                emitBodyWithDetachedComments(node, statements, emitSourceFileWorker);
+            }
+            else {
+                emitSourceFileWorker(node);
+            }
         }
 
         function emitSourceFileWorker(node: SourceFile) {
             const statements = node.statements;
             pushNameGenerationScope();
             emitHelpersIndirect(node);
-            emitList(node, statements, ListFormat.MultiLine, statementOffset);
+            const index = findIndex(statements, statement => !isPrologueDirective(statement));
+            emitList(node, statements, ListFormat.MultiLine, index === -1 ? statements.length : index);
             popNameGenerationScope();
         }
 
@@ -2122,40 +2126,27 @@ namespace ts {
 
         function emitShebangIfNeeded(sourceFileOrBundle: Bundle | SourceFile) {
             if (sourceFileOrBundle.kind === SyntaxKind.SourceFile) {
-                emitShebangInSourceFile(sourceFileOrBundle as SourceFile);
+                const shebang = getShebang(sourceFileOrBundle.text);
+                if (shebang) {
+                    write(shebang);
+                    writeLine();
+                    return true;
+                }
             }
             else {
-                for (const sourceFile of (sourceFileOrBundle as Bundle).sourceFiles) {
+                for (const sourceFile of sourceFileOrBundle.sourceFiles) {
                     // Emit only the first encountered shebang
-                    if (emitShebangInSourceFile(sourceFile)) {
+                    if (emitShebangIfNeeded(sourceFile)) {
                         break;
                     }
                 }
             }
-
-            function emitShebangInSourceFile(sourceFile: SourceFile): boolean {
-                const shebang = getShebang(sourceFile.text);
-                if (shebang) {
-                    write(shebang);
-                    writeLine();
-                    return true;
-                }
-            }
         }
 
         //
         // Helpers
         //
 
-        function getIndexOfFirstNonPrologueDirectives(statements: Statement[]): number {
-            for (let i = 0; i < statements.length; i++) {
-                if (!isPrologueDirective(statements[i])) {
-                    return i;
-                }
-            }
-            return statements.length;
-        }
-
         function emitModifiers(node: Node, modifiers: NodeArray<Modifier>) {
             if (modifiers && modifiers.length) {
                 emitList(node, modifiers, ListFormat.Modifiers);

tests/baselines/reference/jsDocTypes.js

@@ -1,7 +1,6 @@
 //// [tests/cases/conformance/salsa/jsDocTypes.ts] ////
 
 //// [a.js]
-
 /** @type {String} */
 var S;
 

tests/baselines/reference/jsDocTypes.symbols

@@ -1,133 +1,132 @@
 === tests/cases/conformance/salsa/a.js ===
-
 /** @type {String} */
 var S;
->S : Symbol(S, Decl(a.js, 2, 3), Decl(b.ts, 0, 3))
+>S : Symbol(S, Decl(a.js, 1, 3), Decl(b.ts, 0, 3))
 
 /** @type {string} */
 var s;
->s : Symbol(s, Decl(a.js, 5, 3), Decl(b.ts, 1, 3))
+>s : Symbol(s, Decl(a.js, 4, 3), Decl(b.ts, 1, 3))
 
 /** @type {Number} */
 var N;
->N : Symbol(N, Decl(a.js, 8, 3), Decl(b.ts, 2, 3))
+>N : Symbol(N, Decl(a.js, 7, 3), Decl(b.ts, 2, 3))
 
 /** @type {number} */
 var n;
->n : Symbol(n, Decl(a.js, 11, 3), Decl(b.ts, 3, 3))
+>n : Symbol(n, Decl(a.js, 10, 3), Decl(b.ts, 3, 3))
 
 /** @type {Boolean} */
 var B;
->B : Symbol(B, Decl(a.js, 14, 3), Decl(b.ts, 4, 3))
+>B : Symbol(B, Decl(a.js, 13, 3), Decl(b.ts, 4, 3))
 
 /** @type {boolean} */
 var b;
->b : Symbol(b, Decl(a.js, 17, 3), Decl(b.ts, 5, 3))
+>b : Symbol(b, Decl(a.js, 16, 3), Decl(b.ts, 5, 3))
 
 /** @type {Void} */
 var V;
->V : Symbol(V, Decl(a.js, 20, 3), Decl(b.ts, 6, 3))
+>V : Symbol(V, Decl(a.js, 19, 3), Decl(b.ts, 6, 3))
 
 /** @type {void} */
 var v;
->v : Symbol(v, Decl(a.js, 23, 3), Decl(b.ts, 7, 3))
+>v : Symbol(v, Decl(a.js, 22, 3), Decl(b.ts, 7, 3))
 
 /** @type {Undefined} */
 var U;
->U : Symbol(U, Decl(a.js, 26, 3), Decl(b.ts, 8, 3))
+>U : Symbol(U, Decl(a.js, 25, 3), Decl(b.ts, 8, 3))
 
 /** @type {undefined} */
 var u;
->u : Symbol(u, Decl(a.js, 29, 3), Decl(b.ts, 9, 3))
+>u : Symbol(u, Decl(a.js, 28, 3), Decl(b.ts, 9, 3))
 
 /** @type {Null} */
 var Nl;
->Nl : Symbol(Nl, Decl(a.js, 32, 3), Decl(b.ts, 10, 3))
+>Nl : Symbol(Nl, Decl(a.js, 31, 3), Decl(b.ts, 10, 3))
 
 /** @type {null} */
 var nl;
->nl : Symbol(nl, Decl(a.js, 35, 3), Decl(b.ts, 11, 3))
+>nl : Symbol(nl, Decl(a.js, 34, 3), Decl(b.ts, 11, 3))
 
 /** @type {Array} */
 var A;
->A : Symbol(A, Decl(a.js, 38, 3), Decl(b.ts, 12, 3))
+>A : Symbol(A, Decl(a.js, 37, 3), Decl(b.ts, 12, 3))
 
 /** @type {array} */
 var a;
->a : Symbol(a, Decl(a.js, 41, 3), Decl(b.ts, 13, 3))
+>a : Symbol(a, Decl(a.js, 40, 3), Decl(b.ts, 13, 3))
 
 /** @type {Promise} */
 var P;
->P : Symbol(P, Decl(a.js, 44, 3), Decl(b.ts, 14, 3))
+>P : Symbol(P, Decl(a.js, 43, 3), Decl(b.ts, 14, 3))
 
 /** @type {promise} */
 var p;
->p : Symbol(p, Decl(a.js, 47, 3), Decl(b.ts, 15, 3))
+>p : Symbol(p, Decl(a.js, 46, 3), Decl(b.ts, 15, 3))
 
 /** @type {?number} */
 var nullable;
->nullable : Symbol(nullable, Decl(a.js, 50, 3), Decl(b.ts, 16, 3))
+>nullable : Symbol(nullable, Decl(a.js, 49, 3), Decl(b.ts, 16, 3))
 
 /** @type {Object} */
 var Obj;
->Obj : Symbol(Obj, Decl(a.js, 53, 3), Decl(b.ts, 17, 3))
+>Obj : Symbol(Obj, Decl(a.js, 52, 3), Decl(b.ts, 17, 3))
 
 
 
 === tests/cases/conformance/salsa/b.ts ===
 var S: string;
->S : Symbol(S, Decl(a.js, 2, 3), Decl(b.ts, 0, 3))
+>S : Symbol(S, Decl(a.js, 1, 3), Decl(b.ts, 0, 3))
 
 var s: string;
->s : Symbol(s, Decl(a.js, 5, 3), Decl(b.ts, 1, 3))
+>s : Symbol(s, Decl(a.js, 4, 3), Decl(b.ts, 1, 3))
 
 var N: number;
->N : Symbol(N, Decl(a.js, 8, 3), Decl(b.ts, 2, 3))
+>N : Symbol(N, Decl(a.js, 7, 3), Decl(b.ts, 2, 3))
 
 var n: number
->n : Symbol(n, Decl(a.js, 11, 3), Decl(b.ts, 3, 3))
+>n : Symbol(n, Decl(a.js, 10, 3), Decl(b.ts, 3, 3))
 
 var B: boolean;
->B : Symbol(B, Decl(a.js, 14, 3), Decl(b.ts, 4, 3))
+>B : Symbol(B, Decl(a.js, 13, 3), Decl(b.ts, 4, 3))
 
 var b: boolean;
->b : Symbol(b, Decl(a.js, 17, 3), Decl(b.ts, 5, 3))
+>b : Symbol(b, Decl(a.js, 16, 3), Decl(b.ts, 5, 3))
 
 var V :void;
->V : Symbol(V, Decl(a.js, 20, 3), Decl(b.ts, 6, 3))
+>V : Symbol(V, Decl(a.js, 19, 3), Decl(b.ts, 6, 3))
 
 var v: void;
->v : Symbol(v, Decl(a.js, 23, 3), Decl(b.ts, 7, 3))
+>v : Symbol(v, Decl(a.js, 22, 3), Decl(b.ts, 7, 3))
 
 var U: undefined;
->U : Symbol(U, Decl(a.js, 26, 3), Decl(b.ts, 8, 3))
+>U : Symbol(U, Decl(a.js, 25, 3), Decl(b.ts, 8, 3))
 
 var u: undefined;
->u : Symbol(u, Decl(a.js, 29, 3), Decl(b.ts, 9, 3))
+>u : Symbol(u, Decl(a.js, 28, 3), Decl(b.ts, 9, 3))
 
 var Nl: null;
->Nl : Symbol(Nl, Decl(a.js, 32, 3), Decl(b.ts, 10, 3))
+>Nl : Symbol(Nl, Decl(a.js, 31, 3), Decl(b.ts, 10, 3))
 
 var nl: null;
->nl : Symbol(nl, Decl(a.js, 35, 3), Decl(b.ts, 11, 3))
+>nl : Symbol(nl, Decl(a.js, 34, 3), Decl(b.ts, 11, 3))
 
 var A: any[];
->A : Symbol(A, Decl(a.js, 38, 3), Decl(b.ts, 12, 3))
+>A : Symbol(A, Decl(a.js, 37, 3), Decl(b.ts, 12, 3))
 
 var a: any[];
->a : Symbol(a, Decl(a.js, 41, 3), Decl(b.ts, 13, 3))
+>a : Symbol(a, Decl(a.js, 40, 3), Decl(b.ts, 13, 3))
 
 var P: Promise<any>;
->P : Symbol(P, Decl(a.js, 44, 3), Decl(b.ts, 14, 3))
+>P : Symbol(P, Decl(a.js, 43, 3), Decl(b.ts, 14, 3))
 >Promise : Symbol(Promise, Decl(lib.d.ts, --, --))
 
 var p: Promise<any>;
->p : Symbol(p, Decl(a.js, 47, 3), Decl(b.ts, 15, 3))
+>p : Symbol(p, Decl(a.js, 46, 3), Decl(b.ts, 15, 3))
 >Promise : Symbol(Promise, Decl(lib.d.ts, --, --))
 
 var nullable: number | null;
->nullable : Symbol(nullable, Decl(a.js, 50, 3), Decl(b.ts, 16, 3))
+>nullable : Symbol(nullable, Decl(a.js, 49, 3), Decl(b.ts, 16, 3))
 
 var Obj: any;
->Obj : Symbol(Obj, Decl(a.js, 53, 3), Decl(b.ts, 17, 3))
+>Obj : Symbol(Obj, Decl(a.js, 52, 3), Decl(b.ts, 17, 3))
 

tests/baselines/reference/jsDocTypes.types

@@ -1,5 +1,4 @@
 === tests/cases/conformance/salsa/a.js ===
-
 /** @type {String} */
 var S;
 >S : string