Skip to content

Commit 1541599

Browse files
sandersnsandersn
authored
Check base type for special property declarations (microsoft#23671)
If the base type has a property by that name, add it to the list constructor types to make it as authoritative as special assignments found in the constructor.
1 parent aa10243 commit 1541599

10 files changed

+270
-2
lines changed

‎src/compiler/checker.ts

+23-2
Original file line numberDiff line numberDiff line change
@@ -4430,7 +4430,7 @@ namespace ts {
44304430
const special = getSpecialPropertyAssignmentKind(expression);
44314431
if (special === SpecialPropertyAssignmentKind.ThisProperty) {
44324432
const thisContainer = getThisContainer(expression, /*includeArrowFunctions*/ false);
4433-
// Properties defined in a constructor (or javascript constructor function) don't get undefined added.
4433+
// Properties defined in a constructor (or base constructor, or javascript constructor function) don't get undefined added.
44344434
// Function expressions that are assigned to the prototype count as methods.
44354435
declarationInConstructor = thisContainer.kind === SyntaxKind.Constructor ||
44364436
thisContainer.kind === SyntaxKind.FunctionDeclaration ||
@@ -4500,7 +4500,14 @@ namespace ts {
45004500
}
45014501
let type = jsDocType;
45024502
if (!type) {
4503-
// use only the constructor types unless only null | undefined (including widening variants) were assigned there
4503+
// use only the constructor types unless they were only assigned null | undefined (including widening variants)
4504+
if (definedInMethod) {
4505+
const propType = getTypeOfSpecialPropertyOfBaseType(symbol);
4506+
if (propType) {
4507+
(constructorTypes || (constructorTypes = [])).push(propType);
4508+
definedInConstructor = true;
4509+
}
4510+
}
45044511
const sourceTypes = some(constructorTypes, t => !!(t.flags & ~(TypeFlags.Nullable | TypeFlags.ContainsWideningType))) ? constructorTypes : types;
45054512
type = getUnionType(sourceTypes, UnionReduction.Subtype);
45064513
}
@@ -4514,6 +4521,20 @@ namespace ts {
45144521
return widened;
45154522
}
45164523

4524+
/** check for definition in base class if any declaration is in a class */
4525+
function getTypeOfSpecialPropertyOfBaseType(specialProperty: Symbol) {
4526+
const parentDeclaration = forEach(specialProperty.declarations, d => {
4527+
const parent = getThisContainer(d, /*includeArrowFunctions*/ false).parent;
4528+
return isClassLike(parent) && parent;
4529+
});
4530+
if (parentDeclaration) {
4531+
const classType = getDeclaredTypeOfSymbol(getSymbolOfNode(parentDeclaration)) as InterfaceType;
4532+
const baseClassType = classType && getBaseTypes(classType)[0];
4533+
if (baseClassType) {
4534+
return getTypeOfPropertyOfType(baseClassType, specialProperty.escapedName);
4535+
}
4536+
}
4537+
}
45174538

45184539
// Return the type implied by a binding pattern element. This is the type of the initializer of the element if
45194540
// one is present. Otherwise, if the element is itself a binding pattern, it is the type implied by the binding

‎tests/baselines/reference/inferringClassMembersFromAssignments3.symbols

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
class Base {
3+
>Base : Symbol(Base, Decl(a.js, 0, 0))
4+
5+
constructor() {
6+
this.p = 1
7+
>this.p : Symbol(Base.p, Decl(a.js, 1, 19))
8+
>this : Symbol(Base, Decl(a.js, 0, 0))
9+
>p : Symbol(Base.p, Decl(a.js, 1, 19))
10+
}
11+
}
12+
class Derived extends Base {
13+
>Derived : Symbol(Derived, Decl(a.js, 4, 1))
14+
>Base : Symbol(Base, Decl(a.js, 0, 0))
15+
16+
m() {
17+
>m : Symbol(Derived.m, Decl(a.js, 5, 28))
18+
19+
this.p = 1
20+
>this.p : Symbol(Derived.p, Decl(a.js, 6, 9))
21+
>this : Symbol(Derived, Decl(a.js, 4, 1))
22+
>p : Symbol(Derived.p, Decl(a.js, 6, 9))
23+
}
24+
}
25+

‎tests/baselines/reference/inferringClassMembersFromAssignments3.types

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
class Base {
3+
>Base : Base
4+
5+
constructor() {
6+
this.p = 1
7+
>this.p = 1 : 1
8+
>this.p : number
9+
>this : this
10+
>p : number
11+
>1 : 1
12+
}
13+
}
14+
class Derived extends Base {
15+
>Derived : Derived
16+
>Base : Base
17+
18+
m() {
19+
>m : () => void
20+
21+
this.p = 1
22+
>this.p = 1 : 1
23+
>this.p : number
24+
>this : this
25+
>p : number
26+
>1 : 1
27+
}
28+
}
29+

‎tests/baselines/reference/inferringClassMembersFromAssignments4.symbols

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
class Base {
3+
>Base : Symbol(Base, Decl(a.js, 0, 0))
4+
5+
m() {
6+
>m : Symbol(Base.m, Decl(a.js, 0, 12))
7+
8+
this.p = 1
9+
>this.p : Symbol(Base.p, Decl(a.js, 1, 9))
10+
>this : Symbol(Base, Decl(a.js, 0, 0))
11+
>p : Symbol(Base.p, Decl(a.js, 1, 9))
12+
}
13+
}
14+
class Derived extends Base {
15+
>Derived : Symbol(Derived, Decl(a.js, 4, 1))
16+
>Base : Symbol(Base, Decl(a.js, 0, 0))
17+
18+
m() {
19+
>m : Symbol(Derived.m, Decl(a.js, 5, 28))
20+
21+
// should be OK, and p should have type number | undefined from its base
22+
this.p = 1
23+
>this.p : Symbol(Derived.p, Decl(a.js, 6, 9))
24+
>this : Symbol(Derived, Decl(a.js, 4, 1))
25+
>p : Symbol(Derived.p, Decl(a.js, 6, 9))
26+
}
27+
}
28+

‎tests/baselines/reference/inferringClassMembersFromAssignments4.types

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
class Base {
3+
>Base : Base
4+
5+
m() {
6+
>m : () => void
7+
8+
this.p = 1
9+
>this.p = 1 : 1
10+
>this.p : number | undefined
11+
>this : this
12+
>p : number | undefined
13+
>1 : 1
14+
}
15+
}
16+
class Derived extends Base {
17+
>Derived : Derived
18+
>Base : Base
19+
20+
m() {
21+
>m : () => void
22+
23+
// should be OK, and p should have type number | undefined from its base
24+
this.p = 1
25+
>this.p = 1 : 1
26+
>this.p : number | undefined
27+
>this : this
28+
>p : number | undefined
29+
>1 : 1
30+
}
31+
}
32+

‎tests/baselines/reference/inferringClassMembersFromAssignments5.symbols

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
class Base {
3+
>Base : Symbol(Base, Decl(a.js, 0, 0))
4+
5+
m() {
6+
>m : Symbol(Base.m, Decl(a.js, 0, 12))
7+
8+
this.p = 1
9+
>this.p : Symbol(Base.p, Decl(a.js, 1, 9))
10+
>this : Symbol(Base, Decl(a.js, 0, 0))
11+
>p : Symbol(Base.p, Decl(a.js, 1, 9))
12+
}
13+
}
14+
class Derived extends Base {
15+
>Derived : Symbol(Derived, Decl(a.js, 4, 1))
16+
>Base : Symbol(Base, Decl(a.js, 0, 0))
17+
18+
constructor() {
19+
super();
20+
>super : Symbol(Base, Decl(a.js, 0, 0))
21+
22+
// should be OK, and p should have type number from this assignment
23+
this.p = 1
24+
>this.p : Symbol(Derived.p, Decl(a.js, 7, 16))
25+
>this : Symbol(Derived, Decl(a.js, 4, 1))
26+
>p : Symbol(Derived.p, Decl(a.js, 7, 16))
27+
}
28+
test() {
29+
>test : Symbol(Derived.test, Decl(a.js, 10, 5))
30+
31+
return this.p
32+
>this.p : Symbol(Derived.p, Decl(a.js, 7, 16))
33+
>this : Symbol(Derived, Decl(a.js, 4, 1))
34+
>p : Symbol(Derived.p, Decl(a.js, 7, 16))
35+
}
36+
}
37+

‎tests/baselines/reference/inferringClassMembersFromAssignments5.types

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
=== tests/cases/conformance/salsa/a.js ===
2+
class Base {
3+
>Base : Base
4+
5+
m() {
6+
>m : () => void
7+
8+
this.p = 1
9+
>this.p = 1 : 1
10+
>this.p : number | undefined
11+
>this : this
12+
>p : number | undefined
13+
>1 : 1
14+
}
15+
}
16+
class Derived extends Base {
17+
>Derived : Derived
18+
>Base : Base
19+
20+
constructor() {
21+
super();
22+
>super() : void
23+
>super : typeof Base
24+
25+
// should be OK, and p should have type number from this assignment
26+
this.p = 1
27+
>this.p = 1 : 1
28+
>this.p : number
29+
>this : this
30+
>p : number
31+
>1 : 1
32+
}
33+
test() {
34+
>test : () => number
35+
36+
return this.p
37+
>this.p : number
38+
>this : this
39+
>p : number
40+
}
41+
}
42+

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

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// @noEmit: true
2+
// @allowJs: true
3+
// @checkJs: true
4+
// @noImplicitAny: true
5+
// @strictNullChecks: true
6+
// @Filename: a.js
7+
class Base {
8+
constructor() {
9+
this.p = 1
10+
}
11+
}
12+
class Derived extends Base {
13+
m() {
14+
this.p = 1
15+
}
16+
}

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

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// @noEmit: true
2+
// @allowJs: true
3+
// @checkJs: true
4+
// @noImplicitAny: true
5+
// @strictNullChecks: true
6+
// @Filename: a.js
7+
class Base {
8+
m() {
9+
this.p = 1
10+
}
11+
}
12+
class Derived extends Base {
13+
m() {
14+
// should be OK, and p should have type number | undefined from its base
15+
this.p = 1
16+
}
17+
}

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

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// @noEmit: true
2+
// @allowJs: true
3+
// @checkJs: true
4+
// @noImplicitAny: true
5+
// @strictNullChecks: true
6+
// @Filename: a.js
7+
class Base {
8+
m() {
9+
this.p = 1
10+
}
11+
}
12+
class Derived extends Base {
13+
constructor() {
14+
super();
15+
// should be OK, and p should have type number from this assignment
16+
this.p = 1
17+
}
18+
test() {
19+
return this.p
20+
}
21+
}

0 commit comments

Comments
 (0)