From db888b86709b0a55d3105dc882ae98297e853fb6 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Thu, 12 Jul 2018 12:23:24 +0200 Subject: [PATCH 1/7] Retain synthetic comments on exported variables. Variables that do not have a local variable created get transformed into a single exports assignment expression. TypeScript previously just created a new expression and set the text range to retain original comments, but for synthetic comments, merging the emit nodes by setting the original node is required. --- src/compiler/transformers/module/module.ts | 2 +- src/testRunner/unittests/transform.ts | 30 +++++++++++++++++++ ...rectly.transformAddCommentToExportedVar.js | 7 +++++ 3 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToExportedVar.js diff --git a/src/compiler/transformers/module/module.ts b/src/compiler/transformers/module/module.ts index ce405694cb4..6df143d0265 100644 --- a/src/compiler/transformers/module/module.ts +++ b/src/compiler/transformers/module/module.ts @@ -1140,7 +1140,7 @@ namespace ts { } if (expressions) { - statements = append(statements, setTextRange(createExpressionStatement(inlineExpressions(expressions)), node)); + statements = append(statements, setOriginalNode(setTextRange(createExpressionStatement(inlineExpressions(expressions)), node), node)); } } else { diff --git a/src/testRunner/unittests/transform.ts b/src/testRunner/unittests/transform.ts index 6c74c558b16..547b22f2cbc 100644 --- a/src/testRunner/unittests/transform.ts +++ b/src/testRunner/unittests/transform.ts @@ -296,6 +296,36 @@ namespace ts { } } }); + + // https://github.com/Microsoft/TypeScript/issues/17594 + testBaseline("transformAddCommentToExportedVar", () => { + return transpileModule(`export const exportedDirectly = 1; +const exportedSeparately = 2; +export {exportedSeparately}; +`, { + transformers: { + before: [addSyntheticComment], + }, + compilerOptions: { + target: ScriptTarget.ES5, + newLine: NewLineKind.CarriageReturnLineFeed, + } + }).outputText; + + function addSyntheticComment(context: TransformationContext) { + return (sourceFile: SourceFile): SourceFile => { + return visitNode(sourceFile, rootTransform, isSourceFile); + }; + function rootTransform(node: T): VisitResult { + if (isVariableStatement(node)) { + setEmitFlags(node, EmitFlags.NoLeadingComments); + setSyntheticLeadingComments(node, [{ kind: SyntaxKind.MultiLineCommentTrivia, text: "* @type {number} ", pos: -1, end: -1, hasTrailingNewLine: true }]); + return node; + } + return visitEachChild(node, rootTransform, context); + } + } + }); }); } diff --git a/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToExportedVar.js b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToExportedVar.js new file mode 100644 index 00000000000..32be082c1f4 --- /dev/null +++ b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToExportedVar.js @@ -0,0 +1,7 @@ +"use strict"; +Object.defineProperty(exports, "__esModule", { value: true }); +/** @type {number} */ +exports.exportedDirectly = 1; +/** @type {number} */ +var exportedSeparately = 2; +exports.exportedSeparately = exportedSeparately; From 6f114a2c9b49e9f9d66e3a214f9a9c24046352c6 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Thu, 12 Jul 2018 13:11:06 +0200 Subject: [PATCH 2/7] Fix comments for import and export declarations. Previously, TypeScript would only set the text range when transforming import and export declarations, leading it to drop synthetic comments and emit flags. This commit sets the original node on the statements that contains the generated `require()` call (or similar, depending on module system), retaining emit flags and synthetic comments. --- src/compiler/transformers/module/module.ts | 140 ++++++++++-------- src/testRunner/unittests/transform.ts | 35 ++++- ...msCorrectly.transformAddCommentToImport.js | 16 ++ 3 files changed, 128 insertions(+), 63 deletions(-) create mode 100644 tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToImport.js diff --git a/src/compiler/transformers/module/module.ts b/src/compiler/transformers/module/module.ts index 6df143d0265..09bf1b75b9c 100644 --- a/src/compiler/transformers/module/module.ts +++ b/src/compiler/transformers/module/module.ts @@ -767,7 +767,7 @@ namespace ts { if (moduleKind !== ModuleKind.AMD) { if (!node.importClause) { // import "mod"; - return setTextRange(createExpressionStatement(createRequireCall(node)), node); + return setOriginalNode(setTextRange(createExpressionStatement(createRequireCall(node)), node), node); } else { const variables: VariableDeclaration[] = []; @@ -806,15 +806,17 @@ namespace ts { } statements = append(statements, - setTextRange( - createVariableStatement( - /*modifiers*/ undefined, - createVariableDeclarationList( - variables, - languageVersion >= ScriptTarget.ES2015 ? NodeFlags.Const : NodeFlags.None - ) - ), - /*location*/ node + setOriginalNode( + setTextRange( + createVariableStatement( + /*modifiers*/ undefined, + createVariableDeclarationList( + variables, + languageVersion >= ScriptTarget.ES2015 ? NodeFlags.Const : NodeFlags.None + ) + ), + /*location*/ node), + /*original*/ node ) ); } @@ -826,13 +828,15 @@ namespace ts { /*modifiers*/ undefined, createVariableDeclarationList( [ - setTextRange( - createVariableDeclaration( - getSynthesizedClone(namespaceDeclaration.name), - /*type*/ undefined, - getGeneratedNameForNode(node) - ), - /*location*/ node + setOriginalNode( + setTextRange( + createVariableDeclaration( + getSynthesizedClone(namespaceDeclaration.name), + /*type*/ undefined, + getGeneratedNameForNode(node) + ), + /*location*/ node), + /*original*/ node ) ], languageVersion >= ScriptTarget.ES2015 ? NodeFlags.Const : NodeFlags.None @@ -880,33 +884,37 @@ namespace ts { if (moduleKind !== ModuleKind.AMD) { if (hasModifier(node, ModifierFlags.Export)) { statements = append(statements, - setTextRange( - createExpressionStatement( - createExportExpression( - node.name, - createRequireCall(node) - ) - ), + setOriginalNode( + setTextRange( + createExpressionStatement( + createExportExpression( + node.name, + createRequireCall(node) + ) + ), + node), node ) ); } else { statements = append(statements, - setTextRange( - createVariableStatement( - /*modifiers*/ undefined, - createVariableDeclarationList( - [ - createVariableDeclaration( - getSynthesizedClone(node.name), - /*type*/ undefined, - createRequireCall(node) - ) - ], - /*flags*/ languageVersion >= ScriptTarget.ES2015 ? NodeFlags.Const : NodeFlags.None - ) - ), + setOriginalNode( + setTextRange( + createVariableStatement( + /*modifiers*/ undefined, + createVariableDeclarationList( + [ + createVariableDeclaration( + getSynthesizedClone(node.name), + /*type*/ undefined, + createRequireCall(node) + ) + ], + /*flags*/ languageVersion >= ScriptTarget.ES2015 ? NodeFlags.Const : NodeFlags.None + ) + ), + node), node ) ); @@ -915,10 +923,12 @@ namespace ts { else { if (hasModifier(node, ModifierFlags.Export)) { statements = append(statements, - setTextRange( - createExpressionStatement( - createExportExpression(getExportName(node), getLocalName(node)) - ), + setOriginalNode( + setTextRange( + createExpressionStatement( + createExportExpression(getExportName(node), getLocalName(node)) + ), + node), node ) ); @@ -956,18 +966,20 @@ namespace ts { // export { x, y } from "mod"; if (moduleKind !== ModuleKind.AMD) { statements.push( - setTextRange( - createVariableStatement( - /*modifiers*/ undefined, - createVariableDeclarationList([ - createVariableDeclaration( - generatedName, - /*type*/ undefined, - createRequireCall(node) - ) - ]) - ), - /*location*/ node + setOriginalNode( + setTextRange( + createVariableStatement( + /*modifiers*/ undefined, + createVariableDeclarationList([ + createVariableDeclaration( + generatedName, + /*type*/ undefined, + createRequireCall(node) + ) + ]) + ), + /*location*/ node), + /* original */ node ) ); } @@ -977,10 +989,12 @@ namespace ts { specifier.propertyName || specifier.name ); statements.push( - setTextRange( - createExpressionStatement( - createExportExpression(getExportName(specifier), exportedValue) - ), + setOriginalNode( + setTextRange( + createExpressionStatement( + createExportExpression(getExportName(specifier), exportedValue) + ), + specifier), specifier ) ); @@ -990,10 +1004,12 @@ namespace ts { } else { // export * from "mod"; - return setTextRange( - createExpressionStatement( - createExportStarHelper(context, moduleKind !== ModuleKind.AMD ? createRequireCall(node) : generatedName) - ), + return setOriginalNode( + setTextRange( + createExpressionStatement( + createExportStarHelper(context, moduleKind !== ModuleKind.AMD ? createRequireCall(node) : generatedName) + ), + node), node ); } diff --git a/src/testRunner/unittests/transform.ts b/src/testRunner/unittests/transform.ts index 547b22f2cbc..bbba52f2fdd 100644 --- a/src/testRunner/unittests/transform.ts +++ b/src/testRunner/unittests/transform.ts @@ -326,6 +326,39 @@ export {exportedSeparately}; } } }); - }); + + // https://github.com/Microsoft/TypeScript/issues/17594 + testBaseline("transformAddCommentToImport", () => { + return transpileModule(` +// Previous comment on import. +import {Value} from 'somewhere'; +import * as X from 'somewhere'; +// Previous comment on export. +export { /* specifier comment */ X, Y} from 'somewhere'; +export * from 'somewhere'; +export {Value}; +`, { + transformers: { + before: [addSyntheticComment], + }, + compilerOptions: { + target: ScriptTarget.ES5, + newLine: NewLineKind.CarriageReturnLineFeed, + } + }).outputText; + + function addSyntheticComment(context: TransformationContext) { + return (sourceFile: SourceFile): SourceFile => { + return visitNode(sourceFile, rootTransform, isSourceFile); + }; + function rootTransform(node: T): VisitResult { + if (isImportDeclaration(node) || isExportDeclaration(node) || isImportSpecifier(node) || isExportSpecifier(node)) { + setEmitFlags(node, EmitFlags.NoLeadingComments); + setSyntheticLeadingComments(node, [{ kind: SyntaxKind.MultiLineCommentTrivia, text: `comment!`, pos: -1, end: -1, hasTrailingNewLine: true }]); + } + return visitEachChild(node, rootTransform, context); + } + } + }); }); } diff --git a/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToImport.js b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToImport.js new file mode 100644 index 00000000000..9fe48f05e61 --- /dev/null +++ b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToImport.js @@ -0,0 +1,16 @@ +"use strict"; +function __export(m) { + for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p]; +} +Object.defineProperty(exports, "__esModule", { value: true }); +/*comment!*/ +var somewhere_1 = require("somewhere"); +exports.Value = somewhere_1.Value; +/*comment!*/ +var somewhere_2 = require("somewhere"); +/*comment!*/ +exports.X = somewhere_2.X; +/*comment!*/ +exports.Y = somewhere_2.Y; +/*comment!*/ +__export(require("somewhere")); From 1bd79af7602a6b169df165890e69ff1e7eca1ebb Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Thu, 12 Jul 2018 15:56:03 +0200 Subject: [PATCH 3/7] Reduce duplication of addSyntheticComment. --- src/testRunner/unittests/transform.ts | 65 ++++++------------- ...y.transformAddCommentToArrowReturnValue.js | 2 +- ...rectly.transformAddCommentToExportedVar.js | 4 +- ...msCorrectly.transformAddCommentToImport.js | 10 +-- 4 files changed, 28 insertions(+), 53 deletions(-) diff --git a/src/testRunner/unittests/transform.ts b/src/testRunner/unittests/transform.ts index bbba52f2fdd..cae0ee7deb2 100644 --- a/src/testRunner/unittests/transform.ts +++ b/src/testRunner/unittests/transform.ts @@ -268,33 +268,34 @@ namespace ts { return fs.readFileSync("/.src/index.d.ts").toString(); } + function addSyntheticComment(nodeFilter: (node: Node) => boolean) { + return (context: TransformationContext) => { + return (sourceFile: SourceFile): SourceFile => { + return visitNode(sourceFile, rootTransform, isSourceFile); + }; + function rootTransform(node: T): VisitResult { + if (nodeFilter(node)) { + setEmitFlags(node, EmitFlags.NoLeadingComments); + setSyntheticLeadingComments(node, [{ kind: SyntaxKind.MultiLineCommentTrivia, text: "comment", pos: -1, end: -1, hasTrailingNewLine: true }]); + } + return visitEachChild(node, rootTransform, context); + } + }; + } + // https://github.com/Microsoft/TypeScript/issues/24096 testBaseline("transformAddCommentToArrowReturnValue", () => { return transpileModule(`const foo = () => void 0 `, { transformers: { - before: [addSyntheticComment], + before: [addSyntheticComment(isVoidExpression)], }, compilerOptions: { target: ScriptTarget.ES5, newLine: NewLineKind.CarriageReturnLineFeed, } }).outputText; - - function addSyntheticComment(context: TransformationContext) { - return (sourceFile: SourceFile): SourceFile => { - return visitNode(sourceFile, rootTransform, isSourceFile); - }; - function rootTransform(node: T): VisitResult { - if (isVoidExpression(node)) { - setEmitFlags(node, EmitFlags.NoLeadingComments); - setSyntheticLeadingComments(node, [{ kind: SyntaxKind.SingleLineCommentTrivia, text: "// comment!", pos: -1, end: -1, hasTrailingNewLine: true }]); - return node; - } - return visitEachChild(node, rootTransform, context); - } - } }); // https://github.com/Microsoft/TypeScript/issues/17594 @@ -304,27 +305,13 @@ const exportedSeparately = 2; export {exportedSeparately}; `, { transformers: { - before: [addSyntheticComment], + before: [addSyntheticComment(isVariableStatement)], }, compilerOptions: { target: ScriptTarget.ES5, newLine: NewLineKind.CarriageReturnLineFeed, } }).outputText; - - function addSyntheticComment(context: TransformationContext) { - return (sourceFile: SourceFile): SourceFile => { - return visitNode(sourceFile, rootTransform, isSourceFile); - }; - function rootTransform(node: T): VisitResult { - if (isVariableStatement(node)) { - setEmitFlags(node, EmitFlags.NoLeadingComments); - setSyntheticLeadingComments(node, [{ kind: SyntaxKind.MultiLineCommentTrivia, text: "* @type {number} ", pos: -1, end: -1, hasTrailingNewLine: true }]); - return node; - } - return visitEachChild(node, rootTransform, context); - } - } }); // https://github.com/Microsoft/TypeScript/issues/17594 @@ -339,26 +326,14 @@ export * from 'somewhere'; export {Value}; `, { transformers: { - before: [addSyntheticComment], + before: [addSyntheticComment(n => isImportDeclaration(n) || isExportDeclaration(n) || isImportSpecifier(n) || isExportSpecifier(n))], }, compilerOptions: { target: ScriptTarget.ES5, newLine: NewLineKind.CarriageReturnLineFeed, } }).outputText; - - function addSyntheticComment(context: TransformationContext) { - return (sourceFile: SourceFile): SourceFile => { - return visitNode(sourceFile, rootTransform, isSourceFile); - }; - function rootTransform(node: T): VisitResult { - if (isImportDeclaration(node) || isExportDeclaration(node) || isImportSpecifier(node) || isExportSpecifier(node)) { - setEmitFlags(node, EmitFlags.NoLeadingComments); - setSyntheticLeadingComments(node, [{ kind: SyntaxKind.MultiLineCommentTrivia, text: `comment!`, pos: -1, end: -1, hasTrailingNewLine: true }]); - } - return visitEachChild(node, rootTransform, context); - } - } - }); }); + }); + }); } diff --git a/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToArrowReturnValue.js b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToArrowReturnValue.js index 37ff0597054..5697e888ef0 100644 --- a/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToArrowReturnValue.js +++ b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToArrowReturnValue.js @@ -1,4 +1,4 @@ var foo = function () { - //// comment! + /*comment*/ return void 0; }; diff --git a/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToExportedVar.js b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToExportedVar.js index 32be082c1f4..d4f60514b8f 100644 --- a/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToExportedVar.js +++ b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToExportedVar.js @@ -1,7 +1,7 @@ "use strict"; Object.defineProperty(exports, "__esModule", { value: true }); -/** @type {number} */ +/*comment*/ exports.exportedDirectly = 1; -/** @type {number} */ +/*comment*/ var exportedSeparately = 2; exports.exportedSeparately = exportedSeparately; diff --git a/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToImport.js b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToImport.js index 9fe48f05e61..de19b854d20 100644 --- a/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToImport.js +++ b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToImport.js @@ -3,14 +3,14 @@ function __export(m) { for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p]; } Object.defineProperty(exports, "__esModule", { value: true }); -/*comment!*/ +/*comment*/ var somewhere_1 = require("somewhere"); exports.Value = somewhere_1.Value; -/*comment!*/ +/*comment*/ var somewhere_2 = require("somewhere"); -/*comment!*/ +/*comment*/ exports.X = somewhere_2.X; -/*comment!*/ +/*comment*/ exports.Y = somewhere_2.Y; -/*comment!*/ +/*comment*/ __export(require("somewhere")); From 2529b864f4ab8df534aa7da4c1f22999fb020374 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Thu, 12 Jul 2018 15:05:10 +0200 Subject: [PATCH 4/7] Retain synthetic comments on classes and their properties. --- src/compiler/transformers/ts.ts | 2 ++ src/testRunner/unittests/transform.ts | 23 +++++++++++++++++++ ...orrectly.transformAddCommentToClassProp.js | 7 ++++++ ...rrectly.transformAddCommentToProperties.js | 12 ++++++++++ 4 files changed, 44 insertions(+) create mode 100644 tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToClassProp.js create mode 100644 tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToProperties.js diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index 839e740b755..e1bb95481a1 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -1245,6 +1245,7 @@ namespace ts { const statement = createExpressionStatement(transformInitializedProperty(property, receiver)); setSourceMapRange(statement, moveRangePastModifiers(property)); setCommentRange(statement, property); + setOriginalNode(statement, property); statements.push(statement); } } @@ -1262,6 +1263,7 @@ namespace ts { startOnNewLine(expression); setSourceMapRange(expression, moveRangePastModifiers(property)); setCommentRange(expression, property); + setOriginalNode(expression, property); expressions.push(expression); } diff --git a/src/testRunner/unittests/transform.ts b/src/testRunner/unittests/transform.ts index cae0ee7deb2..d2a61fb1248 100644 --- a/src/testRunner/unittests/transform.ts +++ b/src/testRunner/unittests/transform.ts @@ -334,6 +334,29 @@ export {Value}; } }).outputText; }); + + // https://github.com/Microsoft/TypeScript/issues/17594 + testBaseline("transformAddCommentToProperties", () => { + return transpileModule(` +// class comment. +class Clazz { + // original comment 1. + static staticProp: number = 1; + // original comment 2. + instanceProp: number = 2; + // original comment 3. + constructor(readonly field = 1) {} +} +`, { + transformers: { + before: [addSyntheticComment(n => isPropertyDeclaration(n) || isParameterPropertyDeclaration(n) || isClassDeclaration(n) || isConstructorDeclaration(n))], + }, + compilerOptions: { + target: ScriptTarget.ES2015, + newLine: NewLineKind.CarriageReturnLineFeed, + } + }).outputText; + }); }); } diff --git a/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToClassProp.js b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToClassProp.js new file mode 100644 index 00000000000..1e316e31107 --- /dev/null +++ b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToClassProp.js @@ -0,0 +1,7 @@ +class X { + constructor() { + // new comment! + // original comment. + this.foo = 1; + } +} diff --git a/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToProperties.js b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToProperties.js new file mode 100644 index 00000000000..d5f27d7148c --- /dev/null +++ b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToProperties.js @@ -0,0 +1,12 @@ +/*comment*/ +class Clazz { + /*comment*/ + constructor(/*comment*/ + field = 1) { + this.field = field; + /*comment*/ + this.instanceProp = 2; + } +} +/*comment*/ +Clazz.staticProp = 1; From a7224ec612d7256a84692f534eb4d763219e4c67 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Wed, 18 Jul 2018 17:22:18 +0200 Subject: [PATCH 5/7] Do not emit comments if container had a comment suppression. --- src/compiler/comments.ts | 8 +++++--- src/compiler/transformers/es2015.ts | 5 ++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/compiler/comments.ts b/src/compiler/comments.ts index 989935a5edb..7dfda66c965 100644 --- a/src/compiler/comments.ts +++ b/src/compiler/comments.ts @@ -72,11 +72,13 @@ namespace ts { const savedContainerEnd = containerEnd; const savedDeclarationListContainerEnd = declarationListContainerEnd; - if (!skipLeadingComments) { + if (!skipLeadingComments || (pos >= 0 && (emitFlags & EmitFlags.NoLeadingComments) !== 0)) { + // Advance the container position of comments get emitted or if they've been disabled explicitly using NoLeadingComments. containerPos = pos; } - if (!skipTrailingComments) { + if (!skipTrailingComments || (end >= 0 && (emitFlags & EmitFlags.NoTrailingComments) !== 0)) { + // As above. containerEnd = end; // To avoid invalid comment emit in a down-level binding pattern, we @@ -426,4 +428,4 @@ namespace ts { return isRecognizedTripleSlashComment(currentText, commentPos, commentEnd); } } -} \ No newline at end of file +} diff --git a/src/compiler/transformers/es2015.ts b/src/compiler/transformers/es2015.ts index 619cc505c8b..39a659486b5 100644 --- a/src/compiler/transformers/es2015.ts +++ b/src/compiler/transformers/es2015.ts @@ -1121,7 +1121,7 @@ namespace ts { } // Perform the capture. - captureThisForNode(statements, ctor, superCallExpression || createActualThis(), firstStatement); + captureThisForNode(statements, ctor, superCallExpression || createActualThis()); // If we're actually replacing the original statement, we need to signal this to the caller. if (superCallExpression) { @@ -1443,7 +1443,7 @@ namespace ts { } } - function captureThisForNode(statements: Statement[], node: Node, initializer: Expression | undefined, originalStatement?: Statement): void { + function captureThisForNode(statements: Statement[], node: Node, initializer: Expression | undefined): void { enableSubstitutionsForCapturedThis(); const captureThisStatement = createVariableStatement( /*modifiers*/ undefined, @@ -1456,7 +1456,6 @@ namespace ts { ]) ); setEmitFlags(captureThisStatement, EmitFlags.NoComments | EmitFlags.CustomPrologue); - setTextRange(captureThisStatement, originalStatement); setSourceMapRange(captureThisStatement, node); statements.push(captureThisStatement); } From c50a6f73894e2e43006ec837b928aa45e9effae1 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Wed, 18 Jul 2018 16:45:53 +0200 Subject: [PATCH 6/7] Fix comment emit for namespaces & enums. Keep emit flags set on the original node on a namespace or enum node. This prevents dropping flags like NoComments, which caused duplicated comment emits. Additionally, TypeScript would emit synthetic comments twice, once for the variable declaration, once for the module statement. This explicitly clears away synthetic comments on namespaces and enums if their synthetic comments have already been emitted on the corresponding variable statement. --- src/compiler/transformers/ts.ts | 22 ++++++++++++++----- src/testRunner/unittests/transform.ts | 21 ++++++++++++++++++ ...orrectly.transformAddCommentToNamespace.js | 9 ++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToNamespace.js diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index e1bb95481a1..d8caea1fde8 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -2633,7 +2633,8 @@ namespace ts { // 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 (addVarForEnumOrModuleDeclaration(statements, node)) { + const varAdded = addVarForEnumOrModuleDeclaration(statements, node); + if (varAdded) { // We should still emit the comments if we are emitting a system module. if (moduleKind !== ModuleKind.System || currentLexicalScope !== currentSourceFile) { emitFlags |= EmitFlags.NoLeadingComments; @@ -2691,8 +2692,13 @@ namespace ts { ); setOriginalNode(enumStatement, node); + if (varAdded) { + // If a variable was added, synthetic comments are mitted on it, not on the moduleStatement. + setSyntheticLeadingComments(enumStatement, undefined); + setSyntheticTrailingComments(enumStatement, undefined); + } setTextRange(enumStatement, node); - setEmitFlags(enumStatement, emitFlags); + addEmitFlags(enumStatement, emitFlags); statements.push(enumStatement); // Add a DeclarationMarker for the enum to preserve trailing comments and mark @@ -2882,7 +2888,7 @@ namespace ts { // })(m1 || (m1 = {})); // trailing comment module // setCommentRange(statement, node); - setEmitFlags(statement, EmitFlags.NoTrailingComments | EmitFlags.HasEndOfDeclarationMarker); + addEmitFlags(statement, EmitFlags.NoTrailingComments | EmitFlags.HasEndOfDeclarationMarker); statements.push(statement); return true; } @@ -2922,7 +2928,8 @@ namespace ts { // 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 (addVarForEnumOrModuleDeclaration(statements, node)) { + const varAdded = addVarForEnumOrModuleDeclaration(statements, node); + if (varAdded) { // We should still emit the comments if we are emitting a system module. if (moduleKind !== ModuleKind.System || currentLexicalScope !== currentSourceFile) { emitFlags |= EmitFlags.NoLeadingComments; @@ -2979,8 +2986,13 @@ namespace ts { ); setOriginalNode(moduleStatement, node); + if (varAdded) { + // If a variable was added, synthetic comments are mitted on it, not on the moduleStatement. + setSyntheticLeadingComments(moduleStatement, undefined); + setSyntheticTrailingComments(moduleStatement, undefined); + } setTextRange(moduleStatement, node); - setEmitFlags(moduleStatement, emitFlags); + addEmitFlags(moduleStatement, emitFlags); statements.push(moduleStatement); // Add a DeclarationMarker for the namespace to preserve trailing comments and mark diff --git a/src/testRunner/unittests/transform.ts b/src/testRunner/unittests/transform.ts index d2a61fb1248..31422003b44 100644 --- a/src/testRunner/unittests/transform.ts +++ b/src/testRunner/unittests/transform.ts @@ -357,6 +357,27 @@ class Clazz { } }).outputText; }); + + testBaseline("transformAddCommentToNamespace", () => { + return transpileModule(` +// namespace comment. +namespace Foo { + export const x = 1; +} +// another comment. +namespace Foo { + export const y = 1; +} +`, { + transformers: { + before: [addSyntheticComment(n => isModuleDeclaration(n))], + }, + compilerOptions: { + target: ScriptTarget.ES2015, + newLine: NewLineKind.CarriageReturnLineFeed, + } + }).outputText; + }); }); } diff --git a/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToNamespace.js b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToNamespace.js new file mode 100644 index 00000000000..d2a32d437fa --- /dev/null +++ b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToNamespace.js @@ -0,0 +1,9 @@ +/*comment*/ +var Foo; +(function (Foo) { + Foo.x = 1; +})(Foo || (Foo = {})); +/*comment*/ +(function (Foo) { + Foo.y = 1; +})(Foo || (Foo = {})); From 7d44014c5656134d76764adc551a05de83627371 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Tue, 24 Jul 2018 16:37:07 -0700 Subject: [PATCH 7/7] Fix typos --- src/compiler/transformers/ts.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index d8caea1fde8..bf4ae5ce7d3 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -2693,7 +2693,7 @@ namespace ts { setOriginalNode(enumStatement, node); if (varAdded) { - // If a variable was added, synthetic comments are mitted on it, not on the moduleStatement. + // If a variable was added, synthetic comments are emitted on it, not on the moduleStatement. setSyntheticLeadingComments(enumStatement, undefined); setSyntheticTrailingComments(enumStatement, undefined); } @@ -2987,7 +2987,7 @@ namespace ts { setOriginalNode(moduleStatement, node); if (varAdded) { - // If a variable was added, synthetic comments are mitted on it, not on the moduleStatement. + // If a variable was added, synthetic comments are emitted on it, not on the moduleStatement. setSyntheticLeadingComments(moduleStatement, undefined); setSyntheticTrailingComments(moduleStatement, undefined); }