Skip to content

Commit 78b9245

Browse files
craigphickscraigphicks
committed
On branch tsmainb/main/44856
 Changes to be committed: modified: src/compiler/checker.ts new file: tests/baselines/reference/_44856.js new file: tests/baselines/reference/_44856.symbols new file: tests/baselines/reference/_44856.types new file: tests/cases/compiler/_44856.ts - fix for issue #44856 - minor modifications to checker.ts - Preexisting algorithm: - In the case of checking for excess properties being assigned to a union, the logic forks: - If there is an index in the union whose type qualifies it as discriminant index, then don't allow excess properties (roughly) - otherwise, allow excess properites which satisfy some member of the union - Change: - Don't consider an index with a boolean type to qualify for being a discriminant index. - Before: - In *src/compiler/checker.ts*, in the function `createUnionOrIntersectionProperty` the preexisting code is ``` if ((isLiteralType(type) || isPatternLiteralType(type) || type === uniqueLiteralType) { checkFlags |= CheckFlags.HasLiteralType; } ``` - `isLiteralType(type)` returns true for a boolean type, and setting `checkFlags |= CheckFlags.HasLiteralType` results in a higher up decision to qualify the corresponding index as a discriminated index. - After: ``` if ((isLiteralType(type) && (!fromIsDiscriminantProperty || !(type.flags & TypeFlags.Boolean && type.flags & TypeFlags.Union))) || isPatternLiteralType(type) || type === uniqueLiteralType) { checkFlags |= CheckFlags.HasLiteralType; } ``` - the `fromIsDiscriminantProperty` is to provide the context in which `createUnionOrIntersectionProperty`. - That is necessary because `createUnionOrIntersectionProperty` is called in other contexts that require the existing behavior. - Here is one such scenario from `objectSpread.ts` ``` function f<T, U>(t: T, u: U) { return { ...t, ...u, id: 'id' } } let overwriteId: { id: string, a: number, c: number, d: string } = f({ a: 1, id: true }, { c: 1, d: 'no' }) ``` - Without introducing the condition `fromIsDiscriminantProperty`, overwriteId would have type never.
1 parent 4765355 commit 78b9245

File tree

5 files changed

+354
-5
lines changed

5 files changed

+354
-5
lines changed

‎src/compiler/checker.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12313,7 +12313,7 @@ namespace ts {
1231312313
return getReducedType(getApparentType(getReducedType(type)));
1231412314
}
1231512315

12316-
function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined {
12316+
function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean, fromIsDiscriminantProperty?: boolean | undefined): Symbol | undefined {
1231712317
let singleProp: Symbol | undefined;
1231812318
let propSet: ESMap<SymbolId, Symbol> | undefined;
1231912319
let indexTypes: Type[] | undefined;
@@ -12438,7 +12438,7 @@ namespace ts {
1243812438
else if (type !== firstType) {
1243912439
checkFlags |= CheckFlags.HasNonUniformType;
1244012440
}
12441-
if (isLiteralType(type) || isPatternLiteralType(type) || type === uniqueLiteralType) {
12441+
if ((isLiteralType(type) && (!fromIsDiscriminantProperty || !(type.flags & TypeFlags.Boolean && type.flags & TypeFlags.Union))) || isPatternLiteralType(type) || type === uniqueLiteralType) {
1244212442
checkFlags |= CheckFlags.HasLiteralType;
1244312443
}
1244412444
if (type.flags & TypeFlags.Never && type !== uniqueLiteralType) {
@@ -12481,11 +12481,11 @@ namespace ts {
1248112481
// constituents, in which case the isPartial flag is set when the containing type is union type. We need
1248212482
// these partial properties when identifying discriminant properties, but otherwise they are filtered out
1248312483
// and do not appear to be present in the union type.
12484-
function getUnionOrIntersectionProperty(type: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined {
12484+
function getUnionOrIntersectionProperty(type: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean, fromIsDiscriminantProperty?: boolean | undefined): Symbol | undefined {
1248512485
let property = type.propertyCacheWithoutObjectFunctionPropertyAugment?.get(name) ||
1248612486
!skipObjectFunctionPropertyAugment ? type.propertyCache?.get(name) : undefined;
1248712487
if (!property) {
12488-
property = createUnionOrIntersectionProperty(type, name, skipObjectFunctionPropertyAugment);
12488+
property = createUnionOrIntersectionProperty(type, name, skipObjectFunctionPropertyAugment, fromIsDiscriminantProperty);
1248912489
if (property) {
1249012490
const properties = skipObjectFunctionPropertyAugment ?
1249112491
type.propertyCacheWithoutObjectFunctionPropertyAugment ||= createSymbolTable() :
@@ -23219,7 +23219,7 @@ namespace ts {
2321923219

2322023220
function isDiscriminantProperty(type: Type | undefined, name: __String) {
2322123221
if (type && type.flags & TypeFlags.Union) {
23222-
const prop = getUnionOrIntersectionProperty(type as UnionType, name);
23222+
const prop = getUnionOrIntersectionProperty(type as UnionType, name, /* skipObjectFunctionPropertyAugment */ undefined, /* fromIsDiscriminantProperty */ true);
2322323223
if (prop && getCheckFlags(prop) & CheckFlags.SyntheticProperty) {
2322423224
if ((prop as TransientSymbol).isDiscriminantProperty === undefined) {
2322523225
(prop as TransientSymbol).isDiscriminantProperty =

‎tests/baselines/reference/_44856.js

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
//// [_44856.ts]
2+
// The expected rule for properties of union type (|) is
3+
// (1) must contain all required properties of at least one union member
4+
// (2) may contain additional properties that belong to any union member
5+
6+
{
7+
type A2 = {a: string; b: string};
8+
type A3 = {a: string; b: boolean; c: number};
9+
10+
{
11+
// THIS IS THE UNEXPECTED OUTLIER
12+
const b: A2|A3 = {a: '', b: '', c: 1}; // ❌ assignment strictly checked, extra prop from A3 not allowed
13+
}
14+
}
15+
16+
{
17+
type A2 = {a: string; b: string};
18+
type A3 = {a: string; b: boolean; c: boolean};
19+
type A4 = {a: string; b: number; c: number};
20+
21+
{
22+
// THIS IS THE UNEXPECTED OUTLIER
23+
const b: A2|A3 = {a: '', b: '', c: true}; // ❌ assignment strictly checked, extra prop from A3 not allowed
24+
}
25+
{
26+
// BEHAVING AS EXPECTED
27+
const b: A2|A3|A4 = {a: '', b: '', c: true}; // ✔ assignments allow extra props of other union types
28+
}
29+
}
30+
31+
{
32+
type A2 = {a: string; b: string};
33+
type A3 = {a: string; b: boolean; c: boolean};
34+
type A4 = {a: string; b: boolean; c: number};
35+
36+
{
37+
// THIS IS THE UNEXPECTED OUTLIER
38+
const b: A2|A3 = {a: '', b: '', c: true}; // ❌ assignment strictly checked, extra prop from A3 not allowed
39+
}
40+
{
41+
// BEHAVING AS EXPECTED
42+
const b: A2|A3|A4 = {a: '', b: '', c: true}; // ✔ assignments allow extra props of other union types
43+
}
44+
}
45+
46+
47+
//// [_44856.js]
48+
// The expected rule for properties of union type (|) is
49+
// (1) must contain all required properties of at least one union member
50+
// (2) may contain additional properties that belong to any union member
51+
{
52+
{
53+
// THIS IS THE UNEXPECTED OUTLIER
54+
var b = { a: '', b: '', c: 1 }; // ❌ assignment strictly checked, extra prop from A3 not allowed
55+
}
56+
}
57+
{
58+
{
59+
// THIS IS THE UNEXPECTED OUTLIER
60+
var b = { a: '', b: '', c: true }; // ❌ assignment strictly checked, extra prop from A3 not allowed
61+
}
62+
{
63+
// BEHAVING AS EXPECTED
64+
var b = { a: '', b: '', c: true }; // ✔ assignments allow extra props of other union types
65+
}
66+
}
67+
{
68+
{
69+
// THIS IS THE UNEXPECTED OUTLIER
70+
var b = { a: '', b: '', c: true }; // ❌ assignment strictly checked, extra prop from A3 not allowed
71+
}
72+
{
73+
// BEHAVING AS EXPECTED
74+
var b = { a: '', b: '', c: true }; // ✔ assignments allow extra props of other union types
75+
}
76+
}

‎tests/baselines/reference/_44856.symbols

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
=== tests/cases/compiler/_44856.ts ===
2+
// The expected rule for properties of union type (|) is
3+
// (1) must contain all required properties of at least one union member
4+
// (2) may contain additional properties that belong to any union member
5+
6+
{
7+
type A2 = {a: string; b: string};
8+
>A2 : Symbol(A2, Decl(_44856.ts, 4, 1))
9+
>a : Symbol(a, Decl(_44856.ts, 5, 15))
10+
>b : Symbol(b, Decl(_44856.ts, 5, 25))
11+
12+
type A3 = {a: string; b: boolean; c: number};
13+
>A3 : Symbol(A3, Decl(_44856.ts, 5, 37))
14+
>a : Symbol(a, Decl(_44856.ts, 6, 15))
15+
>b : Symbol(b, Decl(_44856.ts, 6, 25))
16+
>c : Symbol(c, Decl(_44856.ts, 6, 37))
17+
18+
{
19+
// THIS IS THE UNEXPECTED OUTLIER
20+
const b: A2|A3 = {a: '', b: '', c: 1}; // ❌ assignment strictly checked, extra prop from A3 not allowed
21+
>b : Symbol(b, Decl(_44856.ts, 10, 11))
22+
>A2 : Symbol(A2, Decl(_44856.ts, 4, 1))
23+
>A3 : Symbol(A3, Decl(_44856.ts, 5, 37))
24+
>a : Symbol(a, Decl(_44856.ts, 10, 24))
25+
>b : Symbol(b, Decl(_44856.ts, 10, 30))
26+
>c : Symbol(c, Decl(_44856.ts, 10, 37))
27+
}
28+
}
29+
30+
{
31+
type A2 = {a: string; b: string};
32+
>A2 : Symbol(A2, Decl(_44856.ts, 14, 1))
33+
>a : Symbol(a, Decl(_44856.ts, 15, 15))
34+
>b : Symbol(b, Decl(_44856.ts, 15, 25))
35+
36+
type A3 = {a: string; b: boolean; c: boolean};
37+
>A3 : Symbol(A3, Decl(_44856.ts, 15, 37))
38+
>a : Symbol(a, Decl(_44856.ts, 16, 15))
39+
>b : Symbol(b, Decl(_44856.ts, 16, 25))
40+
>c : Symbol(c, Decl(_44856.ts, 16, 37))
41+
42+
type A4 = {a: string; b: number; c: number};
43+
>A4 : Symbol(A4, Decl(_44856.ts, 16, 50))
44+
>a : Symbol(a, Decl(_44856.ts, 17, 15))
45+
>b : Symbol(b, Decl(_44856.ts, 17, 25))
46+
>c : Symbol(c, Decl(_44856.ts, 17, 36))
47+
48+
{
49+
// THIS IS THE UNEXPECTED OUTLIER
50+
const b: A2|A3 = {a: '', b: '', c: true}; // ❌ assignment strictly checked, extra prop from A3 not allowed
51+
>b : Symbol(b, Decl(_44856.ts, 21, 11))
52+
>A2 : Symbol(A2, Decl(_44856.ts, 14, 1))
53+
>A3 : Symbol(A3, Decl(_44856.ts, 15, 37))
54+
>a : Symbol(a, Decl(_44856.ts, 21, 24))
55+
>b : Symbol(b, Decl(_44856.ts, 21, 30))
56+
>c : Symbol(c, Decl(_44856.ts, 21, 37))
57+
}
58+
{
59+
// BEHAVING AS EXPECTED
60+
const b: A2|A3|A4 = {a: '', b: '', c: true}; // ✔ assignments allow extra props of other union types
61+
>b : Symbol(b, Decl(_44856.ts, 25, 11))
62+
>A2 : Symbol(A2, Decl(_44856.ts, 14, 1))
63+
>A3 : Symbol(A3, Decl(_44856.ts, 15, 37))
64+
>A4 : Symbol(A4, Decl(_44856.ts, 16, 50))
65+
>a : Symbol(a, Decl(_44856.ts, 25, 27))
66+
>b : Symbol(b, Decl(_44856.ts, 25, 33))
67+
>c : Symbol(c, Decl(_44856.ts, 25, 40))
68+
}
69+
}
70+
71+
{
72+
type A2 = {a: string; b: string};
73+
>A2 : Symbol(A2, Decl(_44856.ts, 29, 3))
74+
>a : Symbol(a, Decl(_44856.ts, 30, 15))
75+
>b : Symbol(b, Decl(_44856.ts, 30, 25))
76+
77+
type A3 = {a: string; b: boolean; c: boolean};
78+
>A3 : Symbol(A3, Decl(_44856.ts, 30, 37))
79+
>a : Symbol(a, Decl(_44856.ts, 31, 15))
80+
>b : Symbol(b, Decl(_44856.ts, 31, 25))
81+
>c : Symbol(c, Decl(_44856.ts, 31, 37))
82+
83+
type A4 = {a: string; b: boolean; c: number};
84+
>A4 : Symbol(A4, Decl(_44856.ts, 31, 50))
85+
>a : Symbol(a, Decl(_44856.ts, 32, 15))
86+
>b : Symbol(b, Decl(_44856.ts, 32, 25))
87+
>c : Symbol(c, Decl(_44856.ts, 32, 37))
88+
89+
{
90+
// THIS IS THE UNEXPECTED OUTLIER
91+
const b: A2|A3 = {a: '', b: '', c: true}; // ❌ assignment strictly checked, extra prop from A3 not allowed
92+
>b : Symbol(b, Decl(_44856.ts, 36, 11))
93+
>A2 : Symbol(A2, Decl(_44856.ts, 29, 3))
94+
>A3 : Symbol(A3, Decl(_44856.ts, 30, 37))
95+
>a : Symbol(a, Decl(_44856.ts, 36, 24))
96+
>b : Symbol(b, Decl(_44856.ts, 36, 30))
97+
>c : Symbol(c, Decl(_44856.ts, 36, 37))
98+
}
99+
{
100+
// BEHAVING AS EXPECTED
101+
const b: A2|A3|A4 = {a: '', b: '', c: true}; // ✔ assignments allow extra props of other union types
102+
>b : Symbol(b, Decl(_44856.ts, 40, 11))
103+
>A2 : Symbol(A2, Decl(_44856.ts, 29, 3))
104+
>A3 : Symbol(A3, Decl(_44856.ts, 30, 37))
105+
>A4 : Symbol(A4, Decl(_44856.ts, 31, 50))
106+
>a : Symbol(a, Decl(_44856.ts, 40, 27))
107+
>b : Symbol(b, Decl(_44856.ts, 40, 33))
108+
>c : Symbol(c, Decl(_44856.ts, 40, 40))
109+
}
110+
}
111+

‎tests/baselines/reference/_44856.types

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
=== tests/cases/compiler/_44856.ts ===
2+
// The expected rule for properties of union type (|) is
3+
// (1) must contain all required properties of at least one union member
4+
// (2) may contain additional properties that belong to any union member
5+
6+
{
7+
type A2 = {a: string; b: string};
8+
>A2 : { a: string; b: string; }
9+
>a : string
10+
>b : string
11+
12+
type A3 = {a: string; b: boolean; c: number};
13+
>A3 : { a: string; b: boolean; c: number; }
14+
>a : string
15+
>b : boolean
16+
>c : number
17+
18+
{
19+
// THIS IS THE UNEXPECTED OUTLIER
20+
const b: A2|A3 = {a: '', b: '', c: 1}; // ❌ assignment strictly checked, extra prop from A3 not allowed
21+
>b : { a: string; b: string; } | { a: string; b: boolean; c: number; }
22+
>{a: '', b: '', c: 1} : { a: string; b: string; c: number; }
23+
>a : string
24+
>'' : ""
25+
>b : string
26+
>'' : ""
27+
>c : number
28+
>1 : 1
29+
}
30+
}
31+
32+
{
33+
type A2 = {a: string; b: string};
34+
>A2 : { a: string; b: string; }
35+
>a : string
36+
>b : string
37+
38+
type A3 = {a: string; b: boolean; c: boolean};
39+
>A3 : { a: string; b: boolean; c: boolean; }
40+
>a : string
41+
>b : boolean
42+
>c : boolean
43+
44+
type A4 = {a: string; b: number; c: number};
45+
>A4 : { a: string; b: number; c: number; }
46+
>a : string
47+
>b : number
48+
>c : number
49+
50+
{
51+
// THIS IS THE UNEXPECTED OUTLIER
52+
const b: A2|A3 = {a: '', b: '', c: true}; // ❌ assignment strictly checked, extra prop from A3 not allowed
53+
>b : { a: string; b: string; } | { a: string; b: boolean; c: boolean; }
54+
>{a: '', b: '', c: true} : { a: string; b: string; c: true; }
55+
>a : string
56+
>'' : ""
57+
>b : string
58+
>'' : ""
59+
>c : true
60+
>true : true
61+
}
62+
{
63+
// BEHAVING AS EXPECTED
64+
const b: A2|A3|A4 = {a: '', b: '', c: true}; // ✔ assignments allow extra props of other union types
65+
>b : { a: string; b: string; } | { a: string; b: boolean; c: boolean; } | { a: string; b: number; c: number; }
66+
>{a: '', b: '', c: true} : { a: string; b: string; c: true; }
67+
>a : string
68+
>'' : ""
69+
>b : string
70+
>'' : ""
71+
>c : true
72+
>true : true
73+
}
74+
}
75+
76+
{
77+
type A2 = {a: string; b: string};
78+
>A2 : { a: string; b: string; }
79+
>a : string
80+
>b : string
81+
82+
type A3 = {a: string; b: boolean; c: boolean};
83+
>A3 : { a: string; b: boolean; c: boolean; }
84+
>a : string
85+
>b : boolean
86+
>c : boolean
87+
88+
type A4 = {a: string; b: boolean; c: number};
89+
>A4 : { a: string; b: boolean; c: number; }
90+
>a : string
91+
>b : boolean
92+
>c : number
93+
94+
{
95+
// THIS IS THE UNEXPECTED OUTLIER
96+
const b: A2|A3 = {a: '', b: '', c: true}; // ❌ assignment strictly checked, extra prop from A3 not allowed
97+
>b : { a: string; b: string; } | { a: string; b: boolean; c: boolean; }
98+
>{a: '', b: '', c: true} : { a: string; b: string; c: true; }
99+
>a : string
100+
>'' : ""
101+
>b : string
102+
>'' : ""
103+
>c : true
104+
>true : true
105+
}
106+
{
107+
// BEHAVING AS EXPECTED
108+
const b: A2|A3|A4 = {a: '', b: '', c: true}; // ✔ assignments allow extra props of other union types
109+
>b : { a: string; b: string; } | { a: string; b: boolean; c: boolean; } | { a: string; b: boolean; c: number; }
110+
>{a: '', b: '', c: true} : { a: string; b: string; c: true; }
111+
>a : string
112+
>'' : ""
113+
>b : string
114+
>'' : ""
115+
>c : true
116+
>true : true
117+
}
118+
}
119+

‎tests/cases/compiler/_44856.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// The expected rule for properties of union type (|) is
2+
// (1) must contain all required properties of at least one union member
3+
// (2) may contain additional properties that belong to any union member
4+
5+
{
6+
type A2 = {a: string; b: string};
7+
type A3 = {a: string; b: boolean; c: number};
8+
9+
{
10+
// THIS IS THE UNEXPECTED OUTLIER
11+
const b: A2|A3 = {a: '', b: '', c: 1}; // ❌ assignment strictly checked, extra prop from A3 not allowed
12+
}
13+
}
14+
15+
{
16+
type A2 = {a: string; b: string};
17+
type A3 = {a: string; b: boolean; c: boolean};
18+
type A4 = {a: string; b: number; c: number};
19+
20+
{
21+
// THIS IS THE UNEXPECTED OUTLIER
22+
const b: A2|A3 = {a: '', b: '', c: true}; // ❌ assignment strictly checked, extra prop from A3 not allowed
23+
}
24+
{
25+
// BEHAVING AS EXPECTED
26+
const b: A2|A3|A4 = {a: '', b: '', c: true}; // ✔ assignments allow extra props of other union types
27+
}
28+
}
29+
30+
{
31+
type A2 = {a: string; b: string};
32+
type A3 = {a: string; b: boolean; c: boolean};
33+
type A4 = {a: string; b: boolean; c: number};
34+
35+
{
36+
// THIS IS THE UNEXPECTED OUTLIER
37+
const b: A2|A3 = {a: '', b: '', c: true}; // ❌ assignment strictly checked, extra prop from A3 not allowed
38+
}
39+
{
40+
// BEHAVING AS EXPECTED
41+
const b: A2|A3|A4 = {a: '', b: '', c: true}; // ✔ assignments allow extra props of other union types
42+
}
43+
}

0 commit comments

Comments
 (0)