From bad155f072b29fb1f40c5aef5b0fb38409cb59f8 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 13 Feb 2018 16:50:53 -0800 Subject: [PATCH] Clean up bindPropertyAssignment The add-intermediate-container-symbols loop is still quite confusing, but it's not as bad as before. --- src/compiler/binder.ts | 53 ++++++++----------- src/compiler/checker.ts | 2 +- .../typeFromPropertyAssignment10.symbols | 28 +++++----- 3 files changed, 37 insertions(+), 46 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 72f4ad03260..b40833db7b3 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -2408,9 +2408,8 @@ namespace ts { } } - function isValidInitializer(initializer: Node) { - return initializer === undefined || - initializer.kind === SyntaxKind.ClassExpression || + function isNamespaceableInitializer(initializer: Node) { + return initializer.kind === SyntaxKind.ClassExpression || initializer.kind === SyntaxKind.FunctionExpression || initializer.kind === SyntaxKind.ObjectLiteralExpression && (initializer as ObjectLiteralExpression).properties.length === 0 || initializer.kind === SyntaxKind.CallExpression && (skipParentheses((initializer as CallExpression).expression).kind === SyntaxKind.FunctionExpression || @@ -2424,34 +2423,28 @@ namespace ts { Debug.assert(propertyAccess.parent.kind === SyntaxKind.BinaryExpression || propertyAccess.parent.kind === SyntaxKind.ExpressionStatement || propertyAccess.parent.kind === SyntaxKind.PropertyAccessExpression); - const isLegalPosition = propertyAccess.parent.kind === SyntaxKind.BinaryExpression ? - propertyAccess.parent.parent.parent.kind === SyntaxKind.SourceFile && isValidInitializer((propertyAccess.parent as BinaryExpression).right) : - propertyAccess.parent.parent.kind === SyntaxKind.SourceFile && isValidInitializer((propertyAccess.parent as VariableDeclaration).initializer); - if (!isPrototypeProperty && (!symbol || !(symbol.flags & SymbolFlags.Namespace)) && isLegalPosition) { + const isToplevelNamespaceableInitializer = propertyAccess.parent.kind === SyntaxKind.BinaryExpression ? + propertyAccess.parent.parent.parent.kind === SyntaxKind.SourceFile && isNamespaceableInitializer((propertyAccess.parent as BinaryExpression).right) : + propertyAccess.parent.parent.kind === SyntaxKind.SourceFile; + if (!isPrototypeProperty && (!symbol || !(symbol.flags & SymbolFlags.Namespace)) && isToplevelNamespaceableInitializer) { const flags = SymbolFlags.Module | SymbolFlags.JSContainer; const excludeFlags = SymbolFlags.ValueModuleExcludes & ~SymbolFlags.JSContainer; - // hm. This is only needed to make namespaced access of types workable. Namespaced access of *values* doesn't work now either, so something is wrong. - // (Note: for the non-nested case, at least, addDeclarationToSymbol is only needed for things that could be further namespaces, because it - // makes the intermediate namespace. However, I think something like it is needed for *all* nested assignments, in case their intermediate namespaces don't exist) - iterateEntityNameExpression(propertyAccess.expression, (id, originalSymbol, available) => { - if (symbol) { - if (available) { - // Note: add declaration to original symbol, not the special-syntax's symbol, so that namespaces work for type lookup - addDeclarationToSymbol(originalSymbol, id, flags); - // TODO: Why can't I overwrite symbol here? I'm having trouble tracking symbol's state. - return originalSymbol; - } - else { - originalSymbol.exports = originalSymbol.exports || createSymbolTable(); - symbol = declareSymbol(originalSymbol.exports, originalSymbol, id, flags, excludeFlags); - return symbol; - } + // addDeclarationToSymbol is only needed for things that could be further containers, because it makes the intermediate namespace symbol. + iterateEntityNameExpression(propertyAccess.expression, (id, original) => { + if (original) { + // Note: add declaration to original symbol, not the special-syntax's symbol, so that namespaces work for type lookup + addDeclarationToSymbol(original, id, flags); + return original; + } + else if (symbol) { + symbol.exports = symbol.exports || createSymbolTable(); + symbol = declareSymbol(symbol.exports, symbol, id, flags, excludeFlags); } else { - Debug.assert(!available); + Debug.assert(!original); symbol = declareSymbol(container.locals, /*parent*/ undefined, id, flags, excludeFlags); - return symbol; } + return symbol; }); } if (!symbol || !(symbol.flags & (SymbolFlags.Function | SymbolFlags.Class | SymbolFlags.NamespaceModule | SymbolFlags.ObjectLiteral))) { @@ -2467,17 +2460,15 @@ namespace ts { declareSymbol(symbolTable, symbol, propertyAccess, SymbolFlags.Property, SymbolFlags.PropertyExcludes); } - function iterateEntityNameExpression(e: EntityNameExpression, action: (e: Identifier, originalSymbol: Symbol, available: boolean) => Symbol): Symbol { + function iterateEntityNameExpression(e: EntityNameExpression, action: (e: Identifier, symbol: Symbol) => Symbol): Symbol { if (isIdentifier(e)) { - const s = lookupSymbolForPropertyAccess(e); - return action(e, s, !!s); + return action(e, lookupSymbolForPropertyAccess(e)); } else { const s = follow(iterateEntityNameExpression(e.expression, action)); Debug.assert(!!s, "lost the chant"); - Debug.assert(!!s.exports, `${s.escapedName} has no exports???`); - const t = s.exports.get(e.name.escapedText); - return action(e.name, t || s, s.exports.has(e.name.escapedText)); + Debug.assert(!!s.exports, "has no exports"); + return action(e.name, s.exports.get(e.name.escapedText)); } } diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 737daa52e24..be751758126 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -14777,7 +14777,7 @@ namespace ts { // TODO: This seems like it might be wrong, or at least should come earlier // (maybe check SymbolFlags.JSContainer? This currently misses normal declarations like `var my = {}`, but shouldn't) if (isInJSFile && node.symbol && node.symbol.exports && node.properties.length === 0) { - let symbol = node.symbol; + const symbol = getMergedSymbol(node.symbol); propertiesTable = symbol.exports; symbol.exports.forEach(symbol => propertiesArray.push(getMergedSymbol(symbol))); return createObjectLiteralType(); diff --git a/tests/baselines/reference/typeFromPropertyAssignment10.symbols b/tests/baselines/reference/typeFromPropertyAssignment10.symbols index 55c3f1dfcef..32430bfd5a1 100644 --- a/tests/baselines/reference/typeFromPropertyAssignment10.symbols +++ b/tests/baselines/reference/typeFromPropertyAssignment10.symbols @@ -13,11 +13,11 @@ Outer.app = Outer.app || {}; === tests/cases/conformance/salsa/someview.js === Outer.app.SomeView = (function () { ->Outer.app.SomeView : Symbol(Outer.app.SomeView, Decl(someview.js, 0, 0)) +>Outer.app.SomeView : Symbol(app.SomeView, Decl(someview.js, 0, 0)) >Outer.app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) >Outer : Symbol(Outer, Decl(module.js, 0, 3), Decl(someview.js, 0, 0), Decl(application.js, 0, 0)) >app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) ->SomeView : Symbol(Outer.app.SomeView, Decl(someview.js, 0, 0)) +>SomeView : Symbol(app.SomeView, Decl(someview.js, 0, 0)) var SomeView = function() { >SomeView : Symbol(SomeView, Decl(someview.js, 1, 7)) @@ -30,11 +30,11 @@ Outer.app.SomeView = (function () { })(); Outer.app.Inner = class { ->Outer.app.Inner : Symbol(Outer.app.Inner, Decl(someview.js, 5, 5)) +>Outer.app.Inner : Symbol(app.Inner, Decl(someview.js, 5, 5)) >Outer.app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) >Outer : Symbol(Outer, Decl(module.js, 0, 3), Decl(someview.js, 0, 0), Decl(application.js, 0, 0)) >app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) ->Inner : Symbol(Outer.app.Inner, Decl(someview.js, 5, 5)) +>Inner : Symbol(app.Inner, Decl(someview.js, 5, 5)) constructor() { /** @type {number} */ @@ -46,11 +46,11 @@ Outer.app.Inner = class { } var example = new Outer.app.Inner(); >example : Symbol(example, Decl(someview.js, 12, 3)) ->Outer.app.Inner : Symbol(Outer.app.Inner, Decl(someview.js, 5, 5)) +>Outer.app.Inner : Symbol(app.Inner, Decl(someview.js, 5, 5)) >Outer.app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) >Outer : Symbol(Outer, Decl(module.js, 0, 3), Decl(someview.js, 0, 0), Decl(application.js, 0, 0)) >app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) ->Inner : Symbol(Outer.app.Inner, Decl(someview.js, 5, 5)) +>Inner : Symbol(app.Inner, Decl(someview.js, 5, 5)) example.y; >example.y : Symbol((Anonymous class).y, Decl(someview.js, 7, 19)) @@ -59,11 +59,11 @@ example.y; /** @param {number} k */ Outer.app.statische = function (k) { ->Outer.app.statische : Symbol(Outer.app.statische, Decl(someview.js, 13, 10)) +>Outer.app.statische : Symbol(app.statische, Decl(someview.js, 13, 10)) >Outer.app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) >Outer : Symbol(Outer, Decl(module.js, 0, 3), Decl(someview.js, 0, 0), Decl(application.js, 0, 0)) >app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) ->statische : Symbol(Outer.app.statische, Decl(someview.js, 13, 10)) +>statische : Symbol(app.statische, Decl(someview.js, 13, 10)) >k : Symbol(k, Decl(someview.js, 15, 32)) return k ** k; @@ -90,11 +90,11 @@ Outer.app.Application = (function () { me.view = new Outer.app.SomeView(); >me : Symbol(me, Decl(application.js, 7, 11)) ->Outer.app.SomeView : Symbol(Outer.app.SomeView, Decl(someview.js, 0, 0)) +>Outer.app.SomeView : Symbol(app.SomeView, Decl(someview.js, 0, 0)) >Outer.app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) >Outer : Symbol(Outer, Decl(module.js, 0, 3), Decl(someview.js, 0, 0), Decl(application.js, 0, 0)) >app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) ->SomeView : Symbol(Outer.app.SomeView, Decl(someview.js, 0, 0)) +>SomeView : Symbol(app.SomeView, Decl(someview.js, 0, 0)) }; return Application; @@ -112,11 +112,11 @@ var app = new Outer.app.Application(); var inner = new Outer.app.Inner(); >inner : Symbol(inner, Decl(main.js, 1, 3)) ->Outer.app.Inner : Symbol(Outer.app.Inner, Decl(someview.js, 5, 5)) +>Outer.app.Inner : Symbol(app.Inner, Decl(someview.js, 5, 5)) >Outer.app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) >Outer : Symbol(Outer, Decl(module.js, 0, 3), Decl(someview.js, 0, 0), Decl(application.js, 0, 0)) >app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) ->Inner : Symbol(Outer.app.Inner, Decl(someview.js, 5, 5)) +>Inner : Symbol(app.Inner, Decl(someview.js, 5, 5)) inner.y; >inner.y : Symbol((Anonymous class).y, Decl(someview.js, 7, 19)) @@ -133,9 +133,9 @@ x.y; >y : Symbol((Anonymous class).y, Decl(someview.js, 7, 19)) Outer.app.statische(101); // Infinity, duh ->Outer.app.statische : Symbol(Outer.app.statische, Decl(someview.js, 13, 10)) +>Outer.app.statische : Symbol(app.statische, Decl(someview.js, 13, 10)) >Outer.app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) >Outer : Symbol(Outer, Decl(module.js, 0, 3), Decl(someview.js, 0, 0), Decl(application.js, 0, 0)) >app : Symbol(app, Decl(module.js, 0, 24), Decl(someview.js, 0, 6), Decl(application.js, 0, 6)) ->statische : Symbol(Outer.app.statische, Decl(someview.js, 13, 10)) +>statische : Symbol(app.statische, Decl(someview.js, 13, 10))