From 754bdccebe5caacb92ecf6680d2604393d5fd559 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 26 May 2016 01:16:29 -0700 Subject: [PATCH 1/8] Added parallel support for runtests-file --- Jakefile.js | 338 ++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 232 insertions(+), 106 deletions(-) diff --git a/Jakefile.js b/Jakefile.js index 8ba49afadfc..37952afaf0f 100644 --- a/Jakefile.js +++ b/Jakefile.js @@ -245,7 +245,7 @@ var librarySourceMap = [ { target: "lib.es2015.d.ts", sources: ["header.d.ts", "es2015.d.ts"] }, { target: "lib.es2016.d.ts", sources: ["header.d.ts", "es2016.d.ts"] }, { target: "lib.es2017.d.ts", sources: ["header.d.ts", "es2017.d.ts"] }, - + // JavaScript + all host library { target: "lib.d.ts", sources: ["header.d.ts", "es5.d.ts"].concat(hostsLibrarySources) }, { target: "lib.es6.d.ts", sources: ["header.d.ts", "es5.d.ts"].concat(es2015LibrarySources, hostsLibrarySources, "dom.iterable.d.ts") } @@ -749,7 +749,7 @@ function deleteTemporaryProjectOutput() { } } -function runTestsAndWriteOutput(file) { +function runTestsAndWriteOutput(file, defaultSubsets) { cleanTestDirs(); var tests = process.env.test || process.env.tests || process.env.t; var light = process.env.light || false; @@ -764,114 +764,231 @@ function runTestsAndWriteOutput(file) { testTimeout = 100000; } - var args = []; - args.push("-R", "tap"); - args.push("--no-colors"); - args.push("-t", testTimeout); - if (tests) { - args.push("-g", '"' + tests + '"'); + var subsetRegexes; + var subsets; + if (defaultSubsets.length === 0) { + subsetRegexes = [tests]; + subsets = [tests]; + } + else { + subsets = tests ? tests.split("|") : defaultSubsets; + subsetRegexes = subsets.map(function (sub) { return "^" + sub + ".*$"; }); + subsetRegexes.push("^(?!" + subsets.join("|") + ").*$"); + subsets.push("other"); } - args.push(run); - - var cmd = "mocha " + args.join(" "); - console.log(cmd); - var p = child_process.spawn( - process.platform === "win32" ? "cmd" : "/bin/sh", - process.platform === "win32" ? ["/c", cmd] : ["-c", cmd], { - windowsVerbatimArguments: true - }); var out = fs.createWriteStream(file); + var outFileNames = subsetRegexes.length !== 1 ? [] : undefined; var tapRange = /^(\d+)\.\.(\d+)(?:$|\r\n?|\n)/; var tapOk = /^ok\s/; var tapNotOk = /^not\sok\s/; - var tapComment = /^#/; + var tapComment = /^#(?: (tests|pass|fail) (\d+)$)?/; var typeError = /^\s+TypeError:/; var debugError = /^\s+Error:\sDebug\sFailure\./; - var progress = new ProgressBar("Running tests..."); - var expectedTestCount = 0; - var testCount = 0; - var failureCount = 0; - var successCount = 0; - var comments = []; - var typeErrorCount = 0; - var debugErrorCount = 0; - - var rl = readline.createInterface({ - input: p.stdout, - terminal: false - }); - - function updateProgress(percentComplete) { - progress.update(percentComplete, - /*foregroundColor*/ failureCount > 0 - ? "red" - : successCount === expectedTestCount - ? "green" - : "cyan", - /*backgroundColor*/ "gray" - ); - } - - rl.on("line", function (line) { - var m = tapRange.exec(line); - if (m) { - expectedTestCount = parseInt(m[2]); - return; - } - - if (tapOk.test(line)) { - out.write(line.replace(/^ok\s+\d+\s+/, "ok ") + os.EOL); - successCount++; - } - else if (tapNotOk.test(line)) { - out.write(line.replace(/^not\s+ok\s+\d+\s+/, "not ok ") + os.EOL); - failureCount++; + var progress = new ProgressBar(); + var totalTypeErrorCount = 0; + var totalDebugErrorCount = 0; + var totalReportedTestCount = 0; + var totalReportedPassCount = 0; + var totalReportedFailCount = 0; + var counter = subsetRegexes.length; + var errorStatus; + subsetRegexes.forEach(function (subsetRegex, i) { + var expectedTestCount = 0; + var testCount = 0; + var failureCount = 0; + var successCount = 0; + var reportedTestCount = 0; + var reportedPassCount = 0; + var reportedFailCount = 0; + var comments = []; + var typeErrorCount = 0; + var debugErrorCount = 0; + var outFileName; + var outFile; + if (subsetRegexes.length === 1) { + outFile = out; } else { - out.write(line + os.EOL); - if (tapComment.test(line)) { - comments.push(line); + outFileName = path.join(os.tmpdir(), path.basename(file) + "." + i); + outFileNames[i] = outFileName; + outFile = fs.createWriteStream(outFileName); + } + + var args = []; + args.push("-R", "tap"); + args.push("--no-colors"); + args.push("-t", testTimeout); + if (subsetRegex) { + args.push("-g", '"' + subsetRegex + '"'); + } + + args.push(run); + + var cmd = "mocha " + args.join(" "); + if (subsetRegexes.length === 1) { + console.log(cmd); + } + + updateProgress(0); + + var p = child_process.spawn( + process.platform === "win32" ? "cmd" : "/bin/sh", + process.platform === "win32" ? ["/c", cmd] : ["-c", cmd], { + windowsVerbatimArguments: true, + env: { NODE_ENV: "development" } + }); + + var rl = readline.createInterface({ + input: p.stdout, + terminal: false + }); + + var start; + var end; + rl.on("line", function (line) { + if (!start) start = Date.now(); + var m = tapRange.exec(line); + if (m) { + expectedTestCount = parseInt(m[2]); + return; } - else if (typeError.test(line)) { - typeErrorCount++; + + if (tapOk.test(line)) { + outFile.write(line.replace(/^ok\s+\d+\s+/, "ok ") + os.EOL); + successCount++; } - else if (debugError.test(line)) { - debugErrorCount++; + else if (tapNotOk.test(line)) { + outFile.write(line.replace(/^not\s+ok\s+\d+\s+/, "not ok ") + os.EOL); + failureCount++; } + else { + m = tapComment.exec(line); + if (m) { + if (m[1] === "tests") { + end = Date.now(); + reportedTestCount = parseInt(m[2]); + } + else if (m[1] === "pass") { + reportedPassCount = parseInt(m[2]); + } + else if (m[1] === "fail") { + reportedFailCount = parseInt(m[2]); + } + else { + outFile.write(line + os.EOL); + } + } + else { + outFile.write(line + os.EOL); + if (typeError.test(line)) { + typeErrorCount++; + } + else if (debugError.test(line)) { + debugErrorCount++; + } + } + return; + } + + testCount++; + + var percentComplete = testCount * 100 / expectedTestCount; + updateProgress(percentComplete); + }); + + p.on("exit", function (status) { + totalReportedTestCount += reportedTestCount; + totalReportedPassCount += reportedPassCount; + totalReportedFailCount += reportedFailCount; + totalTypeErrorCount += typeErrorCount; + totalDebugErrorCount += debugErrorCount; + + var duration = end - start; + var summary = + "pass: " + reportedPassCount + "/" + reportedTestCount + + ", duration: " + (duration / 1000).toFixed(2) + "s"; + + updateProgress(100, summary); + + if (subsetRegexes.length !== 1) { + outFile.close(); + } + + if (status && !errorStatus) { + errorStatus = status; + } + + counter--; + if (counter === 0) { + if (subsetRegexes.length !== 1) { + concatenate(); + } + else { + finish(); + } + } + }); + + function updateProgress(percentComplete, status) { + var title = status || (percentComplete < 100 ? "running..." : "done"); + if (subsetRegexes.length !== 1) { + title = "[" + subsets[i] + "] " + title; + } + + progress.update(percentComplete, + /*foregroundColor*/ failureCount > 0 + ? "red" + : successCount === expectedTestCount + ? "green" + : "cyan", + /*backgroundColor*/ "gray", + title, + i + ); + } + }); + + function concatenate() { + if (outFileNames.length > 0) { + var outFileName = outFileNames.shift(); + var outFile = fs.createReadStream(outFileName); + outFile.pipe(out, { end: false }); + outFile.on("end", function () { + fs.unlinkSync(outFileName); + concatenate(); + }); return; } - testCount++; + finish(); + } - var percentComplete = testCount * 100 / expectedTestCount; - updateProgress(percentComplete); - }); + function finish() { + out.write( + "# tests " + totalReportedTestCount + os.EOL + + "# pass " + totalReportedPassCount + os.EOL + + "# fail " + totalReportedFailCount + os.EOL); + console.log("# tests " + totalReportedTestCount); + console.log("# pass " + totalReportedPassCount); + console.log("# fail " + totalReportedFailCount); - p.on("exit", function (status) { - if (progress.visible) { - updateProgress(100); - process.stdout.write("done." + os.EOL); + if (totalTypeErrorCount) { + console.log("# type errors " + totalTypeErrorCount); } - console.log(comments.join(os.EOL)); - - if (typeErrorCount) { - console.log("# type errors: %s", typeErrorCount); - } - - if (debugErrorCount) { - console.log("# debug errors: %s", debugErrorCount); + if (totalDebugErrorCount) { + console.log("# debug errors " + totalDebugErrorCount); } deleteTemporaryProjectOutput(); - if (status) { - fail("Process exited with code " + status); + if (totalReportedFailCount) { + fail("Test failures reported: " + totalReportedFailCount); } else { complete(); } - }); + } } function runConsoleTests(defaultReporter, defaultSubsets, dirty) { @@ -963,7 +1080,10 @@ task("runtests", ["build-rules", "tests", builtLocalDirectory], function() { }, {async: true}); task("runtests-file", ["build-rules", "tests", builtLocalDirectory], function () { - runTestsAndWriteOutput("tests/baselines/local/testresults.tap"); + runTestsAndWriteOutput("tests/baselines/local/testresults.tap", []); +}, { async: true }); +task("runtests-file-parallel", ["build-rules", "tests", builtLocalDirectory], function () { + runTestsAndWriteOutput("tests/baselines/local/testresults.tap", ["conformance", "compiler", "Projects", "fourslash"]); }, { async: true }); task("runtests-dirty", ["build-rules", "tests", builtLocalDirectory], function () { runConsoleTests("mocha-fivemat-progress-reporter", [], /*dirty*/ true); @@ -1250,8 +1370,9 @@ task("lint-server", ["build-rules"], function() { } }); -function ProgressBar(title) { - this.title = title; +function ProgressBar() { + this._progress = []; + this._lineCount = 0; } ProgressBar.prototype = { progressChars: ["\u0020", "\u2591", "\u2592", "\u2593", "\u2588"], @@ -1280,7 +1401,9 @@ ProgressBar.prototype = { }, reset: "\u001b[0m" }, - update: function (percentComplete, foregroundColor, backgroundColor) { + update: function (percentComplete, foregroundColor, backgroundColor, title, index) { + if (index === undefined) index = 0; + var progress = ""; for (var i = 0; i < 100; i += 4) { progress += this.progressChars[Math.floor(Math.max(0, Math.min(4, percentComplete - i)))]; @@ -1299,20 +1422,13 @@ ProgressBar.prototype = { progress += this.colors.reset; } - if (this._lastProgress !== progress || !this.visible) { - this._print(progress); + if (title) { + progress += " " + title; } - }, - hide: function () { - if (this.visible) { - this._savedProgress = this._lastProgress; - this.clear(); - } - }, - show: function () { - if (this._savedProgress && !this.visible) { - this._print(this._savedProgress); - this._savedProgress = undefined; + + if (this._progress[index] !== progress) { + this._progress[index] = progress; + this._print(index); } }, clear: function () { @@ -1323,12 +1439,22 @@ ProgressBar.prototype = { this.visible = false; } }, - _print: function (progress) { - readline.moveCursor(process.stdout, -process.stdout.columns, 0); - process.stdout.write(this.title ? progress + " " + this.title : progress); - readline.clearLine(process.stdout, 1); - this._lastProgress = progress; - this.visible = true; + _print: function (index) { + readline.moveCursor(process.stdout, -process.stdout.columns, -this._lineCount); + var lineCount = 0; + for (var i = 0; i < this._progress.length; i++) { + if (i === index) { + readline.clearLine(process.stdout, 1); + process.stdout.write(this._progress[i] + os.EOL); + } + else { + readline.moveCursor(process.stdout, -process.stdout.columns, +1); + } + + lineCount++; + } + + this._lineCount = lineCount; } }; From 22f31232282f9a42f0e5fdb598c3938a8fa329ac Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 26 May 2016 01:17:08 -0700 Subject: [PATCH 2/8] Removed most needs to traverse original nodes for emit flags, comments, and source maps. --- src/compiler/factory.ts | 17 ++--- src/compiler/transformer.ts | 117 +++---------------------------- src/compiler/transformers/es6.ts | 5 +- src/compiler/transformers/ts.ts | 7 +- src/compiler/types.ts | 1 - src/compiler/visitor.ts | 2 +- 6 files changed, 26 insertions(+), 123 deletions(-) diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index 40db3736b82..a823306412a 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -22,17 +22,12 @@ namespace ts { return node; } - function updateNode(updated: T, original: T): T { - updated.original = original; - if (original.transformId) { - updated.transformId = original.transformId; - updated.emitFlags = original.emitFlags; - updated.commentRange = original.commentRange; - updated.sourceMapRange = original.sourceMapRange; - } + export function updateNode(updated: T, original: T): T { + setOriginalNode(updated, original); if (original.startsOnNewLine) { updated.startsOnNewLine = true; } + return updated; } @@ -1894,6 +1889,12 @@ namespace ts { export function setOriginalNode(node: T, original: Node): T { node.original = original; + if (original && original.transformId && !node.transformId) { + node.transformId = original.transformId; + node.emitFlags = original.emitFlags; + node.commentRange = original.commentRange; + node.sourceMapRange = original.sourceMapRange; + } return node; } diff --git a/src/compiler/transformer.ts b/src/compiler/transformer.ts index 9e58b02c208..3775ec348f7 100644 --- a/src/compiler/transformer.ts +++ b/src/compiler/transformer.ts @@ -69,15 +69,9 @@ namespace ts { const enabledSyntaxKindFeatures = new Array(SyntaxKind.Count); const sourceTreeNodesWithAnnotations: Node[] = []; - let lastNodeEmitFlagsNode: Node; - let lastNodeEmitFlags: NodeEmitFlags; - let lastSourceMapRangeNode: Node; - let lastSourceMapRange: TextRange; let lastTokenSourceMapRangeNode: Node; let lastTokenSourceMapRangeToken: SyntaxKind; let lastTokenSourceMapRange: TextRange; - let lastCommentMapRangeNode: Node; - let lastCommentMapRange: TextRange; let lexicalEnvironmentStackOffset = 0; let hoistedVariableDeclarations: VariableDeclaration[]; let hoistedFunctionDeclarations: FunctionDeclaration[]; @@ -210,18 +204,12 @@ namespace ts { * @param node The node. */ function beforeSetAnnotation(node: Node) { - if (node.transformId !== transformId) { - node.transformId = transformId; - if ((node.flags & NodeFlags.Synthesized) === 0) { - node.emitFlags = 0; - node.sourceMapRange = undefined; - node.commentRange = undefined; - - // To avoid holding onto transformation artifacts, we keep track of any - // source tree node we are annotating. This allows us to clean them up after - // all transformations have completed. - sourceTreeNodesWithAnnotations.push(node); - } + node.transformId = transformId; + if ((node.flags & NodeFlags.Synthesized) === 0) { + // To avoid holding onto transformation artifacts, we keep track of any + // source tree node we are annotating. This allows us to clean them up after + // all transformations have completed. + sourceTreeNodesWithAnnotations.push(node); } } @@ -234,31 +222,7 @@ namespace ts { * @param node The node. */ function getNodeEmitFlags(node: Node) { - // As a performance optimization, use the cached value of the most recent node. - // This helps for cases where this function is called repeatedly for the same node. - if (lastNodeEmitFlagsNode === node) { - return lastNodeEmitFlags; - } - - // Get the emit flags for a node or from one of its original nodes. - let flags: NodeEmitFlags; - let current = node; - while (current) { - if (current.transformId === transformId) { - const nodeEmitFlags = current.emitFlags; - if (nodeEmitFlags) { - flags = nodeEmitFlags & ~NodeEmitFlags.HasNodeEmitFlags; - break; - } - } - - current = current.original; - } - - // Cache the most recently requested value. - lastNodeEmitFlagsNode = node; - lastNodeEmitFlags = flags; - return flags; + return node.emitFlags & ~NodeEmitFlags.HasNodeEmitFlags; } /** @@ -268,16 +232,7 @@ namespace ts { * @param emitFlags The NodeEmitFlags for the node. */ function setNodeEmitFlags(node: T, emitFlags: NodeEmitFlags) { - // Merge existing flags. - if (emitFlags & NodeEmitFlags.Merge) { - emitFlags = getNodeEmitFlags(node) | (emitFlags & ~NodeEmitFlags.Merge); - } - beforeSetAnnotation(node); - - // Cache the most recently requested value. - lastNodeEmitFlagsNode = node; - lastNodeEmitFlags = emitFlags; node.emitFlags = emitFlags | NodeEmitFlags.HasNodeEmitFlags; return node; } @@ -291,30 +246,7 @@ namespace ts { * @param node The node. */ function getSourceMapRange(node: Node) { - // As a performance optimization, use the cached value of the most recent node. - // This helps for cases where this function is called repeatedly for the same node. - if (lastSourceMapRangeNode === node) { - return lastSourceMapRange || node; - } - - // Get the custom source map range for a node or from one of its original nodes. - let range: TextRange; - let current = node; - while (current) { - if (current.transformId === transformId) { - range = current.sourceMapRange; - if (range !== undefined) { - break; - } - } - - current = current.original; - } - - // Cache the most recently requested value. - lastSourceMapRangeNode = node; - lastSourceMapRange = range; - return range || node; + return node.sourceMapRange || node; } /** @@ -325,10 +257,6 @@ namespace ts { */ function setSourceMapRange(node: T, range: TextRange) { beforeSetAnnotation(node); - - // Cache the most recently requested value. - lastSourceMapRangeNode = node; - lastSourceMapRange = range; node.sourceMapRange = range; return node; } @@ -394,30 +322,7 @@ namespace ts { * @param node The node. */ function getCommentRange(node: Node) { - // As a performance optimization, use the cached value of the most recent node. - // This helps for cases where this function is called repeatedly for the same node. - if (lastCommentMapRangeNode === node) { - return lastCommentMapRange || node; - } - - // Get the custom comment range for a node or from one of its original nodes. - let range: TextRange; - let current = node; - while (current) { - if (current.transformId === transformId) { - range = current.commentRange; - if (range !== undefined) { - break; - } - } - - current = current.original; - } - - // Cache the most recently requested value. - lastCommentMapRangeNode = node; - lastCommentMapRange = range; - return range || node; + return node.commentRange || node; } /** @@ -425,10 +330,6 @@ namespace ts { */ function setCommentRange(node: T, range: TextRange) { beforeSetAnnotation(node); - - // Cache the most recently requested value. - lastCommentMapRangeNode = node; - lastCommentMapRange = range; node.commentRange = range; return node; } diff --git a/src/compiler/transformers/es6.ts b/src/compiler/transformers/es6.ts index 527c3b00572..16d08298f72 100644 --- a/src/compiler/transformers/es6.ts +++ b/src/compiler/transformers/es6.ts @@ -956,6 +956,7 @@ namespace ts { * @param initializer The initializer for the parameter. */ function addDefaultValueAssignmentForInitializer(statements: Statement[], parameter: ParameterDeclaration, name: Identifier, initializer: Expression): void { + initializer = visitNode(initializer, visitor, isExpression); const statement = createIf( createStrictEquality( getSynthesizedClone(name), @@ -966,7 +967,7 @@ namespace ts { createStatement( createAssignment( setNodeEmitFlags(getMutableClone(name), NodeEmitFlags.NoSourceMap), - setNodeEmitFlags(visitNode(initializer, visitor, isExpression), NodeEmitFlags.NoSourceMap | NodeEmitFlags.Merge), + setNodeEmitFlags(initializer, NodeEmitFlags.NoSourceMap | getNodeEmitFlags(initializer)), /*location*/ parameter ) ) @@ -1533,7 +1534,7 @@ namespace ts { // the source map range for the declaration list. const firstDeclaration = firstOrUndefined(declarations); const lastDeclaration = lastOrUndefined(declarations); - setSourceMapRange(node, createRange(firstDeclaration.pos, lastDeclaration.end)); + setSourceMapRange(declarationList, createRange(firstDeclaration.pos, lastDeclaration.end)); } return declarationList; diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index 469c2fe8aba..e0ad54eb377 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -2243,10 +2243,10 @@ namespace ts { // While we emit the source map for the node after skipping decorators and modifiers, // we need to emit the comments for the original range. + setOriginalNode(parameter, node); setCommentRange(parameter, node); setSourceMapRange(parameter, moveRangePastModifiers(node)); setNodeEmitFlags(parameter.name, NodeEmitFlags.NoTrailingSourceMap); - setOriginalNode(parameter, node); return parameter; } @@ -2564,6 +2564,8 @@ namespace ts { ] ); + setOriginalNode(statement, /*original*/ node); + // Adjust the source map emit to match the old emitter. if (node.kind === SyntaxKind.EnumDeclaration) { setSourceMapRange(statement.declarationList, node); @@ -2592,7 +2594,6 @@ namespace ts { // setCommentRange(statement, node); setNodeEmitFlags(statement, NodeEmitFlags.NoTrailingComments); - setOriginalNode(statement, /*original*/ node); statements.push(statement); } @@ -2748,7 +2749,7 @@ namespace ts { } function disableCommentsRecursive(node: Node) { - setNodeEmitFlags(node, NodeEmitFlags.NoComments | NodeEmitFlags.Merge); + setNodeEmitFlags(node, NodeEmitFlags.NoComments | getNodeEmitFlags(node)); forEachChild(node, disableCommentsRecursive); } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index a7c21d4b70e..21b89707460 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3002,7 +3002,6 @@ namespace ts { ExportName = 1 << 17, // Ensure an export prefix is added for an identifier that points to an exported declaration with a local name (see SymbolFlags.ExportHasLocal). LocalName = 1 << 18, // Ensure an export prefix is not added for an identifier that points to an exported declaration. Indented = 1 << 19, // Adds an explicit extra indentation level for class and function bodies when printing (used to match old emitter). - Merge = 1 << 20, // When getting emit options, merge with existing emit options. // SourceMap Specialization. // TODO(rbuckton): These should be removed once source maps are aligned with the old diff --git a/src/compiler/visitor.ts b/src/compiler/visitor.ts index 856571d221b..a7af4e6a1e4 100644 --- a/src/compiler/visitor.ts +++ b/src/compiler/visitor.ts @@ -1044,7 +1044,7 @@ namespace ts { } if (updated !== node) { - updated.original = node; + updateNode(updated, node); } // performance.measure(measureName, markName); From 87fc46c013691b97e846ea84c140ab19602b8746 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 26 May 2016 10:34:34 -0700 Subject: [PATCH 3/8] Moved responsibility for consuming comment ranges. --- src/compiler/comments.ts | 59 ++++------------------------------------ src/compiler/scanner.ts | 16 +++++++---- 2 files changed, 16 insertions(+), 59 deletions(-) diff --git a/src/compiler/comments.ts b/src/compiler/comments.ts index dbe2299ca7a..9ed15335541 100644 --- a/src/compiler/comments.ts +++ b/src/compiler/comments.ts @@ -31,7 +31,7 @@ namespace ts { // This maps start->end for a comment range. See `hasConsumedCommentRange` and // `consumeCommentRange` for usage. - let consumedCommentRanges: Map; + let consumedCommentRanges: Map; let leadingCommentRangePositions: Map; let trailingCommentRangePositions: Map; @@ -157,8 +157,8 @@ namespace ts { leadingCommentRangePositions[pos] = true; const comments = hasDetachedComments(pos) ? getLeadingCommentsWithoutDetachedComments() - : getLeadingCommentRanges(currentText, pos); - return consumeCommentRanges(comments); + : getLeadingCommentRanges(currentText, pos, consumedCommentRanges); + return comments; } function getTrailingCommentsOfPosition(pos: number) { @@ -167,8 +167,8 @@ namespace ts { } trailingCommentRangePositions[pos] = true; - const comments = getTrailingCommentRanges(currentText, pos); - return consumeCommentRanges(comments); + const comments = getTrailingCommentRanges(currentText, pos, consumedCommentRanges); + return comments; } function emitLeadingComments(range: TextRange, comments: CommentRange[]): void; @@ -211,53 +211,6 @@ namespace ts { range = collapseRangeToEnd(range); emitLeadingComments(range, getLeadingComments(range)); } - - function hasConsumedCommentRange(comment: CommentRange) { - return comment.end === consumedCommentRanges[comment.pos]; - } - - function consumeCommentRange(comment: CommentRange) { - if (!hasConsumedCommentRange(comment)) { - consumedCommentRanges[comment.pos] = comment.end; - return true; - } - - return false; - } - - function consumeCommentRanges(comments: CommentRange[]) { - let consumed: CommentRange[]; - if (comments) { - let commentsSkipped = 0; - let commentsConsumed = 0; - for (let i = 0; i < comments.length; i++) { - const comment = comments[i]; - if (consumeCommentRange(comment)) { - commentsConsumed++; - if (commentsSkipped !== 0) { - if (consumed === undefined) { - consumed = [comment]; - } - else { - consumed.push(comment); - } - } - } - else { - commentsSkipped++; - if (commentsConsumed !== 0 && consumed === undefined) { - consumed = comments.slice(0, i); - } - } - } - - if (commentsConsumed) { - return consumed || comments; - } - } - - return noComments; - } } function createCommentWriterWithExtendedDiagnostics(writer: CommentWriter): CommentWriter { @@ -344,7 +297,7 @@ namespace ts { function getLeadingCommentsWithoutDetachedComments() { // get the leading comments from detachedPos const pos = lastOrUndefined(detachedCommentsInfo).detachedCommentEndPos; - const leadingComments = getLeadingCommentRanges(currentText, pos); + const leadingComments = getLeadingCommentRanges(currentText, pos, consumedCommentRanges); if (detachedCommentsInfo.length - 1) { detachedCommentsInfo.pop(); } diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index ec3b1234adf..254923a981e 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -591,7 +591,7 @@ namespace ts { * and the next token are returned. * If true, comments occurring between the given position and the next line break are returned. */ - function getCommentRanges(text: string, pos: number, trailing: boolean): CommentRange[] { + function getCommentRanges(text: string, pos: number, trailing: boolean, consumedCommentRanges?: Map): CommentRange[] { let result: CommentRange[]; let collecting = trailing || pos === 0; while (pos >= 0 && pos < text.length) { @@ -643,13 +643,17 @@ namespace ts { } } - if (collecting) { + if (collecting && (!consumedCommentRanges || !(startPos in consumedCommentRanges))) { if (!result) { result = []; } result.push({ pos: startPos, end: pos, hasTrailingNewLine, kind }); + if (consumedCommentRanges) { + consumedCommentRanges[startPos] = true; + } } + continue; } break; @@ -669,12 +673,12 @@ namespace ts { return result; } - export function getLeadingCommentRanges(text: string, pos: number): CommentRange[] { - return getCommentRanges(text, pos, /*trailing*/ false); + export function getLeadingCommentRanges(text: string, pos: number, consumedCommentRanges?: Map): CommentRange[] { + return getCommentRanges(text, pos, /*trailing*/ false, consumedCommentRanges); } - export function getTrailingCommentRanges(text: string, pos: number): CommentRange[] { - return getCommentRanges(text, pos, /*trailing*/ true); + export function getTrailingCommentRanges(text: string, pos: number, consumedCommentRanges?: Map): CommentRange[] { + return getCommentRanges(text, pos, /*trailing*/ true, consumedCommentRanges); } /** Optionally, get the shebang */ From a1518d324fdd3253152c14f694759abeacdff903 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 26 May 2016 22:12:22 -0700 Subject: [PATCH 4/8] Clean up parallel test runs in runtests-file --- Jakefile.js | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/Jakefile.js b/Jakefile.js index 37952afaf0f..70f62250121 100644 --- a/Jakefile.js +++ b/Jakefile.js @@ -766,15 +766,22 @@ function runTestsAndWriteOutput(file, defaultSubsets) { var subsetRegexes; var subsets; - if (defaultSubsets.length === 0) { + if (tests || defaultSubsets.length === 0) { subsetRegexes = [tests]; subsets = [tests]; } else { - subsets = tests ? tests.split("|") : defaultSubsets; - subsetRegexes = subsets.map(function (sub) { return "^" + sub + ".*$"; }); - subsetRegexes.push("^(?!" + subsets.join("|") + ").*$"); + subsets = []; + subsetRegexes = []; + negations = []; + for (const subset of defaultSubsets) { + subsets.push(subset.name); + subsetRegexes.push(subset.pattern); + negations.push(subset.pattern); + } + subsets.push("other"); + subsetRegexes.push("^(?!" + negations.join("|") + ")"); } var out = fs.createWriteStream(file); @@ -1080,11 +1087,23 @@ task("runtests", ["build-rules", "tests", builtLocalDirectory], function() { }, {async: true}); task("runtests-file", ["build-rules", "tests", builtLocalDirectory], function () { - runTestsAndWriteOutput("tests/baselines/local/testresults.tap", []); -}, { async: true }); -task("runtests-file-parallel", ["build-rules", "tests", builtLocalDirectory], function () { - runTestsAndWriteOutput("tests/baselines/local/testresults.tap", ["conformance", "compiler", "Projects", "fourslash"]); + var subsets = []; + var cores = os.cpus().length; + if (cores > 1) { + subsets.push({ name: "conformance", pattern: "^conformance\\b" }); + subsets.push({ name: "compiler", pattern: "^compiler\\b" }); + subsets.push({ name: "projects", pattern: "^Projects\\b" }); + if (cores > 4) { + subsets.push({ name: "fourslash", pattern: "^fourslash\\b" }); + subsets.push({ name: "fourslash (shims, shims-pp, server)", pattern: "^fourslash-" }); + } + else { + subsets.push({ name: "fourslash", pattern: "^fourslash" }); + } + } + runTestsAndWriteOutput("tests/baselines/local/testresults.tap", subsets); }, { async: true }); + task("runtests-dirty", ["build-rules", "tests", builtLocalDirectory], function () { runConsoleTests("mocha-fivemat-progress-reporter", [], /*dirty*/ true); }, { async: true }); From 0dc261d4a2b601bbfbaf200d0461fe51351220da Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 26 May 2016 22:12:42 -0700 Subject: [PATCH 5/8] Performance improvements in comment emit. --- src/compiler/comments.ts | 390 ++++++++------------- src/compiler/printer.ts | 122 ++++--- src/compiler/scanner.ts | 15 +- src/compiler/transformers/es6.ts | 1 + src/compiler/transformers/module/module.ts | 9 +- src/compiler/transformers/ts.ts | 156 +++++---- src/compiler/utilities.ts | 9 +- 7 files changed, 336 insertions(+), 366 deletions(-) diff --git a/src/compiler/comments.ts b/src/compiler/comments.ts index 9ed15335541..e8db28d90db 100644 --- a/src/compiler/comments.ts +++ b/src/compiler/comments.ts @@ -5,18 +5,9 @@ namespace ts { export interface CommentWriter { reset(): void; setSourceFile(sourceFile: SourceFile): void; - getLeadingComments(range: TextRange): CommentRange[]; - getLeadingComments(range: TextRange, contextNode: Node, ignoreNodeCallback: (contextNode: Node) => boolean, getTextRangeCallback: (contextNode: Node) => TextRange): CommentRange[]; - getTrailingComments(range: TextRange): CommentRange[]; - getTrailingComments(range: TextRange, contextNode: Node, ignoreNodeCallback: (contextNode: Node) => boolean, getTextRangeCallback: (contextNode: Node) => TextRange): CommentRange[]; - getTrailingCommentsOfPosition(pos: number): CommentRange[]; - emitLeadingComments(range: TextRange, comments: CommentRange[]): void; - emitLeadingComments(range: TextRange, comments: CommentRange[], contextNode: Node, getTextRangeCallback: (contextNode: Node) => TextRange): void; - emitTrailingComments(range: TextRange, comments: CommentRange[]): void; - emitLeadingDetachedComments(range: TextRange): void; - emitLeadingDetachedComments(range: TextRange, contextNode: Node, ignoreNodeCallback: (contextNode: Node) => boolean): void; - emitTrailingDetachedComments(range: TextRange): void; - emitTrailingDetachedComments(range: TextRange, contextNode: Node, ignoreNodeCallback: (contextNode: Node) => boolean): void; + emitNodeWithComments(node: Node, emitCallback: (node: Node) => void): void; + emitBodyWithDetachedComments(node: Node, detachedRange: TextRange, emitCallback: (node: Node) => void): void; + emitTrailingCommentsOfPosition(pos: number): void; } export function createCommentWriter(host: EmitHost, writer: EmitTextWriter, sourceMap: SourceMapWriter): CommentWriter { @@ -24,250 +15,177 @@ namespace ts { const newLine = host.getNewLine(); const { emitPos } = sourceMap; + let containerPos = -1; + let containerEnd = -1; + let declarationListContainerEnd = -1; let currentSourceFile: SourceFile; let currentText: string; let currentLineMap: number[]; let detachedCommentsInfo: { nodePos: number, detachedCommentEndPos: number}[]; - // This maps start->end for a comment range. See `hasConsumedCommentRange` and - // `consumeCommentRange` for usage. + // Tracks comment ranges that have already been consumed. let consumedCommentRanges: Map; - let leadingCommentRangePositions: Map; - let trailingCommentRangePositions: Map; - const commentWriter = compilerOptions.removeComments - ? createCommentRemovingWriter() - : createCommentPreservingWriter(); + return { + reset, + setSourceFile, + emitNodeWithComments, + emitBodyWithDetachedComments, + emitTrailingCommentsOfPosition, + }; - return compilerOptions.extendedDiagnostics - ? createCommentWriterWithExtendedDiagnostics(commentWriter) - : commentWriter; - - function createCommentRemovingWriter(): CommentWriter { - return { - reset, - setSourceFile, - getLeadingComments(range: TextRange, contextNode?: Node, ignoreNodeCallback?: (contextNode: Node) => boolean, getTextRangeCallback?: (contextNode: Node) => TextRange): CommentRange[] { return undefined; }, - getTrailingComments(range: TextRange, contextNode?: Node, ignoreNodeCallback?: (contextNode: Node) => boolean, getTextRangeCallback?: (contextNode: Node) => TextRange): CommentRange[] { return undefined; }, - getTrailingCommentsOfPosition(pos: number): CommentRange[] { return undefined; }, - emitLeadingComments(range: TextRange, comments: CommentRange[], contextNode?: Node, getTextRangeCallback?: (contextNode: Node) => TextRange): void { }, - emitTrailingComments(range: TextRange, comments: CommentRange[]): void { }, - emitLeadingDetachedComments, - emitTrailingDetachedComments(range: TextRange, contextNode?: Node, ignoreNodeCallback?: (contextNode: Node) => boolean): void {} - }; - - function emitLeadingDetachedComments(node: TextRange, contextNode?: Node, ignoreNodeCallback?: (contextNode: Node) => boolean): void { - if (ignoreNodeCallback && ignoreNodeCallback(contextNode)) { - return; - } - - emitDetachedCommentsAndUpdateCommentsInfo(node, /*removeComments*/ true); + function emitNodeWithComments(node: Node, emitCallback: (node: Node) => void) { + if (compilerOptions.removeComments) { + emitCallback(node); + return; } - } - function createCommentPreservingWriter(): CommentWriter { - const noComments: CommentRange[] = []; - return { - reset, - setSourceFile, - getLeadingComments, - getTrailingComments, - getTrailingCommentsOfPosition, - emitLeadingComments, - emitTrailingComments, - emitLeadingDetachedComments, - emitTrailingDetachedComments - }; + if (node) { + const { pos, end } = node.commentRange || node; + if ((pos < 0 && end < 0) || (pos === end)) { + // Both pos and end are synthesized, so just emit the node without comments. + emitCallback(node); + } + else { + const emitFlags = node.emitFlags; + const isEmittedNode = node.kind !== SyntaxKind.NotEmittedStatement; + const skipLeadingComments = pos < 0 || (emitFlags & NodeEmitFlags.NoLeadingComments) !== 0; + const skipTrailingComments = end < 0 || (emitFlags & NodeEmitFlags.NoTrailingComments) !== 0; - function getLeadingComments(range: TextRange): CommentRange[]; - function getLeadingComments(range: TextRange, contextNode: Node, ignoreNodeCallback: (contextNode: Node) => boolean, getTextRangeCallback: (contextNode: Node) => TextRange): CommentRange[]; - function getLeadingComments(range: TextRange, contextNode?: Node, ignoreNodeCallback?: (contextNode: Node) => boolean, getTextRangeCallback?: (contextNode: Node) => TextRange) { - let comments: CommentRange[] = []; - let ignored = false; - if (contextNode) { - range = getTextRangeCallback(contextNode) || range; - if (ignoreNodeCallback(contextNode)) { - ignored = true; - // If the node will not be emitted in JS, remove all the comments (normal, - // pinned and `///`) associated with the node, unless it is a triple slash - // comment at the top of the file. - // - // For Example: - // /// - // declare var x; - // /// - // interface F {} - // - // The first `///` will NOT be removed while the second one will be removed - // even though both nodes will not be emitted. - if (range.pos === 0) { - comments = filter(getLeadingCommentsOfPosition(0), isTripleSlashComment); + // Emit leading comments if the position is not synthesized and the node + // has not opted out from emitting leading comments. + if (!skipLeadingComments) { + emitLeadingComments(pos, isEmittedNode); + } + + // Save current container state on the stack. + const savedContainerPos = containerPos; + const savedContainerEnd = containerEnd; + const savedDeclarationListContainerEnd = declarationListContainerEnd; + + if (!skipLeadingComments) { + containerPos = pos; + } + + if (!skipTrailingComments) { + containerEnd = end; + + // To avoid invalid comment emit in a down-level binding pattern, we + // keep track of the last declaration list container's end + if (node.kind === SyntaxKind.VariableDeclarationList) { + declarationListContainerEnd = end; } } - } - if (!ignored) { - comments = getLeadingCommentsOfPosition(range.pos); - } + emitCallback(node); - return comments; - } + // Restore previous container state. + containerPos = savedContainerPos; + containerEnd = savedContainerEnd; + declarationListContainerEnd = savedDeclarationListContainerEnd; - /** - * Determine if the given comment is a triple-slash - **/ - function isTripleSlashComment(comment: CommentRange) { - // Verify this is /// comment, but do the regexp match only when we first can find /// in the comment text - // so that we don't end up computing comment string and doing match for all // comments - if (currentText.charCodeAt(comment.pos + 1) === CharacterCodes.slash && - comment.pos + 2 < comment.end && - currentText.charCodeAt(comment.pos + 2) === CharacterCodes.slash) { - const textSubStr = currentText.substring(comment.pos, comment.end); - return fullTripleSlashReferencePathRegEx.test(textSubStr) - || fullTripleSlashAMDReferencePathRegEx.test(textSubStr); - } - return false; - } - - function getTrailingComments(range: TextRange): CommentRange[]; - function getTrailingComments(range: TextRange, contextNode: Node, ignoreNodeCallback: (contextNode: Node) => boolean, getTextRangeCallback: (contextNode: Node) => TextRange): CommentRange[]; - function getTrailingComments(range: TextRange, contextNode?: Node, ignoreNodeCallback?: (contextNode: Node) => boolean, getTextRangeCallback?: (contextNode: Node) => TextRange) { - let ignored = false; - if (contextNode) { - if (ignoreNodeCallback(contextNode)) { - ignored = true; - } - else { - range = getTextRangeCallback(contextNode) || range; + // Emit trailing comments if the position is not synthesized and the node + // has not opted out from emitting leading comments and is an emitted node. + if (!skipTrailingComments && isEmittedNode) { + emitTrailingComments(end); } } - - let comments: CommentRange[]; - if (!ignored) { - comments = getTrailingCommentsOfPosition(range.end); - } - return comments; - } - - function getLeadingCommentsOfPosition(pos: number) { - if (positionIsSynthesized(pos) || leadingCommentRangePositions[pos]) { - return undefined; - } - - leadingCommentRangePositions[pos] = true; - const comments = hasDetachedComments(pos) - ? getLeadingCommentsWithoutDetachedComments() - : getLeadingCommentRanges(currentText, pos, consumedCommentRanges); - return comments; - } - - function getTrailingCommentsOfPosition(pos: number) { - if (positionIsSynthesized(pos) || trailingCommentRangePositions[pos]) { - return undefined; - } - - trailingCommentRangePositions[pos] = true; - const comments = getTrailingCommentRanges(currentText, pos, consumedCommentRanges); - return comments; - } - - function emitLeadingComments(range: TextRange, comments: CommentRange[]): void; - function emitLeadingComments(range: TextRange, comments: CommentRange[], contextNode: Node, getTextRangeCallback: (contextNode: Node) => TextRange): void; - function emitLeadingComments(range: TextRange, comments: CommentRange[], contextNode?: Node, getTextRangeCallback?: (contextNode: Node) => TextRange) { - if (comments && comments.length > 0) { - if (contextNode) { - range = getTextRangeCallback(contextNode) || range; - } - - emitNewLineBeforeLeadingComments(currentLineMap, writer, range, comments); - - // Leading comments are emitted at /*leading comment1 */space/*leading comment*/space - emitComments(currentText, currentLineMap, writer, comments, /*leadingSeparator*/ false, /*trailingSeparator*/ true, newLine, writeComment); - } - } - - function emitTrailingComments(range: TextRange, comments: CommentRange[]) { - // trailing comments are emitted at space/*trailing comment1 */space/*trailing comment*/ - emitComments(currentText, currentLineMap, writer, comments, /*leadingSeparator*/ true, /*trailingSeparator*/ false, newLine, writeComment); - } - - function emitLeadingDetachedComments(range: TextRange): void; - function emitLeadingDetachedComments(range: TextRange, contextNode: Node, ignoreNodeCallback: (node: Node) => boolean): void; - function emitLeadingDetachedComments(range: TextRange, contextNode?: Node, ignoreNodeCallback?: (node: Node) => boolean): void { - if (contextNode && ignoreNodeCallback(contextNode)) { - return; - } - - emitDetachedCommentsAndUpdateCommentsInfo(range, /*removeComments*/ false); - } - - function emitTrailingDetachedComments(range: TextRange): void; - function emitTrailingDetachedComments(range: TextRange, contextNode: Node, ignoreNodeCallback?: (node: Node) => boolean): void; - function emitTrailingDetachedComments(range: TextRange, contextNode?: Node, ignoreNodeCallback?: (node: Node) => boolean): void { - if (contextNode && ignoreNodeCallback(contextNode)) { - return; - } - - range = collapseRangeToEnd(range); - emitLeadingComments(range, getLeadingComments(range)); } } - function createCommentWriterWithExtendedDiagnostics(writer: CommentWriter): CommentWriter { - const { - reset, - setSourceFile, - getLeadingComments, - getTrailingComments, - getTrailingCommentsOfPosition, - emitLeadingComments, - emitTrailingComments, - emitLeadingDetachedComments, - emitTrailingDetachedComments - } = writer; + function emitBodyWithDetachedComments(node: Node, detachedRange: TextRange, emitCallback: (node: Node) => void) { + const { pos, end } = detachedRange; + const emitFlags = node.emitFlags; + const skipLeadingComments = pos < 0 || (emitFlags & NodeEmitFlags.NoLeadingComments) !== 0; + const skipTrailingComments = end < 0 || (emitFlags & NodeEmitFlags.NoTrailingComments) !== 0 || compilerOptions.removeComments; - return { - reset, - setSourceFile, - getLeadingComments(range: TextRange, contextNode?: Node, ignoreNodeCallback?: (contextNode: Node) => boolean, getTextRangeCallback?: (contextNode: Node) => TextRange): CommentRange[] { - performance.mark("commentStart"); - const comments = getLeadingComments(range, contextNode, ignoreNodeCallback, getTextRangeCallback); - performance.measure("commentTime", "commentStart"); - return comments; - }, - getTrailingComments(range: TextRange, contextNode?: Node, ignoreNodeCallback?: (contextNode: Node) => boolean, getTextRangeCallback?: (contextNode: Node) => TextRange): CommentRange[] { - performance.mark("commentStart"); - const comments = getTrailingComments(range, contextNode, ignoreNodeCallback, getTextRangeCallback); - performance.measure("commentTime", "commentStart"); - return comments; - }, - getTrailingCommentsOfPosition(pos: number): CommentRange[] { - performance.mark("commentStart"); - const comments = getTrailingCommentsOfPosition(pos); - performance.measure("commentTime", "commentStart"); - return comments; - }, - emitLeadingComments(range: TextRange, comments: CommentRange[], contextNode?: Node, getTextRangeCallback?: (contextNode: Node) => TextRange): void { - performance.mark("commentStart"); - emitLeadingComments(range, comments, contextNode, getTextRangeCallback); - performance.measure("commentTime", "commentStart"); - }, - emitTrailingComments(range: TextRange, comments: CommentRange[]): void { - performance.mark("commentStart"); - emitLeadingComments(range, comments); - performance.measure("commentTime", "commentStart"); - }, - emitLeadingDetachedComments(range: TextRange, contextNode?: Node, ignoreNodeCallback?: (contextNode: Node) => boolean): void { - performance.mark("commentStart"); - emitLeadingDetachedComments(range, contextNode, ignoreNodeCallback); - performance.measure("commentTime", "commentStart"); - }, - emitTrailingDetachedComments(range: TextRange, contextNode?: Node, ignoreNodeCallback?: (contextNode: Node) => boolean): void { - performance.mark("commentStart"); - emitTrailingDetachedComments(range, contextNode, ignoreNodeCallback); - performance.measure("commentTime", "commentStart"); + if (!skipLeadingComments) { + emitDetachedCommentsAndUpdateCommentsInfo(detachedRange, compilerOptions.removeComments); + } + + emitCallback(node); + + if (!skipTrailingComments) { + emitLeadingComments(detachedRange.end, /*isEmittedNode*/ true); + } + } + + function emitLeadingComments(pos: number, isEmittedNode: boolean) { + let leadingComments: CommentRange[]; + if (isEmittedNode) { + leadingComments = getLeadingCommentsToEmit(pos); + } + else { + // If the node will not be emitted in JS, remove all the comments(normal, pinned and ///) associated with the node, + // unless it is a triple slash comment at the top of the file. + // For Example: + // /// + // declare var x; + // /// + // interface F {} + // The first /// will NOT be removed while the second one will be removed even though both node will not be emitted + if (pos === 0) { + leadingComments = filter(getLeadingCommentsToEmit(pos), isTripleSlashComment); } - }; + } + + emitNewLineBeforeLeadingCommentsOfPosition(currentLineMap, writer, pos, leadingComments); + + // Leading comments are emitted at /*leading comment1 */space/*leading comment*/space + emitComments(currentText, currentLineMap, writer, leadingComments, /*leadingSeparator*/ false, /*trailingSeparator*/ true, newLine, writeComment); + } + + function emitTrailingComments(pos: number) { + const trailingComments = getTrailingCommentsToEmit(pos); + + // trailing comments are emitted at space/*trailing comment1 */space/*trailing comment*/ + emitComments(currentText, currentLineMap, writer, trailingComments, /*leadingSeparator*/ true, /*trailingSeparator*/ false, newLine, writeComment); + } + + function emitTrailingCommentsOfPosition(pos: number) { + if (compilerOptions.removeComments) { + return; + } + + const trailingComments = getTrailingCommentsToEmit(pos); + + // trailing comments of a position are emitted at /*trailing comment1 */space/*trailing comment*/space + emitComments(currentText, currentLineMap, writer, trailingComments, /*leadingSeparator*/ false, /*trailingSeparator*/ true, newLine, writeComment); + } + + function getLeadingCommentsToEmit(pos: number) { + // Emit the leading comments only if the container's pos doesn't match because the container should take care of emitting these comments + if (containerPos === -1 || pos !== containerPos) { + return hasDetachedComments(pos) + ? getLeadingCommentsWithoutDetachedComments() + : getLeadingCommentRanges(currentText, pos); + } + } + + function getTrailingCommentsToEmit(end: number) { + // Emit the trailing comments only if the container's end doesn't match because the container should take care of emitting these comments + if (containerEnd === -1 || (end !== containerEnd && end !== declarationListContainerEnd)) { + return getTrailingCommentRanges(currentText, end); + } + } + + /** + * Determine if the given comment is a triple-slash + * + * @return true if the comment is a triple-slash comment else false + **/ + function isTripleSlashComment(comment: CommentRange) { + // Verify this is /// comment, but do the regexp match only when we first can find /// in the comment text + // so that we don't end up computing comment string and doing match for all // comments + if (currentText.charCodeAt(comment.pos + 1) === CharacterCodes.slash && + comment.pos + 2 < comment.end && + currentText.charCodeAt(comment.pos + 2) === CharacterCodes.slash) { + const textSubStr = currentText.substring(comment.pos, comment.end); + return textSubStr.match(fullTripleSlashReferencePathRegEx) || + textSubStr.match(fullTripleSlashAMDReferencePathRegEx) ? + true : false; + } + return false; } function reset() { @@ -276,8 +194,6 @@ namespace ts { currentLineMap = undefined; detachedCommentsInfo = undefined; consumedCommentRanges = undefined; - trailingCommentRangePositions = undefined; - leadingCommentRangePositions = undefined; } function setSourceFile(sourceFile: SourceFile) { @@ -286,8 +202,6 @@ namespace ts { currentLineMap = getLineStarts(currentSourceFile); detachedCommentsInfo = undefined; consumedCommentRanges = {}; - leadingCommentRangePositions = {}; - trailingCommentRangePositions = {}; } function hasDetachedComments(pos: number) { @@ -297,7 +211,7 @@ namespace ts { function getLeadingCommentsWithoutDetachedComments() { // get the leading comments from detachedPos const pos = lastOrUndefined(detachedCommentsInfo).detachedCommentEndPos; - const leadingComments = getLeadingCommentRanges(currentText, pos, consumedCommentRanges); + const leadingComments = getLeadingCommentRanges(currentText, pos); if (detachedCommentsInfo.length - 1) { detachedCommentsInfo.pop(); } diff --git a/src/compiler/printer.ts b/src/compiler/printer.ts index 12d399b04d8..e730334515a 100644 --- a/src/compiler/printer.ts +++ b/src/compiler/printer.ts @@ -161,13 +161,9 @@ const _super = (function (geti, seti) { const comments = createCommentWriter(host, writer, sourceMap); const { - getLeadingComments, - getTrailingComments, - getTrailingCommentsOfPosition, - emitLeadingComments, - emitTrailingComments, - emitLeadingDetachedComments, - emitTrailingDetachedComments + emitNodeWithComments, + emitBodyWithDetachedComments, + emitTrailingCommentsOfPosition } = comments; let context: TransformationContext; @@ -175,7 +171,6 @@ const _super = (function (geti, seti) { let setNodeEmitFlags: (node: Node, flags: NodeEmitFlags) => void; let getSourceMapRange: (node: Node) => TextRange; let getTokenSourceMapRange: (node: Node, token: SyntaxKind) => TextRange; - let getCommentRange: (node: Node) => TextRange; let isSubstitutionEnabled: (node: Node) => boolean; let isEmitNotificationEnabled: (node: Node) => boolean; let onSubstituteNode: (node: Node, isExpression: boolean) => Node; @@ -240,7 +235,6 @@ const _super = (function (geti, seti) { setNodeEmitFlags = undefined; getSourceMapRange = undefined; getTokenSourceMapRange = undefined; - getCommentRange = undefined; isSubstitutionEnabled = undefined; isEmitNotificationEnabled = undefined; onSubstituteNode = undefined; @@ -262,7 +256,6 @@ const _super = (function (geti, seti) { setNodeEmitFlags = context.setNodeEmitFlags; getSourceMapRange = context.getSourceMapRange; getTokenSourceMapRange = context.getTokenSourceMapRange; - getCommentRange = context.getCommentRange; isSubstitutionEnabled = context.isSubstitutionEnabled; isEmitNotificationEnabled = context.isEmitNotificationEnabled; onSubstituteNode = context.onSubstituteNode; @@ -276,7 +269,7 @@ const _super = (function (geti, seti) { currentFileIdentifiers = node.identifiers; sourceMap.setSourceFile(node); comments.setSourceFile(node); - emitNodeWithNotificationOption(node, emitWorker); + emitNodeWithNotification(node, emitWorker); return node; } @@ -291,7 +284,7 @@ const _super = (function (geti, seti) { * Emits a node. */ function emit(node: Node) { - emitNodeWithNotificationOption(node, emitWithoutNotificationOption); + emitNodeWithNotification(node, emitWithComments); } /** @@ -315,51 +308,71 @@ const _super = (function (geti, seti) { } /** - * Emits a node without calling onEmitNode. - * NOTE: Do not call this method directly. + * Emits a node with comments. + * + * NOTE: Do not call this method directly. It is part of the emit pipeline + * and should only be called indirectly from emit. */ - function emitWithoutNotificationOption(node: Node) { - emitNodeWithWorker(node, emitWorker); + function emitWithComments(node: Node) { + emitNodeWithComments(node, emitWithSourceMap); + } + + /** + * Emits a node with source maps. + * + * NOTE: Do not call this method directly. It is part of the emit pipeline + * and should only be called indirectly from emitWithComments. + */ + function emitWithSourceMap(node: Node) { + emitNodeWithSourceMap(node, emitWorker); } /** * Emits an expression node. */ function emitExpression(node: Expression) { - emitNodeWithNotificationOption(node, emitExpressionWithoutNotificationOption); + emitNodeWithNotification(node, emitExpressionWithComments); } /** - * Emits an expression without calling onEmitNode. - * NOTE: Do not call this method directly. + * Emits an expression with comments. + * + * NOTE: Do not call this method directly. It is part of the emitExpression pipeline + * and should only be called indirectly from emitExpression. */ - function emitExpressionWithoutNotificationOption(node: Expression) { - emitNodeWithWorker(node, emitExpressionWorker); + function emitExpressionWithComments(node: Expression) { + emitNodeWithComments(node, emitExpressionWithSourceMap); + } + + /** + * Emits an expression with source maps. + * + * NOTE: Do not call this method directly. It is part of the emitExpression pipeline + * and should only be called indirectly from emitExpressionWithComments. + */ + function emitExpressionWithSourceMap(node: Expression) { + emitNodeWithSourceMap(node, emitExpressionWorker); } /** * Emits a node with emit notification if available. */ - function emitNodeWithNotificationOption(node: Node, emit: (node: Node) => void) { + function emitNodeWithNotification(node: Node, emitCallback: (node: Node) => void) { if (node) { if (isEmitNotificationEnabled(node)) { - onEmitNode(node, emit); + onEmitNode(node, emitCallback); } else { - emit(node); + emitCallback(node); } } } - function emitNodeWithWorker(node: Node, emitWorker: (node: Node) => void) { + function emitNodeWithSourceMap(node: Node, emitCallback: (node: Node) => void) { if (node) { - const leadingComments = getLeadingComments(/*range*/ node, /*contextNode*/ node, shouldSkipLeadingCommentsForNode, getCommentRange); - const trailingComments = getTrailingComments(/*range*/ node, /*contextNode*/ node, shouldSkipTrailingCommentsForNode, getCommentRange); - emitLeadingComments(/*range*/ node, leadingComments, /*contextNode*/ node, getCommentRange); emitStart(/*range*/ node, /*contextNode*/ node, shouldSkipLeadingSourceMapForNode, shouldSkipSourceMapForChildren, getSourceMapRange); - emitWorker(node); + emitCallback(node); emitEnd(/*range*/ node, /*contextNode*/ node, shouldSkipTrailingSourceMapForNode, shouldSkipSourceMapForChildren, getSourceMapRange); - emitTrailingComments(node, trailingComments); } } @@ -1642,13 +1655,26 @@ const _super = (function (geti, seti) { } increaseIndent(); - emitLeadingDetachedComments(body.statements, body, shouldSkipLeadingCommentsForNode); + emitBodyWithDetachedComments(body, body.statements, + shouldEmitBlockFunctionBodyOnSingleLine(parentNode, body) + ? emitBlockFunctionBodyOnSingleLine + : emitBlockFunctionBodyWorker); + + decreaseIndent(); + writeToken(SyntaxKind.CloseBraceToken, body.statements.end, body); + } + + function emitBlockFunctionBodyOnSingleLine(body: Block) { + emitBlockFunctionBodyWorker(body, /*emitBlockFunctionBodyOnSingleLine*/ true); + } + + function emitBlockFunctionBodyWorker(body: Block, emitBlockFunctionBodyOnSingleLine?: boolean) { // Emit all the prologue directives (like "use strict"). const statementOffset = emitPrologueDirectives(body.statements, /*startWithNewLine*/ true); const helpersEmitted = emitHelpers(body); - if (statementOffset === 0 && !helpersEmitted && shouldEmitBlockFunctionBodyOnSingleLine(parentNode, body)) { + if (statementOffset === 0 && !helpersEmitted && emitBlockFunctionBodyOnSingleLine) { decreaseIndent(); emitList(body, body.statements, ListFormat.SingleLineFunctionBodyStatements); increaseIndent(); @@ -1656,10 +1682,6 @@ const _super = (function (geti, seti) { else { emitList(body, body.statements, ListFormat.MultiLineFunctionBodyStatements, statementOffset); } - - emitTrailingDetachedComments(body.statements, body, shouldSkipTrailingCommentsForNode); - decreaseIndent(); - writeToken(SyntaxKind.CloseBraceToken, body.statements.end, body); } function emitClassDeclaration(node: ClassDeclaration) { @@ -2004,9 +2026,12 @@ const _super = (function (geti, seti) { // } // "comment1" is not considered to be leading comment for node.initializer // but rather a trailing comment on the previous node. - if (!shouldSkipLeadingCommentsForNode(node.initializer)) { - emitLeadingComments(/*range*/ node.initializer, getTrailingComments(collapseRangeToStart(node.initializer)), /*contextNode*/ node.initializer, getCommentRange); + const initializer = node.initializer; + if (!shouldSkipLeadingCommentsForNode(initializer)) { + const commentRange = initializer.commentRange || initializer; + emitTrailingCommentsOfPosition(commentRange.pos); } + emitExpression(node.initializer); } @@ -2034,8 +2059,10 @@ const _super = (function (geti, seti) { function emitSourceFile(node: SourceFile) { writeLine(); emitShebang(); - emitLeadingDetachedComments(node); + emitBodyWithDetachedComments(node, node.statements, emitSourceFileWorker); + } + function emitSourceFileWorker(node: SourceFile) { const statements = node.statements; const statementOffset = emitPrologueDirectives(statements); if (getNodeEmitFlags(node) & NodeEmitFlags.NoLexicalEnvironment) { @@ -2049,8 +2076,6 @@ const _super = (function (geti, seti) { emitList(node, statements, ListFormat.MultiLine, statementOffset); tempFlags = savedTempFlags; } - - emitTrailingDetachedComments(node.statements); } // Transformation nodes @@ -2326,7 +2351,8 @@ const _super = (function (geti, seti) { } else { // Write the opening line terminator or leading whitespace. - let shouldEmitInterveningComments = true; + const mayEmitInterveningComments = (format & ListFormat.NoInterveningComments) === 0; + let shouldEmitInterveningComments = mayEmitInterveningComments; if (shouldWriteLeadingLineTerminator(parentNode, children, format)) { writeLine(); shouldEmitInterveningComments = false; @@ -2369,10 +2395,11 @@ const _super = (function (geti, seti) { } if (shouldEmitInterveningComments) { - emitLeadingComments(/*node*/ child, getTrailingCommentsOfPosition(child.pos), /*contextNode*/ child, getCommentRange); + const commentRange = child.commentRange || child; + emitTrailingCommentsOfPosition(commentRange.pos); } else { - shouldEmitInterveningComments = true; + shouldEmitInterveningComments = mayEmitInterveningComments; } // Emit this child. @@ -2876,6 +2903,7 @@ const _super = (function (geti, seti) { // Other PreferNewLine = 1 << 15, // Prefer adding a LineTerminator between synthesized nodes. NoTrailingNewLine = 1 << 16, // Do not emit a trailing NewLine for a MultiLine list. + NoInterveningComments = 1 << 17,// Do not emit comments between each node // Precomputed Formats Modifiers = SingleLine | SpaceBetweenSiblings, @@ -2890,7 +2918,7 @@ const _super = (function (geti, seti) { ArrayLiteralExpressionElements = PreserveLines | CommaDelimited | SpaceBetweenSiblings | AllowTrailingComma | Indented | SquareBrackets, CallExpressionArguments = CommaDelimited | SpaceBetweenSiblings | SingleLine | Parenthesis, NewExpressionArguments = CommaDelimited | SpaceBetweenSiblings | SingleLine | Parenthesis | OptionalIfUndefined, - TemplateExpressionSpans = SingleLine, + TemplateExpressionSpans = SingleLine | NoInterveningComments, SingleLineBlockStatements = SpaceBetweenBraces | SpaceBetweenSiblings | SingleLine, MultiLineBlockStatements = Indented | MultiLine, VariableDeclarationList = CommaDelimited | SpaceBetweenSiblings | SingleLine, @@ -2902,8 +2930,8 @@ const _super = (function (geti, seti) { EnumMembers = CommaDelimited | Indented | MultiLine, CaseBlockClauses = Indented | MultiLine, NamedImportsOrExportsElements = CommaDelimited | SpaceBetweenSiblings | AllowTrailingComma | SingleLine | SpaceBetweenBraces, - JsxElementChildren = SingleLine, - JsxElementAttributes = SingleLine | SpaceBetweenSiblings, + JsxElementChildren = SingleLine | NoInterveningComments, + JsxElementAttributes = SingleLine | SpaceBetweenSiblings | NoInterveningComments, CaseOrDefaultClauseStatements = Indented | MultiLine | NoTrailingNewLine | OptionalIfEmpty, HeritageClauseTypes = CommaDelimited | SpaceBetweenSiblings | SingleLine, SourceFileStatements = MultiLine | NoTrailingNewLine, diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index 254923a981e..bbadd63e1f0 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -591,7 +591,7 @@ namespace ts { * and the next token are returned. * If true, comments occurring between the given position and the next line break are returned. */ - function getCommentRanges(text: string, pos: number, trailing: boolean, consumedCommentRanges?: Map): CommentRange[] { + function getCommentRanges(text: string, pos: number, trailing: boolean): CommentRange[] { let result: CommentRange[]; let collecting = trailing || pos === 0; while (pos >= 0 && pos < text.length) { @@ -643,15 +643,12 @@ namespace ts { } } - if (collecting && (!consumedCommentRanges || !(startPos in consumedCommentRanges))) { + if (collecting) { if (!result) { result = []; } result.push({ pos: startPos, end: pos, hasTrailingNewLine, kind }); - if (consumedCommentRanges) { - consumedCommentRanges[startPos] = true; - } } continue; @@ -673,12 +670,12 @@ namespace ts { return result; } - export function getLeadingCommentRanges(text: string, pos: number, consumedCommentRanges?: Map): CommentRange[] { - return getCommentRanges(text, pos, /*trailing*/ false, consumedCommentRanges); + export function getLeadingCommentRanges(text: string, pos: number): CommentRange[] { + return getCommentRanges(text, pos, /*trailing*/ false); } - export function getTrailingCommentRanges(text: string, pos: number, consumedCommentRanges?: Map): CommentRange[] { - return getCommentRanges(text, pos, /*trailing*/ true, consumedCommentRanges); + export function getTrailingCommentRanges(text: string, pos: number): CommentRange[] { + return getCommentRanges(text, pos, /*trailing*/ true); } /** Optionally, get the shebang */ diff --git a/src/compiler/transformers/es6.ts b/src/compiler/transformers/es6.ts index 16d08298f72..b796a3a7596 100644 --- a/src/compiler/transformers/es6.ts +++ b/src/compiler/transformers/es6.ts @@ -1526,6 +1526,7 @@ namespace ts { const declarationList = createVariableDeclarationList(declarations, /*location*/ node); setOriginalNode(declarationList, node); + setCommentRange(declarationList, node); if (node.transformFlags & TransformFlags.ContainsBindingPattern && (isBindingPattern(node.declarations[0].name) diff --git a/src/compiler/transformers/module/module.ts b/src/compiler/transformers/module/module.ts index 121365dd84c..af214a608cd 100644 --- a/src/compiler/transformers/module/module.ts +++ b/src/compiler/transformers/module/module.ts @@ -17,6 +17,7 @@ namespace ts { hoistVariableDeclaration, setNodeEmitFlags, getNodeEmitFlags, + setSourceMapRange, } = context; const compilerOptions = context.getCompilerOptions(); @@ -926,7 +927,13 @@ namespace ts { } function createExportStatement(name: Identifier, value: Expression, location?: TextRange) { - return startOnNewLine(createStatement(createExportAssignment(name, value), location)); + const statement = createStatement(createExportAssignment(name, value)); + statement.startsOnNewLine = true; + if (location) { + setSourceMapRange(statement, location); + } + + return statement; } function createExportAssignment(name: Identifier, value: Expression) { diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index e0ad54eb377..d7998e65a0d 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -2376,17 +2376,16 @@ namespace ts { * Adds a trailing VariableStatement for an enum or module declaration. */ function addVarForEnumExportedFromNamespace(statements: Statement[], node: EnumDeclaration | ModuleDeclaration) { - statements.push( - createVariableStatement( - /*modifiers*/ undefined, - [createVariableDeclaration( - getDeclarationName(node), - /*type*/ undefined, - getExportName(node) - )], - /*location*/ node - ) + const statement = createVariableStatement( + /*modifiers*/ undefined, + [createVariableDeclaration( + getDeclarationName(node), + /*type*/ undefined, + getExportName(node) + )] ); + setSourceMapRange(statement, node); + statements.push(statement); } /** @@ -2402,48 +2401,62 @@ namespace ts { } const statements: Statement[] = []; + + // We request to be advised when the printer is about to print this node. This allows + // us to set up the correct state for later substitutions. + let emitFlags = NodeEmitFlags.AdviseOnEmitNode; + + // If needed, we should emit a variable declaration for the enum. If we emit + // a leading variable declaration, we should not emit leading comments for the + // enum body. if (shouldEmitVarForEnumDeclaration(node)) { addVarForEnumOrModuleDeclaration(statements, node); + + // We should still emit the comments if we are emitting a system module. + if (moduleKind !== ModuleKind.System || currentScope !== currentSourceFile) { + emitFlags |= NodeEmitFlags.NoLeadingComments; + } } - const innerName = getNamespaceContainerName(node); - const paramName = getNamespaceParameterName(node); + // `parameterName` is the declaration name used inside of the enum. + const parameterName = getNamespaceParameterName(node); + + // `containerName` is the expression used inside of the enum for assignments. + const containerName = getNamespaceContainerName(node); + + // `exportName` is the expression used within this node's container for any exported references. const exportName = getExportName(node); // (function (x) { // x[x["y"] = 0] = "y"; // ... // })(x || (x = {})); - statements.push( - setNodeEmitFlags( - setOriginalNode( - createStatement( - createCall( - createFunctionExpression( - /*asteriskToken*/ undefined, - /*name*/ undefined, - /*typeParameters*/ undefined, - [createParameter(paramName)], - /*type*/ undefined, - transformEnumBody(node, innerName) - ), - /*typeArguments*/ undefined, - [createLogicalOr( - exportName, - createAssignment( - exportName, - createObjectLiteral() - ) - )] - ), - /*location*/ node - ), - /*original*/ node + const enumStatement = createStatement( + createCall( + createFunctionExpression( + /*asteriskToken*/ undefined, + /*name*/ undefined, + /*typeParameters*/ undefined, + [createParameter(parameterName)], + /*type*/ undefined, + transformEnumBody(node, containerName) ), - NodeEmitFlags.AdviseOnEmitNode - ) + /*typeArguments*/ undefined, + [createLogicalOr( + exportName, + createAssignment( + exportName, + createObjectLiteral() + ) + )] + ), + /*location*/ node ); + setOriginalNode(enumStatement, node); + setNodeEmitFlags(enumStatement, emitFlags); + statements.push(enumStatement); + if (isNamespaceExport(node)) { addVarForEnumExportedFromNamespace(statements, node); } @@ -2614,8 +2627,19 @@ namespace ts { const statements: Statement[] = []; + // We request to be advised when the printer is about to print this node. This allows + // us to set up the correct state for later substitutions. + let emitFlags = NodeEmitFlags.AdviseOnEmitNode; + + // If needed, we should emit a variable declaration for the module. If we emit + // a leading variable declaration, we should not emit leading comments for the + // module body. if (shouldEmitVarForModuleDeclaration(node)) { addVarForEnumOrModuleDeclaration(statements, node); + // We should still emit the comments if we are emitting a system module. + if (moduleKind !== ModuleKind.System || currentScope !== currentSourceFile) { + emitFlags |= NodeEmitFlags.NoLeadingComments; + } } // `parameterName` is the declaration name used inside of the namespace. @@ -2649,30 +2673,25 @@ namespace ts { // (function (x_1) { // x_1.y = ...; // })(x || (x = {})); - statements.push( - setNodeEmitFlags( - setOriginalNode( - createStatement( - createCall( - createFunctionExpression( - /*asteriskToken*/ undefined, - /*name*/ undefined, - /*typeParameters*/ undefined, - [createParameter(parameterName)], - /*type*/ undefined, - transformModuleBody(node, containerName) - ), - /*typeArguments*/ undefined, - [moduleArg] - ), - /*location*/ node - ), - /*original*/ node + const moduleStatement = createStatement( + createCall( + createFunctionExpression( + /*asteriskToken*/ undefined, + /*name*/ undefined, + /*typeParameters*/ undefined, + [createParameter(parameterName)], + /*type*/ undefined, + transformModuleBody(node, containerName) ), - NodeEmitFlags.AdviseOnEmitNode - ) + /*typeArguments*/ undefined, + [moduleArg] + ), + /*location*/ node ); + setOriginalNode(moduleStatement, node); + setNodeEmitFlags(moduleStatement, emitFlags); + statements.push(moduleStatement); return statements; } @@ -2846,16 +2865,15 @@ namespace ts { } function addExportMemberAssignment(statements: Statement[], node: DeclarationStatement) { - statements.push( - createStatement( - createAssignment( - getExportName(node), - getLocalName(node, /*noSourceMaps*/ true), - /*location*/ createRange(node.name.pos, node.end) - ), - /*location*/ createRange(-1, node.end) - ) + const expression = createAssignment( + getExportName(node), + getLocalName(node, /*noSourceMaps*/ true) ); + setSourceMapRange(expression, createRange(node.name.pos, node.end)); + + const statement = createStatement(expression); + setSourceMapRange(statement, createRange(-1, node.end)); + statements.push(statement); } function createNamespaceExport(exportName: Identifier, exportValue: Expression, location?: TextRange) { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index c4a644dcc1d..7ecb0fa194b 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -695,6 +695,7 @@ namespace ts { export let fullTripleSlashReferenceTypeReferenceDirectiveRegEx = /^(\/\/\/\s*/; export let fullTripleSlashAMDReferencePathRegEx = /^(\/\/\/\s*/; + export function isPartOfTypeNode(node: Node): boolean { if (SyntaxKind.FirstTypeNode <= node.kind && node.kind <= SyntaxKind.LastTypeNode) { return true; @@ -2590,9 +2591,13 @@ namespace ts { } export function emitNewLineBeforeLeadingComments(lineMap: number[], writer: EmitTextWriter, node: TextRange, leadingComments: CommentRange[]) { + emitNewLineBeforeLeadingCommentsOfPosition(lineMap, writer, node.pos, leadingComments); + } + + export function emitNewLineBeforeLeadingCommentsOfPosition(lineMap: number[], writer: EmitTextWriter, pos: number, leadingComments: CommentRange[]) { // If the leading comments start on different line than the start of node, write new line - if (leadingComments && leadingComments.length && node.pos !== leadingComments[0].pos && - getLineOfLocalPositionFromLineMap(lineMap, node.pos) !== getLineOfLocalPositionFromLineMap(lineMap, leadingComments[0].pos)) { + if (leadingComments && leadingComments.length && pos !== leadingComments[0].pos && + getLineOfLocalPositionFromLineMap(lineMap, pos) !== getLineOfLocalPositionFromLineMap(lineMap, leadingComments[0].pos)) { writer.writeLine(); } } From 43e3f357ee93fe813496c4689ff9b798cb297400 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Fri, 27 May 2016 17:07:49 -0700 Subject: [PATCH 6/8] Reduce allocations/gc by avoiding the creation of some CommentRange objects. --- src/compiler/comments.ts | 156 ++++++++++++++++++++++++-------------- src/compiler/core.ts | 34 ++++++--- src/compiler/emitter.ts | 8 +- src/compiler/scanner.ts | 79 ++++++++++++++----- src/compiler/tsc.ts | 12 --- src/compiler/utilities.ts | 38 ++++++---- 6 files changed, 211 insertions(+), 116 deletions(-) diff --git a/src/compiler/comments.ts b/src/compiler/comments.ts index 9c847524520..3c2324d352c 100644 --- a/src/compiler/comments.ts +++ b/src/compiler/comments.ts @@ -23,9 +23,9 @@ namespace ts { let currentText: string; let currentLineMap: number[]; let detachedCommentsInfo: { nodePos: number, detachedCommentEndPos: number}[]; - - // Tracks comment ranges that have already been consumed. - let consumedCommentRanges: Map; + let hasWrittenComment = false; + let hasLastComment: boolean; + let lastCommentEnd: number; return { reset, @@ -122,7 +122,7 @@ namespace ts { const skipTrailingComments = end < 0 || (emitFlags & NodeEmitFlags.NoTrailingComments) !== 0 || compilerOptions.removeComments; if (!skipLeadingComments) { - emitDetachedCommentsAndUpdateCommentsInfo(detachedRange, compilerOptions.removeComments); + emitDetachedCommentsAndUpdateCommentsInfo(detachedRange); } if (extendedDiagnostics) { @@ -144,11 +144,12 @@ namespace ts { } function emitLeadingComments(pos: number, isEmittedNode: boolean) { - let leadingComments: CommentRange[]; + hasWrittenComment = false; + if (isEmittedNode) { - leadingComments = getLeadingCommentsToEmit(pos); + forEachLeadingCommentToEmit(pos, emitLeadingComment); } - else { + else if (pos === 0) { // If the node will not be emitted in JS, remove all the comments(normal, pinned and ///) associated with the node, // unless it is a triple slash comment at the top of the file. // For Example: @@ -157,22 +158,52 @@ namespace ts { // /// // interface F {} // The first /// will NOT be removed while the second one will be removed even though both node will not be emitted - if (pos === 0) { - leadingComments = filter(getLeadingCommentsToEmit(pos), isTripleSlashComment); - } + forEachLeadingCommentToEmit(pos, emitTripleSlashLeadingComment); + } + } + + function emitTripleSlashLeadingComment(commentPos: number, commentEnd: number, kind: SyntaxKind, hasTrailingNewLine: boolean, rangePos: number) { + if (isTripleSlashComment(commentPos, commentEnd)) { + emitLeadingComment(commentPos, commentEnd, kind, hasTrailingNewLine, rangePos); + } + } + + function emitLeadingComment(commentPos: number, commentEnd: number, kind: SyntaxKind, hasTrailingNewLine: boolean, rangePos: number) { + if (!hasWrittenComment) { + emitNewLineBeforeLeadingCommentOfPosition(currentLineMap, writer, rangePos, commentPos); + hasWrittenComment = true; } - emitNewLineBeforeLeadingCommentsOfPosition(currentLineMap, writer, pos, leadingComments); - // Leading comments are emitted at /*leading comment1 */space/*leading comment*/space - emitComments(currentText, currentLineMap, writer, leadingComments, /*leadingSeparator*/ false, /*trailingSeparator*/ true, newLine, writeComment); + emitPos(commentPos); + writeCommentRange(currentText, currentLineMap, writer, commentPos, commentEnd, newLine); + emitPos(commentEnd); + + if (hasTrailingNewLine) { + writer.writeLine(); + } + else { + writer.write(" "); + } } function emitTrailingComments(pos: number) { - const trailingComments = getTrailingCommentsToEmit(pos); + forEachTrailingCommentToEmit(pos, emitTrailingComment); + } - // trailing comments are emitted at space/*trailing comment1 */space/*trailing comment*/ - emitComments(currentText, currentLineMap, writer, trailingComments, /*leadingSeparator*/ true, /*trailingSeparator*/ false, newLine, writeComment); + function emitTrailingComment(commentPos: number, commentEnd: number, kind: SyntaxKind, hasTrailingNewLine: boolean) { + // trailing comments are emitted at space/*trailing comment1 */space/*trailing comment2*/ + if (!writer.isAtStartOfLine()) { + writer.write(" "); + } + + emitPos(commentPos); + writeCommentRange(currentText, currentLineMap, writer, commentPos, commentEnd, newLine); + emitPos(commentEnd); + + if (hasTrailingNewLine) { + writer.writeLine(); + } } function emitTrailingCommentsOfPosition(pos: number) { @@ -185,57 +216,52 @@ namespace ts { commentStart = performance.mark(); } - const trailingComments = getTrailingCommentsToEmit(pos); - - // trailing comments of a position are emitted at /*trailing comment1 */space/*trailing comment*/space - emitComments(currentText, currentLineMap, writer, trailingComments, /*leadingSeparator*/ false, /*trailingSeparator*/ true, newLine, writeComment); + forEachTrailingCommentToEmit(pos, emitTrailingCommentOfPosition); if (extendedDiagnostics) { performance.measure("commentTime", commentStart); } } - function getLeadingCommentsToEmit(pos: number) { + function emitTrailingCommentOfPosition(commentPos: number, commentEnd: number, kind: SyntaxKind, hasTrailingNewLine: boolean) { + // trailing comments of a position are emitted at /*trailing comment1 */space/*trailing comment*/space + + emitPos(commentPos); + writeCommentRange(currentText, currentLineMap, writer, commentPos, commentEnd, newLine); + emitPos(commentEnd); + + if (hasTrailingNewLine) { + writer.writeLine(); + } + else { + writer.write(" "); + } + } + + function forEachLeadingCommentToEmit(pos: number, cb: (commentPos: number, commentEnd: number, kind: SyntaxKind, hasTrailingNewLine: boolean, rangePos: number) => void) { // Emit the leading comments only if the container's pos doesn't match because the container should take care of emitting these comments if (containerPos === -1 || pos !== containerPos) { - return hasDetachedComments(pos) - ? getLeadingCommentsWithoutDetachedComments() - : getLeadingCommentRanges(currentText, pos); + if (hasDetachedComments(pos)) { + forEachLeadingCommentWithoutDetachedComments(cb); + } + else { + forEachLeadingCommentRange(currentText, pos, cb, /*state*/ pos); + } } } - function getTrailingCommentsToEmit(end: number) { + function forEachTrailingCommentToEmit(end: number, cb: (commentPos: number, commentEnd: number, kind: SyntaxKind, hasTrailingNewLine: boolean) => void) { // Emit the trailing comments only if the container's end doesn't match because the container should take care of emitting these comments if (containerEnd === -1 || (end !== containerEnd && end !== declarationListContainerEnd)) { - return getTrailingCommentRanges(currentText, end); + forEachTrailingCommentRange(currentText, end, cb); } } - /** - * Determine if the given comment is a triple-slash - * - * @return true if the comment is a triple-slash comment else false - **/ - function isTripleSlashComment(comment: CommentRange) { - // Verify this is /// comment, but do the regexp match only when we first can find /// in the comment text - // so that we don't end up computing comment string and doing match for all // comments - if (currentText.charCodeAt(comment.pos + 1) === CharacterCodes.slash && - comment.pos + 2 < comment.end && - currentText.charCodeAt(comment.pos + 2) === CharacterCodes.slash) { - const textSubStr = currentText.substring(comment.pos, comment.end); - return textSubStr.match(fullTripleSlashReferencePathRegEx) || - textSubStr.match(fullTripleSlashAMDReferencePathRegEx) ? - true : false; - } - return false; - } - function reset() { currentSourceFile = undefined; currentText = undefined; currentLineMap = undefined; detachedCommentsInfo = undefined; - consumedCommentRanges = undefined; } function setSourceFile(sourceFile: SourceFile) { @@ -243,17 +269,15 @@ namespace ts { currentText = currentSourceFile.text; currentLineMap = getLineStarts(currentSourceFile); detachedCommentsInfo = undefined; - consumedCommentRanges = {}; } function hasDetachedComments(pos: number) { return detachedCommentsInfo !== undefined && lastOrUndefined(detachedCommentsInfo).nodePos === pos; } - function getLeadingCommentsWithoutDetachedComments() { + function forEachLeadingCommentWithoutDetachedComments(cb: (commentPos: number, commentEnd: number, kind: SyntaxKind, hasTrailingNewLine: boolean, rangePos: number) => void) { // get the leading comments from detachedPos const pos = lastOrUndefined(detachedCommentsInfo).detachedCommentEndPos; - const leadingComments = getLeadingCommentRanges(currentText, pos); if (detachedCommentsInfo.length - 1) { detachedCommentsInfo.pop(); } @@ -261,12 +285,11 @@ namespace ts { detachedCommentsInfo = undefined; } - return leadingComments; + forEachLeadingCommentRange(currentText, pos, cb, /*state*/ pos); } - function emitDetachedCommentsAndUpdateCommentsInfo(node: TextRange, removeComments: boolean) { - const currentDetachedCommentInfo = emitDetachedComments(currentText, currentLineMap, writer, writeComment, node, newLine, removeComments); - + function emitDetachedCommentsAndUpdateCommentsInfo(range: TextRange) { + const currentDetachedCommentInfo = emitDetachedComments(currentText, currentLineMap, writer, writeComment, range, newLine, compilerOptions.removeComments); if (currentDetachedCommentInfo) { if (detachedCommentsInfo) { detachedCommentsInfo.push(currentDetachedCommentInfo); @@ -277,10 +300,29 @@ namespace ts { } } - function writeComment(text: string, lineMap: number[], writer: EmitTextWriter, comment: CommentRange, newLine: string) { - emitPos(comment.pos); - writeCommentRange(text, lineMap, writer, comment, newLine); - emitPos(comment.end); + function writeComment(text: string, lineMap: number[], writer: EmitTextWriter, commentPos: number, commentEnd: number, newLine: string) { + emitPos(commentPos); + writeCommentRange(text, lineMap, writer, commentPos, commentEnd, newLine); + emitPos(commentEnd); + } + + /** + * Determine if the given comment is a triple-slash + * + * @return true if the comment is a triple-slash comment else false + **/ + function isTripleSlashComment(commentPos: number, commentEnd: number) { + // Verify this is /// comment, but do the regexp match only when we first can find /// in the comment text + // so that we don't end up computing comment string and doing match for all // comments + if (currentText.charCodeAt(commentPos + 1) === CharacterCodes.slash && + commentPos + 2 < commentEnd && + currentText.charCodeAt(commentPos + 2) === CharacterCodes.slash) { + const textSubStr = currentText.substring(commentPos, commentEnd); + return textSubStr.match(fullTripleSlashReferencePathRegEx) || + textSubStr.match(fullTripleSlashAMDReferencePathRegEx) ? + true : false; + } + return false; } } } \ No newline at end of file diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 063b619c690..f6bef5ebca9 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -1139,8 +1139,8 @@ namespace ts { /** Performance measurements for the compiler. */ /*@internal*/ export namespace performance { - let counters: Map = {}; - let measures: Map = {}; + let counters: Map; + let measures: Map; let enabled = false; /** @@ -1191,22 +1191,32 @@ namespace ts { return enabled && getProperty(measures, measureName) || 0; } - /** - * Resets all marks and measurements in the performance service. - */ - export function reset() { - counters = {}; - measures = {}; - } - /** Enables performance measurements for the compiler. */ export function enable() { - enabled = true; + if (!enabled) { + enabled = true; + counters = { }; + measures = { + programTime: 0, + parseTime: 0, + bindTime: 0, + emitTime: 0, + ioReadTime: 0, + ioWriteTime: 0, + printTime: 0, + commentTime: 0, + sourceMapTime: 0 + }; + } } /** Disables performance measurements for the compiler. */ export function disable() { - enabled = false; + if (enabled) { + enabled = false; + counters = undefined; + measures = undefined; + } } } } diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 8d190dc84a0..c327ef2b5f1 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -8228,10 +8228,10 @@ const _super = (function (geti, seti) { } } - function writeComment(text: string, lineMap: number[], writer: EmitTextWriter, comment: CommentRange, newLine: string) { - emitPos(comment.pos); - writeCommentRange(text, lineMap, writer, comment, newLine); - emitPos(comment.end); + function writeComment(text: string, lineMap: number[], writer: EmitTextWriter, commentPos: number, commentEnd: number, newLine: string) { + emitPos(commentPos); + writeCommentRange(text, lineMap, writer, commentPos, commentEnd, newLine); + emitPos(commentEnd); } function emitShebang() { diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index bbadd63e1f0..c4133b27fb3 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -591,10 +591,15 @@ namespace ts { * and the next token are returned. * If true, comments occurring between the given position and the next line break are returned. */ - function getCommentRanges(text: string, pos: number, trailing: boolean): CommentRange[] { - let result: CommentRange[]; + function iterateCommentRanges(reduce: boolean, text: string, pos: number, trailing: boolean, cb: (pos: number, end: number, kind: SyntaxKind, hasTrailingNewLine: boolean, state: T, memo: U) => U, state: T, initial?: U): U { + let pendingPos: number; + let pendingEnd: number; + let pendingKind: SyntaxKind; + let pendingHasTrailingNewLine: boolean; + let hasPendingCommentRange = false; let collecting = trailing || pos === 0; - while (pos >= 0 && pos < text.length) { + let accumulator = initial; + scan: while (pos >= 0 && pos < text.length) { const ch = text.charCodeAt(pos); switch (ch) { case CharacterCodes.carriageReturn: @@ -604,12 +609,14 @@ namespace ts { case CharacterCodes.lineFeed: pos++; if (trailing) { - return result; + break scan; } + collecting = true; - if (result && result.length) { - lastOrUndefined(result).hasTrailingNewLine = true; + if (hasPendingCommentRange) { + pendingHasTrailingNewLine = true; } + continue; case CharacterCodes.tab: case CharacterCodes.verticalTab: @@ -644,38 +651,76 @@ namespace ts { } if (collecting) { - if (!result) { - result = []; + if (hasPendingCommentRange) { + accumulator = cb(pendingPos, pendingEnd, pendingKind, pendingHasTrailingNewLine, state, accumulator); + if (!reduce && accumulator) { + // If we are not reducing and we have a truthy result, return it. + return accumulator; + } + + hasPendingCommentRange = false; } - result.push({ pos: startPos, end: pos, hasTrailingNewLine, kind }); + pendingPos = startPos; + pendingEnd = pos; + pendingKind = kind; + pendingHasTrailingNewLine = hasTrailingNewLine; + hasPendingCommentRange = true; } continue; } - break; + break scan; default: if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpace(ch) || isLineBreak(ch))) { - if (result && result.length && isLineBreak(ch)) { - lastOrUndefined(result).hasTrailingNewLine = true; + if (hasPendingCommentRange && isLineBreak(ch)) { + pendingHasTrailingNewLine = true; } pos++; continue; } - break; + break scan; } - return result; } - return result; + if (hasPendingCommentRange) { + accumulator = cb(pendingPos, pendingEnd, pendingKind, pendingHasTrailingNewLine, state, accumulator); + } + + return accumulator; + } + + export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: SyntaxKind, hasTrailingNewLine: boolean, state: T) => U, state?: T) { + return iterateCommentRanges(/*reduce*/ false, text, pos, /*trailing*/ false, cb, state); + } + + export function forEachTrailingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: SyntaxKind, hasTrailingNewLine: boolean, state: T) => U, state?: T) { + return iterateCommentRanges(/*reduce*/ false, text, pos, /*trailing*/ true, cb, state); + } + + export function reduceEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: SyntaxKind, hasTrailingNewLine: boolean, state: T, memo: U) => U, state: T, initial: U) { + return iterateCommentRanges(/*reduce*/ true, text, pos, /*trailing*/ false, cb, state, initial); + } + + export function reduceEachTrailingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: SyntaxKind, hasTrailingNewLine: boolean, state: T, memo: U) => U, state: T, initial: U) { + return iterateCommentRanges(/*reduce*/ true, text, pos, /*trailing*/ true, cb, state, initial); + } + + function appendCommentRange(pos: number, end: number, kind: SyntaxKind, hasTrailingNewLine: boolean, state: any, comments: CommentRange[]) { + if (!comments) { + comments = []; + } + + comments.push({ pos, end, hasTrailingNewLine, kind }); + return comments; } export function getLeadingCommentRanges(text: string, pos: number): CommentRange[] { - return getCommentRanges(text, pos, /*trailing*/ false); + return reduceEachLeadingCommentRange(text, pos, appendCommentRange, undefined, undefined); } export function getTrailingCommentRanges(text: string, pos: number): CommentRange[] { - return getCommentRanges(text, pos, /*trailing*/ true); + return reduceEachTrailingCommentRange(text, pos, appendCommentRange, undefined, undefined); } /** Optionally, get the shebang */ diff --git a/src/compiler/tsc.ts b/src/compiler/tsc.ts index 24c54e71170..e2b6a77457d 100644 --- a/src/compiler/tsc.ts +++ b/src/compiler/tsc.ts @@ -11,16 +11,6 @@ namespace ts { value: string; } - interface Mark { - markName: string; - count: number; - } - - interface Measure { - measureName: string; - duration: number; - } - let reportDiagnostic = reportDiagnosticSimply; function reportDiagnostics(diagnostics: Diagnostic[], host: CompilerHost): void { @@ -560,7 +550,6 @@ namespace ts { let statistics: Statistic[]; if (compilerOptions.diagnostics || compilerOptions.extendedDiagnostics) { performance.enable(); - performance.reset(); statistics = []; } @@ -610,7 +599,6 @@ namespace ts { reportStatistics(); performance.disable(); - performance.reset(); } return { program, exitStatus }; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 0feeee4d37f..2744d6b549f 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -2285,6 +2285,7 @@ namespace ts { getLine(): number; getColumn(): number; getIndent(): number; + isAtStartOfLine(): boolean; reset(): void; } @@ -2373,6 +2374,7 @@ namespace ts { getLine: () => lineCount + 1, getColumn: () => lineStart ? indent * getIndentSize() + 1 : output.length - linePos + 1, getText: () => output, + isAtStartOfLine: () => lineStart, reset }; } @@ -2601,8 +2603,16 @@ namespace ts { } } + export function emitNewLineBeforeLeadingCommentOfPosition(lineMap: number[], writer: EmitTextWriter, pos: number, commentPos: number) { + // If the leading comments start on different line than the start of node, write new line + if (pos !== commentPos && + getLineOfLocalPositionFromLineMap(lineMap, pos) !== getLineOfLocalPositionFromLineMap(lineMap, commentPos)) { + writer.writeLine(); + } + } + export function emitComments(text: string, lineMap: number[], writer: EmitTextWriter, comments: CommentRange[], leadingSeparator: boolean, trailingSeparator: boolean, newLine: string, - writeComment: (text: string, lineMap: number[], writer: EmitTextWriter, comment: CommentRange, newLine: string) => void) { + writeComment: (text: string, lineMap: number[], writer: EmitTextWriter, commentPos: number, commentEnd: number, newLine: string) => void) { if (comments && comments.length > 0) { if (leadingSeparator) { writer.write(" "); @@ -2615,7 +2625,7 @@ namespace ts { emitInterveningSeparator = false; } - writeComment(text, lineMap, writer, comment, newLine); + writeComment(text, lineMap, writer, comment.pos, comment.end, newLine); if (comment.hasTrailingNewLine) { writer.writeLine(); } @@ -2635,7 +2645,7 @@ namespace ts { * the next statement by space. */ export function emitDetachedComments(text: string, lineMap: number[], writer: EmitTextWriter, - writeComment: (text: string, lineMap: number[], writer: EmitTextWriter, comment: CommentRange, newLine: string) => void, + writeComment: (text: string, lineMap: number[], writer: EmitTextWriter, commentPos: number, commentEnd: number, newLine: string) => void, node: TextRange, newLine: string, removeComments: boolean) { let leadingComments: CommentRange[]; let currentDetachedCommentInfo: {nodePos: number, detachedCommentEndPos: number}; @@ -2699,20 +2709,20 @@ namespace ts { } - export function writeCommentRange(text: string, lineMap: number[], writer: EmitTextWriter, comment: CommentRange, newLine: string) { - if (text.charCodeAt(comment.pos + 1) === CharacterCodes.asterisk) { - const firstCommentLineAndCharacter = computeLineAndCharacterOfPosition(lineMap, comment.pos); + export function writeCommentRange(text: string, lineMap: number[], writer: EmitTextWriter, commentPos: number, commentEnd: number, newLine: string) { + if (text.charCodeAt(commentPos + 1) === CharacterCodes.asterisk) { + const firstCommentLineAndCharacter = computeLineAndCharacterOfPosition(lineMap, commentPos); const lineCount = lineMap.length; let firstCommentLineIndent: number; - for (let pos = comment.pos, currentLine = firstCommentLineAndCharacter.line; pos < comment.end; currentLine++) { + for (let pos = commentPos, currentLine = firstCommentLineAndCharacter.line; pos < commentEnd; currentLine++) { const nextLineStart = (currentLine + 1) === lineCount ? text.length + 1 : lineMap[currentLine + 1]; - if (pos !== comment.pos) { + if (pos !== commentPos) { // If we are not emitting first line, we need to write the spaces to adjust the alignment if (firstCommentLineIndent === undefined) { - firstCommentLineIndent = calculateIndent(text, lineMap[firstCommentLineAndCharacter.line], comment.pos); + firstCommentLineIndent = calculateIndent(text, lineMap[firstCommentLineAndCharacter.line], commentPos); } // These are number of spaces writer is going to write at current indent @@ -2753,24 +2763,24 @@ namespace ts { } // Write the comment line text - writeTrimmedCurrentLine(text, comment, writer, newLine, pos, nextLineStart); + writeTrimmedCurrentLine(text, commentEnd, writer, newLine, pos, nextLineStart); pos = nextLineStart; } } else { // Single line comment of style //.... - writer.write(text.substring(comment.pos, comment.end)); + writer.write(text.substring(commentPos, commentEnd)); } } - function writeTrimmedCurrentLine(text: string, comment: CommentRange, writer: EmitTextWriter, newLine: string, pos: number, nextLineStart: number) { - const end = Math.min(comment.end, nextLineStart - 1); + function writeTrimmedCurrentLine(text: string, commentEnd: number, writer: EmitTextWriter, newLine: string, pos: number, nextLineStart: number) { + const end = Math.min(commentEnd, nextLineStart - 1); const currentLineText = text.substring(pos, end).replace(/^\s+|\s+$/g, ""); if (currentLineText) { // trimmed forward and ending spaces text writer.write(currentLineText); - if (end !== comment.end) { + if (end !== commentEnd) { writer.writeLine(); } } From e9115cad19b178336a96370a5bf35ea3ea49642b Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Tue, 31 May 2016 16:24:20 -0700 Subject: [PATCH 7/8] Simplify disabling comments recursively, cleanup unused flags. --- src/compiler/comments.ts | 50 ++++++++++++++++++++++++++------- src/compiler/factory.ts | 10 +++---- src/compiler/printer.ts | 16 ++++------- src/compiler/transformer.ts | 8 +++--- src/compiler/transformers/ts.ts | 8 ++---- src/compiler/types.ts | 30 +++++++++----------- 6 files changed, 70 insertions(+), 52 deletions(-) diff --git a/src/compiler/comments.ts b/src/compiler/comments.ts index 3c2324d352c..57611d1498a 100644 --- a/src/compiler/comments.ts +++ b/src/compiler/comments.ts @@ -26,6 +26,7 @@ namespace ts { let hasWrittenComment = false; let hasLastComment: boolean; let lastCommentEnd: number; + let disabled: boolean = compilerOptions.removeComments; return { reset, @@ -36,16 +37,22 @@ namespace ts { }; function emitNodeWithComments(node: Node, emitCallback: (node: Node) => void) { - if (compilerOptions.removeComments) { + if (disabled) { emitCallback(node); return; } if (node) { const { pos, end } = node.commentRange || node; + const emitFlags = node.emitFlags; if ((pos < 0 && end < 0) || (pos === end)) { // Both pos and end are synthesized, so just emit the node without comments. - emitCallback(node); + if (emitFlags & NodeEmitFlags.NoNestedComments) { + disableCommentsAndEmit(node, emitCallback); + } + else { + emitCallback(node); + } } else { let commentStart: number; @@ -53,7 +60,6 @@ namespace ts { commentStart = performance.mark(); } - const emitFlags = node.emitFlags; const isEmittedNode = node.kind !== SyntaxKind.NotEmittedStatement; const skipLeadingComments = pos < 0 || (emitFlags & NodeEmitFlags.NoLeadingComments) !== 0; const skipTrailingComments = end < 0 || (emitFlags & NodeEmitFlags.NoTrailingComments) !== 0; @@ -85,13 +91,19 @@ namespace ts { if (extendedDiagnostics) { performance.measure("commentTime", commentStart); - emitCallback(node); - commentStart = performance.mark(); + } + + if (emitFlags & NodeEmitFlags.NoNestedComments) { + disableCommentsAndEmit(node, emitCallback); } else { emitCallback(node); } + if (extendedDiagnostics) { + commentStart = performance.mark(); + } + // Restore previous container state. containerPos = savedContainerPos; containerEnd = savedContainerEnd; @@ -119,7 +131,7 @@ namespace ts { const { pos, end } = detachedRange; const emitFlags = node.emitFlags; const skipLeadingComments = pos < 0 || (emitFlags & NodeEmitFlags.NoLeadingComments) !== 0; - const skipTrailingComments = end < 0 || (emitFlags & NodeEmitFlags.NoTrailingComments) !== 0 || compilerOptions.removeComments; + const skipTrailingComments = disabled || end < 0 || (emitFlags & NodeEmitFlags.NoTrailingComments) !== 0; if (!skipLeadingComments) { emitDetachedCommentsAndUpdateCommentsInfo(detachedRange); @@ -127,13 +139,19 @@ namespace ts { if (extendedDiagnostics) { performance.measure("commentTime", commentStart); - emitCallback(node); - commentStart = performance.mark(); + } + + if (emitFlags & NodeEmitFlags.NoNestedComments) { + disableCommentsAndEmit(node, emitCallback); } else { emitCallback(node); } + if (extendedDiagnostics) { + commentStart = performance.mark(); + } + if (!skipTrailingComments) { emitLeadingComments(detachedRange.end, /*isEmittedNode*/ true); } @@ -207,7 +225,7 @@ namespace ts { } function emitTrailingCommentsOfPosition(pos: number) { - if (compilerOptions.removeComments) { + if (disabled) { return; } @@ -269,6 +287,18 @@ namespace ts { currentText = currentSourceFile.text; currentLineMap = getLineStarts(currentSourceFile); detachedCommentsInfo = undefined; + disabled = false; + } + + function disableCommentsAndEmit(node: Node, emitCallback: (node: Node) => void): void { + if (disabled) { + emitCallback(node); + } + else { + disabled = true; + emitCallback(node); + disabled = false; + } } function hasDetachedComments(pos: number) { @@ -289,7 +319,7 @@ namespace ts { } function emitDetachedCommentsAndUpdateCommentsInfo(range: TextRange) { - const currentDetachedCommentInfo = emitDetachedComments(currentText, currentLineMap, writer, writeComment, range, newLine, compilerOptions.removeComments); + const currentDetachedCommentInfo = emitDetachedComments(currentText, currentLineMap, writer, writeComment, range, newLine, disabled); if (currentDetachedCommentInfo) { if (detachedCommentsInfo) { detachedCommentsInfo.push(currentDetachedCommentInfo); diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index d4a03468ca0..2ea5de24901 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -1882,11 +1882,11 @@ namespace ts { export function setOriginalNode(node: T, original: Node): T { node.original = original; - if (original && original.transformId && !node.transformId) { - node.transformId = original.transformId; - node.emitFlags = original.emitFlags; - node.commentRange = original.commentRange; - node.sourceMapRange = original.sourceMapRange; + if (original) { + const { emitFlags, commentRange, sourceMapRange } = original; + if (emitFlags) node.emitFlags = emitFlags; + if (commentRange) node.commentRange = commentRange; + if (sourceMapRange) node.sourceMapRange = sourceMapRange; } return node; } diff --git a/src/compiler/printer.ts b/src/compiler/printer.ts index d58c580972b..6a72181373a 100644 --- a/src/compiler/printer.ts +++ b/src/compiler/printer.ts @@ -2065,17 +2065,11 @@ const _super = (function (geti, seti) { function emitSourceFileWorker(node: SourceFile) { const statements = node.statements; const statementOffset = emitPrologueDirectives(statements); - if (getNodeEmitFlags(node) & NodeEmitFlags.NoLexicalEnvironment) { - emitHelpers(node); - emitList(node, statements, ListFormat.MultiLine, statementOffset); - } - else { - const savedTempFlags = tempFlags; - tempFlags = 0; - emitHelpers(node); - emitList(node, statements, ListFormat.MultiLine, statementOffset); - tempFlags = savedTempFlags; - } + const savedTempFlags = tempFlags; + tempFlags = 0; + emitHelpers(node); + emitList(node, statements, ListFormat.MultiLine, statementOffset); + tempFlags = savedTempFlags; } // Transformation nodes diff --git a/src/compiler/transformer.ts b/src/compiler/transformer.ts index 70a73ea90e0..60240372f0f 100644 --- a/src/compiler/transformer.ts +++ b/src/compiler/transformer.ts @@ -300,12 +300,12 @@ namespace ts { * @param node The node. */ function beforeSetAnnotation(node: Node) { - node.transformId = transformId; - if ((node.flags & NodeFlags.Synthesized) === 0) { + if ((node.flags & NodeFlags.Synthesized) === 0 && node.transformId !== transformId) { // To avoid holding onto transformation artifacts, we keep track of any // source tree node we are annotating. This allows us to clean them up after // all transformations have completed. sourceTreeNodesWithAnnotations.push(node); + node.transformId = transformId; } } @@ -318,7 +318,7 @@ namespace ts { * @param node The node. */ function getNodeEmitFlags(node: Node) { - return node.emitFlags & ~NodeEmitFlags.HasNodeEmitFlags; + return node.emitFlags; } /** @@ -329,7 +329,7 @@ namespace ts { */ function setNodeEmitFlags(node: T, emitFlags: NodeEmitFlags) { beforeSetAnnotation(node); - node.emitFlags = emitFlags | NodeEmitFlags.HasNodeEmitFlags; + node.emitFlags = emitFlags; return node; } diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index 0b590801b91..d11b2f5663d 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -2768,11 +2768,6 @@ namespace ts { && resolver.isTopLevelValueImportEqualsWithEntityName(node)); } - function disableCommentsRecursive(node: Node) { - setNodeEmitFlags(node, NodeEmitFlags.NoComments | getNodeEmitFlags(node)); - forEachChild(node, disableCommentsRecursive); - } - /** * Visits an import equals declaration. * @@ -2788,7 +2783,8 @@ namespace ts { } const moduleReference = createExpressionFromEntityName(node.moduleReference); - disableCommentsRecursive(moduleReference); + setNodeEmitFlags(moduleReference, NodeEmitFlags.NoComments | NodeEmitFlags.NoNestedComments); + if (isNamedExternalModuleExport(node) || !isNamespaceExport(node)) { // export var ${name} = ${moduleReference}; // var ${name} = ${moduleReference}; diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 528093e8050..30899f7aa2a 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2984,21 +2984,21 @@ namespace ts { EmitSuperHelper = 1 << 2, // Emit the basic _super helper for async methods. EmitAdvancedSuperHelper = 1 << 3, // Emit the advanced _super helper for async methods. UMDDefine = 1 << 4, // This node should be replaced with the UMD define helper. - NoLexicalEnvironment = 1 << 5, // A new LexicalEnvironment should *not* be introduced when emitting this node, this is primarily used when printing a SystemJS module. - SingleLine = 1 << 6, // The contents of this node should be emitted on a single line. - AdviseOnEmitNode = 1 << 7, // The printer should invoke the onEmitNode callback when printing this node. - NoSubstitution = 1 << 8, // Disables further substitution of an expression. - CapturesThis = 1 << 9, // The function captures a lexical `this` - NoLeadingSourceMap = 1 << 10, // Do not emit a leading source map location for this node. - NoTrailingSourceMap = 1 << 11, // Do not emit a trailing source map location for this node. + SingleLine = 1 << 5, // The contents of this node should be emitted on a single line. + AdviseOnEmitNode = 1 << 6, // The printer should invoke the onEmitNode callback when printing this node. + NoSubstitution = 1 << 7, // Disables further substitution of an expression. + CapturesThis = 1 << 8, // The function captures a lexical `this` + NoLeadingSourceMap = 1 << 9, // Do not emit a leading source map location for this node. + NoTrailingSourceMap = 1 << 10, // Do not emit a trailing source map location for this node. NoSourceMap = NoLeadingSourceMap | NoTrailingSourceMap, // Do not emit a source map location for this node. - NoNestedSourceMaps = 1 << 12, // Do not emit source map locations for children of this node. - NoTokenLeadingSourceMaps = 1 << 13, // Do not emit leading source map location for token nodes. - NoTokenTrailingSourceMaps = 1 << 14, // Do not emit trailing source map location for token nodes. + NoNestedSourceMaps = 1 << 11, // Do not emit source map locations for children of this node. + NoTokenLeadingSourceMaps = 1 << 12, // Do not emit leading source map location for token nodes. + NoTokenTrailingSourceMaps = 1 << 13, // Do not emit trailing source map location for token nodes. NoTokenSourceMaps = NoTokenLeadingSourceMaps | NoTokenTrailingSourceMaps, // Do not emit source map locations for tokens of this node. - NoLeadingComments = 1 << 15, // Do not emit leading comments for this node. - NoTrailingComments = 1 << 16, // Do not emit trailing comments for this node. + NoLeadingComments = 1 << 14, // Do not emit leading comments for this node. + NoTrailingComments = 1 << 15, // Do not emit trailing comments for this node. NoComments = NoLeadingComments | NoTrailingComments, // Do not emit comments for this node. + NoNestedComments = 1 << 16, ExportName = 1 << 17, // Ensure an export prefix is added for an identifier that points to an exported declaration with a local name (see SymbolFlags.ExportHasLocal). LocalName = 1 << 18, // Ensure an export prefix is not added for an identifier that points to an exported declaration. Indented = 1 << 19, // Adds an explicit extra indentation level for class and function bodies when printing (used to match old emitter). @@ -3007,10 +3007,8 @@ namespace ts { // TODO(rbuckton): These should be removed once source maps are aligned with the old // emitter and new baselines are taken. This exists solely to // align with the old emitter. - SourceMapEmitOpenBraceAsToken = 1 << 21, // Emits the open brace of a block function body as a source mapped token. - SourceMapAdjustRestParameterLoop = 1 << 22, // Emits adjusted source map positions for a ForStatement generated when transforming a rest parameter for ES5/3. - - HasNodeEmitFlags = 1 << 31, // Indicates the node has emit flags set. + SourceMapEmitOpenBraceAsToken = 1 << 20, // Emits the open brace of a block function body as a source mapped token. + SourceMapAdjustRestParameterLoop = 1 << 21, // Emits adjusted source map positions for a ForStatement generated when transforming a rest parameter for ES5/3. } /** Additional context provided to `visitEachChild` */ From bf9d2c4bebbeea5bcf581b5b27b2c8ae80b04930 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 1 Jun 2016 15:10:00 -0700 Subject: [PATCH 8/8] Updated comments for iterateCommentRanges --- src/compiler/scanner.ts | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index 5c6c0440243..b39131b770e 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -584,15 +584,24 @@ namespace ts { } /** - * Extract comments from text prefixing the token closest following `pos`. - * The return value is an array containing a TextRange for each comment. - * Single-line comment ranges include the beginning '//' characters but not the ending line break. - * Multi - line comment ranges include the beginning '/* and ending '/' characters. - * The return value is undefined if no comments were found. - * @param trailing - * If false, whitespace is skipped until the first line break and comments between that location - * and the next token are returned. - * If true, comments occurring between the given position and the next line break are returned. + * Invokes a callback for each comment range following the provided position. + * + * Single-line comment ranges include the leading double-slash characters but not the ending + * line break. Multi-line comment ranges include the leading slash-asterisk and trailing + * asterisk-slash characters. + * + * @param reduce If true, accumulates the result of calling the callback in a fashion similar + * to reduceLeft. If false, iteration stops when the callback returns a truthy value. + * @param text The source text to scan. + * @param pos The position at which to start scanning. + * @param trailing If false, whitespace is skipped until the first line break and comments + * between that location and the next token are returned. If true, comments occurring + * between the given position and the next line break are returned. + * @param cb The callback to execute as each comment range is encountered. + * @param state A state value to pass to each iteration of the callback. + * @param initial An initial value to pass when accumulating results (when "reduce" is true). + * @returns If "reduce" is true, the accumulated value. If "reduce" is false, the first truthy + * return value of the callback. */ function iterateCommentRanges(reduce: boolean, text: string, pos: number, trailing: boolean, cb: (pos: number, end: number, kind: SyntaxKind, hasTrailingNewLine: boolean, state: T, memo: U) => U, state: T, initial?: U): U { let pendingPos: number;