From 394ee31a56c40033f31a3a8461ec267463033194 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Sun, 16 Sep 2018 07:46:03 -0700 Subject: [PATCH] Fix cross-file merge of assignment decl valueDeclaration (#26918) * Fix cross-file merge of assignment decl valueDeclaration Previously mergeSymbol in the checker always updated valueDeclaration if target.valueDeclaration was an assignment declaration. The binder only updates target.valueDeclaration if it is an assignment declaration and source.valueDeclaration is *not* an assignment declaration. Now the checker behaves the same way as the binder. * Update baselines * Add a fix for #27099 Makes commonjs merge with globals when appropriate. * Add a separate jsGlobalAugmentations table Instead of trying to filter these augmentations out of the normal symbol table of commonjs modules. --- src/compiler/binder.ts | 7 ++- src/compiler/checker.ts | 5 +- src/compiler/types.ts | 2 + ...thCommonJSAssignmentDeclaration.errors.txt | 11 ++++ ...eWithCommonJSAssignmentDeclaration.symbols | 17 ++++++ ...rgeWithCommonJSAssignmentDeclaration.types | 21 +++++++ .../jsContainerMergeJsContainer.types | 8 +-- .../typeFromPropertyAssignment35.symbols | 51 +++++++++++++++++ .../typeFromPropertyAssignment35.types | 57 +++++++++++++++++++ ...lMergeWithCommonJSAssignmentDeclaration.ts | 8 +++ .../salsa/typeFromPropertyAssignment35.ts | 22 +++++++ 11 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.errors.txt create mode 100644 tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.symbols create mode 100644 tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.types create mode 100644 tests/baselines/reference/typeFromPropertyAssignment35.symbols create mode 100644 tests/baselines/reference/typeFromPropertyAssignment35.types create mode 100644 tests/cases/conformance/salsa/globalMergeWithCommonJSAssignmentDeclaration.ts create mode 100644 tests/cases/conformance/salsa/typeFromPropertyAssignment35.ts diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 81d4857ad073a..af3cf3b022dca 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -2532,7 +2532,9 @@ namespace ts { return symbol; } else { - return declareSymbol(parent ? parent.exports! : container.locals!, parent, id, flags, excludeFlags); + const table = parent ? parent.exports! : + file.jsGlobalAugmentations || (file.jsGlobalAugmentations = createSymbolTable()); + return declareSymbol(table, parent, id, flags, excludeFlags); } }); } @@ -2901,6 +2903,9 @@ namespace ts { if (local) { return local.exportSymbol || local; } + if (isSourceFile(container) && container.jsGlobalAugmentations && container.jsGlobalAugmentations.has(name)) { + return container.jsGlobalAugmentations.get(name); + } return container.symbol && container.symbol.exports && container.symbol.exports.get(name); } diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 2a8b0ce43147e..11f279d27d531 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -850,7 +850,7 @@ namespace ts { target.flags |= source.flags; if (source.valueDeclaration && (!target.valueDeclaration || - isAssignmentDeclaration(target.valueDeclaration) || + isAssignmentDeclaration(target.valueDeclaration) && !isAssignmentDeclaration(source.valueDeclaration) || isEffectiveModuleDeclaration(target.valueDeclaration) && !isEffectiveModuleDeclaration(source.valueDeclaration))) { // other kinds of value declarations take precedence over modules and assignment declarations target.valueDeclaration = source.valueDeclaration; @@ -28611,6 +28611,9 @@ namespace ts { if (!isExternalOrCommonJsModule(file)) { mergeSymbolTable(globals, file.locals!); } + if (file.jsGlobalAugmentations) { + mergeSymbolTable(globals, file.jsGlobalAugmentations); + } if (file.patternAmbientModules && file.patternAmbientModules.length) { patternAmbientModules = concatenate(patternAmbientModules, file.patternAmbientModules); } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 56ddc1de896ef..a5e06c7304bc8 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2593,6 +2593,8 @@ namespace ts { /* @internal */ externalModuleIndicator?: Node; // The first node that causes this file to be a CommonJS module /* @internal */ commonJsModuleIndicator?: Node; + // JS identifier-declarations that are intended to merge with globals + /* @internal */ jsGlobalAugmentations?: SymbolTable; /* @internal */ identifiers: Map; // Map from a string to an interned string /* @internal */ nodeCount: number; diff --git a/tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.errors.txt b/tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.errors.txt new file mode 100644 index 0000000000000..bc67e012a8618 --- /dev/null +++ b/tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.errors.txt @@ -0,0 +1,11 @@ +tests/cases/conformance/salsa/bug27099.js(1,1): error TS2322: Type '1' is not assignable to type 'string'. + + +==== tests/cases/conformance/salsa/bug27099.js (1 errors) ==== + window.name = 1; + ~~~~~~~~~~~ +!!! error TS2322: Type '1' is not assignable to type 'string'. + window.console; // should not have error: Property 'console' does not exist on type 'typeof window'. + module.exports = 'anything'; + + \ No newline at end of file diff --git a/tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.symbols b/tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.symbols new file mode 100644 index 0000000000000..dd9b8a1bab9cc --- /dev/null +++ b/tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.symbols @@ -0,0 +1,17 @@ +=== tests/cases/conformance/salsa/bug27099.js === +window.name = 1; +>window.name : Symbol(Window.name, Decl(lib.dom.d.ts, --, --)) +>window : Symbol(window, Decl(lib.dom.d.ts, --, --), Decl(bug27099.js, 0, 0)) +>name : Symbol(Window.name, Decl(lib.dom.d.ts, --, --)) + +window.console; // should not have error: Property 'console' does not exist on type 'typeof window'. +>window.console : Symbol(WindowConsole.console, Decl(lib.dom.d.ts, --, --)) +>window : Symbol(window, Decl(lib.dom.d.ts, --, --), Decl(bug27099.js, 0, 0)) +>console : Symbol(WindowConsole.console, Decl(lib.dom.d.ts, --, --)) + +module.exports = 'anything'; +>module.exports : Symbol("tests/cases/conformance/salsa/bug27099", Decl(bug27099.js, 0, 0)) +>module : Symbol(export=, Decl(bug27099.js, 1, 15)) +>exports : Symbol(export=, Decl(bug27099.js, 1, 15)) + + diff --git a/tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.types b/tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.types new file mode 100644 index 0000000000000..485c90f6100cf --- /dev/null +++ b/tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.types @@ -0,0 +1,21 @@ +=== tests/cases/conformance/salsa/bug27099.js === +window.name = 1; +>window.name = 1 : 1 +>window.name : string +>window : Window +>name : string +>1 : 1 + +window.console; // should not have error: Property 'console' does not exist on type 'typeof window'. +>window.console : Console +>window : Window +>console : Console + +module.exports = 'anything'; +>module.exports = 'anything' : string +>module.exports : string +>module : { "tests/cases/conformance/salsa/bug27099": string; } +>exports : string +>'anything' : "anything" + + diff --git a/tests/baselines/reference/jsContainerMergeJsContainer.types b/tests/baselines/reference/jsContainerMergeJsContainer.types index 75d2152c57190..73e61aded0df9 100644 --- a/tests/baselines/reference/jsContainerMergeJsContainer.types +++ b/tests/baselines/reference/jsContainerMergeJsContainer.types @@ -6,18 +6,18 @@ const a = {}; a.d = function() {}; >a.d = function() {} : { (): void; prototype: {}; } ->a.d : typeof a.d +>a.d : { (): void; prototype: {}; } >a : typeof a ->d : typeof a.d +>d : { (): void; prototype: {}; } >function() {} : { (): void; prototype: {}; } === tests/cases/conformance/salsa/b.js === a.d.prototype = {}; >a.d.prototype = {} : {} >a.d.prototype : {} ->a.d : typeof a.d +>a.d : { (): void; prototype: {}; } >a : typeof a ->d : typeof a.d +>d : { (): void; prototype: {}; } >prototype : {} >{} : {} diff --git a/tests/baselines/reference/typeFromPropertyAssignment35.symbols b/tests/baselines/reference/typeFromPropertyAssignment35.symbols new file mode 100644 index 0000000000000..d651748d1cc42 --- /dev/null +++ b/tests/baselines/reference/typeFromPropertyAssignment35.symbols @@ -0,0 +1,51 @@ +=== tests/cases/conformance/salsa/bug26877.js === +/** @param {Emu.D} x */ +function ollKorrect(x) { +>ollKorrect : Symbol(ollKorrect, Decl(bug26877.js, 0, 0)) +>x : Symbol(x, Decl(bug26877.js, 1, 20)) + + x._model +>x._model : Symbol(D._model, Decl(bug26877.js, 7, 19)) +>x : Symbol(x, Decl(bug26877.js, 1, 20)) +>_model : Symbol(D._model, Decl(bug26877.js, 7, 19)) + + const y = new Emu.D() +>y : Symbol(y, Decl(bug26877.js, 3, 9)) +>Emu.D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4)) +>Emu : Symbol(Emu, Decl(bug26877.js, 5, 1), Decl(second.js, 0, 3), Decl(second.js, 0, 12)) +>D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4)) + + const z = Emu.D._wrapperInstance; +>z : Symbol(z, Decl(bug26877.js, 4, 9)) +>Emu.D._wrapperInstance : Symbol(Emu.D._wrapperInstance, Decl(second.js, 0, 12)) +>Emu.D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4)) +>Emu : Symbol(Emu, Decl(bug26877.js, 5, 1), Decl(second.js, 0, 3), Decl(second.js, 0, 12)) +>D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4)) +>_wrapperInstance : Symbol(Emu.D._wrapperInstance, Decl(second.js, 0, 12)) +} +Emu.D = class { +>Emu.D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4)) +>Emu : Symbol(Emu, Decl(bug26877.js, 5, 1), Decl(second.js, 0, 3), Decl(second.js, 0, 12)) +>D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4)) + + constructor() { + this._model = 1 +>this._model : Symbol(D._model, Decl(bug26877.js, 7, 19)) +>this : Symbol(D, Decl(bug26877.js, 6, 7)) +>_model : Symbol(D._model, Decl(bug26877.js, 7, 19)) + } +} + +=== tests/cases/conformance/salsa/second.js === +var Emu = {} +>Emu : Symbol(Emu, Decl(bug26877.js, 5, 1), Decl(second.js, 0, 3), Decl(second.js, 0, 12)) + +/** @type {string} */ +Emu.D._wrapperInstance; +>Emu.D._wrapperInstance : Symbol(Emu.D._wrapperInstance, Decl(second.js, 0, 12)) +>Emu.D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4)) +>Emu : Symbol(Emu, Decl(bug26877.js, 5, 1), Decl(second.js, 0, 3), Decl(second.js, 0, 12)) +>D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4)) +>_wrapperInstance : Symbol(Emu.D._wrapperInstance, Decl(second.js, 0, 12)) + + diff --git a/tests/baselines/reference/typeFromPropertyAssignment35.types b/tests/baselines/reference/typeFromPropertyAssignment35.types new file mode 100644 index 0000000000000..678adb1ba5346 --- /dev/null +++ b/tests/baselines/reference/typeFromPropertyAssignment35.types @@ -0,0 +1,57 @@ +=== tests/cases/conformance/salsa/bug26877.js === +/** @param {Emu.D} x */ +function ollKorrect(x) { +>ollKorrect : (x: D) => void +>x : D + + x._model +>x._model : number +>x : D +>_model : number + + const y = new Emu.D() +>y : D +>new Emu.D() : D +>Emu.D : typeof D +>Emu : typeof Emu +>D : typeof D + + const z = Emu.D._wrapperInstance; +>z : string +>Emu.D._wrapperInstance : string +>Emu.D : typeof D +>Emu : typeof Emu +>D : typeof D +>_wrapperInstance : string +} +Emu.D = class { +>Emu.D = class { constructor() { this._model = 1 }} : typeof D +>Emu.D : typeof D +>Emu : typeof Emu +>D : typeof D +>class { constructor() { this._model = 1 }} : typeof D + + constructor() { + this._model = 1 +>this._model = 1 : 1 +>this._model : number +>this : this +>_model : number +>1 : 1 + } +} + +=== tests/cases/conformance/salsa/second.js === +var Emu = {} +>Emu : typeof Emu +>{} : {} + +/** @type {string} */ +Emu.D._wrapperInstance; +>Emu.D._wrapperInstance : string +>Emu.D : typeof D +>Emu : typeof Emu +>D : typeof D +>_wrapperInstance : string + + diff --git a/tests/cases/conformance/salsa/globalMergeWithCommonJSAssignmentDeclaration.ts b/tests/cases/conformance/salsa/globalMergeWithCommonJSAssignmentDeclaration.ts new file mode 100644 index 0000000000000..f1435eea87e04 --- /dev/null +++ b/tests/cases/conformance/salsa/globalMergeWithCommonJSAssignmentDeclaration.ts @@ -0,0 +1,8 @@ +// @noEmit: true +// @allowJs: true +// @checkJs: true +// @Filename: bug27099.js +window.name = 1; +window.console; // should not have error: Property 'console' does not exist on type 'typeof window'. +module.exports = 'anything'; + diff --git a/tests/cases/conformance/salsa/typeFromPropertyAssignment35.ts b/tests/cases/conformance/salsa/typeFromPropertyAssignment35.ts new file mode 100644 index 0000000000000..952fdd7c9c02d --- /dev/null +++ b/tests/cases/conformance/salsa/typeFromPropertyAssignment35.ts @@ -0,0 +1,22 @@ +// @noEmit: true +// @allowJs: true +// @checkJs: true + +// @Filename: bug26877.js +/** @param {Emu.D} x */ +function ollKorrect(x) { + x._model + const y = new Emu.D() + const z = Emu.D._wrapperInstance; +} +Emu.D = class { + constructor() { + this._model = 1 + } +} + +// @Filename: second.js +var Emu = {} +/** @type {string} */ +Emu.D._wrapperInstance; +