From 60d4d0ae543245e47e06a13952e8db2e6f9c7f74 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 20 Apr 2016 16:45:14 -0700 Subject: [PATCH 1/2] Fixes issues with emit for ShorthandPropertyAssignments and exported namespaces --- src/compiler/checker.ts | 44 +++-- src/compiler/printer.ts | 79 ++------ src/compiler/transformer.ts | 58 ++++-- src/compiler/transformers/destructuring.ts | 12 +- src/compiler/transformers/es6.ts | 36 +++- src/compiler/transformers/module/module.ts | 87 ++++++--- src/compiler/transformers/module/system.ts | 37 ++-- src/compiler/transformers/ts.ts | 178 +++++++++++------- src/compiler/types.ts | 14 +- src/compiler/utilities.ts | 4 + .../emptyAssignmentPatterns04_ES5.js | 4 +- .../reference/invalidInstantiatedModule.js | 4 +- 12 files changed, 344 insertions(+), 213 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index a18c3c45680..d0a95783de5 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -16578,10 +16578,10 @@ namespace ts { } // Gets the type of object literal or array literal of destructuring assignment. - // { a } from + // { a } from // for ( { a } of elems) { // } - // [ a ] from + // [ a ] from // [a] = [ some array ...] function getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(expr: Expression): Type { Debug.assert(expr.kind === SyntaxKind.ObjectLiteralExpression || expr.kind === SyntaxKind.ArrayLiteralExpression); @@ -16614,10 +16614,10 @@ namespace ts { } // Gets the property symbol corresponding to the property in destructuring assignment - // 'property1' from + // 'property1' from // for ( { property1: a } of elems) { // } - // 'property1' at location 'a' from: + // 'property1' at location 'a' from: // [a] = [ property1, property2 ] function getPropertySymbolOfDestructuringAssignment(location: Identifier) { // Get the type of the object or array literal and then look for property of given name in the type @@ -16721,19 +16721,28 @@ namespace ts { } } + function isNameOfModuleOrEnumDeclaration(node: Identifier) { + const parent = node.parent; + return isModuleOrEnumDeclaration(parent) && node === parent.name; + } + // When resolved as an expression identifier, if the given node references an exported entity, return the declaration // node of the exported entity's container. Otherwise, return undefined. function getReferencedExportContainer(node: Identifier, prefixLocals?: boolean): SourceFile | ModuleDeclaration | EnumDeclaration { node = getSourceTreeNodeOfType(node, isIdentifier); if (node) { - let symbol = getReferencedValueSymbol(node); + // When resolving the export container for the name of a module or enum + // declaration, we need to start resolution at the declaration's container. + // Otherwise, we could incorrectly resolve the export container as the + // declaration if it contains an exported member with the same name. + let symbol = getReferencedValueSymbol(node, /*startInDeclarationContainer*/ isNameOfModuleOrEnumDeclaration(node)); if (symbol) { if (symbol.flags & SymbolFlags.ExportValue) { // If we reference an exported entity within the same module declaration, then whether // we prefix depends on the kind of entity. SymbolFlags.ExportHasLocal encompasses all the // kinds that we do NOT prefix. const exportSymbol = getMergedSymbol(symbol.exportSymbol); - if (exportSymbol.flags & SymbolFlags.ExportHasLocal && !prefixLocals) { + if (!prefixLocals && exportSymbol.flags & SymbolFlags.ExportHasLocal) { return undefined; } symbol = exportSymbol; @@ -16744,7 +16753,7 @@ namespace ts { return parentSymbol.valueDeclaration; } for (let n = node.parent; n; n = n.parent) { - if ((n.kind === SyntaxKind.ModuleDeclaration || n.kind === SyntaxKind.EnumDeclaration) && getSymbolOfNode(n) === parentSymbol) { + if (isModuleOrEnumDeclaration(n) && getSymbolOfNode(n) === parentSymbol) { return n; } } @@ -17039,10 +17048,23 @@ namespace ts { return hasProperty(globals, name); } - function getReferencedValueSymbol(reference: Identifier): Symbol { - return getNodeLinks(reference).resolvedSymbol || - resolveName(reference, reference.text, SymbolFlags.Value | SymbolFlags.ExportValue | SymbolFlags.Alias, - /*nodeNotFoundMessage*/ undefined, /*nameArg*/ undefined); + function getReferencedValueSymbol(reference: Identifier, startInDeclarationContainer?: boolean): Symbol { + const resolvedSymbol = getNodeLinks(reference).resolvedSymbol; + if (resolvedSymbol) { + return resolvedSymbol; + } + + let location: Node = reference; + if (startInDeclarationContainer) { + // When resolving the name of a declaration as a value, we need to start resolution + // at a point outside of the declaration. + const parent = reference.parent; + if (isDeclaration(parent) && reference === parent.name) { + location = getDeclarationContainer(parent); + } + } + + return resolveName(location, reference.text, SymbolFlags.Value | SymbolFlags.ExportValue | SymbolFlags.Alias, /*nodeNotFoundMessage*/ undefined, /*nameArg*/ undefined); } function getReferencedValueDeclaration(reference: Identifier): Declaration { diff --git a/src/compiler/printer.ts b/src/compiler/printer.ts index 501b03425df..3beb71fc356 100644 --- a/src/compiler/printer.ts +++ b/src/compiler/printer.ts @@ -170,14 +170,11 @@ const _super = (function (geti, seti) { } = comments; let context: TransformationContext; - let startLexicalEnvironment: () => void; - let endLexicalEnvironment: () => Statement[]; let getNodeEmitFlags: (node: Node) => NodeEmitFlags; let setNodeEmitFlags: (node: Node, flags: NodeEmitFlags) => void; - let isExpressionSubstitutionEnabled: (node: Node) => boolean; + let isSubstitutionEnabled: (node: Node) => boolean; let isEmitNotificationEnabled: (node: Node) => boolean; - let expressionSubstitution: (node: Expression) => Expression; - let identifierSubstitution: (node: Identifier) => Identifier; + let onSubstituteNode: (node: Node, isExpression: boolean) => Node; let onEmitNode: (node: Node, emit: (node: Node) => void) => void; let nodeToGeneratedName: string[]; let generatedNameSet: Map; @@ -233,14 +230,11 @@ const _super = (function (geti, seti) { comments.reset(); writer.reset(); - startLexicalEnvironment = undefined; - endLexicalEnvironment = undefined; getNodeEmitFlags = undefined; setNodeEmitFlags = undefined; - isExpressionSubstitutionEnabled = undefined; + isSubstitutionEnabled = undefined; isEmitNotificationEnabled = undefined; - expressionSubstitution = undefined; - identifierSubstitution = undefined; + onSubstituteNode = undefined; onEmitNode = undefined; tempFlags = TempFlags.Auto; currentSourceFile = undefined; @@ -255,14 +249,11 @@ const _super = (function (geti, seti) { function initializePrinter(_context: TransformationContext) { context = _context; - startLexicalEnvironment = context.startLexicalEnvironment; - endLexicalEnvironment = context.endLexicalEnvironment; getNodeEmitFlags = context.getNodeEmitFlags; setNodeEmitFlags = context.setNodeEmitFlags; - isExpressionSubstitutionEnabled = context.isExpressionSubstitutionEnabled; + isSubstitutionEnabled = context.isSubstitutionEnabled; isEmitNotificationEnabled = context.isEmitNotificationEnabled; - expressionSubstitution = context.expressionSubstitution; - identifierSubstitution = context.identifierSubstitution; + onSubstituteNode = context.onSubstituteNode; onEmitNode = context.onEmitNode; return printSourceFile; } @@ -416,6 +407,10 @@ const _super = (function (geti, seti) { } function emitWorker(node: Node): void { + if (tryEmitSubstitute(node, emitWorker, /*isExpression*/ false)) { + return; + } + const kind = node.kind; switch (kind) { // Pseudo-literals @@ -426,10 +421,6 @@ const _super = (function (geti, seti) { // Identifiers case SyntaxKind.Identifier: - if (tryEmitSubstitute(node, identifierSubstitution)) { - return; - } - return emitIdentifier(node); // Reserved words @@ -675,11 +666,11 @@ const _super = (function (geti, seti) { } function emitExpressionWorker(node: Node) { - const kind = node.kind; - if (isExpressionSubstitutionEnabled(node) && tryEmitSubstitute(node, expressionSubstitution)) { + if (tryEmitSubstitute(node, emitExpressionWorker, /*isExpression*/ true)) { return; } + const kind = node.kind; switch (kind) { // Literals case SyntaxKind.NumericLiteral: @@ -1531,7 +1522,6 @@ const _super = (function (geti, seti) { const savedTempFlags = tempFlags; tempFlags = 0; - startLexicalEnvironment(); emitSignatureHead(node); emitBlockFunctionBodyAndEndLexicalEnvironment(node, body); if (indentedFlag) { @@ -1609,7 +1599,6 @@ const _super = (function (geti, seti) { write(" {"); } - const startingLine = writer.getLine(); increaseIndent(); emitLeadingDetachedComments(body.statements, body, shouldSkipLeadingCommentsForNode); @@ -1626,8 +1615,6 @@ const _super = (function (geti, seti) { emitList(body, body.statements, ListFormat.MultiLineFunctionBodyStatements, statementOffset); } - const endingLine = writer.getLine(); - emitLexicalEnvironment(endLexicalEnvironment(), /*newLine*/ startingLine !== endingLine); emitTrailingDetachedComments(body.statements, body, shouldSkipTrailingCommentsForNode); decreaseIndent(); writeToken(SyntaxKind.CloseBraceToken, body.statements.end); @@ -1725,15 +1712,9 @@ const _super = (function (geti, seti) { else { const savedTempFlags = tempFlags; tempFlags = 0; - startLexicalEnvironment(); write("{"); increaseIndent(); - - const startingLine = writer.getLine(); emitBlockStatements(node); - - const endingLine = writer.getLine(); - emitLexicalEnvironment(endLexicalEnvironment(), /*newLine*/ startingLine !== endingLine); write("}"); tempFlags = savedTempFlags; } @@ -2020,10 +2001,8 @@ const _super = (function (geti, seti) { else { const savedTempFlags = tempFlags; tempFlags = 0; - startLexicalEnvironment(); emitHelpers(node); emitList(node, statements, ListFormat.MultiLine, statementOffset); - emitLexicalEnvironment(endLexicalEnvironment(), /*newLine*/ true); tempFlags = savedTempFlags; } @@ -2036,28 +2015,6 @@ const _super = (function (geti, seti) { emitExpression(node.expression); } - function emitLexicalEnvironment(declarations: Statement[], newLine: boolean) { - if (declarations && declarations.length > 0) { - for (const node of declarations) { - if (newLine) { - writeLine(); - } - else { - write(" "); - } - - emit(node); - } - - if (newLine) { - writeLine(); - } - else { - write(" "); - } - } - } - /** * Emits any prologue directives at the start of a Statement list, returning the * number of prologue directives written to the output. @@ -2216,12 +2173,16 @@ const _super = (function (geti, seti) { } } - function tryEmitSubstitute(node: Node, substitution: (node: Node) => Node) { - if (substitution && (getNodeEmitFlags(node) & NodeEmitFlags.NoSubstitution) === 0) { - const substitute = substitution(node); + function tryEmitSubstitute(node: Node, emitNode: (node: Node) => void, isExpression: boolean) { + if ((node).text === "localizedDiagnosticMessages" && isExpression) { + debugger; + } + + if (isSubstitutionEnabled(node) && (getNodeEmitFlags(node) & NodeEmitFlags.NoSubstitution) === 0) { + const substitute = onSubstituteNode(node, isExpression); if (substitute !== node) { setNodeEmitFlags(substitute, NodeEmitFlags.NoSubstitution | getNodeEmitFlags(substitute)); - emitWorker(substitute); + emitNode(substitute); return true; } } diff --git a/src/compiler/transformer.ts b/src/compiler/transformer.ts index 1f4764da688..ab28e493c9d 100644 --- a/src/compiler/transformer.ts +++ b/src/compiler/transformer.ts @@ -19,7 +19,7 @@ namespace ts { }; const enum SyntaxKindFeatureFlags { - ExpressionSubstitution = 1 << 0, + Substitution = 1 << 0, EmitNotifications = 1 << 1, } @@ -62,6 +62,7 @@ namespace ts { let hoistedVariableDeclarations: VariableDeclaration[]; let hoistedFunctionDeclarations: FunctionDeclaration[]; let currentSourceFile: SourceFile; + let lexicalEnvironmentDisabled: boolean; // The transformation context is provided to each transformer as part of transformer // initialization. @@ -75,13 +76,12 @@ namespace ts { hoistFunctionDeclaration, startLexicalEnvironment, endLexicalEnvironment, - identifierSubstitution: node => node, - expressionSubstitution: node => node, - enableExpressionSubstitution, - isExpressionSubstitutionEnabled, - onEmitNode: (node, emit) => emit(node), + onSubstituteNode, + enableSubstitution, + isSubstitutionEnabled, + onEmitNode, enableEmitNotification, - isEmitNotificationEnabled, + isEmitNotificationEnabled }; // Chain together and initialize each transformer. @@ -104,12 +104,23 @@ namespace ts { return transformation(sourceFile); } - function enableExpressionSubstitution(kind: SyntaxKind) { - enabledSyntaxKindFeatures[kind] |= SyntaxKindFeatureFlags.ExpressionSubstitution; + function enableSubstitution(kind: SyntaxKind) { + enabledSyntaxKindFeatures[kind] |= SyntaxKindFeatureFlags.Substitution; } - function isExpressionSubstitutionEnabled(node: Node) { - return (enabledSyntaxKindFeatures[node.kind] & SyntaxKindFeatureFlags.ExpressionSubstitution) !== 0; + function isSubstitutionEnabled(node: Node) { + return (enabledSyntaxKindFeatures[node.kind] & SyntaxKindFeatureFlags.Substitution) !== 0; + } + + /** + * Default hook for node substitutions. + * + * @param node The node to substitute. + * @param isExpression A value indicating whether the node is to be used in an expression + * position. + */ + function onSubstituteNode(node: Node, isExpression: boolean) { + return node; } function enableEmitNotification(kind: SyntaxKind) { @@ -121,6 +132,25 @@ namespace ts { || (getNodeEmitFlags(node) & NodeEmitFlags.AdviseOnEmitNode) !== 0; } + /** + * Default hook for node emit. + * + * @param node The node to emit. + * @param emit A callback used to emit the node in the printer. + */ + function onEmitNode(node: Node, emit: (node: Node) => void) { + // Ensure that lexical environment modifications are disabled during the print phase. + if (!lexicalEnvironmentDisabled) { + const savedLexicalEnvironmentDisabled = lexicalEnvironmentDisabled; + lexicalEnvironmentDisabled = true; + emit(node); + lexicalEnvironmentDisabled = savedLexicalEnvironmentDisabled; + return; + } + + emit(node); + } + /** * Gets flags that control emit behavior of a node. */ @@ -149,6 +179,7 @@ namespace ts { * Records a hoisted variable declaration for the provided name within a lexical environment. */ function hoistVariableDeclaration(name: Identifier): void { + Debug.assert(!lexicalEnvironmentDisabled, "Cannot modify the lexical environment during the print phase."); const decl = createVariableDeclaration(name); if (!hoistedVariableDeclarations) { hoistedVariableDeclarations = [decl]; @@ -162,6 +193,7 @@ namespace ts { * Records a hoisted function declaration within a lexical environment. */ function hoistFunctionDeclaration(func: FunctionDeclaration): void { + Debug.assert(!lexicalEnvironmentDisabled, "Cannot modify the lexical environment during the print phase."); if (!hoistedFunctionDeclarations) { hoistedFunctionDeclarations = [func]; } @@ -175,6 +207,8 @@ namespace ts { * are pushed onto a stack, and the related storage variables are reset. */ function startLexicalEnvironment(): void { + Debug.assert(!lexicalEnvironmentDisabled, "Cannot start a lexical environment during the print phase."); + // Save the current lexical environment. Rather than resizing the array we adjust the // stack size variable. This allows us to reuse existing array slots we've // already allocated between transformations to avoid allocation and GC overhead during @@ -191,6 +225,8 @@ namespace ts { * any hoisted declarations added in this environment are returned. */ function endLexicalEnvironment(): Statement[] { + Debug.assert(!lexicalEnvironmentDisabled, "Cannot end a lexical environment during the print phase."); + let statements: Statement[]; if (hoistedVariableDeclarations || hoistedFunctionDeclarations) { if (hoistedFunctionDeclarations) { diff --git a/src/compiler/transformers/destructuring.ts b/src/compiler/transformers/destructuring.ts index e1374c13ad3..6c1b796a836 100644 --- a/src/compiler/transformers/destructuring.ts +++ b/src/compiler/transformers/destructuring.ts @@ -39,7 +39,7 @@ namespace ts { // // The source map location for the assignment should point to the entire binary // expression. - value = ensureIdentifier(node.right, /*reuseIdentifierExpressions*/ true, location, emitTempVariableAssignment); + value = ensureIdentifier(value, /*reuseIdentifierExpressions*/ true, location, emitTempVariableAssignment, visitor); } else if (nodeIsSynthesized(node)) { // Generally, the source map location for a destructuring assignment is the root @@ -48,7 +48,7 @@ namespace ts { // However, if the root expression is synthesized (as in the case // of the initializer when transforming a ForOfStatement), then the source map // location should point to the right-hand value of the expression. - location = node.right; + location = value; } flattenDestructuring(context, node, value, location, emitAssignment, emitTempVariableAssignment, visitor); @@ -397,16 +397,22 @@ namespace ts { * false if it is necessary to always emit an identifier. * @param location The location to use for source maps and comments. * @param emitTempVariableAssignment A callback used to emit a temporary variable. + * @param visitor An optional callback used to visit the value. */ function ensureIdentifier( value: Expression, reuseIdentifierExpressions: boolean, location: TextRange, - emitTempVariableAssignment: (value: Expression, location: TextRange) => Identifier) { + emitTempVariableAssignment: (value: Expression, location: TextRange) => Identifier, + visitor?: (node: Node) => VisitResult) { if (isIdentifier(value) && reuseIdentifierExpressions) { return value; } else { + if (visitor) { + value = visitNode(value, visitor, isExpression); + } + return emitTempVariableAssignment(value, location); } } diff --git a/src/compiler/transformers/es6.ts b/src/compiler/transformers/es6.ts index adf54d5ea08..0680001ef3b 100644 --- a/src/compiler/transformers/es6.ts +++ b/src/compiler/transformers/es6.ts @@ -148,12 +148,10 @@ namespace ts { } = context; const resolver = context.getEmitResolver(); - const previousIdentifierSubstitution = context.identifierSubstitution; - const previousExpressionSubstitution = context.expressionSubstitution; + const previousOnSubstituteNode = context.onSubstituteNode; const previousOnEmitNode = context.onEmitNode; context.onEmitNode = onEmitNode; - context.identifierSubstitution = substituteIdentifier; - context.expressionSubstitution = substituteExpression; + context.onSubstituteNode = onSubstituteNode; let currentSourceFile: SourceFile; let currentText: string; @@ -2667,7 +2665,7 @@ namespace ts { function enableSubstitutionsForBlockScopedBindings() { if ((enabledSubstitutions & ES6SubstitutionFlags.BlockScopedBindings) === 0) { enabledSubstitutions |= ES6SubstitutionFlags.BlockScopedBindings; - context.enableExpressionSubstitution(SyntaxKind.Identifier); + context.enableSubstitution(SyntaxKind.Identifier); } } @@ -2678,7 +2676,7 @@ namespace ts { function enableSubstitutionsForCapturedThis() { if ((enabledSubstitutions & ES6SubstitutionFlags.CapturedThis) === 0) { enabledSubstitutions |= ES6SubstitutionFlags.CapturedThis; - context.enableExpressionSubstitution(SyntaxKind.ThisKeyword); + context.enableSubstitution(SyntaxKind.ThisKeyword); context.enableEmitNotification(SyntaxKind.Constructor); context.enableEmitNotification(SyntaxKind.MethodDeclaration); context.enableEmitNotification(SyntaxKind.GetAccessor); @@ -2689,12 +2687,31 @@ namespace ts { } } + /** + * Hooks node substitutions. + * + * @param node The node to substitute. + * @param isExpression A value indicating whether the node is to be used in an expression + * position. + */ + function onSubstituteNode(node: Node, isExpression: boolean) { + node = previousOnSubstituteNode(node, isExpression); + + if (isExpression) { + return substituteExpression(node); + } + + if (isIdentifier(node)) { + return substituteIdentifier(node); + } + + return node; + } + /** * Hooks substitutions for non-expression identifiers. */ function substituteIdentifier(node: Identifier) { - node = previousIdentifierSubstitution(node); - // Only substitute the identifier if we have enabled substitutions for block-scoped // bindings. if (enabledSubstitutions & ES6SubstitutionFlags.BlockScopedBindings) { @@ -2732,8 +2749,7 @@ namespace ts { * * @param node An Expression node. */ - function substituteExpression(node: Expression): Expression { - node = previousExpressionSubstitution(node); + function substituteExpression(node: Node) { switch (node.kind) { case SyntaxKind.Identifier: return substituteExpressionIdentifier(node); diff --git a/src/compiler/transformers/module/module.ts b/src/compiler/transformers/module/module.ts index d8db4165faa..9b3385f3b6d 100644 --- a/src/compiler/transformers/module/module.ts +++ b/src/compiler/transformers/module/module.ts @@ -24,9 +24,10 @@ namespace ts { const host = context.getEmitHost(); const languageVersion = getEmitScriptTarget(compilerOptions); const moduleKind = getEmitModuleKind(compilerOptions); - const previousExpressionSubstitution = context.expressionSubstitution; - context.enableExpressionSubstitution(SyntaxKind.Identifier); - context.expressionSubstitution = substituteExpression; + const previousOnSubstituteNode = context.onSubstituteNode; + context.onSubstituteNode = onSubstituteNode; + context.enableSubstitution(SyntaxKind.Identifier); + context.enableSubstitution(SyntaxKind.ShorthandPropertyAssignment); let currentSourceFile: SourceFile; let externalImports: (ImportDeclaration | ImportEqualsDeclaration | ExportDeclaration)[]; @@ -775,8 +776,40 @@ namespace ts { return node.name ? getSynthesizedClone(node.name) : getGeneratedNameForNode(node); } + /** + * Hooks node substitutions. + * + * @param node The node to substitute. + * @param isExpression A value indicating whether the node is to be used in an expression + * position. + */ + function onSubstituteNode(node: Node, isExpression: boolean) { + node = previousOnSubstituteNode(node, isExpression); + if (isExpression) { + return substituteExpression(node); + } + else if (isShorthandPropertyAssignment(node)) { + return substituteShorthandPropertyAssignment(node); + } + return node; + } + + function substituteShorthandPropertyAssignment(node: ShorthandPropertyAssignment): ObjectLiteralElement { + const name = node.name; + const exportedOrImportedName = substituteExpressionIdentifier(name); + if (exportedOrImportedName !== name) { + // A shorthand property with an assignment initializer is probably part of a + // destructuring assignment + if (node.objectAssignmentInitializer) { + const initializer = createAssignment(exportedOrImportedName, node.objectAssignmentInitializer); + return createPropertyAssignment(name, initializer, /*location*/ node); + } + return createPropertyAssignment(name, exportedOrImportedName, /*location*/ node); + } + return node; + } + function substituteExpression(node: Expression) { - node = previousExpressionSubstitution(node); if (isIdentifier(node)) { return substituteExpressionIdentifier(node); } @@ -785,24 +818,34 @@ namespace ts { } function substituteExpressionIdentifier(node: Identifier): Expression { - if (getNodeEmitFlags(node) & NodeEmitFlags.LocalName) { - return node; - } + return trySubstituteExportedName(node) + || trySubstituteImportedName(node) + || node; + } - const container = resolver.getReferencedExportContainer(node, (getNodeEmitFlags(node) & NodeEmitFlags.ExportName) !== 0); - if (container) { - if (container.kind === SyntaxKind.SourceFile) { - return createPropertyAccess( - createIdentifier("exports"), - getSynthesizedClone(node), - /*location*/ node - ); + function trySubstituteExportedName(node: Identifier) { + const emitFlags = getNodeEmitFlags(node); + if ((emitFlags & NodeEmitFlags.LocalName) === 0) { + const container = resolver.getReferencedExportContainer(node, (emitFlags & NodeEmitFlags.ExportName) !== 0); + if (container) { + if (container.kind === SyntaxKind.SourceFile) { + return createPropertyAccess( + createIdentifier("exports"), + getSynthesizedClone(node), + /*location*/ node + ); + } } } - else { + + return undefined; + } + + function trySubstituteImportedName(node: Identifier): Expression { + if ((getNodeEmitFlags(node) & NodeEmitFlags.LocalName) === 0) { const declaration = resolver.getReferencedImportDeclaration(node); if (declaration) { - if (declaration.kind === SyntaxKind.ImportClause) { + if (isImportClause(declaration)) { if (languageVersion >= ScriptTarget.ES5) { return createPropertyAccess( getGeneratedNameForNode(declaration.parent), @@ -811,6 +854,7 @@ namespace ts { ); } else { + // TODO: ES3 transform to handle x.default -> x["default"] return createElementAccess( getGeneratedNameForNode(declaration.parent), createLiteral("default"), @@ -818,10 +862,10 @@ namespace ts { ); } } - else if (declaration.kind === SyntaxKind.ImportSpecifier) { - const name = (declaration).propertyName - || (declaration).name; + else if (isImportSpecifier(declaration)) { + const name = declaration.propertyName || declaration.name; if (name.originalKeywordKind === SyntaxKind.DefaultKeyword && languageVersion <= ScriptTarget.ES3) { + // TODO: ES3 transform to handle x.default -> x["default"] return createElementAccess( getGeneratedNameForNode(declaration.parent.parent.parent), createLiteral(name.text), @@ -838,8 +882,7 @@ namespace ts { } } } - - return node; + return undefined; } function getModuleMemberName(name: Identifier) { diff --git a/src/compiler/transformers/module/system.ts b/src/compiler/transformers/module/system.ts index 3077eec4661..1fc94b3061e 100644 --- a/src/compiler/transformers/module/system.ts +++ b/src/compiler/transformers/module/system.ts @@ -22,19 +22,17 @@ namespace ts { const resolver = context.getEmitResolver(); const host = context.getEmitHost(); const languageVersion = getEmitScriptTarget(compilerOptions); - const previousExpressionSubstitution = context.expressionSubstitution; - context.enableExpressionSubstitution(SyntaxKind.Identifier); - context.enableExpressionSubstitution(SyntaxKind.BinaryExpression); - context.enableExpressionSubstitution(SyntaxKind.PrefixUnaryExpression); - context.enableExpressionSubstitution(SyntaxKind.PostfixUnaryExpression); - context.expressionSubstitution = substituteExpression; - + const previousOnSubstituteNode = context.onSubstituteNode; + const previousOnEmitNode = context.onEmitNode; + context.onSubstituteNode = onSubstituteNode; + context.onEmitNode = onEmitNode; + context.enableSubstitution(SyntaxKind.Identifier); + context.enableSubstitution(SyntaxKind.BinaryExpression); + context.enableSubstitution(SyntaxKind.PrefixUnaryExpression); + context.enableSubstitution(SyntaxKind.PostfixUnaryExpression); context.enableEmitNotification(SyntaxKind.SourceFile); const exportFunctionForFileMap: Identifier[] = []; - const previousOnEmitNode = context.onEmitNode; - context.onEmitNode = onEmitNode; - let currentSourceFile: SourceFile; let externalImports: (ImportDeclaration | ImportEqualsDeclaration | ExportDeclaration)[]; let exportSpecifiers: Map; @@ -974,13 +972,28 @@ namespace ts { } } + /** + * Hooks node substitutions. + * + * @param node The node to substitute. + * @param isExpression A value indicating whether the node is to be used in an expression + * position. + */ + function onSubstituteNode(node: Node, isExpression: boolean) { + node = previousOnSubstituteNode(node, isExpression); + if (isExpression) { + return substituteExpression(node); + } + + return node; + } + /** * Substitute the expression, if necessary. * * @param node The node to substitute. */ - function substituteExpression(node: Expression): Expression { - node = previousExpressionSubstitution(node); + function substituteExpression(node: Expression) { switch (node.kind) { case SyntaxKind.Identifier: return substituteExpressionIdentifier(node); diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index 7eb10f7342e..1e1863158e4 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -38,11 +38,11 @@ namespace ts { // Save the previous transformation hooks. const previousOnEmitNode = context.onEmitNode; - const previousExpressionSubstitution = context.expressionSubstitution; + const previousOnSubstituteNode = context.onSubstituteNode; // Set new transformation hooks. context.onEmitNode = onEmitNode; - context.expressionSubstitution = substituteExpression; + context.onSubstituteNode = onSubstituteNode; // These variables contain state that changes as we descend into the tree. let currentSourceFile: SourceFile; @@ -612,7 +612,7 @@ namespace ts { // Record an alias to avoid class double-binding. let decoratedClassAlias: Identifier; if (resolver.getNodeCheckFlags(node) & NodeCheckFlags.DecoratedClassWithSelfReference) { - enableExpressionSubstitutionForDecoratedClasses(); + enableSubstitutionForDecoratedClasses(); decoratedClassAlias = createUniqueName(node.name && !isGeneratedIdentifier(node.name) ? node.name.text : "default"); decoratedClassAliases[getOriginalNodeId(node)] = decoratedClassAlias; @@ -2099,11 +2099,11 @@ namespace ts { // This step isn't needed if we eventually transform this to ES5. if (languageVersion >= ScriptTarget.ES6) { if (resolver.getNodeCheckFlags(node) & NodeCheckFlags.AsyncMethodWithSuperBinding) { - enableExpressionSubstitutionForAsyncMethodsWithSuper(); + enableSubstitutionForAsyncMethodsWithSuper(); setNodeEmitFlags(block, NodeEmitFlags.EmitAdvancedSuperHelper); } else if (resolver.getNodeCheckFlags(node) & NodeCheckFlags.AsyncMethodWithSuper) { - enableExpressionSubstitutionForAsyncMethodsWithSuper(); + enableSubstitutionForAsyncMethodsWithSuper(); setNodeEmitFlags(block, NodeEmitFlags.EmitSuperHelper); } } @@ -2480,7 +2480,7 @@ namespace ts { } Debug.assert(isIdentifier(node.name), "TypeScript module should have an Identifier name."); - enableExpressionSubstitutionForNamespaceExports(); + enableSubstitutionForNamespaceExports(); const statements: Statement[] = []; @@ -2851,6 +2851,61 @@ namespace ts { : getClassPrototype(node); } + function enableSubstitutionForNonQualifiedEnumMembers() { + if ((enabledSubstitutions & TypeScriptSubstitutionFlags.NonQualifiedEnumMembers) === 0) { + enabledSubstitutions |= TypeScriptSubstitutionFlags.NonQualifiedEnumMembers; + context.enableSubstitution(SyntaxKind.Identifier); + } + } + + function enableSubstitutionForAsyncMethodsWithSuper() { + if ((enabledSubstitutions & TypeScriptSubstitutionFlags.AsyncMethodsWithSuper) === 0) { + enabledSubstitutions |= TypeScriptSubstitutionFlags.AsyncMethodsWithSuper; + + // We need to enable substitutions for call, property access, and element access + // if we need to rewrite super calls. + context.enableSubstitution(SyntaxKind.CallExpression); + context.enableSubstitution(SyntaxKind.PropertyAccessExpression); + context.enableSubstitution(SyntaxKind.ElementAccessExpression); + + // We need to be notified when entering and exiting declarations that bind super. + context.enableEmitNotification(SyntaxKind.ClassDeclaration); + context.enableEmitNotification(SyntaxKind.MethodDeclaration); + context.enableEmitNotification(SyntaxKind.GetAccessor); + context.enableEmitNotification(SyntaxKind.SetAccessor); + context.enableEmitNotification(SyntaxKind.Constructor); + } + } + + function enableSubstitutionForDecoratedClasses() { + if ((enabledSubstitutions & TypeScriptSubstitutionFlags.DecoratedClasses) === 0) { + enabledSubstitutions |= TypeScriptSubstitutionFlags.DecoratedClasses; + + // We need to enable substitutions for identifiers. This allows us to + // substitute class names inside of a class declaration. + context.enableSubstitution(SyntaxKind.Identifier); + context.enableEmitNotification(SyntaxKind.Identifier); + + // Keep track of class aliases. + decoratedClassAliases = {}; + currentDecoratedClassAliases = {}; + } + } + + function enableSubstitutionForNamespaceExports() { + if ((enabledSubstitutions & TypeScriptSubstitutionFlags.NamespaceExports) === 0) { + enabledSubstitutions |= TypeScriptSubstitutionFlags.NamespaceExports; + + // We need to enable substitutions for identifiers and shorthand property assignments. This allows us to + // substitute the names of exported members of a namespace. + context.enableSubstitution(SyntaxKind.Identifier); + context.enableSubstitution(SyntaxKind.ShorthandPropertyAssignment); + + // We need to be notified when entering and exiting namespaces. + context.enableEmitNotification(SyntaxKind.ModuleDeclaration); + } + } + function isClassWithDecorators(node: Node): node is ClassDeclaration { return node.kind === SyntaxKind.ClassDeclaration && node.decorators !== undefined; } @@ -2872,6 +2927,12 @@ namespace ts { return getOriginalNode(node).kind === SyntaxKind.EnumDeclaration; } + /** + * Hook for node emit. + * + * @param node The node to emit. + * @param emit A callback used to emit the node in the printer. + */ function onEmitNode(node: Node, emit: (node: Node) => void): void { const savedApplicableSubstitutions = applicableSubstitutions; const savedCurrentSuperContainer = currentSuperContainer; @@ -2913,9 +2974,43 @@ namespace ts { currentSuperContainer = savedCurrentSuperContainer; } - function substituteExpression(node: Expression): Expression { - node = previousExpressionSubstitution(node); + /** + * Hooks node substitutions. + * + * @param node The node to substitute. + * @param isExpression A value indicating whether the node is to be used in an expression + * position. + */ + function onSubstituteNode(node: Node, isExpression: boolean) { + node = previousOnSubstituteNode(node, isExpression); + if (isExpression) { + return substituteExpression(node); + } + else if (isShorthandPropertyAssignment(node)) { + return substituteShorthandPropertyAssignment(node); + } + return node; + } + + function substituteShorthandPropertyAssignment(node: ShorthandPropertyAssignment): ObjectLiteralElement { + if (enabledSubstitutions & TypeScriptSubstitutionFlags.NamespaceExports) { + const name = node.name; + const exportedName = trySubstituteNamespaceExportedName(name); + if (exportedName) { + // A shorthand property with an assignment initializer is probably part of a + // destructuring assignment + if (node.objectAssignmentInitializer) { + const initializer = createAssignment(exportedName, node.objectAssignmentInitializer); + return createPropertyAssignment(name, initializer, /*location*/ node); + } + return createPropertyAssignment(name, exportedName, /*location*/ node); + } + } + return node; + } + + function substituteExpression(node: Expression) { switch (node.kind) { case SyntaxKind.Identifier: return substituteExpressionIdentifier(node); @@ -2961,17 +3056,12 @@ namespace ts { } function trySubstituteNamespaceExportedName(node: Identifier): Expression { - if (enabledSubstitutions & applicableSubstitutions) { - // If this is explicitly a local name, do not substitute. - if (getNodeEmitFlags(node) & NodeEmitFlags.LocalName) { - return node; - } - + // If this is explicitly a local name, do not substitute. + if (enabledSubstitutions & applicableSubstitutions && (getNodeEmitFlags(node) & NodeEmitFlags.LocalName) === 0) { // If we are nested within a namespace declaration, we may need to qualifiy // an identifier that is exported from a merged namespace. - const original = getSourceTreeNodeOfType(node, isIdentifier); - const container = resolver.getReferencedExportContainer(original, /*prefixLocals*/ false); - if (container && original !== container.name) { + const container = resolver.getReferencedExportContainer(node, /*prefixLocals*/ false); + if (container) { const substitute = (applicableSubstitutions & TypeScriptSubstitutionFlags.NamespaceExports && container.kind === SyntaxKind.ModuleDeclaration) || (applicableSubstitutions & TypeScriptSubstitutionFlags.NonQualifiedEnumMembers && container.kind === SyntaxKind.EnumDeclaration); @@ -3034,60 +3124,6 @@ namespace ts { return node; } - function enableSubstitutionForNonQualifiedEnumMembers() { - if ((enabledSubstitutions & TypeScriptSubstitutionFlags.NonQualifiedEnumMembers) === 0) { - enabledSubstitutions |= TypeScriptSubstitutionFlags.NonQualifiedEnumMembers; - context.enableExpressionSubstitution(SyntaxKind.Identifier); - } - } - - function enableExpressionSubstitutionForAsyncMethodsWithSuper() { - if ((enabledSubstitutions & TypeScriptSubstitutionFlags.AsyncMethodsWithSuper) === 0) { - enabledSubstitutions |= TypeScriptSubstitutionFlags.AsyncMethodsWithSuper; - - // We need to enable substitutions for call, property access, and element access - // if we need to rewrite super calls. - context.enableExpressionSubstitution(SyntaxKind.CallExpression); - context.enableExpressionSubstitution(SyntaxKind.PropertyAccessExpression); - context.enableExpressionSubstitution(SyntaxKind.ElementAccessExpression); - - // We need to be notified when entering and exiting declarations that bind super. - context.enableEmitNotification(SyntaxKind.ClassDeclaration); - context.enableEmitNotification(SyntaxKind.MethodDeclaration); - context.enableEmitNotification(SyntaxKind.GetAccessor); - context.enableEmitNotification(SyntaxKind.SetAccessor); - context.enableEmitNotification(SyntaxKind.Constructor); - } - } - - function enableExpressionSubstitutionForDecoratedClasses() { - if ((enabledSubstitutions & TypeScriptSubstitutionFlags.DecoratedClasses) === 0) { - enabledSubstitutions |= TypeScriptSubstitutionFlags.DecoratedClasses; - - // We need to enable substitutions for identifiers. This allows us to - // substitute class names inside of a class declaration. - context.enableExpressionSubstitution(SyntaxKind.Identifier); - context.enableEmitNotification(SyntaxKind.Identifier); - - // Keep track of class aliases. - decoratedClassAliases = {}; - currentDecoratedClassAliases = {}; - } - } - - function enableExpressionSubstitutionForNamespaceExports() { - if ((enabledSubstitutions & TypeScriptSubstitutionFlags.NamespaceExports) === 0) { - enabledSubstitutions |= TypeScriptSubstitutionFlags.NamespaceExports; - - // We need to enable substitutions for identifiers. This allows us to - // substitute the names of exported members of a namespace. - context.enableExpressionSubstitution(SyntaxKind.Identifier); - - // We need to be notified when entering and exiting namespaces. - context.enableEmitNotification(SyntaxKind.ModuleDeclaration); - } - } - function createSuperAccessInAsyncMethod(argumentExpression: Expression, flags: NodeCheckFlags, location: TextRange): LeftHandSideExpression { if (flags & NodeCheckFlags.AsyncMethodWithSuperBinding) { return createPropertyAccess( diff --git a/src/compiler/types.ts b/src/compiler/types.ts index fe437f7205f..cbf3a0b89fc 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2541,7 +2541,7 @@ namespace ts { // When options come from a config file, its path is recorded here configFilePath?: string; /* @internal */ - // Path used to used to compute primary search locations + // Path used to used to compute primary search locations typesRoot?: string; types?: string[]; @@ -2968,29 +2968,23 @@ namespace ts { hoistFunctionDeclaration(node: FunctionDeclaration): void; hoistVariableDeclaration(node: Identifier): void; - /** - * Hook used by transformers to substitute non-expression identifiers - * just before theyare emitted by the pretty printer. - */ - identifierSubstitution?: (node: Identifier) => Identifier; - /** * Enables expression substitutions in the pretty printer for * the provided SyntaxKind. */ - enableExpressionSubstitution(kind: SyntaxKind): void; + enableSubstitution(kind: SyntaxKind): void; /** * Determines whether expression substitutions are enabled for the * provided node. */ - isExpressionSubstitutionEnabled(node: Node): boolean; + isSubstitutionEnabled(node: Node): boolean; /** * Hook used by transformers to substitute expressions just before they * are emitted by the pretty printer. */ - expressionSubstitution?: (node: Expression) => Expression; + onSubstituteNode?: (node: Node, isExpression: boolean) => Node; /** * Enables before/after emit notifications in the pretty printer for diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 31e4277f75f..2c62e0ffa5d 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3589,6 +3589,10 @@ namespace ts { return node.kind === SyntaxKind.ExportSpecifier; } + export function isModuleOrEnumDeclaration(node: Node): node is ModuleDeclaration | EnumDeclaration { + return node.kind === SyntaxKind.ModuleDeclaration || node.kind === SyntaxKind.EnumDeclaration; + } + function isDeclarationKind(kind: SyntaxKind) { return kind === SyntaxKind.ArrowFunction || kind === SyntaxKind.BindingElement diff --git a/tests/baselines/reference/emptyAssignmentPatterns04_ES5.js b/tests/baselines/reference/emptyAssignmentPatterns04_ES5.js index f76420844a3..e6b3cc7e3f2 100644 --- a/tests/baselines/reference/emptyAssignmentPatterns04_ES5.js +++ b/tests/baselines/reference/emptyAssignmentPatterns04_ES5.js @@ -9,8 +9,8 @@ let x, y, z, a1, a2, a3; //// [emptyAssignmentPatterns04_ES5.js] var a; var x, y, z, a1, a2, a3; -(_a = {} = a, x = _a.x, y = _a.y, z = _a.z, _a); -(_b = [] = a, a1 = _b[0], a2 = _b[1], a3 = _b[2], _b); +(_a = a, x = _a.x, y = _a.y, z = _a.z, _a); +(_b = a, a1 = _b[0], a2 = _b[1], a3 = _b[2], _b); var _a, _b; diff --git a/tests/baselines/reference/invalidInstantiatedModule.js b/tests/baselines/reference/invalidInstantiatedModule.js index 708e2c3b807..65328dc15b5 100644 --- a/tests/baselines/reference/invalidInstantiatedModule.js +++ b/tests/baselines/reference/invalidInstantiatedModule.js @@ -21,9 +21,9 @@ var M; var Point = (function () { function Point() { } - return M.Point; + return Point; }()); - M.Point = M.Point; + M.Point = Point; M.Point = 1; // Error })(M || (M = {})); var M2; From 0da185da194c65464bb6b055f57492f9faa5536c Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Fri, 22 Apr 2016 10:56:48 -0700 Subject: [PATCH 2/2] PR Feedback --- src/compiler/printer.ts | 4 ---- src/compiler/transformers/destructuring.ts | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/compiler/printer.ts b/src/compiler/printer.ts index 3beb71fc356..137fce46345 100644 --- a/src/compiler/printer.ts +++ b/src/compiler/printer.ts @@ -2174,10 +2174,6 @@ const _super = (function (geti, seti) { } function tryEmitSubstitute(node: Node, emitNode: (node: Node) => void, isExpression: boolean) { - if ((node).text === "localizedDiagnosticMessages" && isExpression) { - debugger; - } - if (isSubstitutionEnabled(node) && (getNodeEmitFlags(node) & NodeEmitFlags.NoSubstitution) === 0) { const substitute = onSubstituteNode(node, isExpression); if (substitute !== node) { diff --git a/src/compiler/transformers/destructuring.ts b/src/compiler/transformers/destructuring.ts index 6c1b796a836..c8ee58d8afc 100644 --- a/src/compiler/transformers/destructuring.ts +++ b/src/compiler/transformers/destructuring.ts @@ -405,6 +405,7 @@ namespace ts { location: TextRange, emitTempVariableAssignment: (value: Expression, location: TextRange) => Identifier, visitor?: (node: Node) => VisitResult) { + if (isIdentifier(value) && reuseIdentifierExpressions) { return value; }