Skip to content

Commit c9f1902

Browse files
sandersnsandersn
authored
Fix non-toplevel prototype assignment (microsoft#27096)
* Fix non-toplevel prototype assignment binder was using the wrong node to lookup the containing class type for prototype assignment, so it incorrectly put the prototype declaration on the class' symbol. This correction to the binder in turn required a change in getJSClassType in the checker. It now has to look at the "prototype" property for the prototype instead of looking on the class symbol's exports (which makes no sense). * Refactor per PR suggestion
1 parent 989a717 commit c9f1902

6 files changed

Lines changed: 382 additions & 9 deletions

File tree

‎src/compiler/binder.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2465,7 +2465,7 @@ namespace ts {
24652465
node.left.parent = node;
24662466
node.right.parent = node;
24672467
const lhs = node.left as PropertyAccessEntityNameExpression;
2468-
bindPropertyAssignment(lhs, lhs, /*isPrototypeProperty*/ false);
2468+
bindPropertyAssignment(lhs.expression, lhs, /*isPrototypeProperty*/ false);
24692469
}
24702470

24712471
/**
@@ -2522,7 +2522,7 @@ namespace ts {
25222522
const isToplevel = isBinaryExpression(propertyAccess.parent)
25232523
? getParentOfBinaryExpression(propertyAccess.parent).parent.kind === SyntaxKind.SourceFile
25242524
: propertyAccess.parent.parent.kind === SyntaxKind.SourceFile;
2525-
if (!isPrototypeProperty && (!namespaceSymbol || !(namespaceSymbol.flags & SymbolFlags.Namespace)) && isToplevel) {
2525+
if (isToplevel && !isPrototypeProperty && (!namespaceSymbol || !(namespaceSymbol.flags & SymbolFlags.Namespace))) {
25262526
// make symbols or add declarations for intermediate containers
25272527
const flags = SymbolFlags.Module | SymbolFlags.Assignment;
25282528
const excludeFlags = SymbolFlags.ValueModuleExcludes & ~SymbolFlags.Assignment;

‎src/compiler/checker.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20203,18 +20203,15 @@ namespace ts {
2020320203
assigned || inferred;
2020420204
}
2020520205

20206-
function getAssignedClassType(symbol: Symbol) {
20206+
function getAssignedClassType(symbol: Symbol): Type | undefined {
2020720207
const decl = symbol.valueDeclaration;
2020820208
const assignmentSymbol = decl && decl.parent &&
2020920209
(isFunctionDeclaration(decl) && getSymbolOfNode(decl) ||
2021020210
isBinaryExpression(decl.parent) && getSymbolOfNode(decl.parent.left) ||
2021120211
isVariableDeclaration(decl.parent) && getSymbolOfNode(decl.parent));
20212-
if (assignmentSymbol) {
20213-
const prototype = forEach(assignmentSymbol.declarations, getAssignedJSPrototype);
20214-
if (prototype) {
20215-
return checkExpression(prototype);
20216-
}
20217-
}
20212+
const prototype = assignmentSymbol && assignmentSymbol.exports && assignmentSymbol.exports.get("prototype" as __String);
20213+
const init = prototype && getAssignedJSPrototype(prototype.valueDeclaration);
20214+
return init ? checkExpression(init) : undefined;
2021820215
}
2021920216

2022020217
function getAssignedJSPrototype(node: Node) {

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
tests/cases/conformance/salsa/a.js(28,24): error TS2339: Property 'addon' does not exist on type '{ set: () => void; get(): void; }'.
2+
3+
4+
==== tests/cases/conformance/salsa/a.js (1 errors) ====
5+
// non top-level:
6+
// all references to _map, set, get, addon should be ok
7+
(function container() {
8+
/** @constructor */
9+
var Multimap = function() {
10+
this._map = {};
11+
this._map
12+
this.set
13+
this.get
14+
this.addon
15+
};
16+
17+
Multimap.prototype = {
18+
set: function() {
19+
this._map
20+
this.set
21+
this.get
22+
this.addon
23+
},
24+
get() {
25+
this._map
26+
this.set
27+
this.get
28+
this.addon
29+
}
30+
}
31+
32+
Multimap.prototype.addon = function () {
33+
~~~~~
34+
!!! error TS2339: Property 'addon' does not exist on type '{ set: () => void; get(): void; }'.
35+
this._map
36+
this.set
37+
this.get
38+
this.addon
39+
}
40+
41+
var mm = new Multimap();
42+
mm._map
43+
mm.set
44+
mm.get
45+
mm.addon
46+
});
47+

‎tests/baselines/reference/typeFromPrototypeAssignment2.symbols‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
// non top-level:
3+
// all references to _map, set, get, addon should be ok
4+
(function container() {
5+
>container : Symbol(container, Decl(a.js, 2, 1))
6+
7+
/** @constructor */
8+
var Multimap = function() {
9+
>Multimap : Symbol(Multimap, Decl(a.js, 4, 7))
10+
11+
this._map = {};
12+
>this._map : Symbol(Multimap._map, Decl(a.js, 4, 31))
13+
>_map : Symbol(Multimap._map, Decl(a.js, 4, 31))
14+
15+
this._map
16+
>this._map : Symbol(Multimap._map, Decl(a.js, 4, 31))
17+
>_map : Symbol(Multimap._map, Decl(a.js, 4, 31))
18+
19+
this.set
20+
>this.set : Symbol(set, Decl(a.js, 12, 26))
21+
>set : Symbol(set, Decl(a.js, 12, 26))
22+
23+
this.get
24+
>this.get : Symbol(get, Decl(a.js, 18, 10))
25+
>get : Symbol(get, Decl(a.js, 18, 10))
26+
27+
this.addon
28+
>this.addon : Symbol(Multimap.addon, Decl(a.js, 25, 5))
29+
>addon : Symbol(Multimap.addon, Decl(a.js, 25, 5))
30+
31+
};
32+
33+
Multimap.prototype = {
34+
>Multimap.prototype : Symbol(Multimap.prototype, Decl(a.js, 10, 6))
35+
>Multimap : Symbol(Multimap, Decl(a.js, 4, 7))
36+
>prototype : Symbol(Multimap.prototype, Decl(a.js, 10, 6))
37+
38+
set: function() {
39+
>set : Symbol(set, Decl(a.js, 12, 26))
40+
41+
this._map
42+
>this._map : Symbol(Multimap._map, Decl(a.js, 4, 31))
43+
>_map : Symbol(Multimap._map, Decl(a.js, 4, 31))
44+
45+
this.set
46+
>this.set : Symbol(set, Decl(a.js, 12, 26))
47+
>set : Symbol(set, Decl(a.js, 12, 26))
48+
49+
this.get
50+
>this.get : Symbol(get, Decl(a.js, 18, 10))
51+
>get : Symbol(get, Decl(a.js, 18, 10))
52+
53+
this.addon
54+
>this.addon : Symbol(Multimap.addon, Decl(a.js, 25, 5))
55+
>addon : Symbol(Multimap.addon, Decl(a.js, 25, 5))
56+
57+
},
58+
get() {
59+
>get : Symbol(get, Decl(a.js, 18, 10))
60+
61+
this._map
62+
>this._map : Symbol(Multimap._map, Decl(a.js, 4, 31))
63+
>_map : Symbol(Multimap._map, Decl(a.js, 4, 31))
64+
65+
this.set
66+
>this.set : Symbol(set, Decl(a.js, 12, 26))
67+
>set : Symbol(set, Decl(a.js, 12, 26))
68+
69+
this.get
70+
>this.get : Symbol(get, Decl(a.js, 18, 10))
71+
>get : Symbol(get, Decl(a.js, 18, 10))
72+
73+
this.addon
74+
>this.addon : Symbol(Multimap.addon, Decl(a.js, 25, 5))
75+
>addon : Symbol(Multimap.addon, Decl(a.js, 25, 5))
76+
}
77+
}
78+
79+
Multimap.prototype.addon = function () {
80+
>Multimap.prototype : Symbol(Multimap.addon, Decl(a.js, 25, 5))
81+
>Multimap : Symbol(Multimap, Decl(a.js, 4, 7))
82+
>prototype : Symbol(Multimap.prototype, Decl(a.js, 10, 6))
83+
>addon : Symbol(Multimap.addon, Decl(a.js, 25, 5))
84+
85+
this._map
86+
>this._map : Symbol(Multimap._map, Decl(a.js, 4, 31))
87+
>_map : Symbol(Multimap._map, Decl(a.js, 4, 31))
88+
89+
this.set
90+
>this.set : Symbol(set, Decl(a.js, 12, 26))
91+
>set : Symbol(set, Decl(a.js, 12, 26))
92+
93+
this.get
94+
>this.get : Symbol(get, Decl(a.js, 18, 10))
95+
>get : Symbol(get, Decl(a.js, 18, 10))
96+
97+
this.addon
98+
>this.addon : Symbol(Multimap.addon, Decl(a.js, 25, 5))
99+
>addon : Symbol(Multimap.addon, Decl(a.js, 25, 5))
100+
}
101+
102+
var mm = new Multimap();
103+
>mm : Symbol(mm, Decl(a.js, 34, 7))
104+
>Multimap : Symbol(Multimap, Decl(a.js, 4, 7))
105+
106+
mm._map
107+
>mm._map : Symbol(Multimap._map, Decl(a.js, 4, 31))
108+
>mm : Symbol(mm, Decl(a.js, 34, 7))
109+
>_map : Symbol(Multimap._map, Decl(a.js, 4, 31))
110+
111+
mm.set
112+
>mm.set : Symbol(set, Decl(a.js, 12, 26))
113+
>mm : Symbol(mm, Decl(a.js, 34, 7))
114+
>set : Symbol(set, Decl(a.js, 12, 26))
115+
116+
mm.get
117+
>mm.get : Symbol(get, Decl(a.js, 18, 10))
118+
>mm : Symbol(mm, Decl(a.js, 34, 7))
119+
>get : Symbol(get, Decl(a.js, 18, 10))
120+
121+
mm.addon
122+
>mm.addon : Symbol(Multimap.addon, Decl(a.js, 25, 5))
123+
>mm : Symbol(mm, Decl(a.js, 34, 7))
124+
>addon : Symbol(Multimap.addon, Decl(a.js, 25, 5))
125+
126+
});
127+

‎tests/baselines/reference/typeFromPrototypeAssignment2.types‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
// non top-level:
3+
// all references to _map, set, get, addon should be ok
4+
(function container() {
5+
>(function container() { /** @constructor */ var Multimap = function() { this._map = {}; this._map this.set this.get this.addon }; Multimap.prototype = { set: function() { this._map this.set this.get this.addon }, get() { this._map this.set this.get this.addon } } Multimap.prototype.addon = function () { this._map this.set this.get this.addon } var mm = new Multimap(); mm._map mm.set mm.get mm.addon}) : () => void
6+
>function container() { /** @constructor */ var Multimap = function() { this._map = {}; this._map this.set this.get this.addon }; Multimap.prototype = { set: function() { this._map this.set this.get this.addon }, get() { this._map this.set this.get this.addon } } Multimap.prototype.addon = function () { this._map this.set this.get this.addon } var mm = new Multimap(); mm._map mm.set mm.get mm.addon} : () => void
7+
>container : () => void
8+
9+
/** @constructor */
10+
var Multimap = function() {
11+
>Multimap : typeof Multimap
12+
>function() { this._map = {}; this._map this.set this.get this.addon } : typeof Multimap
13+
14+
this._map = {};
15+
>this._map = {} : {}
16+
>this._map : {}
17+
>this : Multimap & { set: () => void; get(): void; }
18+
>_map : {}
19+
>{} : {}
20+
21+
this._map
22+
>this._map : {}
23+
>this : Multimap & { set: () => void; get(): void; }
24+
>_map : {}
25+
26+
this.set
27+
>this.set : () => void
28+
>this : Multimap & { set: () => void; get(): void; }
29+
>set : () => void
30+
31+
this.get
32+
>this.get : () => void
33+
>this : Multimap & { set: () => void; get(): void; }
34+
>get : () => void
35+
36+
this.addon
37+
>this.addon : () => void
38+
>this : Multimap & { set: () => void; get(): void; }
39+
>addon : () => void
40+
41+
};
42+
43+
Multimap.prototype = {
44+
>Multimap.prototype = { set: function() { this._map this.set this.get this.addon }, get() { this._map this.set this.get this.addon } } : { set: () => void; get(): void; }
45+
>Multimap.prototype : { set: () => void; get(): void; }
46+
>Multimap : typeof Multimap
47+
>prototype : { set: () => void; get(): void; }
48+
>{ set: function() { this._map this.set this.get this.addon }, get() { this._map this.set this.get this.addon } } : { set: () => void; get(): void; }
49+
50+
set: function() {
51+
>set : () => void
52+
>function() { this._map this.set this.get this.addon } : () => void
53+
54+
this._map
55+
>this._map : {}
56+
>this : Multimap & { set: () => void; get(): void; }
57+
>_map : {}
58+
59+
this.set
60+
>this.set : () => void
61+
>this : Multimap & { set: () => void; get(): void; }
62+
>set : () => void
63+
64+
this.get
65+
>this.get : () => void
66+
>this : Multimap & { set: () => void; get(): void; }
67+
>get : () => void
68+
69+
this.addon
70+
>this.addon : () => void
71+
>this : Multimap & { set: () => void; get(): void; }
72+
>addon : () => void
73+
74+
},
75+
get() {
76+
>get : () => void
77+
78+
this._map
79+
>this._map : {}
80+
>this : Multimap & { set: () => void; get(): void; }
81+
>_map : {}
82+
83+
this.set
84+
>this.set : () => void
85+
>this : Multimap & { set: () => void; get(): void; }
86+
>set : () => void
87+
88+
this.get
89+
>this.get : () => void
90+
>this : Multimap & { set: () => void; get(): void; }
91+
>get : () => void
92+
93+
this.addon
94+
>this.addon : () => void
95+
>this : Multimap & { set: () => void; get(): void; }
96+
>addon : () => void
97+
}
98+
}
99+
100+
Multimap.prototype.addon = function () {
101+
>Multimap.prototype.addon = function () { this._map this.set this.get this.addon } : () => void
102+
>Multimap.prototype.addon : any
103+
>Multimap.prototype : { set: () => void; get(): void; }
104+
>Multimap : typeof Multimap
105+
>prototype : { set: () => void; get(): void; }
106+
>addon : any
107+
>function () { this._map this.set this.get this.addon } : () => void
108+
109+
this._map
110+
>this._map : {}
111+
>this : Multimap & { set: () => void; get(): void; }
112+
>_map : {}
113+
114+
this.set
115+
>this.set : () => void
116+
>this : Multimap & { set: () => void; get(): void; }
117+
>set : () => void
118+
119+
this.get
120+
>this.get : () => void
121+
>this : Multimap & { set: () => void; get(): void; }
122+
>get : () => void
123+
124+
this.addon
125+
>this.addon : () => void
126+
>this : Multimap & { set: () => void; get(): void; }
127+
>addon : () => void
128+
}
129+
130+
var mm = new Multimap();
131+
>mm : Multimap & { set: () => void; get(): void; }
132+
>new Multimap() : Multimap & { set: () => void; get(): void; }
133+
>Multimap : typeof Multimap
134+
135+
mm._map
136+
>mm._map : {}
137+
>mm : Multimap & { set: () => void; get(): void; }
138+
>_map : {}
139+
140+
mm.set
141+
>mm.set : () => void
142+
>mm : Multimap & { set: () => void; get(): void; }
143+
>set : () => void
144+
145+
mm.get
146+
>mm.get : () => void
147+
>mm : Multimap & { set: () => void; get(): void; }
148+
>get : () => void
149+
150+
mm.addon
151+
>mm.addon : () => void
152+
>mm : Multimap & { set: () => void; get(): void; }
153+
>addon : () => void
154+
155+
});
156+

0 commit comments

Comments
 (0)