Skip to content

Commit f6a850b

Browse files
ahejlsbergahejlsberg
authored
Merge pull request #10188 from Microsoft/discriminantPropertyCheck
Discriminant property checks
2 parents acfdfe0 + 1375505 commit f6a850b

File tree

5 files changed

+311
-10
lines changed

5 files changed

+311
-10
lines changed

‎src/compiler/checker.ts‎

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4416,10 +4416,19 @@ namespace ts {
44164416
}
44174417
const propTypes: Type[] = [];
44184418
const declarations: Declaration[] = [];
4419+
let commonType: Type = undefined;
4420+
let hasCommonType = true;
44194421
for (const prop of props) {
44204422
if (prop.declarations) {
44214423
addRange(declarations, prop.declarations);
44224424
}
4425+
const type = getTypeOfSymbol(prop);
4426+
if (!commonType) {
4427+
commonType = type;
4428+
}
4429+
else if (type !== commonType) {
4430+
hasCommonType = false;
4431+
}
44234432
propTypes.push(getTypeOfSymbol(prop));
44244433
}
44254434
const result = <TransientSymbol>createSymbol(
@@ -4429,6 +4438,7 @@ namespace ts {
44294438
commonFlags,
44304439
name);
44314440
result.containingType = containingType;
4441+
result.hasCommonType = hasCommonType;
44324442
result.declarations = declarations;
44334443
result.isReadonly = isReadonly;
44344444
result.type = containingType.flags & TypeFlags.Union ? getUnionType(propTypes) : getIntersectionType(propTypes);
@@ -7793,8 +7803,39 @@ namespace ts {
77937803
return false;
77947804
}
77957805

7796-
function rootContainsMatchingReference(source: Node, target: Node) {
7797-
return target.kind === SyntaxKind.PropertyAccessExpression && containsMatchingReference(source, (<PropertyAccessExpression>target).expression);
7806+
// Return true if target is a property access xxx.yyy, source is a property access xxx.zzz, the declared
7807+
// type of xxx is a union type, and yyy is a property that is possibly a discriminant. We consider a property
7808+
// a possible discriminant if its type differs in the constituents of containing union type, and if every
7809+
// choice is a unit type or a union of unit types.
7810+
function containsMatchingReferenceDiscriminant(source: Node, target: Node) {
7811+
return target.kind === SyntaxKind.PropertyAccessExpression &&
7812+
containsMatchingReference(source, (<PropertyAccessExpression>target).expression) &&
7813+
isDiscriminantProperty(getDeclaredTypeOfReference((<PropertyAccessExpression>target).expression), (<PropertyAccessExpression>target).name.text);
7814+
}
7815+
7816+
function getDeclaredTypeOfReference(expr: Node): Type {
7817+
if (expr.kind === SyntaxKind.Identifier) {
7818+
return getTypeOfSymbol(getResolvedSymbol(<Identifier>expr));
7819+
}
7820+
if (expr.kind === SyntaxKind.PropertyAccessExpression) {
7821+
const type = getDeclaredTypeOfReference((<PropertyAccessExpression>expr).expression);
7822+
return type && getTypeOfPropertyOfType(type, (<PropertyAccessExpression>expr).name.text);
7823+
}
7824+
return undefined;
7825+
}
7826+
7827+
function isDiscriminantProperty(type: Type, name: string) {
7828+
if (type && type.flags & TypeFlags.Union) {
7829+
const prop = getPropertyOfType(type, name);
7830+
if (prop && prop.flags & SymbolFlags.SyntheticProperty) {
7831+
if ((<TransientSymbol>prop).isDiscriminantProperty === undefined) {
7832+
(<TransientSymbol>prop).isDiscriminantProperty = !(<TransientSymbol>prop).hasCommonType &&
7833+
isUnitUnionType(getTypeOfSymbol(prop));
7834+
}
7835+
return (<TransientSymbol>prop).isDiscriminantProperty;
7836+
}
7837+
}
7838+
return false;
77987839
}
77997840

78007841
function isOrContainsMatchingReference(source: Node, target: Node) {
@@ -8223,7 +8264,7 @@ namespace ts {
82238264
if (isMatchingReference(reference, expr)) {
82248265
type = narrowTypeBySwitchOnDiscriminant(type, flow.switchStatement, flow.clauseStart, flow.clauseEnd);
82258266
}
8226-
else if (isMatchingPropertyAccess(expr)) {
8267+
else if (isMatchingReferenceDiscriminant(expr)) {
82278268
type = narrowTypeByDiscriminant(type, <PropertyAccessExpression>expr, t => narrowTypeBySwitchOnDiscriminant(t, flow.switchStatement, flow.clauseStart, flow.clauseEnd));
82288269
}
82298270
return createFlowType(type, isIncomplete(flowType));
@@ -8301,10 +8342,11 @@ namespace ts {
83018342
return cache[key] = getUnionType(antecedentTypes);
83028343
}
83038344

8304-
function isMatchingPropertyAccess(expr: Expression) {
8345+
function isMatchingReferenceDiscriminant(expr: Expression) {
83058346
return expr.kind === SyntaxKind.PropertyAccessExpression &&
8347+
declaredType.flags & TypeFlags.Union &&
83068348
isMatchingReference(reference, (<PropertyAccessExpression>expr).expression) &&
8307-
(declaredType.flags & TypeFlags.Union) !== 0;
8349+
isDiscriminantProperty(declaredType, (<PropertyAccessExpression>expr).name.text);
83088350
}
83098351

83108352
function narrowTypeByDiscriminant(type: Type, propAccess: PropertyAccessExpression, narrowType: (t: Type) => Type): Type {
@@ -8318,10 +8360,10 @@ namespace ts {
83188360
if (isMatchingReference(reference, expr)) {
83198361
return getTypeWithFacts(type, assumeTrue ? TypeFacts.Truthy : TypeFacts.Falsy);
83208362
}
8321-
if (isMatchingPropertyAccess(expr)) {
8363+
if (isMatchingReferenceDiscriminant(expr)) {
83228364
return narrowTypeByDiscriminant(type, <PropertyAccessExpression>expr, t => getTypeWithFacts(t, assumeTrue ? TypeFacts.Truthy : TypeFacts.Falsy));
83238365
}
8324-
if (rootContainsMatchingReference(reference, expr)) {
8366+
if (containsMatchingReferenceDiscriminant(reference, expr)) {
83258367
return declaredType;
83268368
}
83278369
return type;
@@ -8350,13 +8392,13 @@ namespace ts {
83508392
if (isMatchingReference(reference, right)) {
83518393
return narrowTypeByEquality(type, operator, left, assumeTrue);
83528394
}
8353-
if (isMatchingPropertyAccess(left)) {
8395+
if (isMatchingReferenceDiscriminant(left)) {
83548396
return narrowTypeByDiscriminant(type, <PropertyAccessExpression>left, t => narrowTypeByEquality(t, operator, right, assumeTrue));
83558397
}
8356-
if (isMatchingPropertyAccess(right)) {
8398+
if (isMatchingReferenceDiscriminant(right)) {
83578399
return narrowTypeByDiscriminant(type, <PropertyAccessExpression>right, t => narrowTypeByEquality(t, operator, left, assumeTrue));
83588400
}
8359-
if (rootContainsMatchingReference(reference, left) || rootContainsMatchingReference(reference, right)) {
8401+
if (containsMatchingReferenceDiscriminant(reference, left) || containsMatchingReferenceDiscriminant(reference, right)) {
83608402
return declaredType;
83618403
}
83628404
break;

‎src/compiler/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2160,6 +2160,8 @@ namespace ts {
21602160
mapper?: TypeMapper; // Type mapper for instantiation alias
21612161
referenced?: boolean; // True if alias symbol has been referenced as a value
21622162
containingType?: UnionOrIntersectionType; // Containing union or intersection type for synthetic property
2163+
hasCommonType?: boolean; // True if constituents of synthetic property all have same type
2164+
isDiscriminantProperty?: boolean; // True if discriminant synthetic property
21632165
resolvedExports?: SymbolTable; // Resolved exports of module
21642166
exportsChecked?: boolean; // True if exports of external module have been checked
21652167
isDeclarationWithCollidingName?: boolean; // True if symbol is block scoped redeclaration

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
tests/cases/compiler/discriminantPropertyCheck.ts(30,9): error TS2532: Object is possibly 'undefined'.
2+
tests/cases/compiler/discriminantPropertyCheck.ts(66,9): error TS2532: Object is possibly 'undefined'.
3+
4+
5+
==== tests/cases/compiler/discriminantPropertyCheck.ts (2 errors) ====
6+
7+
type Item = Item1 | Item2;
8+
9+
interface Base {
10+
bar: boolean;
11+
}
12+
13+
interface Item1 extends Base {
14+
kind: "A";
15+
foo: string | undefined;
16+
baz: boolean;
17+
qux: true;
18+
}
19+
20+
interface Item2 extends Base {
21+
kind: "B";
22+
foo: string | undefined;
23+
baz: boolean;
24+
qux: false;
25+
}
26+
27+
function goo1(x: Item) {
28+
if (x.kind === "A" && x.foo !== undefined) {
29+
x.foo.length;
30+
}
31+
}
32+
33+
function goo2(x: Item) {
34+
if (x.foo !== undefined && x.kind === "A") {
35+
x.foo.length; // Error, intervening discriminant guard
36+
~~~~~
37+
!!! error TS2532: Object is possibly 'undefined'.
38+
}
39+
}
40+
41+
function foo1(x: Item) {
42+
if (x.bar && x.foo !== undefined) {
43+
x.foo.length;
44+
}
45+
}
46+
47+
function foo2(x: Item) {
48+
if (x.foo !== undefined && x.bar) {
49+
x.foo.length;
50+
}
51+
}
52+
53+
function foo3(x: Item) {
54+
if (x.baz && x.foo !== undefined) {
55+
x.foo.length;
56+
}
57+
}
58+
59+
function foo4(x: Item) {
60+
if (x.foo !== undefined && x.baz) {
61+
x.foo.length;
62+
}
63+
}
64+
65+
function foo5(x: Item) {
66+
if (x.qux && x.foo !== undefined) {
67+
x.foo.length;
68+
}
69+
}
70+
71+
function foo6(x: Item) {
72+
if (x.foo !== undefined && x.qux) {
73+
x.foo.length; // Error, intervening discriminant guard
74+
~~~~~
75+
!!! error TS2532: Object is possibly 'undefined'.
76+
}
77+
}

‎tests/baselines/reference/discriminantPropertyCheck.js‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
//// [discriminantPropertyCheck.ts]
2+
3+
type Item = Item1 | Item2;
4+
5+
interface Base {
6+
bar: boolean;
7+
}
8+
9+
interface Item1 extends Base {
10+
kind: "A";
11+
foo: string | undefined;
12+
baz: boolean;
13+
qux: true;
14+
}
15+
16+
interface Item2 extends Base {
17+
kind: "B";
18+
foo: string | undefined;
19+
baz: boolean;
20+
qux: false;
21+
}
22+
23+
function goo1(x: Item) {
24+
if (x.kind === "A" && x.foo !== undefined) {
25+
x.foo.length;
26+
}
27+
}
28+
29+
function goo2(x: Item) {
30+
if (x.foo !== undefined && x.kind === "A") {
31+
x.foo.length; // Error, intervening discriminant guard
32+
}
33+
}
34+
35+
function foo1(x: Item) {
36+
if (x.bar && x.foo !== undefined) {
37+
x.foo.length;
38+
}
39+
}
40+
41+
function foo2(x: Item) {
42+
if (x.foo !== undefined && x.bar) {
43+
x.foo.length;
44+
}
45+
}
46+
47+
function foo3(x: Item) {
48+
if (x.baz && x.foo !== undefined) {
49+
x.foo.length;
50+
}
51+
}
52+
53+
function foo4(x: Item) {
54+
if (x.foo !== undefined && x.baz) {
55+
x.foo.length;
56+
}
57+
}
58+
59+
function foo5(x: Item) {
60+
if (x.qux && x.foo !== undefined) {
61+
x.foo.length;
62+
}
63+
}
64+
65+
function foo6(x: Item) {
66+
if (x.foo !== undefined && x.qux) {
67+
x.foo.length; // Error, intervening discriminant guard
68+
}
69+
}
70+
71+
//// [discriminantPropertyCheck.js]
72+
function goo1(x) {
73+
if (x.kind === "A" && x.foo !== undefined) {
74+
x.foo.length;
75+
}
76+
}
77+
function goo2(x) {
78+
if (x.foo !== undefined && x.kind === "A") {
79+
x.foo.length; // Error, intervening discriminant guard
80+
}
81+
}
82+
function foo1(x) {
83+
if (x.bar && x.foo !== undefined) {
84+
x.foo.length;
85+
}
86+
}
87+
function foo2(x) {
88+
if (x.foo !== undefined && x.bar) {
89+
x.foo.length;
90+
}
91+
}
92+
function foo3(x) {
93+
if (x.baz && x.foo !== undefined) {
94+
x.foo.length;
95+
}
96+
}
97+
function foo4(x) {
98+
if (x.foo !== undefined && x.baz) {
99+
x.foo.length;
100+
}
101+
}
102+
function foo5(x) {
103+
if (x.qux && x.foo !== undefined) {
104+
x.foo.length;
105+
}
106+
}
107+
function foo6(x) {
108+
if (x.foo !== undefined && x.qux) {
109+
x.foo.length; // Error, intervening discriminant guard
110+
}
111+
}

‎tests/cases/compiler/discriminantPropertyCheck.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// @strictNullChecks: true
2+
3+
type Item = Item1 | Item2;
4+
5+
interface Base {
6+
bar: boolean;
7+
}
8+
9+
interface Item1 extends Base {
10+
kind: "A";
11+
foo: string | undefined;
12+
baz: boolean;
13+
qux: true;
14+
}
15+
16+
interface Item2 extends Base {
17+
kind: "B";
18+
foo: string | undefined;
19+
baz: boolean;
20+
qux: false;
21+
}
22+
23+
function goo1(x: Item) {
24+
if (x.kind === "A" && x.foo !== undefined) {
25+
x.foo.length;
26+
}
27+
}
28+
29+
function goo2(x: Item) {
30+
if (x.foo !== undefined && x.kind === "A") {
31+
x.foo.length; // Error, intervening discriminant guard
32+
}
33+
}
34+
35+
function foo1(x: Item) {
36+
if (x.bar && x.foo !== undefined) {
37+
x.foo.length;
38+
}
39+
}
40+
41+
function foo2(x: Item) {
42+
if (x.foo !== undefined && x.bar) {
43+
x.foo.length;
44+
}
45+
}
46+
47+
function foo3(x: Item) {
48+
if (x.baz && x.foo !== undefined) {
49+
x.foo.length;
50+
}
51+
}
52+
53+
function foo4(x: Item) {
54+
if (x.foo !== undefined && x.baz) {
55+
x.foo.length;
56+
}
57+
}
58+
59+
function foo5(x: Item) {
60+
if (x.qux && x.foo !== undefined) {
61+
x.foo.length;
62+
}
63+
}
64+
65+
function foo6(x: Item) {
66+
if (x.foo !== undefined && x.qux) {
67+
x.foo.length; // Error, intervening discriminant guard
68+
}
69+
}

0 commit comments

Comments
 (0)