From 8a4b6e03abd87817a4161baabd01400bccb8c33f Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 8 Oct 2018 12:56:03 -0700 Subject: [PATCH] Fix class/constructor-function merge (#27366) The check for prototype assignment on constructor functions assumes that the prototype property, if present, comes from an assignment declaration, such as: ```js SomeClass.prototype = { /* methods go here */ } ``` In this case, however, when class SomeClass and var SomeClass merge (because this is allowed), prototype is the synthetic property from class SomeClass, which has no valueDeclaration. The fix is to check that prototype has a valueDeclaration before checking whether the valueDeclaration is in fact a prototype-assignment declaration. --- src/compiler/checker.ts | 2 +- .../constructorFunctionMergeWithClass.symbols | 21 ++++++++++++++ .../constructorFunctionMergeWithClass.types | 29 +++++++++++++++++++ .../constructorFunctionMergeWithClass.ts | 14 +++++++++ 4 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/constructorFunctionMergeWithClass.symbols create mode 100644 tests/baselines/reference/constructorFunctionMergeWithClass.types create mode 100644 tests/cases/conformance/salsa/constructorFunctionMergeWithClass.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index ae44d873861..18ec5bf9d6d 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -20459,7 +20459,7 @@ namespace ts { isBinaryExpression(decl.parent) && getSymbolOfNode(decl.parent.left) || isVariableDeclaration(decl.parent) && getSymbolOfNode(decl.parent)); const prototype = assignmentSymbol && assignmentSymbol.exports && assignmentSymbol.exports.get("prototype" as __String); - const init = prototype && getAssignedJSPrototype(prototype.valueDeclaration); + const init = prototype && prototype.valueDeclaration && getAssignedJSPrototype(prototype.valueDeclaration); return init ? checkExpression(init) : undefined; } diff --git a/tests/baselines/reference/constructorFunctionMergeWithClass.symbols b/tests/baselines/reference/constructorFunctionMergeWithClass.symbols new file mode 100644 index 00000000000..ecc45e94054 --- /dev/null +++ b/tests/baselines/reference/constructorFunctionMergeWithClass.symbols @@ -0,0 +1,21 @@ +=== tests/cases/conformance/salsa/file1.js === +var SomeClass = function () { +>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19)) + + this.otherProp = 0; +>otherProp : Symbol(SomeClass.otherProp, Decl(file1.js, 0, 29)) + +}; + +new SomeClass(); +>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19)) + +=== tests/cases/conformance/salsa/file2.js === +class SomeClass { } +>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19)) + +SomeClass.prop = 0 +>SomeClass.prop : Symbol(SomeClass.prop, Decl(file2.js, 0, 19)) +>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19)) +>prop : Symbol(SomeClass.prop, Decl(file2.js, 0, 19)) + diff --git a/tests/baselines/reference/constructorFunctionMergeWithClass.types b/tests/baselines/reference/constructorFunctionMergeWithClass.types new file mode 100644 index 00000000000..7bed55aacce --- /dev/null +++ b/tests/baselines/reference/constructorFunctionMergeWithClass.types @@ -0,0 +1,29 @@ +=== tests/cases/conformance/salsa/file1.js === +var SomeClass = function () { +>SomeClass : typeof SomeClass +>function () { this.otherProp = 0;} : typeof SomeClass + + this.otherProp = 0; +>this.otherProp = 0 : 0 +>this.otherProp : any +>this : any +>otherProp : any +>0 : 0 + +}; + +new SomeClass(); +>new SomeClass() : SomeClass +>SomeClass : typeof SomeClass + +=== tests/cases/conformance/salsa/file2.js === +class SomeClass { } +>SomeClass : SomeClass + +SomeClass.prop = 0 +>SomeClass.prop = 0 : 0 +>SomeClass.prop : number +>SomeClass : typeof SomeClass +>prop : number +>0 : 0 + diff --git a/tests/cases/conformance/salsa/constructorFunctionMergeWithClass.ts b/tests/cases/conformance/salsa/constructorFunctionMergeWithClass.ts new file mode 100644 index 00000000000..b6a0a50f029 --- /dev/null +++ b/tests/cases/conformance/salsa/constructorFunctionMergeWithClass.ts @@ -0,0 +1,14 @@ +// @allowJs: true +// @noEmit: true +// @checkJs: true + +// @Filename: file1.js +var SomeClass = function () { + this.otherProp = 0; +}; + +new SomeClass(); + +// @Filename: file2.js +class SomeClass { } +SomeClass.prop = 0