Skip to content

Commit bec204c

Browse files
ahejlsbergMaria Solano
andauthored
Revise discriminateTypeByDiscriminableItems function (#53709)
Co-authored-by: Maria Solano <[email protected]>
1 parent 2db688e commit bec204c

6 files changed

+82
-62
lines changed

src/compiler/checker.ts

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22609,41 +22609,41 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
2260922609
}
2261022610

2261122611
function getBestMatchingType(source: Type, target: UnionOrIntersectionType, isRelatedTo = compareTypesAssignable) {
22612-
return findMatchingDiscriminantType(source, target, isRelatedTo, /*skipPartial*/ true) ||
22612+
return findMatchingDiscriminantType(source, target, isRelatedTo) ||
2261322613
findMatchingTypeReferenceOrTypeAliasReference(source, target) ||
2261422614
findBestTypeForObjectLiteral(source, target) ||
2261522615
findBestTypeForInvokable(source, target) ||
2261622616
findMostOverlappyType(source, target);
2261722617
}
2261822618

22619-
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue?: undefined, skipPartial?: boolean): Type | undefined;
22620-
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue: Type, skipPartial?: boolean): Type;
22621-
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue?: Type, skipPartial?: boolean) {
22622-
// undefined=unknown, true=discriminated, false=not discriminated
22623-
// The state of each type progresses from left to right. Discriminated types stop at 'true'.
22624-
const discriminable = target.types.map(_ => undefined) as (boolean | undefined)[];
22619+
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary) {
22620+
const types = target.types;
22621+
const include: Ternary[] = types.map(t => t.flags & TypeFlags.Primitive ? Ternary.False : Ternary.True);
2262522622
for (const [getDiscriminatingType, propertyName] of discriminators) {
22626-
const targetProp = getUnionOrIntersectionProperty(target, propertyName);
22627-
if (skipPartial && targetProp && getCheckFlags(targetProp) & CheckFlags.ReadPartial) {
22628-
continue;
22629-
}
22630-
let i = 0;
22631-
for (const type of target.types) {
22632-
const targetType = getTypeOfPropertyOfType(type, propertyName);
22633-
if (targetType && related(getDiscriminatingType(), targetType)) {
22634-
discriminable[i] = discriminable[i] === undefined ? true : discriminable[i];
22623+
// If the remaining target types include at least one with a matching discriminant, eliminate those that
22624+
// have non-matching discriminants. This ensures that we ignore erroneous discriminators and gradually
22625+
// refine the target set without eliminating every constituent (which would lead to `never`).
22626+
let matched = false;
22627+
for (let i = 0; i < types.length; i++) {
22628+
if (include[i]) {
22629+
const targetType = getTypeOfPropertyOfType(types[i], propertyName);
22630+
if (targetType && related(getDiscriminatingType(), targetType)) {
22631+
matched = true;
22632+
}
22633+
else {
22634+
include[i] = Ternary.Maybe;
22635+
}
2263522636
}
22636-
else {
22637-
discriminable[i] = false;
22637+
}
22638+
// Turn each Ternary.Maybe into Ternary.False if there was a match. Otherwise, revert to Ternary.True.
22639+
for (let i = 0; i < types.length; i++) {
22640+
if (include[i] === Ternary.Maybe) {
22641+
include[i] = matched ? Ternary.False : Ternary.True;
2263822642
}
22639-
i++;
2264022643
}
2264122644
}
22642-
const match = discriminable.indexOf(/*searchElement*/ true);
22643-
if (match === -1) {
22644-
return defaultValue;
22645-
}
22646-
return getUnionType(target.types.filter((_, index) => discriminable[index]));
22645+
const filtered = contains(include, Ternary.False) ? getUnionType(types.filter((_, i) => include[i])) : target;
22646+
return filtered.flags & TypeFlags.Never ? target : filtered;
2264722647
}
2264822648

2264922649
/**
@@ -29290,8 +29290,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
2929029290
s => [() => undefinedType, s.escapedName] as [() => Type, __String]
2929129291
)
2929229292
),
29293-
isTypeAssignableTo,
29294-
contextualType
29293+
isTypeAssignableTo
2929529294
);
2929629295
}
2929729296

@@ -29307,8 +29306,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
2930729306
s => [() => undefinedType, s.escapedName] as [() => Type, __String]
2930829307
)
2930929308
),
29310-
isTypeAssignableTo,
29311-
contextualType
29309+
isTypeAssignableTo
2931229310
);
2931329311
}
2931429312

@@ -48753,7 +48751,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
4875348751
}
4875448752

4875548753
// Keep this up-to-date with the same logic within `getApparentTypeOfContextualType`, since they should behave similarly
48756-
function findMatchingDiscriminantType(source: Type, target: Type, isRelatedTo: (source: Type, target: Type) => Ternary, skipPartial?: boolean) {
48754+
function findMatchingDiscriminantType(source: Type, target: Type, isRelatedTo: (source: Type, target: Type) => Ternary) {
4875748755
if (target.flags & TypeFlags.Union && source.flags & (TypeFlags.Intersection | TypeFlags.Object)) {
4875848756
const match = getMatchingUnionConstituentForType(target as UnionType, source);
4875948757
if (match) {
@@ -48763,7 +48761,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
4876348761
if (sourceProperties) {
4876448762
const sourcePropertiesFiltered = findDiscriminantProperties(sourceProperties, target);
4876548763
if (sourcePropertiesFiltered) {
48766-
return discriminateTypeByDiscriminableItems(target as UnionType, map(sourcePropertiesFiltered, p => ([() => getTypeOfSymbol(p), p.escapedName] as [() => Type, __String])), isRelatedTo, /*defaultValue*/ undefined, skipPartial);
48764+
const discriminated = discriminateTypeByDiscriminableItems(target as UnionType, map(sourcePropertiesFiltered, p => ([() => getTypeOfSymbol(p), p.escapedName] as [() => Type, __String])), isRelatedTo);
48765+
if (discriminated !== target) {
48766+
return discriminated;
48767+
}
4876748768
}
4876848769
}
4876948770
}

tests/baselines/reference/assignmentCompatWithDiscriminatedUnion.errors.txt

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignmentCompatWithDiscriminatedUnion.ts(44,5): error TS2322: Type 'S' is not assignable to type 'T'.
2-
Type 'S' is not assignable to type '{ a: 2; b: 3; }'.
3-
Types of property 'a' are incompatible.
4-
Type '0 | 2' is not assignable to type '2'.
5-
Type '0' is not assignable to type '2'.
2+
Type 'S' is not assignable to type '{ a: 0; b: 4 | 1; } | { a: 1; b: 2 | 4; }'.
3+
Type 'S' is not assignable to type '{ a: 1; b: 2 | 4; }'.
4+
Types of property 'a' are incompatible.
5+
Type '0 | 2' is not assignable to type '1'.
6+
Type '0' is not assignable to type '1'.
67
tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignmentCompatWithDiscriminatedUnion.ts(58,5): error TS2322: Type 'S' is not assignable to type 'T'.
7-
Property 'c' is missing in type 'S' but required in type '{ a: 2; b: 4 | 3; c: string; }'.
8+
Type 'S' is not assignable to type '{ a: 0; b: 4 | 1; } | { a: 2; b: 4 | 3; c: string; }'.
9+
Property 'c' is missing in type 'S' but required in type '{ a: 2; b: 4 | 3; c: string; }'.
810
tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignmentCompatWithDiscriminatedUnion.ts(82,5): error TS2322: Type 'S' is not assignable to type 'T'.
9-
Type 'S' is not assignable to type '{ a: N; b: N; c: 2; }'.
10-
Types of property 'c' are incompatible.
11-
Type 'N' is not assignable to type '2'.
12-
Type '0' is not assignable to type '2'.
1311

1412

1513
==== tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignmentCompatWithDiscriminatedUnion.ts (3 errors) ====
@@ -59,10 +57,11 @@ tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignme
5957
t = s;
6058
~
6159
!!! error TS2322: Type 'S' is not assignable to type 'T'.
62-
!!! error TS2322: Type 'S' is not assignable to type '{ a: 2; b: 3; }'.
63-
!!! error TS2322: Types of property 'a' are incompatible.
64-
!!! error TS2322: Type '0 | 2' is not assignable to type '2'.
65-
!!! error TS2322: Type '0' is not assignable to type '2'.
60+
!!! error TS2322: Type 'S' is not assignable to type '{ a: 0; b: 4 | 1; } | { a: 1; b: 2 | 4; }'.
61+
!!! error TS2322: Type 'S' is not assignable to type '{ a: 1; b: 2 | 4; }'.
62+
!!! error TS2322: Types of property 'a' are incompatible.
63+
!!! error TS2322: Type '0 | 2' is not assignable to type '1'.
64+
!!! error TS2322: Type '0' is not assignable to type '1'.
6665
}
6766

6867
// Unmatched non-discriminants
@@ -79,7 +78,8 @@ tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignme
7978
t = s;
8079
~
8180
!!! error TS2322: Type 'S' is not assignable to type 'T'.
82-
!!! error TS2322: Property 'c' is missing in type 'S' but required in type '{ a: 2; b: 4 | 3; c: string; }'.
81+
!!! error TS2322: Type 'S' is not assignable to type '{ a: 0; b: 4 | 1; } | { a: 2; b: 4 | 3; c: string; }'.
82+
!!! error TS2322: Property 'c' is missing in type 'S' but required in type '{ a: 2; b: 4 | 3; c: string; }'.
8383
!!! related TS2728 tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignmentCompatWithDiscriminatedUnion.ts:52:36: 'c' is declared here.
8484
}
8585

@@ -107,10 +107,6 @@ tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignme
107107
t = s;
108108
~
109109
!!! error TS2322: Type 'S' is not assignable to type 'T'.
110-
!!! error TS2322: Type 'S' is not assignable to type '{ a: N; b: N; c: 2; }'.
111-
!!! error TS2322: Types of property 'c' are incompatible.
112-
!!! error TS2322: Type 'N' is not assignable to type '2'.
113-
!!! error TS2322: Type '0' is not assignable to type '2'.
114110
}
115111

116112
// https://github.com/Microsoft/TypeScript/issues/14865

tests/baselines/reference/excessPropertyCheckWithMultipleDiscriminants.errors.txt

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(83,5): erro
1010
Object literal may only specify known properties, and 'b' does not exist in type 'Common | (Common & A)'.
1111
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(93,5): error TS2322: Type '{ type: "A"; n: number; a: number; b: number; }' is not assignable to type 'CommonWithDisjointOverlappingOptionals'.
1212
Object literal may only specify known properties, and 'b' does not exist in type 'Common | A'.
13-
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(130,5): error TS2322: Type '"string"' is not assignable to type '"number"'.
14-
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(136,5): error TS2322: Type '"string"' is not assignable to type '"number"'.
13+
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(131,5): error TS2322: Type '{ type: "string"; autoIncrement: boolean; required: true; }' is not assignable to type 'Attribute'.
14+
Object literal may only specify known properties, and 'autoIncrement' does not exist in type 'StringAttribute | OneToOneAttribute'.
15+
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(137,5): error TS2322: Type '{ type: "string"; autoIncrement: boolean; required: true; }' is not assignable to type 'Attribute2'.
16+
Object literal may only specify known properties, and 'autoIncrement' does not exist in type 'StringAttribute'.
1517

1618

1719
==== tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts (8 errors) ====
@@ -163,17 +165,19 @@ tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(136,5): err
163165
// both should error due to excess properties
164166
const attributes: Attribute = {
165167
type: 'string',
166-
~~~~
167-
!!! error TS2322: Type '"string"' is not assignable to type '"number"'.
168168
autoIncrement: true,
169+
~~~~~~~~~~~~~
170+
!!! error TS2322: Type '{ type: "string"; autoIncrement: boolean; required: true; }' is not assignable to type 'Attribute'.
171+
!!! error TS2322: Object literal may only specify known properties, and 'autoIncrement' does not exist in type 'StringAttribute | OneToOneAttribute'.
169172
required: true,
170173
};
171174

172175
const attributes2: Attribute2 = {
173176
type: 'string',
174-
~~~~
175-
!!! error TS2322: Type '"string"' is not assignable to type '"number"'.
176177
autoIncrement: true,
178+
~~~~~~~~~~~~~
179+
!!! error TS2322: Type '{ type: "string"; autoIncrement: boolean; required: true; }' is not assignable to type 'Attribute2'.
180+
!!! error TS2322: Object literal may only specify known properties, and 'autoIncrement' does not exist in type 'StringAttribute'.
177181
required: true,
178182
};
179183

tests/baselines/reference/jsxComponentTypeErrors.errors.txt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ tests/cases/compiler/jsxComponentTypeErrors.tsx(27,16): error TS2786: 'ClassComp
1717
tests/cases/compiler/jsxComponentTypeErrors.tsx(28,16): error TS2786: 'MixedComponent' cannot be used as a JSX component.
1818
Its element type 'ClassComponent | { type: string | undefined; }' is not a valid JSX element.
1919
Type 'ClassComponent' is not assignable to type 'Element | ElementClass | null'.
20-
Type 'ClassComponent' is not assignable to type 'ElementClass'.
20+
Type 'ClassComponent' is not assignable to type 'Element | ElementClass'.
21+
Type 'ClassComponent' is not assignable to type 'ElementClass'.
2122
tests/cases/compiler/jsxComponentTypeErrors.tsx(37,16): error TS2786: 'obj.MemberFunctionComponent' cannot be used as a JSX component.
2223
Its return type '{}' is not a valid JSX element.
2324
Property 'type' is missing in type '{}' but required in type 'Element'.
@@ -79,7 +80,8 @@ tests/cases/compiler/jsxComponentTypeErrors.tsx(38,16): error TS2786: 'obj. Memb
7980
!!! error TS2786: 'MixedComponent' cannot be used as a JSX component.
8081
!!! error TS2786: Its element type 'ClassComponent | { type: string | undefined; }' is not a valid JSX element.
8182
!!! error TS2786: Type 'ClassComponent' is not assignable to type 'Element | ElementClass | null'.
82-
!!! error TS2786: Type 'ClassComponent' is not assignable to type 'ElementClass'.
83+
!!! error TS2786: Type 'ClassComponent' is not assignable to type 'Element | ElementClass'.
84+
!!! error TS2786: Type 'ClassComponent' is not assignable to type 'ElementClass'.
8385

8486
const obj = {
8587
MemberFunctionComponent() {

tests/baselines/reference/objectLiteralNormalization.errors.txt

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@ tests/cases/conformance/expressions/objectLiterals/objectLiteralNormalization.ts
55
tests/cases/conformance/expressions/objectLiterals/objectLiteralNormalization.ts(9,1): error TS2322: Type '{ c: true; }' is not assignable to type '{ a: number; b?: undefined; c?: undefined; } | { a: number; b: string; c?: undefined; } | { a: number; b: string; c: boolean; }'.
66
Type '{ c: true; }' is missing the following properties from type '{ a: number; b: string; c: boolean; }': a, b
77
tests/cases/conformance/expressions/objectLiterals/objectLiteralNormalization.ts(17,1): error TS2322: Type '{ a: string; b: number; }' is not assignable to type '{ a: number; b: number; } | { a: string; b?: undefined; } | { a?: undefined; b?: undefined; }'.
8-
Type '{ a: string; b: number; }' is not assignable to type '{ a?: undefined; b?: undefined; }'.
9-
Types of property 'a' are incompatible.
10-
Type 'string' is not assignable to type 'undefined'.
8+
Types of property 'b' are incompatible.
9+
Type 'number' is not assignable to type 'undefined'.
1110
tests/cases/conformance/expressions/objectLiterals/objectLiteralNormalization.ts(18,1): error TS2322: Type '{ a: number; }' is not assignable to type '{ a: number; b: number; } | { a: string; b?: undefined; } | { a?: undefined; b?: undefined; }'.
1211
Property 'b' is missing in type '{ a: number; }' but required in type '{ a: number; b: number; }'.
1312
tests/cases/conformance/expressions/objectLiterals/objectLiteralNormalization.ts(48,20): error TS2322: Type '2' is not assignable to type '1'.
@@ -44,9 +43,8 @@ tests/cases/conformance/expressions/objectLiterals/objectLiteralNormalization.ts
4443
a2 = { a: "def", b: 20 }; // Error
4544
~~
4645
!!! error TS2322: Type '{ a: string; b: number; }' is not assignable to type '{ a: number; b: number; } | { a: string; b?: undefined; } | { a?: undefined; b?: undefined; }'.
47-
!!! error TS2322: Type '{ a: string; b: number; }' is not assignable to type '{ a?: undefined; b?: undefined; }'.
48-
!!! error TS2322: Types of property 'a' are incompatible.
49-
!!! error TS2322: Type 'string' is not assignable to type 'undefined'.
46+
!!! error TS2322: Types of property 'b' are incompatible.
47+
!!! error TS2322: Type 'number' is not assignable to type 'undefined'.
5048
a2 = { a: 1 }; // Error
5149
~~
5250
!!! error TS2322: Type '{ a: number; }' is not assignable to type '{ a: number; b: number; } | { a: string; b?: undefined; } | { a?: undefined; b?: undefined; }'.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
//// type Foo = { a: 0, b: 'x' } | { a: 0, b: 'y' } | { a: 1, b: 'z' };
4+
//// const foo: Foo = { a: 0, b: '/*1*/' }
5+
////
6+
//// type Bar = { a: 0, b: 'fx' } | { a: 0, b: 'fy' } | { a: 1, b: 'fz' };
7+
//// const bar: Bar = { a: 0, b: 'f/*2*/' }
8+
////
9+
//// type Baz = { x: 0, y: 0, z: 'a' } | { x: 0, y: 1, z: 'b' } | { x: 1, y: 0, z: 'c' } | { x: 1, y: 1, z: 'd' };
10+
//// const baz1: Baz = { z: '/*3*/' };
11+
//// const baz2: Baz = { x: 0, z: '/*4*/' };
12+
//// const baz3: Baz = { x: 0, y: 1, z: '/*5*/' };
13+
//// const baz4: Baz = { x: 2, y: 1, z: '/*6*/' };
14+
verify.completions({ marker: "1", triggerCharacter: "'", includes: [ "x", "y" ], excludes: [ "z" ] });
15+
verify.completions({ marker: "2", triggerCharacter: "'", includes: [ "fx", "fy" ], excludes: [ "fz" ] });
16+
verify.completions({ marker: "3", triggerCharacter: "'", includes: [ "a", "b", "c", "d" ] });
17+
verify.completions({ marker: "4", triggerCharacter: "'", includes: [ "a", "b" ], excludes: [ "c", "d" ] });
18+
verify.completions({ marker: "5", triggerCharacter: "'", includes: [ "b" ], excludes: [ "a", "c", "d" ] });
19+
verify.completions({ marker: "6", triggerCharacter: "'", includes: [ "b", "d" ], excludes: [ "a", "c" ] });

0 commit comments

Comments
 (0)