Skip to content

Commit 394ee31

Browse files
sandersnsandersn
authored
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.
1 parent 70ce7ab commit 394ee31

11 files changed

+203
-6
lines changed

‎src/compiler/binder.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2532,7 +2532,9 @@ namespace ts {
25322532
return symbol;
25332533
}
25342534
else {
2535-
return declareSymbol(parent ? parent.exports! : container.locals!, parent, id, flags, excludeFlags);
2535+
const table = parent ? parent.exports! :
2536+
file.jsGlobalAugmentations || (file.jsGlobalAugmentations = createSymbolTable());
2537+
return declareSymbol(table, parent, id, flags, excludeFlags);
25362538
}
25372539
});
25382540
}
@@ -2901,6 +2903,9 @@ namespace ts {
29012903
if (local) {
29022904
return local.exportSymbol || local;
29032905
}
2906+
if (isSourceFile(container) && container.jsGlobalAugmentations && container.jsGlobalAugmentations.has(name)) {
2907+
return container.jsGlobalAugmentations.get(name);
2908+
}
29042909
return container.symbol && container.symbol.exports && container.symbol.exports.get(name);
29052910
}
29062911

‎src/compiler/checker.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,7 @@ namespace ts {
850850
target.flags |= source.flags;
851851
if (source.valueDeclaration &&
852852
(!target.valueDeclaration ||
853-
isAssignmentDeclaration(target.valueDeclaration) ||
853+
isAssignmentDeclaration(target.valueDeclaration) && !isAssignmentDeclaration(source.valueDeclaration) ||
854854
isEffectiveModuleDeclaration(target.valueDeclaration) && !isEffectiveModuleDeclaration(source.valueDeclaration))) {
855855
// other kinds of value declarations take precedence over modules and assignment declarations
856856
target.valueDeclaration = source.valueDeclaration;
@@ -28611,6 +28611,9 @@ namespace ts {
2861128611
if (!isExternalOrCommonJsModule(file)) {
2861228612
mergeSymbolTable(globals, file.locals!);
2861328613
}
28614+
if (file.jsGlobalAugmentations) {
28615+
mergeSymbolTable(globals, file.jsGlobalAugmentations);
28616+
}
2861428617
if (file.patternAmbientModules && file.patternAmbientModules.length) {
2861528618
patternAmbientModules = concatenate(patternAmbientModules, file.patternAmbientModules);
2861628619
}

‎src/compiler/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2593,6 +2593,8 @@ namespace ts {
25932593
/* @internal */ externalModuleIndicator?: Node;
25942594
// The first node that causes this file to be a CommonJS module
25952595
/* @internal */ commonJsModuleIndicator?: Node;
2596+
// JS identifier-declarations that are intended to merge with globals
2597+
/* @internal */ jsGlobalAugmentations?: SymbolTable;
25962598

25972599
/* @internal */ identifiers: Map<string>; // Map from a string to an interned string
25982600
/* @internal */ nodeCount: number;

‎tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.errors.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
tests/cases/conformance/salsa/bug27099.js(1,1): error TS2322: Type '1' is not assignable to type 'string'.
2+
3+
4+
==== tests/cases/conformance/salsa/bug27099.js (1 errors) ====
5+
window.name = 1;
6+
~~~~~~~~~~~
7+
!!! error TS2322: Type '1' is not assignable to type 'string'.
8+
window.console; // should not have error: Property 'console' does not exist on type 'typeof window'.
9+
module.exports = 'anything';
10+
11+

‎tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.symbols

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
=== tests/cases/conformance/salsa/bug27099.js ===
2+
window.name = 1;
3+
>window.name : Symbol(Window.name, Decl(lib.dom.d.ts, --, --))
4+
>window : Symbol(window, Decl(lib.dom.d.ts, --, --), Decl(bug27099.js, 0, 0))
5+
>name : Symbol(Window.name, Decl(lib.dom.d.ts, --, --))
6+
7+
window.console; // should not have error: Property 'console' does not exist on type 'typeof window'.
8+
>window.console : Symbol(WindowConsole.console, Decl(lib.dom.d.ts, --, --))
9+
>window : Symbol(window, Decl(lib.dom.d.ts, --, --), Decl(bug27099.js, 0, 0))
10+
>console : Symbol(WindowConsole.console, Decl(lib.dom.d.ts, --, --))
11+
12+
module.exports = 'anything';
13+
>module.exports : Symbol("tests/cases/conformance/salsa/bug27099", Decl(bug27099.js, 0, 0))
14+
>module : Symbol(export=, Decl(bug27099.js, 1, 15))
15+
>exports : Symbol(export=, Decl(bug27099.js, 1, 15))
16+
17+

‎tests/baselines/reference/globalMergeWithCommonJSAssignmentDeclaration.types

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
=== tests/cases/conformance/salsa/bug27099.js ===
2+
window.name = 1;
3+
>window.name = 1 : 1
4+
>window.name : string
5+
>window : Window
6+
>name : string
7+
>1 : 1
8+
9+
window.console; // should not have error: Property 'console' does not exist on type 'typeof window'.
10+
>window.console : Console
11+
>window : Window
12+
>console : Console
13+
14+
module.exports = 'anything';
15+
>module.exports = 'anything' : string
16+
>module.exports : string
17+
>module : { "tests/cases/conformance/salsa/bug27099": string; }
18+
>exports : string
19+
>'anything' : "anything"
20+
21+

‎tests/baselines/reference/jsContainerMergeJsContainer.types

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,18 @@ const a = {};
66

77
a.d = function() {};
88
>a.d = function() {} : { (): void; prototype: {}; }
9-
>a.d : typeof a.d
9+
>a.d : { (): void; prototype: {}; }
1010
>a : typeof a
11-
>d : typeof a.d
11+
>d : { (): void; prototype: {}; }
1212
>function() {} : { (): void; prototype: {}; }
1313

1414
=== tests/cases/conformance/salsa/b.js ===
1515
a.d.prototype = {};
1616
>a.d.prototype = {} : {}
1717
>a.d.prototype : {}
18-
>a.d : typeof a.d
18+
>a.d : { (): void; prototype: {}; }
1919
>a : typeof a
20-
>d : typeof a.d
20+
>d : { (): void; prototype: {}; }
2121
>prototype : {}
2222
>{} : {}
2323

‎tests/baselines/reference/typeFromPropertyAssignment35.symbols

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
=== tests/cases/conformance/salsa/bug26877.js ===
2+
/** @param {Emu.D} x */
3+
function ollKorrect(x) {
4+
>ollKorrect : Symbol(ollKorrect, Decl(bug26877.js, 0, 0))
5+
>x : Symbol(x, Decl(bug26877.js, 1, 20))
6+
7+
x._model
8+
>x._model : Symbol(D._model, Decl(bug26877.js, 7, 19))
9+
>x : Symbol(x, Decl(bug26877.js, 1, 20))
10+
>_model : Symbol(D._model, Decl(bug26877.js, 7, 19))
11+
12+
const y = new Emu.D()
13+
>y : Symbol(y, Decl(bug26877.js, 3, 9))
14+
>Emu.D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4))
15+
>Emu : Symbol(Emu, Decl(bug26877.js, 5, 1), Decl(second.js, 0, 3), Decl(second.js, 0, 12))
16+
>D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4))
17+
18+
const z = Emu.D._wrapperInstance;
19+
>z : Symbol(z, Decl(bug26877.js, 4, 9))
20+
>Emu.D._wrapperInstance : Symbol(Emu.D._wrapperInstance, Decl(second.js, 0, 12))
21+
>Emu.D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4))
22+
>Emu : Symbol(Emu, Decl(bug26877.js, 5, 1), Decl(second.js, 0, 3), Decl(second.js, 0, 12))
23+
>D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4))
24+
>_wrapperInstance : Symbol(Emu.D._wrapperInstance, Decl(second.js, 0, 12))
25+
}
26+
Emu.D = class {
27+
>Emu.D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4))
28+
>Emu : Symbol(Emu, Decl(bug26877.js, 5, 1), Decl(second.js, 0, 3), Decl(second.js, 0, 12))
29+
>D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4))
30+
31+
constructor() {
32+
this._model = 1
33+
>this._model : Symbol(D._model, Decl(bug26877.js, 7, 19))
34+
>this : Symbol(D, Decl(bug26877.js, 6, 7))
35+
>_model : Symbol(D._model, Decl(bug26877.js, 7, 19))
36+
}
37+
}
38+
39+
=== tests/cases/conformance/salsa/second.js ===
40+
var Emu = {}
41+
>Emu : Symbol(Emu, Decl(bug26877.js, 5, 1), Decl(second.js, 0, 3), Decl(second.js, 0, 12))
42+
43+
/** @type {string} */
44+
Emu.D._wrapperInstance;
45+
>Emu.D._wrapperInstance : Symbol(Emu.D._wrapperInstance, Decl(second.js, 0, 12))
46+
>Emu.D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4))
47+
>Emu : Symbol(Emu, Decl(bug26877.js, 5, 1), Decl(second.js, 0, 3), Decl(second.js, 0, 12))
48+
>D : Symbol(Emu.D, Decl(bug26877.js, 5, 1), Decl(second.js, 2, 4))
49+
>_wrapperInstance : Symbol(Emu.D._wrapperInstance, Decl(second.js, 0, 12))
50+
51+

‎tests/baselines/reference/typeFromPropertyAssignment35.types

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
=== tests/cases/conformance/salsa/bug26877.js ===
2+
/** @param {Emu.D} x */
3+
function ollKorrect(x) {
4+
>ollKorrect : (x: D) => void
5+
>x : D
6+
7+
x._model
8+
>x._model : number
9+
>x : D
10+
>_model : number
11+
12+
const y = new Emu.D()
13+
>y : D
14+
>new Emu.D() : D
15+
>Emu.D : typeof D
16+
>Emu : typeof Emu
17+
>D : typeof D
18+
19+
const z = Emu.D._wrapperInstance;
20+
>z : string
21+
>Emu.D._wrapperInstance : string
22+
>Emu.D : typeof D
23+
>Emu : typeof Emu
24+
>D : typeof D
25+
>_wrapperInstance : string
26+
}
27+
Emu.D = class {
28+
>Emu.D = class { constructor() { this._model = 1 }} : typeof D
29+
>Emu.D : typeof D
30+
>Emu : typeof Emu
31+
>D : typeof D
32+
>class { constructor() { this._model = 1 }} : typeof D
33+
34+
constructor() {
35+
this._model = 1
36+
>this._model = 1 : 1
37+
>this._model : number
38+
>this : this
39+
>_model : number
40+
>1 : 1
41+
}
42+
}
43+
44+
=== tests/cases/conformance/salsa/second.js ===
45+
var Emu = {}
46+
>Emu : typeof Emu
47+
>{} : {}
48+
49+
/** @type {string} */
50+
Emu.D._wrapperInstance;
51+
>Emu.D._wrapperInstance : string
52+
>Emu.D : typeof D
53+
>Emu : typeof Emu
54+
>D : typeof D
55+
>_wrapperInstance : string
56+
57+

‎tests/cases/conformance/salsa/globalMergeWithCommonJSAssignmentDeclaration.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// @noEmit: true
2+
// @allowJs: true
3+
// @checkJs: true
4+
// @Filename: bug27099.js
5+
window.name = 1;
6+
window.console; // should not have error: Property 'console' does not exist on type 'typeof window'.
7+
module.exports = 'anything';
8+

‎tests/cases/conformance/salsa/typeFromPropertyAssignment35.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// @noEmit: true
2+
// @allowJs: true
3+
// @checkJs: true
4+
5+
// @Filename: bug26877.js
6+
/** @param {Emu.D} x */
7+
function ollKorrect(x) {
8+
x._model
9+
const y = new Emu.D()
10+
const z = Emu.D._wrapperInstance;
11+
}
12+
Emu.D = class {
13+
constructor() {
14+
this._model = 1
15+
}
16+
}
17+
18+
// @Filename: second.js
19+
var Emu = {}
20+
/** @type {string} */
21+
Emu.D._wrapperInstance;
22+

0 commit comments

Comments
 (0)