Skip to content

Commit d187de2

Browse files
sandersnsandersn
authored
Better JS container binding (microsoft#24367)
* Static assignments to class expressions work * Bind static properties of functions too Also update SymbolLinks in getTypeOfFuncClassEnumModule so that the type gets cached correctly. * Remove initializer handling:obj literals+type lookup Also include a couple of improved baselines * Fix 1-nested js containers:binding+cross-file merge * Consolidate check into one utility The utility is horrible and needs to change, but at least it's in one place. Next step is to make the utility like getDeclarationOfAlias, except getDeclarationOfJSAlias. * Defaulted assignments now (mostly) work * Default assignment definitely work, and IIFEs kind of do * n-nested undeclared containers now seem to work Merging even seems to work ok. * Handle prototype+prototype property assignments Perhaps in the wrong way. I have an idea how to simplify them. * Remove prototype special-case 1. It's not completely removed; the checker code in getJavascriptClassType needs to be fixed, among other places. 2. I didn't actually remove the code so that it will be easier to see what used to be there on Monday. Regardless, the code will be much simpler and seems to be mostly improved with very little work so far. * Allow more merges+accept baselines * Update more baselines * Fix js initializer check in bindPropertyAssignment * Fix codefixes * Rest of strictNullChecks cleanup + other cleanup 1. Remove a few TODOs 2. Remove extraneous SymbolFlag 3. Simplify isSameDefaultedName * Binder cleanup * Checker cleanup * Almost done with utilities cleanup * Utilities cleanup * Require js initializer to be (1) JS (2) initializer Change getDeclarationOfJSInitializer to require that the provided js initializer be in a javascript file, and that it is the initializer of the retrieved declaration. * Use getSymbolOfNode instead of accessing symbol directly * Ugh. Start over with just test cases * Handle additional cases in getTypeOfVariableOrParameterOrProperty These are cases in a really embarrassing check, in which we admit that the symbol flags steered us wrong and switch to getTypeOfFuncClassEnumModule instead (which never asserts). * Add test case for microsoft#24111 * Address PR comments
1 parent 70ad5a3 commit d187de2

53 files changed

Lines changed: 653 additions & 479 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/compiler/binder.ts‎

Lines changed: 51 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,6 +1698,7 @@ namespace ts {
16981698
symbol.parent = container.symbol;
16991699
}
17001700
addDeclarationToSymbol(symbol, node, symbolFlags);
1701+
return symbol;
17011702
}
17021703

17031704
function bindBlockScopedDeclaration(node: Declaration, symbolFlags: SymbolFlags, symbolExcludes: SymbolFlags) {
@@ -2317,13 +2318,11 @@ namespace ts {
23172318
// expression is the declaration
23182319
setCommonJsModuleIndicator(node);
23192320
const lhs = node.left as PropertyAccessEntityNameExpression;
2320-
const symbol = forEachIdentifierInEntityName(lhs.expression, (id, original) => {
2321-
if (!original) {
2322-
return undefined;
2321+
const symbol = forEachIdentifierInEntityName(lhs.expression, /*parent*/ undefined, (id, symbol) => {
2322+
if (symbol) {
2323+
addDeclarationToSymbol(symbol, id, SymbolFlags.Module | SymbolFlags.JSContainer);
23232324
}
2324-
const s = getJSInitializerSymbol(original)!;
2325-
addDeclarationToSymbol(s, id, SymbolFlags.Module | SymbolFlags.JSContainer);
2326-
return s;
2325+
return symbol;
23272326
});
23282327
if (symbol) {
23292328
const flags = isClassExpression(node.right) ?
@@ -2359,12 +2358,12 @@ namespace ts {
23592358
switch (thisContainer.kind) {
23602359
case SyntaxKind.FunctionDeclaration:
23612360
case SyntaxKind.FunctionExpression:
2362-
let constructorSymbol = thisContainer.symbol;
2361+
let constructorSymbol: Symbol | undefined = thisContainer.symbol;
23632362
// For `f.prototype.m = function() { this.x = 0; }`, `this.x = 0` should modify `f`'s members, not the function expression.
23642363
if (isBinaryExpression(thisContainer.parent) && thisContainer.parent.operatorToken.kind === SyntaxKind.EqualsToken) {
23652364
const l = thisContainer.parent.left;
23662365
if (isPropertyAccessEntityNameExpression(l) && isPrototypeAccess(l.expression)) {
2367-
constructorSymbol = getJSInitializerSymbolFromName(l.expression.expression, thisParentContainer)!;
2366+
constructorSymbol = lookupSymbolForPropertyAccess(l.expression.expression, thisParentContainer);
23682367
}
23692368
}
23702369

@@ -2463,46 +2462,67 @@ namespace ts {
24632462
bindPropertyAssignment(node.expression, node, /*isPrototypeProperty*/ false);
24642463
}
24652464

2466-
function getJSInitializerSymbolFromName(name: EntityNameExpression, lookupContainer?: Node): Symbol | undefined {
2467-
return getJSInitializerSymbol(lookupSymbolForPropertyAccess(name, lookupContainer));
2468-
}
2469-
24702465
function bindPropertyAssignment(name: EntityNameExpression, propertyAccess: PropertyAccessEntityNameExpression, isPrototypeProperty: boolean) {
2471-
let symbol = getJSInitializerSymbolFromName(name);
2466+
let namespaceSymbol = lookupSymbolForPropertyAccess(name);
24722467
const isToplevelNamespaceableInitializer = isBinaryExpression(propertyAccess.parent)
24732468
? getParentOfBinaryExpression(propertyAccess.parent).parent.kind === SyntaxKind.SourceFile &&
24742469
!!getJavascriptInitializer(getInitializerOfBinaryExpression(propertyAccess.parent), isPrototypeAccess(propertyAccess.parent.left))
24752470
: propertyAccess.parent.parent.kind === SyntaxKind.SourceFile;
2476-
if (!isPrototypeProperty && (!symbol || !(symbol.flags & SymbolFlags.Namespace)) && isToplevelNamespaceableInitializer) {
2471+
if (!isPrototypeProperty && (!namespaceSymbol || !(namespaceSymbol.flags & SymbolFlags.Namespace)) && isToplevelNamespaceableInitializer) {
24772472
// make symbols or add declarations for intermediate containers
24782473
const flags = SymbolFlags.Module | SymbolFlags.JSContainer;
24792474
const excludeFlags = SymbolFlags.ValueModuleExcludes & ~SymbolFlags.JSContainer;
2480-
forEachIdentifierInEntityName(propertyAccess.expression, (id, original) => {
2481-
if (original) {
2482-
// Note: add declaration to original symbol, not the special-syntax's symbol, so that namespaces work for type lookup
2483-
addDeclarationToSymbol(original, id, flags);
2484-
return original;
2475+
namespaceSymbol = forEachIdentifierInEntityName(propertyAccess.expression, namespaceSymbol, (id, symbol, parent) => {
2476+
if (symbol) {
2477+
addDeclarationToSymbol(symbol, id, flags);
2478+
return symbol;
24852479
}
24862480
else {
2487-
return symbol = declareSymbol(symbol ? symbol.exports! : container.locals!, symbol, id, flags, excludeFlags);
2481+
return declareSymbol(parent ? parent.exports! : container.locals!, parent, id, flags, excludeFlags);
24882482
}
24892483
});
24902484
}
2491-
if (!symbol || !(symbol.flags & (SymbolFlags.Function | SymbolFlags.Class | SymbolFlags.NamespaceModule | SymbolFlags.ObjectLiteral))) {
2485+
if (!namespaceSymbol || !isJavascriptContainer(namespaceSymbol)) {
24922486
return;
24932487
}
24942488

24952489
// Set up the members collection if it doesn't exist already
24962490
const symbolTable = isPrototypeProperty ?
2497-
(symbol.members || (symbol.members = createSymbolTable())) :
2498-
(symbol.exports || (symbol.exports = createSymbolTable()));
2491+
(namespaceSymbol.members || (namespaceSymbol.members = createSymbolTable())) :
2492+
(namespaceSymbol.exports || (namespaceSymbol.exports = createSymbolTable()));
24992493

25002494
// Declare the method/property
25012495
const jsContainerFlag = isToplevelNamespaceableInitializer ? SymbolFlags.JSContainer : 0;
2502-
const isMethod = isFunctionLikeDeclaration(getAssignedJavascriptInitializer(propertyAccess)!); // TODO: GH#18217
2496+
const isMethod = isFunctionLikeDeclaration(getAssignedJavascriptInitializer(propertyAccess)!);
25032497
const symbolFlags = (isMethod ? SymbolFlags.Method : SymbolFlags.Property) | jsContainerFlag;
25042498
const symbolExcludes = (isMethod ? SymbolFlags.MethodExcludes : SymbolFlags.PropertyExcludes) & ~jsContainerFlag;
2505-
declareSymbol(symbolTable, symbol, propertyAccess, symbolFlags, symbolExcludes);
2499+
declareSymbol(symbolTable, namespaceSymbol, propertyAccess, symbolFlags, symbolExcludes);
2500+
}
2501+
2502+
/**
2503+
* Javascript containers are:
2504+
* - Functions
2505+
* - classes
2506+
* - namespaces
2507+
* - variables initialized with function expressions
2508+
* - with class expressions
2509+
* - with empty object literals
2510+
* - with non-empty object literals if assigned to the prototype property
2511+
*/
2512+
function isJavascriptContainer(symbol: Symbol): boolean {
2513+
const node = symbol.valueDeclaration;
2514+
if (symbol.flags & (SymbolFlags.Function | SymbolFlags.Class | SymbolFlags.NamespaceModule)) {
2515+
return true;
2516+
}
2517+
const init = isVariableDeclaration(node) ? node.initializer :
2518+
isBinaryExpression(node) ? node.right :
2519+
isPropertyAccessExpression(node) && isBinaryExpression(node.parent) ? node.parent.right :
2520+
undefined;
2521+
if (init) {
2522+
const isPrototypeAssignment = isPrototypeAccess(isVariableDeclaration(node) ? node.name : isBinaryExpression(node) ? node.left : node);
2523+
return !!getJavascriptInitializer(isBinaryExpression(init) && init.operatorToken.kind === SyntaxKind.BarBarToken ? init.right : init, isPrototypeAssignment);
2524+
}
2525+
return false;
25062526
}
25072527

25082528
function getParentOfBinaryExpression(expr: BinaryExpression) {
@@ -2517,22 +2537,22 @@ namespace ts {
25172537
return lookupSymbolForNameWorker(lookupContainer, node.escapedText);
25182538
}
25192539
else {
2520-
const symbol = getJSInitializerSymbol(lookupSymbolForPropertyAccess(node.expression));
2540+
const symbol = lookupSymbolForPropertyAccess(node.expression);
25212541
return symbol && symbol.exports && symbol.exports.get(node.name.escapedText);
25222542
}
25232543
}
25242544

2525-
function forEachIdentifierInEntityName(e: EntityNameExpression, action: (e: Identifier, symbol: Symbol | undefined) => Symbol | undefined): Symbol | undefined {
2545+
function forEachIdentifierInEntityName(e: EntityNameExpression, parent: Symbol | undefined, action: (e: Identifier, symbol: Symbol | undefined, parent: Symbol | undefined) => Symbol | undefined): Symbol | undefined {
25262546
if (isExportsOrModuleExportsOrAlias(file, e)) {
25272547
return file.symbol;
25282548
}
25292549
else if (isIdentifier(e)) {
2530-
return action(e, lookupSymbolForPropertyAccess(e));
2550+
return action(e, lookupSymbolForPropertyAccess(e), parent);
25312551
}
25322552
else {
2533-
const s = getJSInitializerSymbol(forEachIdentifierInEntityName(e.expression, action));
2553+
const s = forEachIdentifierInEntityName(e.expression, parent, action);
25342554
if (!s || !s.exports) return Debug.fail();
2535-
return action(e.name, s.exports.get(e.name.escapedText));
2555+
return action(e.name, s.exports.get(e.name.escapedText), s);
25362556
}
25372557
}
25382558

@@ -2596,7 +2616,7 @@ namespace ts {
25962616
bindBlockScopedVariableDeclaration(node);
25972617
}
25982618
else if (isParameterDeclaration(node)) {
2599-
// It is safe to walk up parent chain to find whether the node is a destructing parameter declaration
2619+
// It is safe to walk up parent chain to find whether the node is a destructuring parameter declaration
26002620
// because its parent chain has already been set up, since parents are set before descending into children.
26012621
//
26022622
// If node is a binding element in parameter declaration, we need to use ParameterExcludes.

0 commit comments

Comments
 (0)