From b0aebb4c1e54c29f392edd1cd4617101a50122b1 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 8 Feb 2018 15:43:10 -0800 Subject: [PATCH] Recursive object-literal-assignment declarations --- src/compiler/binder.ts | 74 ++++++++++------- src/compiler/utilities.ts | 21 +++-- .../typeFromPropertyAssignment9.symbols | 76 +++++++++-------- .../typeFromPropertyAssignment9.types | 81 +++++++++++-------- .../salsa/typeFromPropertyAssignment9.ts | 18 +++-- .../codeFixUndeclaredAcrossFiles1.ts | 4 +- 6 files changed, 156 insertions(+), 118 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index a31b1d3ffbf..0e91e752376 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -2033,16 +2033,16 @@ namespace ts { const specialKind = getSpecialPropertyAssignmentKind(node as BinaryExpression); switch (specialKind) { case SpecialPropertyAssignmentKind.ExportsProperty: - bindExportsPropertyAssignment(node); + bindExportsPropertyAssignment(node as BinaryExpression); break; case SpecialPropertyAssignmentKind.ModuleExports: - bindModuleExportsAssignment(node); + bindModuleExportsAssignment(node as BinaryExpression); break; case SpecialPropertyAssignmentKind.PrototypeProperty: - bindPrototypePropertyAssignment(node); + bindPrototypePropertyAssignment(node as BinaryExpression); break; case SpecialPropertyAssignmentKind.ThisProperty: - bindThisPropertyAssignment(node); + bindThisPropertyAssignment(node as BinaryExpression); break; case SpecialPropertyAssignmentKind.Property: bindStaticBinaryExpressionAssignment(node as BinaryExpression); @@ -2296,6 +2296,7 @@ namespace ts { } function bindExportsPropertyAssignment(node: BinaryExpression) { + // TODO: Maybe this should support nested exports too // When we create a property via 'exports.foo = bar', the 'exports.foo' property access // expression is the declaration setCommonJsModuleIndicator(node); @@ -2370,23 +2371,21 @@ namespace ts { constructorFunction.parent = classPrototype; classPrototype.parent = leftSideOfAssignment; - bindPropertyAssignment(constructorFunction.escapedText, leftSideOfAssignment, /*isPrototypeProperty*/ true); + bindPropertyAssignment(constructorFunction, leftSideOfAssignment, /*isPrototypeProperty*/ true); } function bindStaticBinaryExpressionAssignment(node: BinaryExpression) { const lhs = node.left as PropertyAccessExpression; - if (isIdentifier(lhs.expression)) { - lhs.parent = node; - if (container === file && isNameOfExportsOrModuleExportsAliasDeclaration(file, lhs.expression)) { - // This can be an alias for the 'exports' or 'module.exports' names, e.g. - // var util = module.exports; - // util.property = function ... - bindExportsPropertyAssignment(node); - } - else { - bindStaticPropertyAssignment(lhs); - } + lhs.parent = node; + if (isIdentifier(lhs.expression) && container === file && isNameOfExportsOrModuleExportsAliasDeclaration(file, lhs.expression)) { + // This can be an alias for the 'exports' or 'module.exports' names, e.g. + // var util = module.exports; + // util.property = function ... + bindExportsPropertyAssignment(node); + } + else { + bindStaticPropertyAssignment(lhs); } } /** @@ -2394,24 +2393,37 @@ namespace ts { * Also works for expression statements preceded by JSDoc, like / ** @type number * / x.y; */ function bindStaticPropertyAssignment(node: PropertyAccessExpression) { - // Look up the property in the local scope, since static assignments should follow the declaration - if (isIdentifier(node.expression)) { - // Fix up parent pointers since we're going to use these nodes before we bind into them - node.expression.parent = node; - bindPropertyAssignment(node.expression.escapedText, node, /*isPrototypeProperty*/ false); + // Fix up parent pointers since we're going to use these nodes before we bind into them + node.expression.parent = node; + bindPropertyAssignment(node.expression as Identifier | PropertyAccessExpression, node, /*isPrototypeProperty*/ false); + } + + function lookupSymbolForPropertyAccess(node: Identifier | PropertyAccessExpression): Symbol | undefined { + if (isIdentifier(node)) { + return lookupSymbolForNameWorker(container, node.escapedText); + } + else { + let symbol = lookupSymbolForPropertyAccess(node.expression as Identifier | PropertyAccessExpression); + symbol = symbol && isDeclarationOfJavascriptContainerExpression(symbol) ? (symbol.valueDeclaration as VariableDeclaration).initializer.symbol : + symbol && isDeclarationOfDefaultedJavascriptContainerExpression(symbol) ? ((symbol.valueDeclaration as VariableDeclaration).initializer as BinaryExpression).right.symbol : + // TODO: Might want the next line below for even further nestings? + // symbol && isAssignmentOfDefaultedJavascriptContainerExpression(symbol) ? ((symbol.valueDeclaration.parent as BinaryExpression).right as BinaryExpression).symbol : + symbol; + return symbol && symbol.exports && symbol.exports.get(node.name.escapedText); } } - function lookupSymbolForName(name: __String) { - return lookupSymbolForNameWorker(container, name); - } - - function bindPropertyAssignment(name: __String, propertyAccess: PropertyAccessExpression, isPrototypeProperty: boolean) { - const symbol = lookupSymbolForName(name); + function bindPropertyAssignment(name: Identifier | PropertyAccessExpression, propertyAccess: PropertyAccessExpression, isPrototypeProperty: boolean) { + // Look up the property in the local scope, since property assignments should follow the declaration + const symbol = lookupSymbolForPropertyAccess(name); + // TODO: Should be able to structure this with less duplication let targetSymbol = symbol && isDeclarationOfJavascriptContainerExpression(symbol) ? (symbol.valueDeclaration as VariableDeclaration).initializer.symbol : symbol && isDeclarationOfDefaultedJavascriptContainerExpression(symbol) ? ((symbol.valueDeclaration as VariableDeclaration).initializer as BinaryExpression).right.symbol : + symbol && isAssignmentOfDefaultedJavascriptContainerExpression(symbol) ? (((symbol.valueDeclaration.parent as BinaryExpression).right as BinaryExpression).right as BinaryExpression).symbol : symbol; - Debug.assert(propertyAccess.parent.kind === SyntaxKind.BinaryExpression || propertyAccess.parent.kind === SyntaxKind.ExpressionStatement); + Debug.assert(propertyAccess.parent.kind === SyntaxKind.BinaryExpression || + propertyAccess.parent.kind === SyntaxKind.ExpressionStatement || + propertyAccess.parent.kind === SyntaxKind.PropertyAccessExpression); let isLegalPosition: boolean; if (propertyAccess.parent.kind === SyntaxKind.BinaryExpression) { const initializerKind = (propertyAccess.parent as BinaryExpression).right.kind; @@ -2422,14 +2434,16 @@ namespace ts { isLegalPosition = propertyAccess.parent.parent.kind === SyntaxKind.SourceFile; } if (!isPrototypeProperty && (!targetSymbol || !(targetSymbol.flags & SymbolFlags.Namespace)) && isLegalPosition) { - Debug.assert(isIdentifier(propertyAccess.expression)); - const identifier = propertyAccess.expression as Identifier; + const identifier = isIdentifier(propertyAccess.expression) ? propertyAccess.expression : (propertyAccess.expression as PropertyAccessExpression).expression as Identifier; const flags = SymbolFlags.Module | SymbolFlags.JSContainer; const excludeFlags = SymbolFlags.ValueModuleExcludes & ~SymbolFlags.JSContainer; if (targetSymbol) { + // TODO: Not sure this is correct for nested declarations -- maybe this should be added to targetSymbol + // Note: add declaration to original symbol, not the special-syntax's symbol, so that namespaces work for type lookup addDeclarationToSymbol(symbol, identifier, flags); } else { + // TODO: Not sure this branch is correct for nested declarations targetSymbol = declareSymbol(container.locals, /*parent*/ undefined, identifier, flags, excludeFlags); } } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 63dc6c96c54..966909f5284 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -1490,17 +1490,12 @@ namespace ts { * This function does not test if the node is in a JavaScript file or not. */ export function isDeclarationOfJavascriptContainerExpression(s: Symbol) { - if (s.valueDeclaration && isVariableDeclaration(s.valueDeclaration)) { - return isVariableDeclaration(s.valueDeclaration) && isJavascriptContainerExpression(s.valueDeclaration.initializer); - } - return false; - } - - export function isJavascriptContainerExpression(expression: Expression | undefined) { - return expression && - (expression.kind === SyntaxKind.FunctionExpression || - expression.kind === SyntaxKind.ClassExpression || - isObjectLiteralExpression(expression) && expression.properties.length === 0) + return s.valueDeclaration && + isVariableDeclaration(s.valueDeclaration) && + s.valueDeclaration.initializer && + (s.valueDeclaration.initializer.kind === SyntaxKind.FunctionExpression || + s.valueDeclaration.initializer.kind === SyntaxKind.ClassExpression || + isObjectLiteralExpression(s.valueDeclaration.initializer) && s.valueDeclaration.initializer.properties.length === 0); } export function isDeclarationOfDefaultedJavascriptContainerExpression(s: Symbol) { @@ -1584,9 +1579,11 @@ namespace ts { return SpecialPropertyAssignmentKind.PrototypeProperty; } } + if(isEntityNameExpression(lhs.expression)) { + return SpecialPropertyAssignmentKind.Property; + } } - return SpecialPropertyAssignmentKind.None; } diff --git a/tests/baselines/reference/typeFromPropertyAssignment9.symbols b/tests/baselines/reference/typeFromPropertyAssignment9.symbols index 9af8fffdc88..c1afbe82036 100644 --- a/tests/baselines/reference/typeFromPropertyAssignment9.symbols +++ b/tests/baselines/reference/typeFromPropertyAssignment9.symbols @@ -1,54 +1,62 @@ === tests/cases/conformance/salsa/a.js === // TODO: JSDoc would provide a contextual type, so ... I should test that +// (a number of existing tests fail because of that, I think) // TODO: Try initializer of function or class I guess (though classes aren't context sensitive) +// TODO: Duplicated declarations should be OK (if they have the same type (??)) var my = my || {}; ->my : Symbol(my, Decl(a.js, 2, 3), Decl(a.js, 2, 18)) ->my : Symbol(my, Decl(a.js, 2, 3), Decl(a.js, 2, 18)) +>my : Symbol(my, Decl(a.js, 4, 3), Decl(a.js, 4, 18)) +>my : Symbol(my, Decl(a.js, 4, 3), Decl(a.js, 4, 18)) my.m = function() { ->my.m : Symbol(m, Decl(a.js, 2, 18)) ->my : Symbol(my, Decl(a.js, 2, 3), Decl(a.js, 2, 18)) ->m : Symbol(m, Decl(a.js, 2, 18)) +>my.m : Symbol(m, Decl(a.js, 4, 18)) +>my : Symbol(my, Decl(a.js, 4, 3), Decl(a.js, 4, 18)) +>m : Symbol(m, Decl(a.js, 4, 18)) return 1; } my.n = 1; ->my.n : Symbol(n, Decl(a.js, 5, 1)) ->my : Symbol(my, Decl(a.js, 2, 3), Decl(a.js, 2, 18)) ->n : Symbol(n, Decl(a.js, 5, 1)) +>my.n : Symbol(n, Decl(a.js, 7, 1)) +>my : Symbol(my, Decl(a.js, 4, 3), Decl(a.js, 4, 18)) +>n : Symbol(n, Decl(a.js, 7, 1)) my.o = {}; ->my.o : Symbol(o, Decl(a.js, 6, 9)) ->my : Symbol(my, Decl(a.js, 2, 3), Decl(a.js, 2, 18)) ->o : Symbol(o, Decl(a.js, 6, 9)) +>my.o : Symbol(o, Decl(a.js, 8, 9)) +>my : Symbol(my, Decl(a.js, 4, 3), Decl(a.js, 4, 18)) +>o : Symbol(o, Decl(a.js, 8, 9)) -my.possible = my.possible || {}; ->my.possible : Symbol(possible, Decl(a.js, 7, 10)) ->my : Symbol(my, Decl(a.js, 2, 3), Decl(a.js, 2, 18)) ->possible : Symbol(possible, Decl(a.js, 7, 10)) ->my.possible : Symbol(possible, Decl(a.js, 7, 10)) ->my : Symbol(my, Decl(a.js, 2, 3), Decl(a.js, 2, 18)) ->possible : Symbol(possible, Decl(a.js, 7, 10)) +my.predicate = my.predicate || {}; +>my.predicate : Symbol(predicate, Decl(a.js, 9, 10), Decl(a.js, 10, 34)) +>my : Symbol(my, Decl(a.js, 4, 3), Decl(a.js, 4, 18)) +>predicate : Symbol(predicate, Decl(a.js, 9, 10), Decl(a.js, 10, 34)) +>my.predicate : Symbol(predicate, Decl(a.js, 9, 10), Decl(a.js, 10, 34)) +>my : Symbol(my, Decl(a.js, 4, 3), Decl(a.js, 4, 18)) +>predicate : Symbol(predicate, Decl(a.js, 9, 10), Decl(a.js, 10, 34)) -my.m; ->my.m : Symbol(m, Decl(a.js, 2, 18)) ->my : Symbol(my, Decl(a.js, 2, 3), Decl(a.js, 2, 18)) ->m : Symbol(m, Decl(a.js, 2, 18)) +my.predicate.query = function () { +>my.predicate.query : Symbol(query, Decl(a.js, 10, 34)) +>my.predicate : Symbol(predicate, Decl(a.js, 9, 10), Decl(a.js, 10, 34)) +>my : Symbol(my, Decl(a.js, 4, 3), Decl(a.js, 4, 18)) +>predicate : Symbol(predicate, Decl(a.js, 9, 10), Decl(a.js, 10, 34)) +>query : Symbol(query, Decl(a.js, 10, 34)) -my.o; ->my.o : Symbol(o, Decl(a.js, 6, 9)) ->my : Symbol(my, Decl(a.js, 2, 3), Decl(a.js, 2, 18)) ->o : Symbol(o, Decl(a.js, 6, 9)) + var me = this; +>me : Symbol(me, Decl(a.js, 12, 7)) +>this : Symbol(__object, Decl(a.js, 10, 30)) -my.n; ->my.n : Symbol(n, Decl(a.js, 5, 1)) ->my : Symbol(my, Decl(a.js, 2, 3), Decl(a.js, 2, 18)) ->n : Symbol(n, Decl(a.js, 5, 1)) + me.property = false; +>me : Symbol(me, Decl(a.js, 12, 7)) -my.possible; ->my.possible : Symbol(possible, Decl(a.js, 7, 10)) ->my : Symbol(my, Decl(a.js, 2, 3), Decl(a.js, 2, 18)) ->possible : Symbol(possible, Decl(a.js, 7, 10)) +}; +var q = new my.predicate.query(); +>q : Symbol(q, Decl(a.js, 15, 3)) +>my.predicate.query : Symbol(query, Decl(a.js, 10, 34)) +>my.predicate : Symbol(predicate, Decl(a.js, 9, 10), Decl(a.js, 10, 34)) +>my : Symbol(my, Decl(a.js, 4, 3), Decl(a.js, 4, 18)) +>predicate : Symbol(predicate, Decl(a.js, 9, 10), Decl(a.js, 10, 34)) +>query : Symbol(query, Decl(a.js, 10, 34)) +// my.predicate.result = {}; +// my.predicate.sorted = my.predicate.sorted || {}; +// TODO: EVEN MORE NESTING diff --git a/tests/baselines/reference/typeFromPropertyAssignment9.types b/tests/baselines/reference/typeFromPropertyAssignment9.types index e57d8b70bf7..88607d08cf1 100644 --- a/tests/baselines/reference/typeFromPropertyAssignment9.types +++ b/tests/baselines/reference/typeFromPropertyAssignment9.types @@ -1,16 +1,18 @@ === tests/cases/conformance/salsa/a.js === // TODO: JSDoc would provide a contextual type, so ... I should test that +// (a number of existing tests fail because of that, I think) // TODO: Try initializer of function or class I guess (though classes aren't context sensitive) +// TODO: Duplicated declarations should be OK (if they have the same type (??)) var my = my || {}; ->my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } ->my || {} : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } ->my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } ->{} : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } +>my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; predicate: { [x: string]: any; query: () => void; }; } +>my || {} : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; predicate: { [x: string]: any; query: () => void; }; } +>my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; predicate: { [x: string]: any; query: () => void; }; } +>{} : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; predicate: { [x: string]: any; query: () => void; }; } my.m = function() { >my.m = function() { return 1;} : () => number >my.m : () => number ->my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } +>my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; predicate: { [x: string]: any; query: () => void; }; } >m : () => number >function() { return 1;} : () => number @@ -20,47 +22,60 @@ my.m = function() { my.n = 1; >my.n = 1 : 1 >my.n : number ->my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } +>my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; predicate: { [x: string]: any; query: () => void; }; } >n : number >1 : 1 my.o = {}; >my.o = {} : { [x: string]: any; } >my.o : { [x: string]: any; } ->my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } +>my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; predicate: { [x: string]: any; query: () => void; }; } >o : { [x: string]: any; } >{} : { [x: string]: any; } -my.possible = my.possible || {}; ->my.possible = my.possible || {} : { [x: string]: any; } ->my.possible : { [x: string]: any; } ->my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } ->possible : { [x: string]: any; } ->my.possible || {} : { [x: string]: any; } ->my.possible : { [x: string]: any; } ->my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } ->possible : { [x: string]: any; } ->{} : { [x: string]: any; } +my.predicate = my.predicate || {}; +>my.predicate = my.predicate || {} : { [x: string]: any; query: () => void; } +>my.predicate : { [x: string]: any; query: () => void; } +>my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; predicate: { [x: string]: any; query: () => void; }; } +>predicate : { [x: string]: any; query: () => void; } +>my.predicate || {} : { [x: string]: any; query: () => void; } +>my.predicate : { [x: string]: any; query: () => void; } +>my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; predicate: { [x: string]: any; query: () => void; }; } +>predicate : { [x: string]: any; query: () => void; } +>{} : { [x: string]: any; query: () => void; } -my.m; ->my.m : () => number ->my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } ->m : () => number +my.predicate.query = function () { +>my.predicate.query = function () { var me = this; me.property = false;} : () => void +>my.predicate.query : () => void +>my.predicate : { [x: string]: any; query: () => void; } +>my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; predicate: { [x: string]: any; query: () => void; }; } +>predicate : { [x: string]: any; query: () => void; } +>query : () => void +>function () { var me = this; me.property = false;} : () => void -my.o; ->my.o : { [x: string]: any; } ->my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } ->o : { [x: string]: any; } + var me = this; +>me : { [x: string]: any; query: () => void; } +>this : { [x: string]: any; query: () => void; } -my.n; ->my.n : number ->my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } ->n : number + me.property = false; +>me.property = false : false +>me.property : any +>me : { [x: string]: any; query: () => void; } +>property : any +>false : false -my.possible; ->my.possible : { [x: string]: any; } ->my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; possible: { [x: string]: any; }; } ->possible : { [x: string]: any; } +}; +var q = new my.predicate.query(); +>q : any +>new my.predicate.query() : any +>my.predicate.query : () => void +>my.predicate : { [x: string]: any; query: () => void; } +>my : { [x: string]: any; m: () => number; n: number; o: { [x: string]: any; }; predicate: { [x: string]: any; query: () => void; }; } +>predicate : { [x: string]: any; query: () => void; } +>query : () => void +// my.predicate.result = {}; +// my.predicate.sorted = my.predicate.sorted || {}; +// TODO: EVEN MORE NESTING diff --git a/tests/cases/conformance/salsa/typeFromPropertyAssignment9.ts b/tests/cases/conformance/salsa/typeFromPropertyAssignment9.ts index 0f0b2fa272d..d3a339b8804 100644 --- a/tests/cases/conformance/salsa/typeFromPropertyAssignment9.ts +++ b/tests/cases/conformance/salsa/typeFromPropertyAssignment9.ts @@ -5,18 +5,22 @@ // @Filename: a.js // TODO: JSDoc would provide a contextual type, so ... I should test that +// (a number of existing tests fail because of that, I think) // TODO: Try initializer of function or class I guess (though classes aren't context sensitive) +// TODO: Duplicated declarations should be OK (if they have the same type (??)) var my = my || {}; my.m = function() { return 1; } my.n = 1; my.o = {}; -my.possible = my.possible || {}; - -my.m; -my.o; -my.n; -my.possible; - +my.predicate = my.predicate || {}; +my.predicate.query = function () { + var me = this; + me.property = false; +}; +var q = new my.predicate.query(); +// my.predicate.result = {}; +// my.predicate.sorted = my.predicate.sorted || {}; +// TODO: EVEN MORE NESTING diff --git a/tests/cases/fourslash/codeFixUndeclaredAcrossFiles1.ts b/tests/cases/fourslash/codeFixUndeclaredAcrossFiles1.ts index a946b60f173..29dcc2a5048 100644 --- a/tests/cases/fourslash/codeFixUndeclaredAcrossFiles1.ts +++ b/tests/cases/fourslash/codeFixUndeclaredAcrossFiles1.ts @@ -23,7 +23,7 @@ verify.getAndApplyCodeFix(/*errorCode*/undefined, 0); verify.getAndApplyCodeFix(/*errorCode*/undefined, 0); verify.rangeIs(` - y: {}; + y: { [x: string]: any; }; m1(): any { throw new Error("Method not implemented."); } @@ -31,4 +31,4 @@ verify.rangeIs(` static m0(arg0: any, arg1: any, arg2: any): any { throw new Error("Method not implemented."); } -`); \ No newline at end of file +`);