diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index b3f02dc24b9a1..d8017d601adb0 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -299,8 +299,10 @@ namespace ts { const name = isDefaultExport && parent ? "default" : getDeclarationName(node); let symbol: Symbol; - if (name !== undefined) { - + if (name === undefined) { + symbol = createSymbol(SymbolFlags.None, "__missing"); + } + else { // Check and see if the symbol table already has a symbol with this name. If not, // create a new symbol with this name and add it to the table. Note that we don't // give the new symbol any flags *yet*. This ensures that it will not conflict @@ -312,6 +314,11 @@ namespace ts { // declaration we have for this symbol, and then create a new symbol for this // declaration. // + // Note that when properties declared in Javascript constructors + // (marked by isReplaceableByMethod) conflict with another symbol, the property loses. + // Always. This allows the common Javascript pattern of overwriting a prototype method + // with an bound instance method of the same type: `this.method = this.method.bind(this)` + // // If we created a new symbol, either because we didn't have a symbol with this name // in the symbol table, or we conflicted with an existing symbol, then just add this // node as the sole declaration of the new symbol. @@ -326,33 +333,37 @@ namespace ts { } if (symbol.flags & excludes) { - if (node.name) { - node.name.parent = node; + if (symbol.isReplaceableByMethod) { + // Javascript constructor-declared symbols can be discarded in favor of + // prototype symbols like methods. + symbol = symbolTable[name] = createSymbol(SymbolFlags.None, name); } + else { + if (node.name) { + node.name.parent = node; + } - // Report errors every position with duplicate declaration - // Report errors on previous encountered declarations - let message = symbol.flags & SymbolFlags.BlockScopedVariable - ? Diagnostics.Cannot_redeclare_block_scoped_variable_0 - : Diagnostics.Duplicate_identifier_0; + // Report errors every position with duplicate declaration + // Report errors on previous encountered declarations + let message = symbol.flags & SymbolFlags.BlockScopedVariable + ? Diagnostics.Cannot_redeclare_block_scoped_variable_0 + : Diagnostics.Duplicate_identifier_0; - forEach(symbol.declarations, declaration => { - if (declaration.flags & NodeFlags.Default) { - message = Diagnostics.A_module_cannot_have_multiple_default_exports; - } - }); + forEach(symbol.declarations, declaration => { + if (declaration.flags & NodeFlags.Default) { + message = Diagnostics.A_module_cannot_have_multiple_default_exports; + } + }); - forEach(symbol.declarations, declaration => { - file.bindDiagnostics.push(createDiagnosticForNode(declaration.name || declaration, message, getDisplayName(declaration))); - }); - file.bindDiagnostics.push(createDiagnosticForNode(node.name || node, message, getDisplayName(node))); + forEach(symbol.declarations, declaration => { + file.bindDiagnostics.push(createDiagnosticForNode(declaration.name || declaration, message, getDisplayName(declaration))); + }); + file.bindDiagnostics.push(createDiagnosticForNode(node.name || node, message, getDisplayName(node))); - symbol = createSymbol(SymbolFlags.None, name); + symbol = createSymbol(SymbolFlags.None, name); + } } } - else { - symbol = createSymbol(SymbolFlags.None, "__missing"); - } addDeclarationToSymbol(symbol, node, includes); symbol.parent = parent; @@ -1966,31 +1977,25 @@ namespace ts { } function bindThisPropertyAssignment(node: BinaryExpression) { - // Declare a 'member' in case it turns out the container was an ES5 class or ES6 constructor - let assignee: Node; + Debug.assert(isInJavaScriptFile(node)); + // Declare a 'member' if the container is an ES5 class or ES6 constructor if (container.kind === SyntaxKind.FunctionDeclaration || container.kind === SyntaxKind.FunctionExpression) { - assignee = container; + container.symbol.members = container.symbol.members || createMap(); + // 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); } else if (container.kind === SyntaxKind.Constructor) { - if (isInJavaScriptFile(node)) { - // this.foo assignment in a JavaScript class - // Bind this property to the containing class - const saveContainer = container; - container = container.parent; - bindPropertyOrMethodOrAccessor(node, SymbolFlags.Property, SymbolFlags.None); - container = saveContainer; - return; + // this.foo assignment in a JavaScript class + // Bind this property to the containing class + const saveContainer = container; + container = container.parent; + const symbol = bindPropertyOrMethodOrAccessor(node, SymbolFlags.Property, SymbolFlags.None); + if (symbol) { + // constructor-declared symbols can be overwritten by subsequent method declarations + (symbol as Symbol).isReplaceableByMethod = true; } - else { - assignee = container.parent; - } - } - else { - return; + container = saveContainer; } - assignee.symbol.members = assignee.symbol.members || createMap(); - // It's acceptable for multiple 'this' assignments of the same identifier to occur - declareSymbol(assignee.symbol.members, assignee.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property); } function bindPrototypePropertyAssignment(node: BinaryExpression) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 1e3e78ea76e1c..b5e63ea6e077b 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2157,6 +2157,7 @@ namespace ts { /* @internal */ exportSymbol?: Symbol; // Exported symbol associated with this symbol /* @internal */ constEnumOnlyModule?: boolean; // True if module contains only const enums or other modules with only const enums /* @internal */ isReferenced?: boolean; // True if the symbol is referenced elsewhere + /* @internal */ isReplaceableByMethod?: boolean; // Can this Javascript class property be replaced by a method symbol? /* @internal */ isAssigned?: boolean; // True if the symbol is a parameter with assignments } diff --git a/tests/baselines/reference/multipleDeclarations.js b/tests/baselines/reference/multipleDeclarations.js index 5f5ac29e90964..7fb7e0bca026f 100644 --- a/tests/baselines/reference/multipleDeclarations.js +++ b/tests/baselines/reference/multipleDeclarations.js @@ -1,11 +1,38 @@ //// [input.js] - function C() { this.m = null; } C.prototype.m = function() { this.nothing(); -}; +} +class X { + constructor() { + this.m = this.m.bind(this); + this.mistake = 'frankly, complete nonsense'; + } + m() { + } + mistake() { + } +} +let x = new X(); +X.prototype.mistake = false; +x.m(); +x.mistake; +class Y { + mistake() { + } + m() { + } + constructor() { + this.m = this.m.bind(this); + this.mistake = 'even more nonsense'; + } +} +Y.prototype.mistake = true; +let y = new Y(); +y.m(); +y.mistake(); //// [output.js] @@ -15,3 +42,33 @@ function C() { C.prototype.m = function () { this.nothing(); }; +var X = (function () { + function X() { + this.m = this.m.bind(this); + this.mistake = 'frankly, complete nonsense'; + } + X.prototype.m = function () { + }; + X.prototype.mistake = function () { + }; + return X; +}()); +var x = new X(); +X.prototype.mistake = false; +x.m(); +x.mistake; +var Y = (function () { + function Y() { + this.m = this.m.bind(this); + this.mistake = 'even more nonsense'; + } + Y.prototype.mistake = function () { + }; + Y.prototype.m = function () { + }; + return Y; +}()); +Y.prototype.mistake = true; +var y = new Y(); +y.m(); +y.mistake(); diff --git a/tests/baselines/reference/multipleDeclarations.symbols b/tests/baselines/reference/multipleDeclarations.symbols index a256943dd506c..9e49c2fc00cc4 100644 --- a/tests/baselines/reference/multipleDeclarations.symbols +++ b/tests/baselines/reference/multipleDeclarations.symbols @@ -1,19 +1,106 @@ === tests/cases/conformance/salsa/input.js === - function C() { >C : Symbol(C, Decl(input.js, 0, 0)) this.m = null; ->m : Symbol(C.m, Decl(input.js, 1, 14), Decl(input.js, 3, 1)) +>m : Symbol(C.m, Decl(input.js, 0, 14), Decl(input.js, 2, 1)) } C.prototype.m = function() { ->C.prototype : Symbol(C.m, Decl(input.js, 1, 14), Decl(input.js, 3, 1)) +>C.prototype : Symbol(C.m, Decl(input.js, 0, 14), Decl(input.js, 2, 1)) >C : Symbol(C, Decl(input.js, 0, 0)) >prototype : Symbol(Function.prototype, Decl(lib.d.ts, --, --)) ->m : Symbol(C.m, Decl(input.js, 1, 14), Decl(input.js, 3, 1)) +>m : Symbol(C.m, Decl(input.js, 0, 14), Decl(input.js, 2, 1)) this.nothing(); >this : Symbol(C, Decl(input.js, 0, 0)) +} +class X { +>X : Symbol(X, Decl(input.js, 5, 1)) + + constructor() { + this.m = this.m.bind(this); +>this.m : Symbol(X.m, Decl(input.js, 10, 5)) +>this : Symbol(X, Decl(input.js, 5, 1)) +>m : Symbol(X.m, Decl(input.js, 7, 19)) +>this.m.bind : Symbol(Function.bind, Decl(lib.d.ts, --, --)) +>this.m : Symbol(X.m, Decl(input.js, 10, 5)) +>this : Symbol(X, Decl(input.js, 5, 1)) +>m : Symbol(X.m, Decl(input.js, 10, 5)) +>bind : Symbol(Function.bind, Decl(lib.d.ts, --, --)) +>this : Symbol(X, Decl(input.js, 5, 1)) + + this.mistake = 'frankly, complete nonsense'; +>this.mistake : Symbol(X.mistake, Decl(input.js, 12, 5)) +>this : Symbol(X, Decl(input.js, 5, 1)) +>mistake : Symbol(X.mistake, Decl(input.js, 8, 35)) + } + m() { +>m : Symbol(X.m, Decl(input.js, 10, 5)) + } + mistake() { +>mistake : Symbol(X.mistake, Decl(input.js, 12, 5)) + } +} +let x = new X(); +>x : Symbol(x, Decl(input.js, 16, 3)) +>X : Symbol(X, Decl(input.js, 5, 1)) + +X.prototype.mistake = false; +>X.prototype.mistake : Symbol(X.mistake, Decl(input.js, 12, 5)) +>X : Symbol(X, Decl(input.js, 5, 1)) +>prototype : Symbol(X.prototype) + +x.m(); +>x.m : Symbol(X.m, Decl(input.js, 10, 5)) +>x : Symbol(x, Decl(input.js, 16, 3)) +>m : Symbol(X.m, Decl(input.js, 10, 5)) + +x.mistake; +>x.mistake : Symbol(X.mistake, Decl(input.js, 12, 5)) +>x : Symbol(x, Decl(input.js, 16, 3)) +>mistake : Symbol(X.mistake, Decl(input.js, 12, 5)) + +class Y { +>Y : Symbol(Y, Decl(input.js, 19, 10)) + + mistake() { +>mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35)) + } + m() { +>m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19)) + } + constructor() { + this.m = this.m.bind(this); +>this.m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19)) +>this : Symbol(Y, Decl(input.js, 19, 10)) +>m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19)) +>this.m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19)) +>this : Symbol(Y, Decl(input.js, 19, 10)) +>m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19)) +>this : Symbol(Y, Decl(input.js, 19, 10)) + + this.mistake = 'even more nonsense'; +>this.mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35)) +>this : Symbol(Y, Decl(input.js, 19, 10)) +>mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35)) + } +} +Y.prototype.mistake = true; +>Y.prototype.mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35)) +>Y : Symbol(Y, Decl(input.js, 19, 10)) +>prototype : Symbol(Y.prototype) + +let y = new Y(); +>y : Symbol(y, Decl(input.js, 31, 3)) +>Y : Symbol(Y, Decl(input.js, 19, 10)) + +y.m(); +>y.m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19)) +>y : Symbol(y, Decl(input.js, 31, 3)) +>m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19)) -}; +y.mistake(); +>y.mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35)) +>y : Symbol(y, Decl(input.js, 31, 3)) +>mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35)) diff --git a/tests/baselines/reference/multipleDeclarations.types b/tests/baselines/reference/multipleDeclarations.types index 7c0a3de70c9c6..900d03195d486 100644 --- a/tests/baselines/reference/multipleDeclarations.types +++ b/tests/baselines/reference/multipleDeclarations.types @@ -1,5 +1,4 @@ === tests/cases/conformance/salsa/input.js === - function C() { >C : () => void @@ -24,6 +23,117 @@ C.prototype.m = function() { >this.nothing : any >this : { m: () => void; } >nothing : any +} +class X { +>X : X + + constructor() { + this.m = this.m.bind(this); +>this.m = this.m.bind(this) : any +>this.m : () => void +>this : this +>m : () => void +>this.m.bind(this) : any +>this.m.bind : (this: Function, thisArg: any, ...argArray: any[]) => any +>this.m : () => void +>this : this +>m : () => void +>bind : (this: Function, thisArg: any, ...argArray: any[]) => any +>this : this + + this.mistake = 'frankly, complete nonsense'; +>this.mistake = 'frankly, complete nonsense' : string +>this.mistake : () => void +>this : this +>mistake : () => void +>'frankly, complete nonsense' : string + } + m() { +>m : () => void + } + mistake() { +>mistake : () => void + } +} +let x = new X(); +>x : X +>new X() : X +>X : typeof X + +X.prototype.mistake = false; +>X.prototype.mistake = false : boolean +>X.prototype.mistake : () => void +>X.prototype : X +>X : typeof X +>prototype : X +>mistake : () => void +>false : boolean + +x.m(); +>x.m() : void +>x.m : () => void +>x : X +>m : () => void + +x.mistake; +>x.mistake : () => void +>x : X +>mistake : () => void + +class Y { +>Y : Y + + mistake() { +>mistake : any + } + m() { +>m : any + } + constructor() { + this.m = this.m.bind(this); +>this.m = this.m.bind(this) : any +>this.m : any +>this : this +>m : any +>this.m.bind(this) : any +>this.m.bind : any +>this.m : any +>this : this +>m : any +>bind : any +>this : this + + this.mistake = 'even more nonsense'; +>this.mistake = 'even more nonsense' : string +>this.mistake : any +>this : this +>mistake : any +>'even more nonsense' : string + } +} +Y.prototype.mistake = true; +>Y.prototype.mistake = true : boolean +>Y.prototype.mistake : any +>Y.prototype : Y +>Y : typeof Y +>prototype : Y +>mistake : any +>true : boolean + +let y = new Y(); +>y : Y +>new Y() : Y +>Y : typeof Y + +y.m(); +>y.m() : any +>y.m : any +>y : Y +>m : any -}; +y.mistake(); +>y.mistake() : any +>y.mistake : any +>y : Y +>mistake : any diff --git a/tests/cases/conformance/salsa/multipleDeclarations.ts b/tests/cases/conformance/salsa/multipleDeclarations.ts index 6899be2ec89a2..ba2e14e11e934 100644 --- a/tests/cases/conformance/salsa/multipleDeclarations.ts +++ b/tests/cases/conformance/salsa/multipleDeclarations.ts @@ -1,10 +1,37 @@ // @filename: input.js // @out: output.js // @allowJs: true - function C() { this.m = null; } C.prototype.m = function() { this.nothing(); -}; +} +class X { + constructor() { + this.m = this.m.bind(this); + this.mistake = 'frankly, complete nonsense'; + } + m() { + } + mistake() { + } +} +let x = new X(); +X.prototype.mistake = false; +x.m(); +x.mistake; +class Y { + mistake() { + } + m() { + } + constructor() { + this.m = this.m.bind(this); + this.mistake = 'even more nonsense'; + } +} +Y.prototype.mistake = true; +let y = new Y(); +y.m(); +y.mistake();