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 b1f3a57999c..bffdc5809b0 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); } diff --git a/src/compiler/transformers/module/module.ts b/src/compiler/transformers/module/module.ts index ce405694cb4..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 ); } @@ -1140,7 +1156,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/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index 839e740b755..bf4ae5ce7d3 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); } @@ -2631,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; @@ -2689,8 +2692,13 @@ namespace ts { ); setOriginalNode(enumStatement, node); + if (varAdded) { + // If a variable was added, synthetic comments are emitted 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 @@ -2880,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; } @@ -2920,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; @@ -2977,8 +2986,13 @@ namespace ts { ); setOriginalNode(moduleStatement, node); + if (varAdded) { + // If a variable was added, synthetic comments are emitted 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 6c74c558b16..31422003b44 100644 --- a/src/testRunner/unittests/transform.ts +++ b/src/testRunner/unittests/transform.ts @@ -268,33 +268,115 @@ 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 + testBaseline("transformAddCommentToExportedVar", () => { + return transpileModule(`export const exportedDirectly = 1; +const exportedSeparately = 2; +export {exportedSeparately}; +`, { + transformers: { + before: [addSyntheticComment(isVariableStatement)], + }, + compilerOptions: { + target: ScriptTarget.ES5, + newLine: NewLineKind.CarriageReturnLineFeed, + } + }).outputText; + }); + + // 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(n => isImportDeclaration(n) || isExportDeclaration(n) || isImportSpecifier(n) || isExportSpecifier(n))], + }, + compilerOptions: { + target: ScriptTarget.ES5, + newLine: NewLineKind.CarriageReturnLineFeed, + } + }).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; + }); + + 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.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.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.transformAddCommentToExportedVar.js b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToExportedVar.js new file mode 100644 index 00000000000..d4f60514b8f --- /dev/null +++ b/tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToExportedVar.js @@ -0,0 +1,7 @@ +"use strict"; +Object.defineProperty(exports, "__esModule", { value: true }); +/*comment*/ +exports.exportedDirectly = 1; +/*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 new file mode 100644 index 00000000000..de19b854d20 --- /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")); 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 = {})); 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;