Skip to content

Commit d38c616

Browse files
PranavSenthilnathanPranavSenthilnathan
authored
Fix resolution of properties from prototype assignment in JS (#29302)
* fix type derived from prototype assignment * accept new baselines * remove direct intersection with object literal assigned to prototype * add tests * change webpack submodule commit * fix submodule commits * comment and simplify getJSDocTypeReference * remove circularity guards that aren't hit anymore
1 parent 20285e6 commit d38c616

10 files changed

+189
-72
lines changed

‎src/compiler/checker.ts

Lines changed: 21 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -8704,21 +8704,17 @@ namespace ts {
87048704
* the type of this reference is just the type of the value we resolved to.
87058705
*/
87068706
function getJSDocTypeReference(node: NodeWithTypeArguments, symbol: Symbol, typeArguments: Type[] | undefined): Type | undefined {
8707-
if (!pushTypeResolution(symbol, TypeSystemPropertyName.JSDocTypeReference)) {
8708-
return errorType;
8709-
}
8710-
const assignedType = getAssignedClassType(symbol);
8711-
const valueType = getTypeOfSymbol(symbol);
8712-
const referenceType = valueType.symbol && valueType.symbol !== symbol && !isInferredClassType(valueType) && getTypeReferenceTypeWorker(node, valueType.symbol, typeArguments);
8713-
if (!popTypeResolution()) {
8714-
getSymbolLinks(symbol).resolvedJSDocType = errorType;
8715-
error(node, Diagnostics.JSDoc_type_0_circularly_references_itself, symbolToString(symbol));
8716-
return errorType;
8717-
}
8718-
if (referenceType || assignedType) {
8719-
// TODO: GH#18217 (should the `|| assignedType` be at a lower precedence?)
8720-
const type = (referenceType && assignedType ? getIntersectionType([assignedType, referenceType]) : referenceType || assignedType)!;
8721-
return getSymbolLinks(symbol).resolvedJSDocType = type;
8707+
// In the case of an assignment of a function expression (binary expressions, variable declarations, etc.), we will get the
8708+
// correct instance type for the symbol on the LHS by finding the type for RHS. For example if we want to get the type of the symbol `foo`:
8709+
// var foo = function() {}
8710+
// We will find the static type of the assigned anonymous function.
8711+
const staticType = getTypeOfSymbol(symbol);
8712+
const instanceType =
8713+
staticType.symbol &&
8714+
staticType.symbol !== symbol && // Make sure this is an assignment like expression by checking that symbol -> type -> symbol doesn't roundtrips.
8715+
getTypeReferenceTypeWorker(node, staticType.symbol, typeArguments); // Get the instance type of the RHS symbol.
8716+
if (instanceType) {
8717+
return getSymbolLinks(symbol).resolvedJSDocType = instanceType;
87228718
}
87238719
}
87248720

@@ -8739,8 +8735,11 @@ namespace ts {
87398735

87408736
if (symbol.flags & SymbolFlags.Function &&
87418737
isJSDocTypeReference(node) &&
8742-
(symbol.members || getJSDocClassTag(symbol.valueDeclaration))) {
8743-
return getInferredClassType(symbol);
8738+
isJSConstructor(symbol.valueDeclaration)) {
8739+
const resolved = resolveStructuredTypeMembers(<ObjectType>getTypeOfSymbol(symbol));
8740+
if (resolved.callSignatures.length === 1) {
8741+
return getReturnTypeOfSignature(resolved.callSignatures[0]);
8742+
}
87448743
}
87458744
}
87468745

@@ -16877,7 +16876,7 @@ namespace ts {
1687716876
else if (isInJS &&
1687816877
(container.kind === SyntaxKind.FunctionExpression || container.kind === SyntaxKind.FunctionDeclaration) &&
1687916878
getJSDocClassTag(container)) {
16880-
const classType = getJSClassType(container.symbol);
16879+
const classType = getJSClassType(getMergedSymbol(container.symbol));
1688116880
if (classType) {
1688216881
return getFlowTypeOfReference(node, classType);
1688316882
}
@@ -21057,7 +21056,7 @@ namespace ts {
2105721056

2105821057
// If the symbol of the node has members, treat it like a constructor.
2105921058
const symbol = getSymbolOfNode(func);
21060-
return !!symbol && symbol.members !== undefined;
21059+
return !!symbol && (symbol.members !== undefined || symbol.exports !== undefined && symbol.exports.get("prototype" as __String) !== undefined);
2106121060
}
2106221061
return false;
2106321062
}
@@ -21076,10 +21075,6 @@ namespace ts {
2107621075
inferred = getInferredClassType(symbol);
2107721076
}
2107821077
const assigned = getAssignedClassType(symbol);
21079-
const valueType = getTypeOfSymbol(symbol);
21080-
if (valueType.symbol && !isInferredClassType(valueType) && isJSConstructor(valueType.symbol.valueDeclaration)) {
21081-
inferred = getInferredClassType(valueType.symbol);
21082-
}
2108321078
return assigned && inferred ?
2108421079
getIntersectionType([inferred, assigned]) :
2108521080
assigned || inferred;
@@ -21119,12 +21114,6 @@ namespace ts {
2111921114
return links.inferredClassType;
2112021115
}
2112121116

21122-
function isInferredClassType(type: Type) {
21123-
return type.symbol
21124-
&& getObjectFlags(type) & ObjectFlags.Anonymous
21125-
&& getSymbolLinks(type.symbol).inferredClassType === type;
21126-
}
21127-
2112821117
/**
2112921118
* Syntactically and semantically checks a call or new expression.
2113021119
* @param node The call/new expression to be checked.
@@ -21146,21 +21135,10 @@ namespace ts {
2114621135
declaration.kind !== SyntaxKind.Constructor &&
2114721136
declaration.kind !== SyntaxKind.ConstructSignature &&
2114821137
declaration.kind !== SyntaxKind.ConstructorType &&
21149-
!isJSDocConstructSignature(declaration)) {
21138+
!isJSDocConstructSignature(declaration) &&
21139+
!isJSConstructor(declaration)) {
2115021140

21151-
// When resolved signature is a call signature (and not a construct signature) the result type is any, unless
21152-
// the declaring function had members created through 'x.prototype.y = expr' or 'this.y = expr' psuedodeclarations
21153-
// in a JS file
21154-
// Note:JS inferred classes might come from a variable declaration instead of a function declaration.
21155-
// In this case, using getResolvedSymbol directly is required to avoid losing the members from the declaration.
21156-
let funcSymbol = checkExpression(node.expression).symbol;
21157-
if (!funcSymbol && node.expression.kind === SyntaxKind.Identifier) {
21158-
funcSymbol = getResolvedSymbol(node.expression as Identifier);
21159-
}
21160-
const type = funcSymbol && getJSClassType(funcSymbol);
21161-
if (type) {
21162-
return signature.target ? instantiateType(type, signature.mapper) : type;
21163-
}
21141+
// When resolved signature is a call signature (and not a construct signature) the result type is any
2116421142
if (noImplicitAny) {
2116521143
error(node, Diagnostics.new_expression_whose_target_lacks_a_construct_signature_implicitly_has_an_any_type);
2116621144
}

‎tests/baselines/reference/classCanExtendConstructorFunction.types

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,8 @@ soup.flavour
269269
>flavour : number
270270

271271
var chowder = new Chowder({ claim: "ignorant" });
272-
>chowder : any
273-
>new Chowder({ claim: "ignorant" }) : any
272+
>chowder : Chowder
273+
>new Chowder({ claim: "ignorant" }) : Chowder
274274
>Chowder : typeof Chowder
275275
>{ claim: "ignorant" } : { claim: "ignorant"; }
276276
>claim : "ignorant"
@@ -279,7 +279,7 @@ var chowder = new Chowder({ claim: "ignorant" });
279279
chowder.flavour.claim
280280
>chowder.flavour.claim : any
281281
>chowder.flavour : any
282-
>chowder : any
282+
>chowder : Chowder
283283
>flavour : any
284284
>claim : any
285285

‎tests/baselines/reference/jsContainerMergeJsContainer.types

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,19 @@ const a = {};
55
>{} : {}
66

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

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

‎tests/baselines/reference/typeFromPropertyAssignment14.types

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,18 @@ var Outer = {};
55

66
=== tests/cases/conformance/salsa/work.js ===
77
Outer.Inner = function () {}
8-
>Outer.Inner = function () {} : { (): void; prototype: { x: number; m(): void; }; }
9-
>Outer.Inner : { (): void; prototype: { x: number; m(): void; }; }
8+
>Outer.Inner = function () {} : typeof Inner
9+
>Outer.Inner : typeof Inner
1010
>Outer : typeof Outer
11-
>Inner : { (): void; prototype: { x: number; m(): void; }; }
12-
>function () {} : { (): void; prototype: { x: number; m(): void; }; }
11+
>Inner : typeof Inner
12+
>function () {} : typeof Inner
1313

1414
Outer.Inner.prototype = {
1515
>Outer.Inner.prototype = { x: 1, m() { }} : { x: number; m(): void; }
1616
>Outer.Inner.prototype : { x: number; m(): void; }
17-
>Outer.Inner : { (): void; prototype: { x: number; m(): void; }; }
17+
>Outer.Inner : typeof Inner
1818
>Outer : typeof Outer
19-
>Inner : { (): void; prototype: { x: number; m(): void; }; }
19+
>Inner : typeof Inner
2020
>prototype : { x: number; m(): void; }
2121
>{ x: 1, m() { }} : { x: number; m(): void; }
2222

@@ -47,9 +47,9 @@ inner.m()
4747
var inno = new Outer.Inner()
4848
>inno : { x: number; m(): void; }
4949
>new Outer.Inner() : { x: number; m(): void; }
50-
>Outer.Inner : { (): void; prototype: { x: number; m(): void; }; }
50+
>Outer.Inner : typeof Inner
5151
>Outer : typeof Outer
52-
>Inner : { (): void; prototype: { x: number; m(): void; }; }
52+
>Inner : typeof Inner
5353

5454
inno.x
5555
>inno.x : number

‎tests/baselines/reference/typeFromPropertyAssignment16.types

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,18 @@ var Outer = {};
44
>{} : {}
55

66
Outer.Inner = function () {}
7-
>Outer.Inner = function () {} : { (): void; prototype: { x: number; m(): void; }; }
8-
>Outer.Inner : { (): void; prototype: { x: number; m(): void; }; }
7+
>Outer.Inner = function () {} : typeof Inner
8+
>Outer.Inner : typeof Inner
99
>Outer : typeof Outer
10-
>Inner : { (): void; prototype: { x: number; m(): void; }; }
11-
>function () {} : { (): void; prototype: { x: number; m(): void; }; }
10+
>Inner : typeof Inner
11+
>function () {} : typeof Inner
1212

1313
Outer.Inner.prototype = {
1414
>Outer.Inner.prototype = { x: 1, m() { }} : { x: number; m(): void; }
1515
>Outer.Inner.prototype : { x: number; m(): void; }
16-
>Outer.Inner : { (): void; prototype: { x: number; m(): void; }; }
16+
>Outer.Inner : typeof Inner
1717
>Outer : typeof Outer
18-
>Inner : { (): void; prototype: { x: number; m(): void; }; }
18+
>Inner : typeof Inner
1919
>prototype : { x: number; m(): void; }
2020
>{ x: 1, m() { }} : { x: number; m(): void; }
2121

@@ -45,9 +45,9 @@ inner.m()
4545
var inno = new Outer.Inner()
4646
>inno : { x: number; m(): void; }
4747
>new Outer.Inner() : { x: number; m(): void; }
48-
>Outer.Inner : { (): void; prototype: { x: number; m(): void; }; }
48+
>Outer.Inner : typeof Inner
4949
>Outer : typeof Outer
50-
>Inner : { (): void; prototype: { x: number; m(): void; }; }
50+
>Inner : typeof Inner
5151

5252
inno.x
5353
>inno.x : number

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
tests/cases/conformance/salsa/bug26885.js(2,5): error TS2683: 'this' implicitly has type 'any' because it does not have a type annotation.
2+
tests/cases/conformance/salsa/bug26885.js(11,16): error TS7017: Element implicitly has an 'any' type because type '{}' has no index signature.
3+
4+
5+
==== tests/cases/conformance/salsa/bug26885.js (2 errors) ====
6+
function Multimap3() {
7+
this._map = {};
8+
~~~~
9+
!!! error TS2683: 'this' implicitly has type 'any' because it does not have a type annotation.
10+
};
11+
12+
Multimap3.prototype = {
13+
/**
14+
* @param {string} key
15+
* @returns {number} the value ok
16+
*/
17+
get(key) {
18+
return this._map[key + ''];
19+
~~~~~~~~~~~~~~~~~~~
20+
!!! error TS7017: Element implicitly has an 'any' type because type '{}' has no index signature.
21+
}
22+
}
23+
24+
/** @type {Multimap3} */
25+
const map = new Multimap3();
26+
const n = map.get('hi')

‎tests/baselines/reference/typeFromPrototypeAssignment3.symbols

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
=== tests/cases/conformance/salsa/bug26885.js ===
2+
function Multimap3() {
3+
>Multimap3 : Symbol(Multimap3, Decl(bug26885.js, 0, 0), Decl(bug26885.js, 2, 2))
4+
5+
this._map = {};
6+
>_map : Symbol(Multimap3._map, Decl(bug26885.js, 0, 22))
7+
8+
};
9+
10+
Multimap3.prototype = {
11+
>Multimap3.prototype : Symbol(Multimap3.prototype, Decl(bug26885.js, 2, 2))
12+
>Multimap3 : Symbol(Multimap3, Decl(bug26885.js, 0, 0), Decl(bug26885.js, 2, 2))
13+
>prototype : Symbol(Multimap3.prototype, Decl(bug26885.js, 2, 2))
14+
15+
/**
16+
* @param {string} key
17+
* @returns {number} the value ok
18+
*/
19+
get(key) {
20+
>get : Symbol(get, Decl(bug26885.js, 4, 23))
21+
>key : Symbol(key, Decl(bug26885.js, 9, 8))
22+
23+
return this._map[key + ''];
24+
>this._map : Symbol(Multimap3._map, Decl(bug26885.js, 0, 22))
25+
>_map : Symbol(Multimap3._map, Decl(bug26885.js, 0, 22))
26+
>key : Symbol(key, Decl(bug26885.js, 9, 8))
27+
}
28+
}
29+
30+
/** @type {Multimap3} */
31+
const map = new Multimap3();
32+
>map : Symbol(map, Decl(bug26885.js, 15, 5))
33+
>Multimap3 : Symbol(Multimap3, Decl(bug26885.js, 0, 0), Decl(bug26885.js, 2, 2))
34+
35+
const n = map.get('hi')
36+
>n : Symbol(n, Decl(bug26885.js, 16, 5))
37+
>map.get : Symbol(get, Decl(bug26885.js, 4, 23))
38+
>map : Symbol(map, Decl(bug26885.js, 15, 5))
39+
>get : Symbol(get, Decl(bug26885.js, 4, 23))
40+

‎tests/baselines/reference/typeFromPrototypeAssignment3.types

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
=== tests/cases/conformance/salsa/bug26885.js ===
2+
function Multimap3() {
3+
>Multimap3 : typeof Multimap3
4+
5+
this._map = {};
6+
>this._map = {} : {}
7+
>this._map : any
8+
>this : any
9+
>_map : any
10+
>{} : {}
11+
12+
};
13+
14+
Multimap3.prototype = {
15+
>Multimap3.prototype = { /** * @param {string} key * @returns {number} the value ok */ get(key) { return this._map[key + '']; }} : { get(key: string): number; }
16+
>Multimap3.prototype : { get(key: string): number; }
17+
>Multimap3 : typeof Multimap3
18+
>prototype : { get(key: string): number; }
19+
>{ /** * @param {string} key * @returns {number} the value ok */ get(key) { return this._map[key + '']; }} : { get(key: string): number; }
20+
21+
/**
22+
* @param {string} key
23+
* @returns {number} the value ok
24+
*/
25+
get(key) {
26+
>get : (key: string) => number
27+
>key : string
28+
29+
return this._map[key + ''];
30+
>this._map[key + ''] : any
31+
>this._map : {}
32+
>this : Multimap3 & { get(key: string): number; }
33+
>_map : {}
34+
>key + '' : string
35+
>key : string
36+
>'' : ""
37+
}
38+
}
39+
40+
/** @type {Multimap3} */
41+
const map = new Multimap3();
42+
>map : Multimap3 & { get(key: string): number; }
43+
>new Multimap3() : Multimap3 & { get(key: string): number; }
44+
>Multimap3 : typeof Multimap3
45+
46+
const n = map.get('hi')
47+
>n : number
48+
>map.get('hi') : number
49+
>map.get : (key: string) => number
50+
>map : Multimap3 & { get(key: string): number; }
51+
>get : (key: string) => number
52+
>'hi' : "hi"
53+

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
11
tests/cases/conformance/jsdoc/bug27346.js(2,4): error TS8030: The type of a function declaration must match the function's signature.
2-
tests/cases/conformance/jsdoc/bug27346.js(2,11): error TS2587: JSDoc type 'MyClass' circularly references itself.
32

43

5-
==== tests/cases/conformance/jsdoc/bug27346.js (2 errors) ====
4+
==== tests/cases/conformance/jsdoc/bug27346.js (1 errors) ====
65
/**
76
* @type {MyClass}
87
~~~~~~~~~~~~~~~
98
!!! error TS8030: The type of a function declaration must match the function's signature.
10-
~~~~~~~
11-
!!! error TS2587: JSDoc type 'MyClass' circularly references itself.
129
*/
1310
function MyClass() { }
1411
MyClass.prototype = {};

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// @noEmit: true
2+
// @allowJs: true
3+
// @checkJs: true
4+
// @Filename: bug26885.js
5+
// @strict: true
6+
7+
function Multimap3() {
8+
this._map = {};
9+
};
10+
11+
Multimap3.prototype = {
12+
/**
13+
* @param {string} key
14+
* @returns {number} the value ok
15+
*/
16+
get(key) {
17+
return this._map[key + ''];
18+
}
19+
}
20+
21+
/** @type {Multimap3} */
22+
const map = new Multimap3();
23+
const n = map.get('hi')

0 commit comments

Comments
 (0)