From 239f214b1ca34138470a90100a4716e6135a692b Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 7 Mar 2018 16:26:29 -0800 Subject: [PATCH] Address PR comments 1. Add documentation 2. Better organisation of concerns in utility functions 3. Better handling of module.exports and exports in the binder's new code. --- src/compiler/binder.ts | 30 +++++++++++++--------- src/compiler/utilities.ts | 54 +++++++++++++++++++++++++++++---------- 2 files changed, 59 insertions(+), 25 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 803595f38f1..d03b055c9e4 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -2295,10 +2295,7 @@ namespace ts { // expression is the declaration setCommonJsModuleIndicator(node); const lhs = node.left as PropertyAccessEntityNameExpression; - const symbol = forEachIdentifierInEntityName(lhs.expression, (id, original, e) => { - if (isExportsOrModuleExportsOrAlias(file, e) || (isIdentifier(e) && e.escapedText === "module" && original === undefined)) { - return file.symbol; - } + const symbol = forEachIdentifierInEntityName(lhs.expression, (id, original) => { if (!original) { return undefined; } @@ -2422,9 +2419,15 @@ namespace ts { function bindPropertyAssignment(name: EntityNameExpression, propertyAccess: PropertyAccessEntityNameExpression, isPrototypeProperty: boolean) { let symbol = getJSInitializerSymbol(lookupSymbolForPropertyAccess(name)); - const isToplevelNamespaceableInitializer = isBinaryExpression(propertyAccess.parent) ? - propertyAccess.parent.parent.parent.kind === SyntaxKind.SourceFile && getJavascriptInitializer(propertyAccess.parent.right) : - propertyAccess.parent.parent.kind === SyntaxKind.SourceFile; + let isToplevelNamespaceableInitializer: boolean; + if (isBinaryExpression(propertyAccess.parent)) { + const isPrototypeAssignment = isPropertyAccessExpression(propertyAccess.parent.left) && propertyAccess.parent.left.name.escapedText === "prototype"; + isToplevelNamespaceableInitializer = propertyAccess.parent.parent.parent.kind === SyntaxKind.SourceFile && + !!getJavascriptInitializer(propertyAccess.parent.right, isPrototypeAssignment); + } + else { + isToplevelNamespaceableInitializer = propertyAccess.parent.parent.kind === SyntaxKind.SourceFile; + } if (!isPrototypeProperty && (!symbol || !(symbol.flags & SymbolFlags.Namespace)) && isToplevelNamespaceableInitializer) { // make symbols or add declarations for intermediate containers const flags = SymbolFlags.Module | SymbolFlags.JSContainer; @@ -2465,14 +2468,17 @@ namespace ts { } } - function forEachIdentifierInEntityName(e: EntityNameExpression, action: (e: Identifier, symbol: Symbol, k: EntityNameExpression) => Symbol): Symbol { - if (isIdentifier(e)) { - return action(e, lookupSymbolForPropertyAccess(e), e); + function forEachIdentifierInEntityName(e: EntityNameExpression, action: (e: Identifier, symbol: Symbol) => Symbol): Symbol { + if (isExportsOrModuleExportsOrAlias(file, e)) { + return file.symbol; + } + else if (isIdentifier(e)) { + return action(e, lookupSymbolForPropertyAccess(e)); } else { const s = getJSInitializerSymbol(forEachIdentifierInEntityName(e.expression, action)); Debug.assert(!!s && !!s.exports); - return action(e.name, s.exports.get(e.name.escapedText), e); + return action(e.name, s.exports.get(e.name.escapedText)); } } @@ -2704,7 +2710,7 @@ namespace ts { function isExportsOrModuleExportsOrAliasOrAssignment(sourceFile: SourceFile, node: Expression): boolean { return isExportsOrModuleExportsOrAlias(sourceFile, node) || - (isAssignmentExpression(node, /*excludeCompoundAssignements*/ true) && ( + (isAssignmentExpression(node, /*excludeCompoundAssignment*/ true) && ( isExportsOrModuleExportsOrAliasOrAssignment(sourceFile, node.left) || isExportsOrModuleExportsOrAliasOrAssignment(sourceFile, node.right))); } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 354848eb693..da4904ff996 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -1476,6 +1476,13 @@ namespace ts { return getSourceTextOfNodeFromSourceFile(sourceFile, str).charCodeAt(0) === CharacterCodes.doubleQuote; } + /** + * Given the symbol of a declaration, find the symbol of its Javascript container-like initializer, + * if it has one. Otherwise just return the original symbol. + * + * Container-like initializer behave like namespaces, so the binder needs to add contained symbols + * to their exports. An example is a function with assignments to `this` inside. + */ export function getJSInitializerSymbol(symbol: Symbol) { if (!symbol || !symbol.valueDeclaration) { return symbol; @@ -1485,22 +1492,37 @@ namespace ts { return e ? e.symbol : symbol; } + /** Get the declaration initializer, when the initializer is container-like (See getJavascriptInitializer) */ export function getDeclaredJavascriptInitializer(node: Node) { if (node && isVariableDeclaration(node) && node.initializer) { - return getJavascriptInitializer(node.initializer) || - isIdentifier(node.name) && getDefaultedJavascriptInitializer(node.name, node.initializer); + return getJavascriptInitializer(node.initializer, /*isPrototypeAssignment*/ false) || + isIdentifier(node.name) && getDefaultedJavascriptInitializer(node.name, node.initializer, /*isPrototypeAssignment*/ false); } } + /** + * Get the assignment 'initializer' -- the righthand side-- when the initializer is container-like (See getJavascriptInitializer). + * We treat the right hand side of assignments with container-like initalizers as declarations. + */ export function getAssignedJavascriptInitializer(node: Node) { - return node && - (isBinaryExpression(node) && node.operatorToken.kind === SyntaxKind.BarBarToken || isPropertyAccessExpression(node)) && - node.parent && isBinaryExpression(node.parent) && - node.parent.operatorToken.kind === SyntaxKind.EqualsToken && - (getJavascriptInitializer(node.parent.right) || getDefaultedJavascriptInitializer(node.parent.left as EntityNameExpression, node.parent.right)); + if (node && node.parent && isBinaryExpression(node.parent) && node.parent.operatorToken.kind === SyntaxKind.EqualsToken) { + const isPrototypeAssignment = isPropertyAccessExpression(node.parent.left) && node.parent.left.name.escapedText === "prototype"; + return getJavascriptInitializer(node.parent.right, isPrototypeAssignment) || + getDefaultedJavascriptInitializer(node.parent.left as EntityNameExpression, node.parent.right, isPrototypeAssignment); + } } - export function getJavascriptInitializer(initializer: Expression) { + /** + * Recognized Javascript container-like initializers are: + * 1. (function() {})() -- IIFEs + * 2. function() { } -- Function expressions + * 3. class { } -- Class expressions + * 4. {} -- Empty object literals + * 5. { ... } -- Non-empty object literals, when used to initialize a prototype, like `C.prototype = { m() { } }` + * + * This function returns the provided initializer, or undefined if it is not valid. + */ + export function getJavascriptInitializer(initializer: Expression, isPrototypeAssignment: boolean) { if (isCallExpression(initializer)) { const e = skipParentheses(initializer.expression); return e.kind === SyntaxKind.FunctionExpression || e.kind === SyntaxKind.ArrowFunction ? initializer : undefined; @@ -1508,15 +1530,21 @@ namespace ts { if (initializer.kind === SyntaxKind.FunctionExpression || initializer.kind === SyntaxKind.ClassExpression) { return initializer; } - if (isObjectLiteralExpression(initializer) && - (initializer.properties.length === 0 || - isBinaryExpression(initializer.parent) && isPropertyAccessExpression(initializer.parent.left) && initializer.parent.left.name.escapedText === "prototype")) { + if (isObjectLiteralExpression(initializer) && (initializer.properties.length === 0 || isPrototypeAssignment)) { return initializer; } } - function getDefaultedJavascriptInitializer(name: EntityNameExpression, initializer: Expression) { - const e = isBinaryExpression(initializer) && getJavascriptInitializer(initializer.right); + /** + * A defaulted Javascript initializer matches the pattern + * `Lhs = Lhs || JavascriptInitializer` + * or `var Lhs = Lhs || JavascriptInitializer` + * + * The second Lhs is required to be the same as the first except that it may be prefixed with + * 'window.', 'global.' or 'self.' The second Lhs is otherwise ignored by the binder and checker. + */ + function getDefaultedJavascriptInitializer(name: EntityNameExpression, initializer: Expression, isPrototypeAssignment: boolean) { + const e = isBinaryExpression(initializer) && initializer.operatorToken.kind === SyntaxKind.BarBarToken && getJavascriptInitializer(initializer.right, isPrototypeAssignment); if (e && isSameEntityName(name, (initializer as BinaryExpression).left as EntityNameExpression)) { return e; }