microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2026-05-17 21:06:50 -05:00
Code Issues Packages Projects Releases 212 Wiki Activity

Revised emit for computed property names, including with decorators (#19430)

Browse Source 
* Revised emit for computed property names

* Fix downlevel name generation scopes

* Accept slightly more conservative baseline

* First feedback pass

* Reduce number of nonrequired variable declarations and assignments

* Remove side-effect-free identifier references

* skip partially emitted expressions

* Comments, move starsOnNewLine to emitNode

* Put expressions on newlines when inlined in class expressions for consistency

* Update new ref

* Fix typo in comment
This commit is contained in:
Wesley Wigham
2017-11-06 12:51:34 -08:00
committed by GitHub
parent ddbd654ecd
commit 4f48bf80fe
29 changed files with 2777 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/compiler/binder.ts
View File

@@ -2963,6 +2963,7 @@ namespace ts {
|| hasModifier(node, ModifierFlags.TypeScriptModifier)
|| node.typeParameters
|| node.type
|| (node.name && isComputedPropertyName(node.name)) // While computed method names aren't typescript, the TS transform must visit them to emit property declarations correctly
|| !node.body) {
transformFlags |= TransformFlags.AssertTypeScript;
}
@@ -2993,6 +2994,7 @@ namespace ts {
if (node.decorators
|| hasModifier(node, ModifierFlags.TypeScriptModifier)
|| node.type
|| (node.name && isComputedPropertyName(node.name)) // While computed accessor names aren't typescript, the TS transform must visit them to emit property declarations correctly
|| !node.body) {
transformFlags |= TransformFlags.AssertTypeScript;
}

64
src/compiler/emitter.ts
View File

@@ -1733,26 +1733,15 @@ namespace ts {
increaseIndent();
}
if (getEmitFlags(node) & EmitFlags.ReuseTempVariableScope) {
emitSignatureHead(node);
if (onEmitNode) {
onEmitNode(EmitHint.Unspecified, body, emitBlockCallback);
}
else {
emitBlockFunctionBody(body);
}
pushNameGenerationScope(node);
emitSignatureHead(node);
if (onEmitNode) {
onEmitNode(EmitHint.Unspecified, body, emitBlockCallback);
}
else {
pushNameGenerationScope();
emitSignatureHead(node);
if (onEmitNode) {
onEmitNode(EmitHint.Unspecified, body, emitBlockCallback);
}
else {
emitBlockFunctionBody(body);
}
popNameGenerationScope();
emitBlockFunctionBody(body);
}
popNameGenerationScope(node);
if (indentedFlag) {
decreaseIndent();
@@ -1871,11 +1860,9 @@ namespace ts {
emitTypeParameters(node, node.typeParameters);
emitList(node, node.heritageClauses, ListFormat.ClassHeritageClauses);
pushNameGenerationScope();
write(" {");
emitList(node, node.members, ListFormat.ClassMembers);
write("}");
popNameGenerationScope();
if (indentedFlag) {
decreaseIndent();
@@ -1909,11 +1896,9 @@ namespace ts {
emitModifiers(node, node.modifiers);
write("enum ");
emit(node.name);
pushNameGenerationScope();
write(" {");
emitList(node, node.members, ListFormat.EnumMembers);
write("}");
popNameGenerationScope();
}
function emitModuleDeclaration(node: ModuleDeclaration) {
@@ -1935,11 +1920,11 @@ namespace ts {
}
function emitModuleBlock(node: ModuleBlock) {
pushNameGenerationScope();
pushNameGenerationScope(node);
write("{");
emitBlockStatements(node, /*forceSingleLine*/ isEmptyBlock(node));
write("}");
popNameGenerationScope();
popNameGenerationScope(node);
}
function emitCaseBlock(node: CaseBlock) {
@@ -2284,11 +2269,11 @@ namespace ts {
function emitSourceFileWorker(node: SourceFile) {
const statements = node.statements;
pushNameGenerationScope();
pushNameGenerationScope(node);
emitHelpersIndirect(node);
const index = findIndex(statements, statement => !isPrologueDirective(statement));
emitList(node, statements, ListFormat.MultiLine, index === -1 ? statements.length : index);
popNameGenerationScope();
popNameGenerationScope(node);
}
// Transformation nodes
@@ -2751,7 +2736,7 @@ namespace ts {
}
}
else {
return nextNode.startsOnNewLine;
return getStartsOnNewLine(nextNode);
}
}
@@ -2782,7 +2767,7 @@ namespace ts {
function synthesizedNodeStartsOnNewLine(node: Node, format?: ListFormat) {
if (nodeIsSynthesized(node)) {
const startsOnNewLine = node.startsOnNewLine;
const startsOnNewLine = getStartsOnNewLine(node);
if (startsOnNewLine === undefined) {
return (format & ListFormat.PreferNewLine) !== 0;
}
@@ -2799,7 +2784,7 @@ namespace ts {
node2 = skipSynthesizedParentheses(node2);
// Always use a newline for synthesized code if the synthesizer desires it.
if (node2.startsOnNewLine) {
if (getStartsOnNewLine(node2)) {
return true;
}
@@ -2858,7 +2843,10 @@ namespace ts {
/**
* Push a new name generation scope.
*/
function pushNameGenerationScope() {
function pushNameGenerationScope(node: Node | undefined) {
if (node && getEmitFlags(node) & EmitFlags.ReuseTempVariableScope) {
return;
}
tempFlagsStack.push(tempFlags);
tempFlags = 0;
}
@@ -2866,7 +2854,10 @@ namespace ts {
/**
* Pop the current name generation scope.
*/
function popNameGenerationScope() {
function popNameGenerationScope(node: Node | undefined) {
if (node && getEmitFlags(node) & EmitFlags.ReuseTempVariableScope) {
return;
}
tempFlags = tempFlagsStack.pop();
}
@@ -2877,8 +2868,17 @@ namespace ts {
if (name.autoGenerateKind === GeneratedIdentifierKind.Node) {
// Node names generate unique names based on their original node
// and are cached based on that node's id.
const node = getNodeForGeneratedName(name);
return generateNameCached(node);
if (name.skipNameGenerationScope) {
const savedTempFlags = tempFlags;
popNameGenerationScope(/*node*/ undefined);
const result = generateNameCached(getNodeForGeneratedName(name));
pushNameGenerationScope(/*node*/ undefined);
tempFlags = savedTempFlags;
return result;
}
else {
return generateNameCached(getNodeForGeneratedName(name));
}
}
else {
// Auto, Loop, and Unique names are cached based on their unique

41
src/compiler/factory.ts
View File

@@ -13,9 +13,6 @@ namespace ts {
if (updated !== original) {
setOriginalNode(updated, original);
setTextRange(updated, original);
if (original.startsOnNewLine) {
updated.startsOnNewLine = true;
}
aggregateTransformFlags(updated);
}
return updated;
@@ -168,11 +165,14 @@ namespace ts {
}
/** Create a unique name generated for a node. */
export function getGeneratedNameForNode(node: Node): Identifier {
export function getGeneratedNameForNode(node: Node): Identifier;
/*@internal*/ export function getGeneratedNameForNode(node: Node, shouldSkipNameGenerationScope?: boolean): Identifier;
export function getGeneratedNameForNode(node: Node, shouldSkipNameGenerationScope?: boolean): Identifier {
const name = createIdentifier("");
name.autoGenerateKind = GeneratedIdentifierKind.Node;
name.autoGenerateId = nextAutoGenerateId;
name.original = node;
name.skipNameGenerationScope = !!shouldSkipNameGenerationScope;
nextAutoGenerateId++;
return name;
}
@@ -2683,6 +2683,24 @@ namespace ts {
return node;
}
/**
* Gets a custom text range to use when emitting comments.
*/
/*@internal*/
export function getStartsOnNewLine(node: Node) {
const emitNode = node.emitNode;
return emitNode && emitNode.startsOnNewLine;
}
/**
* Sets a custom text range to use when emitting comments.
*/
/*@internal*/
export function setStartsOnNewLine<T extends Node>(node: T, newLine: boolean) {
getOrCreateEmitNode(node).startsOnNewLine = newLine;
return node;
}
/**
* Gets a custom text range to use when emitting comments.
*/
@@ -2841,7 +2859,8 @@ namespace ts {
sourceMapRange,
tokenSourceMapRanges,
constantValue,
helpers
helpers,
startsOnNewLine,
} = sourceEmitNode;
if (!destEmitNode) destEmitNode = {};
// We are using `.slice()` here in case `destEmitNode.leadingComments` is pushed to later.
@@ -2853,6 +2872,7 @@ namespace ts {
if (tokenSourceMapRanges) destEmitNode.tokenSourceMapRanges = mergeTokenSourceMapRanges(tokenSourceMapRanges, destEmitNode.tokenSourceMapRanges);
if (constantValue !== undefined) destEmitNode.constantValue = constantValue;
if (helpers) destEmitNode.helpers = addRange(destEmitNode.helpers, helpers);
if (startsOnNewLine !== undefined) destEmitNode.startsOnNewLine = startsOnNewLine;
return destEmitNode;
}
@@ -3014,7 +3034,7 @@ namespace ts {
if (children.length > 1) {
for (const child of children) {
child.startsOnNewLine = true;
startOnNewLine(child);
argumentsList.push(child);
}
}
@@ -3045,7 +3065,7 @@ namespace ts {
if (children && children.length > 0) {
if (children.length > 1) {
for (const child of children) {
child.startsOnNewLine = true;
startOnNewLine(child);
argumentsList.push(child);
}
}
@@ -3620,8 +3640,8 @@ namespace ts {
);
setOriginalNode(updated, node);
setTextRange(updated, node);
if (node.startsOnNewLine) {
updated.startsOnNewLine = true;
if (getStartsOnNewLine(node)) {
setStartsOnNewLine(updated, /*newLine*/ true);
}
aggregateTransformFlags(updated);
return updated;
@@ -4250,8 +4270,7 @@ namespace ts {
}
export function startOnNewLine<T extends Node>(node: T): T {
node.startsOnNewLine = true;
return node;
return setStartsOnNewLine(node, /*newLine*/ true);
}
export function getExternalHelpersModuleName(node: SourceFile) {

17
src/compiler/transformers/es2015.ts
View File

@@ -787,9 +787,7 @@ namespace ts {
// To preserve the behavior of the old emitter, we explicitly indent
// the body of the function here if it was requested in an earlier
// transformation.
if (getEmitFlags(node) & EmitFlags.Indented) {
setEmitFlags(classFunction, EmitFlags.Indented);
}
setEmitFlags(classFunction, (getEmitFlags(node) & EmitFlags.Indented) | EmitFlags.ReuseTempVariableScope);
// "inner" and "outer" below are added purely to preserve source map locations from
// the old emitter
@@ -1327,7 +1325,8 @@ namespace ts {
EmitFlags.SingleLine | EmitFlags.NoTrailingSourceMap | EmitFlags.NoTokenSourceMaps
)
);
statement.startsOnNewLine = true;
startOnNewLine(statement);
setTextRange(statement, parameter);
setEmitFlags(statement, EmitFlags.NoTokenSourceMaps | EmitFlags.NoTrailingSourceMap | EmitFlags.CustomPrologue);
statements.push(statement);
@@ -1683,7 +1682,7 @@ namespace ts {
]
);
if (startsOnNewLine) {
call.startsOnNewLine = true;
startOnNewLine(call);
}
exitSubtree(ancestorFacts, HierarchyFacts.PropagateNewTargetMask, hierarchyFacts & HierarchyFacts.PropagateNewTargetMask ? HierarchyFacts.NewTarget : HierarchyFacts.None);
@@ -2602,7 +2601,7 @@ namespace ts {
);
if (node.multiLine) {
assignment.startsOnNewLine = true;
startOnNewLine(assignment);
}
expressions.push(assignment);
@@ -3083,7 +3082,7 @@ namespace ts {
);
setTextRange(expression, property);
if (startsOnNewLine) {
expression.startsOnNewLine = true;
startOnNewLine(expression);
}
return expression;
}
@@ -3105,7 +3104,7 @@ namespace ts {
);
setTextRange(expression, property);
if (startsOnNewLine) {
expression.startsOnNewLine = true;
startOnNewLine(expression);
}
return expression;
}
@@ -3128,7 +3127,7 @@ namespace ts {
);
setTextRange(expression, method);
if (startsOnNewLine) {
expression.startsOnNewLine = true;
startOnNewLine(expression);
}
exitSubtree(ancestorFacts, HierarchyFacts.PropagateNewTargetMask, hierarchyFacts & HierarchyFacts.PropagateNewTargetMask ? HierarchyFacts.NewTarget : HierarchyFacts.None);
return expression;

5
src/compiler/transformers/generators.ts
View File

@@ -1077,7 +1077,7 @@ namespace ts {
const visited = visitNode(expression, visitor, isExpression);
if (visited) {
if (multiLine) {
visited.startsOnNewLine = true;
startOnNewLine(visited);
}
expressions.push(visited);
}
@@ -2683,8 +2683,7 @@ namespace ts {
if (clauses) {
const labelExpression = createPropertyAccess(state, "label");
const switchStatement = createSwitch(labelExpression, createCaseBlock(clauses));
switchStatement.startsOnNewLine = true;
return [switchStatement];
return [startOnNewLine(switchStatement)];
}
if (statements) {

96
src/compiler/transformers/ts.ts
View File

@@ -86,6 +86,12 @@ namespace ts {
*/
let applicableSubstitutions: TypeScriptSubstitutionFlags;
/**
* Tracks what computed name expressions originating from elided names must be inlined
* at the next execution site, in document order
*/
let pendingExpressions: Expression[] | undefined;
return transformSourceFile;
/**
@@ -395,9 +401,11 @@ namespace ts {
case SyntaxKind.TypeAliasDeclaration:
// TypeScript type-only declarations are elided.
return undefined;
case SyntaxKind.PropertyDeclaration:
// TypeScript property declarations are elided.
// TypeScript property declarations are elided. However their names are still visited, and can potentially be retained if they could have sideeffects
return visitPropertyDeclaration(node as PropertyDeclaration);
case SyntaxKind.NamespaceExportDeclaration:
// TypeScript namespace export declarations are elided.
@@ -584,6 +592,9 @@ namespace ts {
* @param node The node to transform.
*/
function visitClassDeclaration(node: ClassDeclaration): VisitResult<Statement> {
const savedPendingExpressions = pendingExpressions;
pendingExpressions = undefined;
const staticProperties = getInitializedProperties(node, /*isStatic*/ true);
const facts = getClassFacts(node, staticProperties);
@@ -598,6 +609,12 @@ namespace ts {
let statements: Statement[] = [classStatement];
// Write any pending expressions from elided or moved computed property names
if (some(pendingExpressions)) {
statements.push(createStatement(inlineExpressions(pendingExpressions)));
}
pendingExpressions = savedPendingExpressions;
// Emit static property assignment. Because classDeclaration is lexically evaluated,
// it is safe to emit static property assignment after classDeclaration
// From ES6 specification:
@@ -856,6 +873,9 @@ namespace ts {
* @param node The node to transform.
*/
function visitClassExpression(node: ClassExpression): Expression {
const savedPendingExpressions = pendingExpressions;
pendingExpressions = undefined;
const staticProperties = getInitializedProperties(node, /*isStatic*/ true);
const heritageClauses = visitNodes(node.heritageClauses, visitor, isHeritageClause);
const members = transformClassMembers(node, some(heritageClauses, c => c.token === SyntaxKind.ExtendsKeyword));
@@ -871,7 +891,7 @@ namespace ts {
setOriginalNode(classExpression, node);
setTextRange(classExpression, node);
if (staticProperties.length > 0) {
if (some(staticProperties) || some(pendingExpressions)) {
const expressions: Expression[] = [];
const temp = createTempVariable(hoistVariableDeclaration);
if (resolver.getNodeCheckFlags(node) & NodeCheckFlags.ClassWithConstructorReference) {
@@ -884,11 +904,15 @@ namespace ts {
// the body of a class with static initializers.
setEmitFlags(classExpression, EmitFlags.Indented | getEmitFlags(classExpression));
expressions.push(startOnNewLine(createAssignment(temp, classExpression)));
// Add any pending expressions leftover from elided or relocated computed property names
addRange(expressions, map(pendingExpressions, startOnNewLine));
pendingExpressions = savedPendingExpressions;
addRange(expressions, generateInitializedPropertyExpressions(staticProperties, temp));
expressions.push(startOnNewLine(temp));
return inlineExpressions(expressions);
}
pendingExpressions = savedPendingExpressions;
return classExpression;
}
@@ -1202,7 +1226,7 @@ namespace ts {
const expressions: Expression[] = [];
for (const property of properties) {
const expression = transformInitializedProperty(property, receiver);
expression.startsOnNewLine = true;
startOnNewLine(expression);
setSourceMapRange(expression, moveRangePastModifiers(property));
setCommentRange(expression, property);
expressions.push(expression);
@@ -1218,7 +1242,10 @@ namespace ts {
* @param receiver The object receiving the property assignment.
*/
function transformInitializedProperty(property: PropertyDeclaration, receiver: LeftHandSideExpression) {
const propertyName = visitPropertyNameOfClassElement(property);
// We generate a name here in order to reuse the value cached by the relocated computed name expression (which uses the same generated name)
const propertyName = isComputedPropertyName(property.name) && !isSimpleInlineableExpression(property.name.expression)
? updateComputedPropertyName(property.name, getGeneratedNameForNode(property.name, !hasModifier(property, ModifierFlags.Static)))
: property.name;
const initializer = visitNode(property.initializer, visitor, isExpression);
const memberAccess = createMemberAccessForPropertyName(receiver, propertyName, /*location*/ propertyName);
@@ -2041,6 +2068,16 @@ namespace ts {
);
}
/**
* A simple inlinable expression is an expression which can be copied into multiple locations
* without risk of repeating any sideeffects and whose value could not possibly change between
* any such locations
*/
function isSimpleInlineableExpression(expression: Expression) {
return !isIdentifier(expression) && isSimpleCopiableExpression(expression) ||
isWellKnownSymbolSyntactically(expression);
}
/**
* Gets an expression that represents a property name. For a computed property, a
* name is generated for the node.
@@ -2050,7 +2087,7 @@ namespace ts {
function getExpressionForPropertyName(member: ClassElement | EnumMember, generateNameForComputedPropertyName: boolean): Expression {
const name = member.name;
if (isComputedPropertyName(name)) {
return generateNameForComputedPropertyName
return generateNameForComputedPropertyName && !isSimpleInlineableExpression((<ComputedPropertyName>name).expression)
? getGeneratedNameForNode(name)
: (<ComputedPropertyName>name).expression;
}
@@ -2062,6 +2099,26 @@ namespace ts {
}
}
/**
* If the name is a computed property, this function transforms it, then either returns an expression which caches the
* value of the result or the expression itself if the value is either unused or safe to inline into multiple locations
* @param shouldHoist Does the expression need to be reused? (ie, for an initializer or a decorator)
* @param omitSimple Should expressions with no observable side-effects be elided? (ie, the expression is not hoisted for a decorator or initializer and is a literal)
*/
function getPropertyNameExpressionIfNeeded(name: PropertyName, shouldHoist: boolean, omitSimple: boolean): Expression {
if (isComputedPropertyName(name)) {
const expression = visitNode(name.expression, visitor, isExpression);
const innerExpression = skipPartiallyEmittedExpressions(expression);
const inlinable = isSimpleInlineableExpression(innerExpression);
if (!inlinable && shouldHoist) {
const generatedName = getGeneratedNameForNode(name);
hoistVariableDeclaration(generatedName);
return createAssignment(generatedName, expression);
}
return (omitSimple && (inlinable || isIdentifier(innerExpression))) ? undefined : expression;
}
}
/**
* Visits the property name of a class element, for use when emitting property
* initializers. For a computed property on a node with decorators, a temporary
@@ -2071,15 +2128,14 @@ namespace ts {
*/
function visitPropertyNameOfClassElement(member: ClassElement): PropertyName {
const name = member.name;
if (isComputedPropertyName(name)) {
let expression = visitNode(name.expression, visitor, isExpression);
if (member.decorators) {
const generatedName = getGeneratedNameForNode(name);
hoistVariableDeclaration(generatedName);
expression = createAssignment(generatedName, expression);
let expr = getPropertyNameExpressionIfNeeded(name, some(member.decorators), /*omitSimple*/ false);
if (expr) { // expr only exists if `name` is a computed property name
// Inline any pending expressions from previous elided or relocated computed property name expressions in order to preserve execution order
if (some(pendingExpressions)) {
expr = inlineExpressions([...pendingExpressions, expr]);
pendingExpressions.length = 0;
}
return updateComputedPropertyName(name, expression);
return updateComputedPropertyName(name as ComputedPropertyName, expr);
}
else {
return name;
@@ -2136,6 +2192,14 @@ namespace ts {
return !nodeIsMissing(node.body);
}
function visitPropertyDeclaration(node: PropertyDeclaration): undefined {
const expr = getPropertyNameExpressionIfNeeded(node.name, some(node.decorators) || !!node.initializer, /*omitSimple*/ true);
if (expr && !isSimpleInlineableExpression(expr)) {
(pendingExpressions || (pendingExpressions = [])).push(expr);
}
return undefined;
}
function visitConstructor(node: ConstructorDeclaration) {
if (!shouldEmitFunctionLikeDeclaration(node)) {
return undefined;
@@ -2156,7 +2220,7 @@ namespace ts {
* This function will be called when one of the following conditions are met:
* - The node is an overload
* - The node is marked as abstract, public, private, protected, or readonly
* - The node has both a decorator and a computed property name
* - The node has a computed property name
*
* @param node The method node.
*/
@@ -2200,7 +2264,7 @@ namespace ts {
*
* This function will be called when one of the following conditions are met:
* - The node is marked as abstract, public, private, or protected
* - The node has both a decorator and a computed property name
* - The node has a computed property name
*
* @param node The get accessor node.
*/
@@ -2231,7 +2295,7 @@ namespace ts {
*
* This function will be called when one of the following conditions are met:
* - The node is marked as abstract, public, private, or protected
* - The node has both a decorator and a computed property name
* - The node has a computed property name
*
* @param node The set accessor node.
*/

3
src/compiler/types.ts
View File

@@ -520,7 +520,6 @@ namespace ts {
/* @internal */ id?: number; // Unique id (used to look up NodeLinks)
parent?: Node; // Parent node (initialized by binding)
/* @internal */ original?: Node; // The original node if this is an updated node.
/* @internal */ startsOnNewLine?: boolean; // Whether a synthesized node should start on a new line (used by transforms).
/* @internal */ symbol?: Symbol; // Symbol declared by node (initialized by binding)
/* @internal */ locals?: SymbolTable; // Locals associated with node (initialized by binding)
/* @internal */ nextContainer?: Node; // Next container in declaration order (initialized by binding)
@@ -630,6 +629,7 @@ namespace ts {
isInJSDocNamespace?: boolean; // if the node is a member in a JSDoc namespace
/*@internal*/ typeArguments?: NodeArray<TypeNode>; // Only defined on synthesized nodes. Though not syntactically valid, used in emitting diagnostics.
/*@internal*/ jsdocDotPos?: number; // Identifier occurs in JSDoc-style generic: Id.<T>
/*@internal*/ skipNameGenerationScope?: boolean; // Should skip a name generation scope when generating the name for this identifier
}
// Transient identifier node (marked by id === -1)
@@ -4330,6 +4330,7 @@ namespace ts {
constantValue?: string | number; // The constant value of an expression
externalHelpersModuleName?: Identifier; // The local name for an imported helpers module
helpers?: EmitHelper[]; // Emit helpers for the node
startsOnNewLine?: boolean; // If the node should begin on a new line
}
export const enum EmitFlags {