Skip to content

Commit afa4842

Browse files
authored
Merge pull request #16363 from Microsoft/excess-property-checks-for-discriminated-unions
Excess property checks for discriminated unions
2 parents 3b9bbb3 + e1bc916 commit afa4842

7 files changed

+599
-14
lines changed

src/compiler/checker.ts

+31-7
Original file line numberDiff line numberDiff line change
@@ -9062,6 +9062,13 @@ namespace ts {
90629062
(isTypeSubsetOf(globalObjectType, target) || (!isComparingJsxAttributes && isEmptyObjectType(target)))) {
90639063
return false;
90649064
}
9065+
if (target.flags & TypeFlags.Union) {
9066+
const discriminantType = findMatchingDiscriminantType(source, target as UnionType);
9067+
if (discriminantType) {
9068+
// check excess properties against discriminant type only, not the entire union
9069+
return hasExcessProperties(source, discriminantType, reportErrors);
9070+
}
9071+
}
90659072
for (const prop of getPropertiesOfObjectType(source)) {
90669073
if (!isKnownProperty(target, prop.escapedName, isComparingJsxAttributes)) {
90679074
if (reportErrors) {
@@ -9138,20 +9145,24 @@ namespace ts {
91389145
}
91399146

91409147
function findMatchingDiscriminantType(source: Type, target: UnionOrIntersectionType) {
9148+
let match: Type;
91419149
const sourceProperties = getPropertiesOfObjectType(source);
91429150
if (sourceProperties) {
9143-
for (const sourceProperty of sourceProperties) {
9144-
if (isDiscriminantProperty(target, sourceProperty.escapedName)) {
9145-
const sourceType = getTypeOfSymbol(sourceProperty);
9146-
for (const type of target.types) {
9147-
const targetType = getTypeOfPropertyOfType(type, sourceProperty.escapedName);
9148-
if (targetType && isRelatedTo(sourceType, targetType)) {
9149-
return type;
9151+
const sourceProperty = findSingleDiscriminantProperty(sourceProperties, target);
9152+
if (sourceProperty) {
9153+
const sourceType = getTypeOfSymbol(sourceProperty);
9154+
for (const type of target.types) {
9155+
const targetType = getTypeOfPropertyOfType(type, sourceProperty.escapedName);
9156+
if (targetType && isRelatedTo(sourceType, targetType)) {
9157+
if (match) {
9158+
return undefined;
91509159
}
9160+
match = type;
91519161
}
91529162
}
91539163
}
91549164
}
9165+
return match;
91559166
}
91569167

91579168
function typeRelatedToEachType(source: Type, target: IntersectionType, reportErrors: boolean): Ternary {
@@ -11154,6 +11165,19 @@ namespace ts {
1115411165
return false;
1115511166
}
1115611167

11168+
function findSingleDiscriminantProperty(sourceProperties: Symbol[], target: Type): Symbol | undefined {
11169+
let result: Symbol;
11170+
for (const sourceProperty of sourceProperties) {
11171+
if (isDiscriminantProperty(target, sourceProperty.escapedName)) {
11172+
if (result) {
11173+
return undefined;
11174+
}
11175+
result = sourceProperty;
11176+
}
11177+
}
11178+
return result;
11179+
}
11180+
1115711181
function isOrContainsMatchingReference(source: Node, target: Node) {
1115811182
return isMatchingReference(source, target) || containsMatchingReference(source, target);
1115911183
}
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
tests/cases/compiler/discriminatedUnionErrorMessage.ts(8,5): error TS2322: Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Shape'.
2-
Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Square'.
3-
Property 'size' is missing in type '{ kind: "sq"; x: number; y: number; }'.
1+
tests/cases/compiler/discriminatedUnionErrorMessage.ts(10,5): error TS2322: Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Shape'.
2+
Object literal may only specify known properties, and 'x' does not exist in type 'Square'.
43

54

65
==== tests/cases/compiler/discriminatedUnionErrorMessage.ts (1 errors) ====
@@ -12,12 +11,11 @@ tests/cases/compiler/discriminatedUnionErrorMessage.ts(8,5): error TS2322: Type
1211
| Rectangle
1312
| Circle;
1413
let shape: Shape = {
15-
~~~~~
16-
!!! error TS2322: Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Shape'.
17-
!!! error TS2322: Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Square'.
18-
!!! error TS2322: Property 'size' is missing in type '{ kind: "sq"; x: number; y: number; }'.
1914
kind: "sq",
2015
x: 12,
16+
~~~~~
17+
!!! error TS2322: Type '{ kind: "sq"; x: number; y: number; }' is not assignable to type 'Shape'.
18+
!!! error TS2322: Object literal may only specify known properties, and 'x' does not exist in type 'Square'.
2119
y: 13,
2220
}
2321

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
tests/cases/compiler/excessPropertyCheckWithUnions.ts(10,30): error TS2322: Type '{ tag: "T"; a1: string; }' is not assignable to type 'ADT'.
2+
Object literal may only specify known properties, and 'a1' does not exist in type '{ tag: "T"; }'.
3+
tests/cases/compiler/excessPropertyCheckWithUnions.ts(11,21): error TS2322: Type '{ tag: "A"; d20: 12; }' is not assignable to type 'ADT'.
4+
Object literal may only specify known properties, and 'd20' does not exist in type '{ tag: "A"; a1: string; }'.
5+
tests/cases/compiler/excessPropertyCheckWithUnions.ts(12,1): error TS2322: Type '{ tag: "D"; }' is not assignable to type 'ADT'.
6+
Type '{ tag: "D"; }' is not assignable to type '{ tag: "D"; d20: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20; }'.
7+
Property 'd20' is missing in type '{ tag: "D"; }'.
8+
tests/cases/compiler/excessPropertyCheckWithUnions.ts(33,28): error TS2322: Type '{ tag: "A"; x: string; extra: number; }' is not assignable to type 'Ambiguous'.
9+
Object literal may only specify known properties, and 'extra' does not exist in type 'Ambiguous'.
10+
tests/cases/compiler/excessPropertyCheckWithUnions.ts(34,26): error TS2322: Type '{ tag: "A"; y: number; extra: number; }' is not assignable to type 'Ambiguous'.
11+
Object literal may only specify known properties, and 'extra' does not exist in type 'Ambiguous'.
12+
tests/cases/compiler/excessPropertyCheckWithUnions.ts(39,1): error TS2322: Type '{ tag: "A"; }' is not assignable to type 'Ambiguous'.
13+
Type '{ tag: "A"; }' is not assignable to type '{ tag: "C"; }'.
14+
Types of property 'tag' are incompatible.
15+
Type '"A"' is not assignable to type '"C"'.
16+
tests/cases/compiler/excessPropertyCheckWithUnions.ts(40,1): error TS2322: Type '{ tag: "A"; z: true; }' is not assignable to type 'Ambiguous'.
17+
Type '{ tag: "A"; z: true; }' is not assignable to type '{ tag: "C"; }'.
18+
Types of property 'tag' are incompatible.
19+
Type '"A"' is not assignable to type '"C"'.
20+
21+
22+
==== tests/cases/compiler/excessPropertyCheckWithUnions.ts (7 errors) ====
23+
type ADT = {
24+
tag: "A",
25+
a1: string
26+
} | {
27+
tag: "D",
28+
d20: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20
29+
} | {
30+
tag: "T",
31+
}
32+
let wrong: ADT = { tag: "T", a1: "extra" }
33+
~~~~~~~~~~~
34+
!!! error TS2322: Type '{ tag: "T"; a1: string; }' is not assignable to type 'ADT'.
35+
!!! error TS2322: Object literal may only specify known properties, and 'a1' does not exist in type '{ tag: "T"; }'.
36+
wrong = { tag: "A", d20: 12 }
37+
~~~~~~~
38+
!!! error TS2322: Type '{ tag: "A"; d20: 12; }' is not assignable to type 'ADT'.
39+
!!! error TS2322: Object literal may only specify known properties, and 'd20' does not exist in type '{ tag: "A"; a1: string; }'.
40+
wrong = { tag: "D" }
41+
~~~~~
42+
!!! error TS2322: Type '{ tag: "D"; }' is not assignable to type 'ADT'.
43+
!!! error TS2322: Type '{ tag: "D"; }' is not assignable to type '{ tag: "D"; d20: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20; }'.
44+
!!! error TS2322: Property 'd20' is missing in type '{ tag: "D"; }'.
45+
46+
type Ambiguous = {
47+
tag: "A",
48+
x: string
49+
} | {
50+
tag: "A",
51+
y: number
52+
} | {
53+
tag: "B",
54+
z: boolean
55+
} | {
56+
tag: "C"
57+
}
58+
let amb: Ambiguous
59+
// no error for ambiguous tag, even when it could satisfy both constituents at once
60+
amb = { tag: "A", x: "hi" }
61+
amb = { tag: "A", y: 12 }
62+
amb = { tag: "A", x: "hi", y: 12 }
63+
64+
// correctly error on excess property 'extra', even when ambiguous
65+
amb = { tag: "A", x: "hi", extra: 12 }
66+
~~~~~~~~~
67+
!!! error TS2322: Type '{ tag: "A"; x: string; extra: number; }' is not assignable to type 'Ambiguous'.
68+
!!! error TS2322: Object literal may only specify known properties, and 'extra' does not exist in type 'Ambiguous'.
69+
amb = { tag: "A", y: 12, extra: 12 }
70+
~~~~~~~~~
71+
!!! error TS2322: Type '{ tag: "A"; y: number; extra: number; }' is not assignable to type 'Ambiguous'.
72+
!!! error TS2322: Object literal may only specify known properties, and 'extra' does not exist in type 'Ambiguous'.
73+
74+
// assignability errors still work.
75+
// But note that the error for `z: true` is the fallback one of reporting on
76+
// the last constituent since assignability error reporting can't find a single best discriminant either.
77+
amb = { tag: "A" }
78+
~~~
79+
!!! error TS2322: Type '{ tag: "A"; }' is not assignable to type 'Ambiguous'.
80+
!!! error TS2322: Type '{ tag: "A"; }' is not assignable to type '{ tag: "C"; }'.
81+
!!! error TS2322: Types of property 'tag' are incompatible.
82+
!!! error TS2322: Type '"A"' is not assignable to type '"C"'.
83+
amb = { tag: "A", z: true }
84+
~~~
85+
!!! error TS2322: Type '{ tag: "A"; z: true; }' is not assignable to type 'Ambiguous'.
86+
!!! error TS2322: Type '{ tag: "A"; z: true; }' is not assignable to type '{ tag: "C"; }'.
87+
!!! error TS2322: Types of property 'tag' are incompatible.
88+
!!! error TS2322: Type '"A"' is not assignable to type '"C"'.
89+
90+
type Overlapping =
91+
| { a: 1, b: 1, first: string }
92+
| { a: 2, second: string }
93+
| { b: 3, third: string }
94+
let over: Overlapping
95+
96+
// these two are not reported because there are two discriminant properties
97+
over = { a: 1, b: 1, first: "ok", second: "error" }
98+
over = { a: 1, b: 1, first: "ok", third: "error" }
99+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
//// [excessPropertyCheckWithUnions.ts]
2+
type ADT = {
3+
tag: "A",
4+
a1: string
5+
} | {
6+
tag: "D",
7+
d20: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20
8+
} | {
9+
tag: "T",
10+
}
11+
let wrong: ADT = { tag: "T", a1: "extra" }
12+
wrong = { tag: "A", d20: 12 }
13+
wrong = { tag: "D" }
14+
15+
type Ambiguous = {
16+
tag: "A",
17+
x: string
18+
} | {
19+
tag: "A",
20+
y: number
21+
} | {
22+
tag: "B",
23+
z: boolean
24+
} | {
25+
tag: "C"
26+
}
27+
let amb: Ambiguous
28+
// no error for ambiguous tag, even when it could satisfy both constituents at once
29+
amb = { tag: "A", x: "hi" }
30+
amb = { tag: "A", y: 12 }
31+
amb = { tag: "A", x: "hi", y: 12 }
32+
33+
// correctly error on excess property 'extra', even when ambiguous
34+
amb = { tag: "A", x: "hi", extra: 12 }
35+
amb = { tag: "A", y: 12, extra: 12 }
36+
37+
// assignability errors still work.
38+
// But note that the error for `z: true` is the fallback one of reporting on
39+
// the last constituent since assignability error reporting can't find a single best discriminant either.
40+
amb = { tag: "A" }
41+
amb = { tag: "A", z: true }
42+
43+
type Overlapping =
44+
| { a: 1, b: 1, first: string }
45+
| { a: 2, second: string }
46+
| { b: 3, third: string }
47+
let over: Overlapping
48+
49+
// these two are not reported because there are two discriminant properties
50+
over = { a: 1, b: 1, first: "ok", second: "error" }
51+
over = { a: 1, b: 1, first: "ok", third: "error" }
52+
53+
54+
//// [excessPropertyCheckWithUnions.js]
55+
var wrong = { tag: "T", a1: "extra" };
56+
wrong = { tag: "A", d20: 12 };
57+
wrong = { tag: "D" };
58+
var amb;
59+
// no error for ambiguous tag, even when it could satisfy both constituents at once
60+
amb = { tag: "A", x: "hi" };
61+
amb = { tag: "A", y: 12 };
62+
amb = { tag: "A", x: "hi", y: 12 };
63+
// correctly error on excess property 'extra', even when ambiguous
64+
amb = { tag: "A", x: "hi", extra: 12 };
65+
amb = { tag: "A", y: 12, extra: 12 };
66+
// assignability errors still work.
67+
// But note that the error for `z: true` is the fallback one of reporting on
68+
// the last constituent since assignability error reporting can't find a single best discriminant either.
69+
amb = { tag: "A" };
70+
amb = { tag: "A", z: true };
71+
var over;
72+
// these two are not reported because there are two discriminant properties
73+
over = { a: 1, b: 1, first: "ok", second: "error" };
74+
over = { a: 1, b: 1, first: "ok", third: "error" };

0 commit comments

Comments
 (0)