From c50a6f73894e2e43006ec837b928aa45e9effae1 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Wed, 18 Jul 2018 16:45:53 +0200 Subject: [PATCH] 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 = {}));