From 1d773a1e09548e319bbff342258eb2a7d003d8ac 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 20f0472c1d9..2b98d80a7e8 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -20331,7 +20331,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