Skip to content

Commit b24050a

Browse files
sandersnsandersn
authored
Excess discriminated types match all discriminable properties (microsoft#32755)
* Target types in excess property checking must match all discriminable properties This allows fewer types to be discriminated in excess properties, which fixes some examples. * Add excess property test * Fix semicolon lint * Remove extra semicolon! * Improve EPC for unions with multiple discriminants
1 parent b70f894 commit b24050a

11 files changed

+642
-8
lines changed

‎src/compiler/checker.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14269,20 +14269,25 @@ namespace ts {
1426914269
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary): Type | undefined;
1427014270
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue: Type): Type;
1427114271
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue?: Type) {
14272-
let match: Type | undefined;
14272+
// undefined=unknown, true=discriminated, false=not discriminated
14273+
// The state of each type progresses from left to right. Discriminated types stop at 'true'.
14274+
const discriminable = target.types.map(_ => undefined) as (boolean | undefined)[];
1427314275
for (const [getDiscriminatingType, propertyName] of discriminators) {
14276+
let i = 0;
1427414277
for (const type of target.types) {
1427514278
const targetType = getTypeOfPropertyOfType(type, propertyName);
1427614279
if (targetType && related(getDiscriminatingType(), targetType)) {
14277-
if (match) {
14278-
if (type === match) continue; // Finding multiple fields which discriminate to the same type is fine
14279-
return defaultValue;
14280-
}
14281-
match = type;
14280+
discriminable[i] = discriminable[i] === undefined ? true : discriminable[i];
1428214281
}
14282+
else {
14283+
discriminable[i] = false;
14284+
}
14285+
i++;
1428314286
}
1428414287
}
14285-
return match || defaultValue;
14288+
const match = discriminable.indexOf(/*searchElement*/ true);
14289+
// make sure exactly 1 matches before returning it
14290+
return match === -1 || discriminable.indexOf(/*searchElement*/ true, match + 1) !== -1 ? defaultValue : target.types[match];
1428614291
}
1428714292

1428814293
/**

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(30,5): error TS2322: Type '{ type: "number"; value: number; multipleOf: number; format: string; }' is not assignable to type 'Primitive'.
2+
Object literal may only specify known properties, and 'multipleOf' does not exist in type 'Float'.
3+
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(41,5): error TS2322: Type '{ p1: "left"; p2: false; p3: number; p4: string; }' is not assignable to type 'DisjointDiscriminants'.
4+
Object literal may only specify known properties, and 'p3' does not exist in type '{ p1: "left"; p2: boolean; }'.
5+
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(57,5): error TS2322: Type '{ p1: "right"; p2: false; p3: number; p4: string; }' is not assignable to type 'DisjointDiscriminants'.
6+
Object literal may only specify known properties, and 'p3' does not exist in type '{ p1: "right"; p2: false; p4: string; }'.
7+
8+
9+
==== tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts (3 errors) ====
10+
// Repro from #32657
11+
12+
interface Base<T> {
13+
value: T;
14+
}
15+
16+
interface Int extends Base<number> {
17+
type: "integer";
18+
multipleOf?: number;
19+
}
20+
21+
interface Float extends Base<number> {
22+
type: "number";
23+
}
24+
25+
interface Str extends Base<string> {
26+
type: "string";
27+
format?: string;
28+
}
29+
30+
interface Bool extends Base<boolean> {
31+
type: "boolean";
32+
}
33+
34+
type Primitive = Int | Float | Str | Bool;
35+
36+
const foo: Primitive = {
37+
type: "number",
38+
value: 10,
39+
multipleOf: 5, // excess property
40+
~~~~~~~~~~~~~
41+
!!! error TS2322: Type '{ type: "number"; value: number; multipleOf: number; format: string; }' is not assignable to type 'Primitive'.
42+
!!! error TS2322: Object literal may only specify known properties, and 'multipleOf' does not exist in type 'Float'.
43+
format: "what?"
44+
}
45+
46+
47+
type DisjointDiscriminants = { p1: 'left'; p2: true; p3: number } | { p1: 'right'; p2: false; p4: string } | { p1: 'left'; p2: boolean };
48+
49+
// This has excess error because variant three is the only applicable case.
50+
const a: DisjointDiscriminants = {
51+
p1: 'left',
52+
p2: false,
53+
p3: 42,
54+
~~~~~~
55+
!!! error TS2322: Type '{ p1: "left"; p2: false; p3: number; p4: string; }' is not assignable to type 'DisjointDiscriminants'.
56+
!!! error TS2322: Object literal may only specify known properties, and 'p3' does not exist in type '{ p1: "left"; p2: boolean; }'.
57+
p4: "hello"
58+
};
59+
60+
// This has no excess error because variant one and three are both applicable.
61+
const b: DisjointDiscriminants = {
62+
p1: 'left',
63+
p2: true,
64+
p3: 42,
65+
p4: "hello"
66+
};
67+
68+
// This has excess error because variant two is the only applicable case
69+
const c: DisjointDiscriminants = {
70+
p1: 'right',
71+
p2: false,
72+
p3: 42,
73+
~~~~~~
74+
!!! error TS2322: Type '{ p1: "right"; p2: false; p3: number; p4: string; }' is not assignable to type 'DisjointDiscriminants'.
75+
!!! error TS2322: Object literal may only specify known properties, and 'p3' does not exist in type '{ p1: "right"; p2: false; p4: string; }'.
76+
p4: "hello"
77+
};
78+

‎tests/baselines/reference/excessPropertyCheckWithMultipleDiscriminants.js

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
//// [excessPropertyCheckWithMultipleDiscriminants.ts]
2+
// Repro from #32657
3+
4+
interface Base<T> {
5+
value: T;
6+
}
7+
8+
interface Int extends Base<number> {
9+
type: "integer";
10+
multipleOf?: number;
11+
}
12+
13+
interface Float extends Base<number> {
14+
type: "number";
15+
}
16+
17+
interface Str extends Base<string> {
18+
type: "string";
19+
format?: string;
20+
}
21+
22+
interface Bool extends Base<boolean> {
23+
type: "boolean";
24+
}
25+
26+
type Primitive = Int | Float | Str | Bool;
27+
28+
const foo: Primitive = {
29+
type: "number",
30+
value: 10,
31+
multipleOf: 5, // excess property
32+
format: "what?"
33+
}
34+
35+
36+
type DisjointDiscriminants = { p1: 'left'; p2: true; p3: number } | { p1: 'right'; p2: false; p4: string } | { p1: 'left'; p2: boolean };
37+
38+
// This has excess error because variant three is the only applicable case.
39+
const a: DisjointDiscriminants = {
40+
p1: 'left',
41+
p2: false,
42+
p3: 42,
43+
p4: "hello"
44+
};
45+
46+
// This has no excess error because variant one and three are both applicable.
47+
const b: DisjointDiscriminants = {
48+
p1: 'left',
49+
p2: true,
50+
p3: 42,
51+
p4: "hello"
52+
};
53+
54+
// This has excess error because variant two is the only applicable case
55+
const c: DisjointDiscriminants = {
56+
p1: 'right',
57+
p2: false,
58+
p3: 42,
59+
p4: "hello"
60+
};
61+
62+
63+
//// [excessPropertyCheckWithMultipleDiscriminants.js]
64+
// Repro from #32657
65+
var foo = {
66+
type: "number",
67+
value: 10,
68+
multipleOf: 5,
69+
format: "what?"
70+
};
71+
// This has excess error because variant three is the only applicable case.
72+
var a = {
73+
p1: 'left',
74+
p2: false,
75+
p3: 42,
76+
p4: "hello"
77+
};
78+
// This has no excess error because variant one and three are both applicable.
79+
var b = {
80+
p1: 'left',
81+
p2: true,
82+
p3: 42,
83+
p4: "hello"
84+
};
85+
// This has excess error because variant two is the only applicable case
86+
var c = {
87+
p1: 'right',
88+
p2: false,
89+
p3: 42,
90+
p4: "hello"
91+
};

‎tests/baselines/reference/excessPropertyCheckWithMultipleDiscriminants.symbols

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
=== tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts ===
2+
// Repro from #32657
3+
4+
interface Base<T> {
5+
>Base : Symbol(Base, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 0, 0))
6+
>T : Symbol(T, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 2, 15))
7+
8+
value: T;
9+
>value : Symbol(Base.value, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 2, 19))
10+
>T : Symbol(T, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 2, 15))
11+
}
12+
13+
interface Int extends Base<number> {
14+
>Int : Symbol(Int, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 4, 1))
15+
>Base : Symbol(Base, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 0, 0))
16+
17+
type: "integer";
18+
>type : Symbol(Int.type, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 6, 36))
19+
20+
multipleOf?: number;
21+
>multipleOf : Symbol(Int.multipleOf, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 7, 20))
22+
}
23+
24+
interface Float extends Base<number> {
25+
>Float : Symbol(Float, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 9, 1))
26+
>Base : Symbol(Base, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 0, 0))
27+
28+
type: "number";
29+
>type : Symbol(Float.type, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 11, 38))
30+
}
31+
32+
interface Str extends Base<string> {
33+
>Str : Symbol(Str, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 13, 1))
34+
>Base : Symbol(Base, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 0, 0))
35+
36+
type: "string";
37+
>type : Symbol(Str.type, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 15, 36))
38+
39+
format?: string;
40+
>format : Symbol(Str.format, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 16, 19))
41+
}
42+
43+
interface Bool extends Base<boolean> {
44+
>Bool : Symbol(Bool, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 18, 1))
45+
>Base : Symbol(Base, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 0, 0))
46+
47+
type: "boolean";
48+
>type : Symbol(Bool.type, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 20, 38))
49+
}
50+
51+
type Primitive = Int | Float | Str | Bool;
52+
>Primitive : Symbol(Primitive, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 22, 1))
53+
>Int : Symbol(Int, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 4, 1))
54+
>Float : Symbol(Float, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 9, 1))
55+
>Str : Symbol(Str, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 13, 1))
56+
>Bool : Symbol(Bool, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 18, 1))
57+
58+
const foo: Primitive = {
59+
>foo : Symbol(foo, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 26, 5))
60+
>Primitive : Symbol(Primitive, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 22, 1))
61+
62+
type: "number",
63+
>type : Symbol(type, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 26, 24))
64+
65+
value: 10,
66+
>value : Symbol(value, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 27, 19))
67+
68+
multipleOf: 5, // excess property
69+
>multipleOf : Symbol(multipleOf, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 28, 14))
70+
71+
format: "what?"
72+
>format : Symbol(format, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 29, 18))
73+
}
74+
75+
76+
type DisjointDiscriminants = { p1: 'left'; p2: true; p3: number } | { p1: 'right'; p2: false; p4: string } | { p1: 'left'; p2: boolean };
77+
>DisjointDiscriminants : Symbol(DisjointDiscriminants, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 31, 1))
78+
>p1 : Symbol(p1, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 34, 30))
79+
>p2 : Symbol(p2, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 34, 42))
80+
>p3 : Symbol(p3, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 34, 52))
81+
>p1 : Symbol(p1, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 34, 69))
82+
>p2 : Symbol(p2, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 34, 82))
83+
>p4 : Symbol(p4, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 34, 93))
84+
>p1 : Symbol(p1, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 34, 110))
85+
>p2 : Symbol(p2, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 34, 122))
86+
87+
// This has excess error because variant three is the only applicable case.
88+
const a: DisjointDiscriminants = {
89+
>a : Symbol(a, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 37, 5))
90+
>DisjointDiscriminants : Symbol(DisjointDiscriminants, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 31, 1))
91+
92+
p1: 'left',
93+
>p1 : Symbol(p1, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 37, 34))
94+
95+
p2: false,
96+
>p2 : Symbol(p2, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 38, 15))
97+
98+
p3: 42,
99+
>p3 : Symbol(p3, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 39, 14))
100+
101+
p4: "hello"
102+
>p4 : Symbol(p4, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 40, 11))
103+
104+
};
105+
106+
// This has no excess error because variant one and three are both applicable.
107+
const b: DisjointDiscriminants = {
108+
>b : Symbol(b, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 45, 5))
109+
>DisjointDiscriminants : Symbol(DisjointDiscriminants, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 31, 1))
110+
111+
p1: 'left',
112+
>p1 : Symbol(p1, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 45, 34))
113+
114+
p2: true,
115+
>p2 : Symbol(p2, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 46, 15))
116+
117+
p3: 42,
118+
>p3 : Symbol(p3, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 47, 13))
119+
120+
p4: "hello"
121+
>p4 : Symbol(p4, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 48, 11))
122+
123+
};
124+
125+
// This has excess error because variant two is the only applicable case
126+
const c: DisjointDiscriminants = {
127+
>c : Symbol(c, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 53, 5))
128+
>DisjointDiscriminants : Symbol(DisjointDiscriminants, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 31, 1))
129+
130+
p1: 'right',
131+
>p1 : Symbol(p1, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 53, 34))
132+
133+
p2: false,
134+
>p2 : Symbol(p2, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 54, 16))
135+
136+
p3: 42,
137+
>p3 : Symbol(p3, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 55, 14))
138+
139+
p4: "hello"
140+
>p4 : Symbol(p4, Decl(excessPropertyCheckWithMultipleDiscriminants.ts, 56, 11))
141+
142+
};
143+

0 commit comments

Comments
 (0)