diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index e1eb0b4b7360d..e717efa5dba70 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -119,6 +119,7 @@ namespace ts { let languageVersion: ScriptTarget; let parent: Node; let container: Node; + let containerContainer: Node; // Container one level up let blockScopeContainer: Node; let inferenceContainer: Node; let lastContainer: Node; @@ -187,6 +188,7 @@ namespace ts { languageVersion = undefined; parent = undefined; container = undefined; + containerContainer = undefined; blockScopeContainer = undefined; inferenceContainer = undefined; lastContainer = undefined; @@ -224,13 +226,7 @@ namespace ts { symbol.flags |= symbolFlags; node.symbol = symbol; - - if (!symbol.declarations) { - symbol.declarations = [node]; - } - else { - symbol.declarations.push(node); - } + symbol.declarations = append(symbol.declarations, node); if (symbolFlags & SymbolFlags.HasExports && !symbol.exports) { symbol.exports = createSymbolTable(); @@ -486,8 +482,11 @@ namespace ts { // and block-container. Then after we pop out of processing the children, we restore // these saved values. const saveContainer = container; + const saveContainerContainer = containerContainer; const savedBlockScopeContainer = blockScopeContainer; + containerContainer = container; + // Depending on what kind of node this is, we may have to adjust the current container // and block-container. If the current node is a container, then it is automatically // considered the current block-container as well. Also, for containers that we know @@ -580,7 +579,9 @@ namespace ts { else { bindChildren(node); } + container = saveContainer; + containerContainer = saveContainerContainer; blockScopeContainer = savedBlockScopeContainer; } @@ -2327,14 +2328,23 @@ namespace ts { function bindThisPropertyAssignment(node: BinaryExpression | PropertyAccessExpression) { Debug.assert(isInJavaScriptFile(node)); - const container = getThisContainer(node, /*includeArrowFunctions*/ false); - switch (container.kind) { + const thisContainer = getThisContainer(node, /*includeArrowFunctions*/ false); + switch (thisContainer.kind) { case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: + let constructorSymbol = thisContainer.symbol; + // For `f.prototype.m = function() { this.x = 0; }`, `this.x = 0` should modify `f`'s members, not the function expression. + if (isBinaryExpression(thisContainer.parent) && thisContainer.parent.operatorToken.kind === SyntaxKind.EqualsToken) { + const l = thisContainer.parent.left; + if (isPropertyAccessExpression(l) && isPropertyAccessExpression(l.expression) && l.expression.name.escapedText === "prototype" && isEntityNameExpression(l.expression.expression)) { + constructorSymbol = getJSInitializerSymbolFromName(l.expression.expression, containerContainer); + } + } + // Declare a 'member' if the container is an ES5 class or ES6 constructor - container.symbol.members = container.symbol.members || createSymbolTable(); + constructorSymbol.members = constructorSymbol.members || createSymbolTable(); // It's acceptable for multiple 'this' assignments of the same identifier to occur - declareSymbol(container.symbol.members, container.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property); + declareSymbol(constructorSymbol.members, constructorSymbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property); break; case SyntaxKind.Constructor: @@ -2344,10 +2354,13 @@ namespace ts { case SyntaxKind.SetAccessor: // this.foo assignment in a JavaScript class // Bind this property to the containing class - const containingClass = container.parent; - const symbolTable = hasModifier(container, ModifierFlags.Static) ? containingClass.symbol.exports : containingClass.symbol.members; + const containingClass = thisContainer.parent; + const symbolTable = hasModifier(thisContainer, ModifierFlags.Static) ? containingClass.symbol.exports : containingClass.symbol.members; declareSymbol(symbolTable, containingClass.symbol, node, SymbolFlags.Property, SymbolFlags.None, /*isReplaceableByMethod*/ true); break; + + default: + Debug.fail(Debug.showSyntaxKind(thisContainer)); } } @@ -2417,8 +2430,12 @@ namespace ts { bindPropertyAssignment(node.expression, node, /*isPrototypeProperty*/ false); } + function getJSInitializerSymbolFromName(name: EntityNameExpression, lookupContainer?: Node): Symbol { + return getJSInitializerSymbol(lookupSymbolForPropertyAccess(name, lookupContainer)); + } + function bindPropertyAssignment(name: EntityNameExpression, propertyAccess: PropertyAccessEntityNameExpression, isPrototypeProperty: boolean) { - let symbol = getJSInitializerSymbol(lookupSymbolForPropertyAccess(name)); + let symbol = getJSInitializerSymbolFromName(name); let isToplevelNamespaceableInitializer: boolean; if (isBinaryExpression(propertyAccess.parent)) { const isPrototypeAssignment = isPropertyAccessExpression(propertyAccess.parent.left) && propertyAccess.parent.left.name.escapedText === "prototype"; @@ -2458,9 +2475,9 @@ namespace ts { declareSymbol(symbolTable, symbol, propertyAccess, symbolFlags, symbolExcludes); } - function lookupSymbolForPropertyAccess(node: EntityNameExpression): Symbol | undefined { + function lookupSymbolForPropertyAccess(node: EntityNameExpression, lookupContainer: Node = container): Symbol | undefined { if (isIdentifier(node)) { - return lookupSymbolForNameWorker(container, node.escapedText); + return lookupSymbolForNameWorker(lookupContainer, node.escapedText); } else { const symbol = getJSInitializerSymbol(lookupSymbolForPropertyAccess(node.expression)); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 72eb95a23aea9..0dca91f3fd445 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3324,7 +3324,7 @@ namespace ts { escapedName: __String; // Name of symbol declarations?: Declaration[]; // Declarations associated with this symbol valueDeclaration?: Declaration; // First value declaration of the symbol - members?: SymbolTable; // Class, interface or literal instance members + members?: SymbolTable; // Class, interface or object literal instance members exports?: SymbolTable; // Module exports globalExports?: SymbolTable; // Conditional global UMD exports /* @internal */ id?: number; // Unique id (used to look up SymbolLinks) diff --git a/tests/baselines/reference/constructorFunctions2.symbols b/tests/baselines/reference/constructorFunctions2.symbols index 1046aa32e9a62..a303f5c6aaa8a 100644 --- a/tests/baselines/reference/constructorFunctions2.symbols +++ b/tests/baselines/reference/constructorFunctions2.symbols @@ -23,12 +23,29 @@ const B = function() { this.id = 1; } >B : Symbol(B, Decl(index.js, 3, 5)) >id : Symbol(B.id, Decl(index.js, 3, 22)) -const b = new B().id; ->b : Symbol(b, Decl(index.js, 4, 5)) ->new B().id : Symbol(B.id, Decl(index.js, 3, 22)) +B.prototype.m = function() { this.x = 2; } +>B.prototype : Symbol(B.m, Decl(index.js, 3, 37)) >B : Symbol(B, Decl(index.js, 3, 5)) +>prototype : Symbol(Function.prototype, Decl(lib.d.ts, --, --)) +>m : Symbol(B.m, Decl(index.js, 3, 37)) +>this.x : Symbol(B.x, Decl(index.js, 4, 28)) +>this : Symbol(B, Decl(index.js, 3, 9)) +>x : Symbol(B.x, Decl(index.js, 4, 28)) + +const b = new B(); +>b : Symbol(b, Decl(index.js, 5, 5)) +>B : Symbol(B, Decl(index.js, 3, 5)) + +b.id; +>b.id : Symbol(B.id, Decl(index.js, 3, 22)) +>b : Symbol(b, Decl(index.js, 5, 5)) >id : Symbol(B.id, Decl(index.js, 3, 22)) +b.x; +>b.x : Symbol(B.x, Decl(index.js, 4, 28)) +>b : Symbol(b, Decl(index.js, 5, 5)) +>x : Symbol(B.x, Decl(index.js, 4, 28)) + === tests/cases/conformance/salsa/other.js === function A() { this.id = 1; } >A : Symbol(A, Decl(other.js, 0, 0)) diff --git a/tests/baselines/reference/constructorFunctions2.types b/tests/baselines/reference/constructorFunctions2.types index e96053b4f4901..1575b35610416 100644 --- a/tests/baselines/reference/constructorFunctions2.types +++ b/tests/baselines/reference/constructorFunctions2.types @@ -30,13 +30,35 @@ const B = function() { this.id = 1; } >id : any >1 : 1 -const b = new B().id; ->b : number ->new B().id : number ->new B() : { id: number; } +B.prototype.m = function() { this.x = 2; } +>B.prototype.m = function() { this.x = 2; } : () => void +>B.prototype.m : any +>B.prototype : any >B : () => void +>prototype : any +>m : any +>function() { this.x = 2; } : () => void +>this.x = 2 : 2 +>this.x : number +>this : { id: number; m: () => void; x: number; } +>x : number +>2 : 2 + +const b = new B(); +>b : { id: number; m: () => void; x: number; } +>new B() : { id: number; m: () => void; x: number; } +>B : () => void + +b.id; +>b.id : number +>b : { id: number; m: () => void; x: number; } >id : number +b.x; +>b.x : number +>b : { id: number; m: () => void; x: number; } +>x : number + === tests/cases/conformance/salsa/other.js === function A() { this.id = 1; } >A : () => void diff --git a/tests/cases/conformance/salsa/constructorFunctions2.ts b/tests/cases/conformance/salsa/constructorFunctions2.ts index 13a6e7b9a3a85..439fe5e49113c 100644 --- a/tests/cases/conformance/salsa/constructorFunctions2.ts +++ b/tests/cases/conformance/salsa/constructorFunctions2.ts @@ -11,7 +11,10 @@ const A = require("./other"); const a = new A().id; const B = function() { this.id = 1; } -const b = new B().id; +B.prototype.m = function() { this.x = 2; } +const b = new B(); +b.id; +b.x; // @filename: other.js function A() { this.id = 1; } diff --git a/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts b/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts index fe21e58544426..ef2249d2d43c7 100644 --- a/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts +++ b/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts @@ -23,7 +23,6 @@ verify.codeFix({ description: "Convert function to an ES2015 class", - index: 0, // TODO: GH#22240 newFileContent: `class fn { constructor() { diff --git a/tests/cases/fourslash/convertFunctionToEs6Class_falsePositive.ts b/tests/cases/fourslash/convertFunctionToEs6Class_falsePositive.ts new file mode 100644 index 0000000000000..f817ffb8e04ad --- /dev/null +++ b/tests/cases/fourslash/convertFunctionToEs6Class_falsePositive.ts @@ -0,0 +1,26 @@ +/// + +// @allowJs: true +// @Filename: /a.js +////function [|f|]() {} +////f.prototype.bar = [|function|](){ +//// this.x = 1; +////}; + +// Only a suggestion for `f`, not for the function expression. See GH#22240 +verify.getSuggestionDiagnostics([{ + message: "This constructor function may be converted to a class declaration.", + code: 80002, +}]); + +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: +`class f { + constructor() { } + bar() { + this.x = 1; + } +} +`, +}); diff --git a/tests/cases/fourslash/findAllRefsConstructorFunctions.ts b/tests/cases/fourslash/findAllRefsConstructorFunctions.ts new file mode 100644 index 0000000000000..77fa674f65bdf --- /dev/null +++ b/tests/cases/fourslash/findAllRefsConstructorFunctions.ts @@ -0,0 +1,14 @@ +/// + +// @allowJs: true + +// @Filename: /a.js +////function f() { +//// this.[|{| "isWriteAccess": true, "isDefinition": true |}x|] = 0; +////} +////f.prototype.setX = function() { +//// this.[|{| "isWriteAccess": true, "isDefinition": true |}x|] = 1; +////} +////f.prototype.useX = function() { this.[|x|]; } + +verify.singleReferenceGroup("(property) f.x: number");