From 51e40112ec22b4f4b1389727e155972fa1c8e381 Mon Sep 17 00:00:00 2001 From: Yui T Date: Thu, 2 Mar 2017 10:51:51 -0800 Subject: [PATCH] Address PR comments --- src/compiler/emitter.ts | 71 +++++++++---------- tests/baselines/reference/jsDocTypes.js | 1 - tests/baselines/reference/jsDocTypes.symbols | 73 ++++++++++---------- tests/baselines/reference/jsDocTypes.types | 1 - 4 files changed, 67 insertions(+), 79 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 1b307920c55..0038ead4591 100644 --- a/src/compiler/emitter.ts +++ b/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; // 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, 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); - } - else { - for (const sourceFile of (sourceFileOrBundle as Bundle).sourceFiles) { - // Emit only the first encountered shebang - if (emitShebangInSourceFile(sourceFile)) { - break; - } - } - } - - function emitShebangInSourceFile(sourceFile: SourceFile): boolean { - const shebang = getShebang(sourceFile.text); + const shebang = getShebang(sourceFileOrBundle.text); if (shebang) { write(shebang); writeLine(); return true; } } + else { + for (const sourceFile of sourceFileOrBundle.sourceFiles) { + // Emit only the first encountered shebang + if (emitShebangIfNeeded(sourceFile)) { + break; + } + } + } } // // 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) { if (modifiers && modifiers.length) { emitList(node, modifiers, ListFormat.Modifiers); diff --git a/tests/baselines/reference/jsDocTypes.js b/tests/baselines/reference/jsDocTypes.js index a6df2f361c6..aab32275f72 100644 --- a/tests/baselines/reference/jsDocTypes.js +++ b/tests/baselines/reference/jsDocTypes.js @@ -1,7 +1,6 @@ //// [tests/cases/conformance/salsa/jsDocTypes.ts] //// //// [a.js] - /** @type {String} */ var S; diff --git a/tests/baselines/reference/jsDocTypes.symbols b/tests/baselines/reference/jsDocTypes.symbols index 9bde032cb44..eb795295ea0 100644 --- a/tests/baselines/reference/jsDocTypes.symbols +++ b/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; ->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; ->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)) diff --git a/tests/baselines/reference/jsDocTypes.types b/tests/baselines/reference/jsDocTypes.types index c0214edf627..116e9f0a245 100644 --- a/tests/baselines/reference/jsDocTypes.types +++ b/tests/baselines/reference/jsDocTypes.types @@ -1,5 +1,4 @@ === tests/cases/conformance/salsa/a.js === - /** @type {String} */ var S; >S : string