From 075a0c058be3ab4a896f90ee6cf91a187e43b558 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 27 Jan 2020 09:29:03 -0800 Subject: [PATCH 1/2] Fix getExpandoSymbol for already-expando symbols getExpandoSymbol is intended to get the symbol of the expando in a declaration like ```js var C = class { } ``` However, when this occurs in a `module.exports` assignment, alias resolution already jumps to the exported symbol -- in this case the expando symbol: ``js // @filename: first.js module.exports = class { } // @filename: other.js /* @type {import("./first")} */ var C ``` `getExpandoSymbol` is then called on `class { }` instead of `module.exports = class { }`. Previously, `getExpandoSymbol` would incorrectly treat `class { }` the same as the export assignment and get the expando ... from the expando itself. This resulted in merging the expando with itself, which causes bogus errors and could cause other problems. Now `getExpandoSymbol` checks that the expando "initializer" is different from the declaration. --- src/compiler/checker.ts | 2 +- ...ReferenceToImportOfClassExpression.symbols | 49 ++++++++++++++++ ...peReferenceToImportOfClassExpression.types | 55 ++++++++++++++++++ ...erenceToImportOfFunctionExpression.symbols | 51 ++++++++++++++++ ...eferenceToImportOfFunctionExpression.types | 58 +++++++++++++++++++ ...cTypeReferenceToImportOfClassExpression.ts | 28 +++++++++ ...peReferenceToImportOfFunctionExpression.ts | 29 ++++++++++ 7 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/jsdocTypeReferenceToImportOfClassExpression.symbols create mode 100644 tests/baselines/reference/jsdocTypeReferenceToImportOfClassExpression.types create mode 100644 tests/baselines/reference/jsdocTypeReferenceToImportOfFunctionExpression.symbols create mode 100644 tests/baselines/reference/jsdocTypeReferenceToImportOfFunctionExpression.types create mode 100644 tests/cases/conformance/jsdoc/jsdocTypeReferenceToImportOfClassExpression.ts create mode 100644 tests/cases/conformance/jsdoc/jsdocTypeReferenceToImportOfFunctionExpression.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 602a37992b4a4..bbd7a00566d18 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2767,7 +2767,7 @@ namespace ts { return undefined; } const init = isVariableDeclaration(decl) ? getDeclaredExpandoInitializer(decl) : getAssignedExpandoInitializer(decl); - if (init) { + if (init && init !== decl as unknown as Expression) { const initSymbol = getSymbolOfNode(init); if (initSymbol) { return mergeJSSymbols(initSymbol, symbol); diff --git a/tests/baselines/reference/jsdocTypeReferenceToImportOfClassExpression.symbols b/tests/baselines/reference/jsdocTypeReferenceToImportOfClassExpression.symbols new file mode 100644 index 0000000000000..e077215aeddbf --- /dev/null +++ b/tests/baselines/reference/jsdocTypeReferenceToImportOfClassExpression.symbols @@ -0,0 +1,49 @@ +=== tests/cases/conformance/jsdoc/MC.js === +const MW = require("./MW"); +>MW : Symbol(MW, Decl(MC.js, 0, 5)) +>require : Symbol(require) +>"./MW" : Symbol("tests/cases/conformance/jsdoc/MW", Decl(MW.js, 0, 0)) + +/** @typedef {number} Cictema */ + +module.exports = class MC { +>module.exports : Symbol("tests/cases/conformance/jsdoc/MC", Decl(MC.js, 0, 0)) +>module : Symbol(export=, Decl(MC.js, 0, 27)) +>exports : Symbol(export=, Decl(MC.js, 0, 27)) +>MC : Symbol(MC, Decl(MC.js, 4, 16)) + + watch() { +>watch : Symbol(MC.watch, Decl(MC.js, 4, 27)) + + return new MW(this); +>MW : Symbol(MW, Decl(MC.js, 0, 5)) +>this : Symbol(MC, Decl(MC.js, 4, 16)) + } +}; + +=== tests/cases/conformance/jsdoc/MW.js === +/** @typedef {import("./MC")} MC */ + +class MW { +>MW : Symbol(MW, Decl(MW.js, 0, 0)) + + /** + * @param {MC} compiler the compiler + */ + constructor(compiler) { +>compiler : Symbol(compiler, Decl(MW.js, 6, 14)) + + this.compiler = compiler; +>this.compiler : Symbol(MW.compiler, Decl(MW.js, 6, 25)) +>this : Symbol(MW, Decl(MW.js, 0, 0)) +>compiler : Symbol(MW.compiler, Decl(MW.js, 6, 25)) +>compiler : Symbol(compiler, Decl(MW.js, 6, 14)) + } +} + +module.exports = MW; +>module.exports : Symbol("tests/cases/conformance/jsdoc/MW", Decl(MW.js, 0, 0)) +>module : Symbol(export=, Decl(MW.js, 9, 1)) +>exports : Symbol(export=, Decl(MW.js, 9, 1)) +>MW : Symbol(MW, Decl(MW.js, 0, 0)) + diff --git a/tests/baselines/reference/jsdocTypeReferenceToImportOfClassExpression.types b/tests/baselines/reference/jsdocTypeReferenceToImportOfClassExpression.types new file mode 100644 index 0000000000000..98f18bc8fb086 --- /dev/null +++ b/tests/baselines/reference/jsdocTypeReferenceToImportOfClassExpression.types @@ -0,0 +1,55 @@ +=== tests/cases/conformance/jsdoc/MC.js === +const MW = require("./MW"); +>MW : typeof import("tests/cases/conformance/jsdoc/MW") +>require("./MW") : typeof import("tests/cases/conformance/jsdoc/MW") +>require : any +>"./MW" : "./MW" + +/** @typedef {number} Cictema */ + +module.exports = class MC { +>module.exports = class MC { watch() { return new MW(this); }} : typeof import("tests/cases/conformance/jsdoc/MC") +>module.exports : typeof import("tests/cases/conformance/jsdoc/MC") +>module : { "\"tests/cases/conformance/jsdoc/MC\"": typeof import("tests/cases/conformance/jsdoc/MC"); } +>exports : typeof import("tests/cases/conformance/jsdoc/MC") +>class MC { watch() { return new MW(this); }} : typeof import("tests/cases/conformance/jsdoc/MC") +>MC : typeof import("tests/cases/conformance/jsdoc/MC") + + watch() { +>watch : () => import("tests/cases/conformance/jsdoc/MW") + + return new MW(this); +>new MW(this) : import("tests/cases/conformance/jsdoc/MW") +>MW : typeof import("tests/cases/conformance/jsdoc/MW") +>this : this + } +}; + +=== tests/cases/conformance/jsdoc/MW.js === +/** @typedef {import("./MC")} MC */ + +class MW { +>MW : MW + + /** + * @param {MC} compiler the compiler + */ + constructor(compiler) { +>compiler : import("tests/cases/conformance/jsdoc/MC") + + this.compiler = compiler; +>this.compiler = compiler : import("tests/cases/conformance/jsdoc/MC") +>this.compiler : import("tests/cases/conformance/jsdoc/MC") +>this : this +>compiler : import("tests/cases/conformance/jsdoc/MC") +>compiler : import("tests/cases/conformance/jsdoc/MC") + } +} + +module.exports = MW; +>module.exports = MW : typeof MW +>module.exports : typeof MW +>module : { "\"tests/cases/conformance/jsdoc/MW\"": typeof MW; } +>exports : typeof MW +>MW : typeof MW + diff --git a/tests/baselines/reference/jsdocTypeReferenceToImportOfFunctionExpression.symbols b/tests/baselines/reference/jsdocTypeReferenceToImportOfFunctionExpression.symbols new file mode 100644 index 0000000000000..d34dcbdaf10af --- /dev/null +++ b/tests/baselines/reference/jsdocTypeReferenceToImportOfFunctionExpression.symbols @@ -0,0 +1,51 @@ +=== tests/cases/conformance/jsdoc/MC.js === +const MW = require("./MW"); +>MW : Symbol(MW, Decl(MC.js, 0, 5)) +>require : Symbol(require) +>"./MW" : Symbol("tests/cases/conformance/jsdoc/MW", Decl(MW.js, 0, 0)) + +/** @typedef {number} Meyerhauser */ + +/** @class */ +module.exports = function MC() { +>module.exports : Symbol("tests/cases/conformance/jsdoc/MC", Decl(MC.js, 0, 0)) +>module : Symbol(export=, Decl(MC.js, 0, 27)) +>exports : Symbol(export=, Decl(MC.js, 0, 27)) +>MC : Symbol(MC, Decl(MC.js, 5, 16)) + + /** @type {any} */ + var x = {} +>x : Symbol(x, Decl(MC.js, 7, 7)) + + return new MW(x); +>MW : Symbol(MW, Decl(MC.js, 0, 5)) +>x : Symbol(x, Decl(MC.js, 7, 7)) + +}; + +=== tests/cases/conformance/jsdoc/MW.js === +/** @typedef {import("./MC")} MC */ + +class MW { +>MW : Symbol(MW, Decl(MW.js, 0, 0)) + + /** + * @param {MC} compiler the compiler + */ + constructor(compiler) { +>compiler : Symbol(compiler, Decl(MW.js, 6, 14)) + + this.compiler = compiler; +>this.compiler : Symbol(MW.compiler, Decl(MW.js, 6, 25)) +>this : Symbol(MW, Decl(MW.js, 0, 0)) +>compiler : Symbol(MW.compiler, Decl(MW.js, 6, 25)) +>compiler : Symbol(compiler, Decl(MW.js, 6, 14)) + } +} + +module.exports = MW; +>module.exports : Symbol("tests/cases/conformance/jsdoc/MW", Decl(MW.js, 0, 0)) +>module : Symbol(export=, Decl(MW.js, 9, 1)) +>exports : Symbol(export=, Decl(MW.js, 9, 1)) +>MW : Symbol(MW, Decl(MW.js, 0, 0)) + diff --git a/tests/baselines/reference/jsdocTypeReferenceToImportOfFunctionExpression.types b/tests/baselines/reference/jsdocTypeReferenceToImportOfFunctionExpression.types new file mode 100644 index 0000000000000..1747fecbf6c38 --- /dev/null +++ b/tests/baselines/reference/jsdocTypeReferenceToImportOfFunctionExpression.types @@ -0,0 +1,58 @@ +=== tests/cases/conformance/jsdoc/MC.js === +const MW = require("./MW"); +>MW : typeof import("tests/cases/conformance/jsdoc/MW") +>require("./MW") : typeof import("tests/cases/conformance/jsdoc/MW") +>require : any +>"./MW" : "./MW" + +/** @typedef {number} Meyerhauser */ + +/** @class */ +module.exports = function MC() { +>module.exports = function MC() { /** @type {any} */ var x = {} return new MW(x);} : typeof MC +>module.exports : typeof MC +>module : { "\"tests/cases/conformance/jsdoc/MC\"": typeof MC; } +>exports : typeof MC +>function MC() { /** @type {any} */ var x = {} return new MW(x);} : typeof MC +>MC : typeof MC + + /** @type {any} */ + var x = {} +>x : any +>{} : {} + + return new MW(x); +>new MW(x) : import("tests/cases/conformance/jsdoc/MW") +>MW : typeof import("tests/cases/conformance/jsdoc/MW") +>x : any + +}; + +=== tests/cases/conformance/jsdoc/MW.js === +/** @typedef {import("./MC")} MC */ + +class MW { +>MW : MW + + /** + * @param {MC} compiler the compiler + */ + constructor(compiler) { +>compiler : typeof MC + + this.compiler = compiler; +>this.compiler = compiler : typeof MC +>this.compiler : typeof MC +>this : this +>compiler : typeof MC +>compiler : typeof MC + } +} + +module.exports = MW; +>module.exports = MW : typeof MW +>module.exports : typeof MW +>module : { "\"tests/cases/conformance/jsdoc/MW\"": typeof MW; } +>exports : typeof MW +>MW : typeof MW + diff --git a/tests/cases/conformance/jsdoc/jsdocTypeReferenceToImportOfClassExpression.ts b/tests/cases/conformance/jsdoc/jsdocTypeReferenceToImportOfClassExpression.ts new file mode 100644 index 0000000000000..b1efa06be66ac --- /dev/null +++ b/tests/cases/conformance/jsdoc/jsdocTypeReferenceToImportOfClassExpression.ts @@ -0,0 +1,28 @@ +// @noEmit: true +// @allowjs: true +// @checkjs: true + +// @Filename: MW.js +/** @typedef {import("./MC")} MC */ + +class MW { + /** + * @param {MC} compiler the compiler + */ + constructor(compiler) { + this.compiler = compiler; + } +} + +module.exports = MW; + +// @Filename: MC.js +const MW = require("./MW"); + +/** @typedef {number} Cictema */ + +module.exports = class MC { + watch() { + return new MW(this); + } +}; diff --git a/tests/cases/conformance/jsdoc/jsdocTypeReferenceToImportOfFunctionExpression.ts b/tests/cases/conformance/jsdoc/jsdocTypeReferenceToImportOfFunctionExpression.ts new file mode 100644 index 0000000000000..f38a7425fda07 --- /dev/null +++ b/tests/cases/conformance/jsdoc/jsdocTypeReferenceToImportOfFunctionExpression.ts @@ -0,0 +1,29 @@ +// @noEmit: true +// @allowjs: true +// @checkjs: true + +// @Filename: MW.js +/** @typedef {import("./MC")} MC */ + +class MW { + /** + * @param {MC} compiler the compiler + */ + constructor(compiler) { + this.compiler = compiler; + } +} + +module.exports = MW; + +// @Filename: MC.js +const MW = require("./MW"); + +/** @typedef {number} Meyerhauser */ + +/** @class */ +module.exports = function MC() { + /** @type {any} */ + var x = {} + return new MW(x); +}; From 7d8588e43f23d56686291fad8e5bc6b869340209 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 27 Jan 2020 09:39:40 -0800 Subject: [PATCH 2/2] Better check for getExpandoSymbol of expando Check the declaration directly instead of the initialiser. --- src/compiler/checker.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index bbd7a00566d18..1a0fb3c60cdc6 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2763,11 +2763,11 @@ namespace ts { */ function getExpandoSymbol(symbol: Symbol): Symbol | undefined { const decl = symbol.valueDeclaration; - if (!decl || !isInJSFile(decl) || symbol.flags & SymbolFlags.TypeAlias) { + if (!decl || !isInJSFile(decl) || symbol.flags & SymbolFlags.TypeAlias || getExpandoInitializer(decl, /*isPrototypeAssignment*/ false)) { return undefined; } const init = isVariableDeclaration(decl) ? getDeclaredExpandoInitializer(decl) : getAssignedExpandoInitializer(decl); - if (init && init !== decl as unknown as Expression) { + if (init) { const initSymbol = getSymbolOfNode(init); if (initSymbol) { return mergeJSSymbols(initSymbol, symbol);