maxmustermann/TypeScript
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge pull request #25604 from mprobst/fix-exported-var-comments

Browse source 
Retain synthetic comments on exports.
This commit is contained in:
Mohamed Hegazy 2018-07-24 17:41:44 -07:00 committed by GitHub
parent 23eb591e01 7d44014c56
commit fd2eb4918d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 253 additions and 89 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
src/compiler/comments.ts
View file

@ -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);
}
}
}
}

5
src/compiler/transformers/es2015.ts
View file

@ -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);
}

142
src/compiler/transformers/module/module.ts
View file

@ -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 {

24
src/compiler/transformers/ts.ts
View file

@ -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

110
src/testRunner/unittests/transform.ts
View file

@ -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<T extends Node>(node: T): VisitResult<T> {
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<T extends Node>(node: T): VisitResult<T> {
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;
});
});
}

2
tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToArrowReturnValue.js
View file

@ -1,4 +1,4 @@
var foo = function () {
//// comment!
/*comment*/
return void 0;
};

7
tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToClassProp.js Normal file
View file

@ -0,0 +1,7 @@
class X {
constructor() {
// new comment!
// original comment.
this.foo = 1;
}
}

7
tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToExportedVar.js Normal file
View file

@ -0,0 +1,7 @@
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
/*comment*/
exports.exportedDirectly = 1;
/*comment*/
var exportedSeparately = 2;
exports.exportedSeparately = exportedSeparately;

16
tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToImport.js Normal file
View file

@ -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"));

9
tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToNamespace.js Normal file
View file

@ -0,0 +1,9 @@
/*comment*/
var Foo;
(function (Foo) {
Foo.x = 1;
})(Foo || (Foo = {}));
/*comment*/
(function (Foo) {
Foo.y = 1;
})(Foo || (Foo = {}));

12
tests/baselines/reference/transformApi/transformsCorrectly.transformAddCommentToProperties.js Normal file
View file

@ -0,0 +1,12 @@
/*comment*/
class Clazz {
/*comment*/
constructor(/*comment*/
field = 1) {
this.field = field;
/*comment*/
this.instanceProp = 2;
}
}
/*comment*/
Clazz.staticProp = 1;