Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise discriminateTypeByDiscriminableItems function #53709

Merged
merged 5 commits into from
Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 31 additions & 30 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22607,41 +22607,41 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function getBestMatchingType(source: Type, target: UnionOrIntersectionType, isRelatedTo = compareTypesAssignable) {
return findMatchingDiscriminantType(source, target, isRelatedTo, /*skipPartial*/ true) ||
return findMatchingDiscriminantType(source, target, isRelatedTo) ||
findMatchingTypeReferenceOrTypeAliasReference(source, target) ||
findBestTypeForObjectLiteral(source, target) ||
findBestTypeForInvokable(source, target) ||
findMostOverlappyType(source, target);
}

function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue?: undefined, skipPartial?: boolean): Type | undefined;
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue: Type, skipPartial?: boolean): Type;
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue?: Type, skipPartial?: boolean) {
// undefined=unknown, true=discriminated, false=not discriminated
// The state of each type progresses from left to right. Discriminated types stop at 'true'.
const discriminable = target.types.map(_ => undefined) as (boolean | undefined)[];
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary) {
const types = target.types;
const include: Ternary[] = types.map(t => t.flags & TypeFlags.Primitive ? Ternary.False : Ternary.True);
for (const [getDiscriminatingType, propertyName] of discriminators) {
const targetProp = getUnionOrIntersectionProperty(target, propertyName);
if (skipPartial && targetProp && getCheckFlags(targetProp) & CheckFlags.ReadPartial) {
continue;
}
let i = 0;
for (const type of target.types) {
const targetType = getTypeOfPropertyOfType(type, propertyName);
if (targetType && related(getDiscriminatingType(), targetType)) {
discriminable[i] = discriminable[i] === undefined ? true : discriminable[i];
// If the remaining target types include at least one with a matching discriminant, eliminate those that
// have non-matching discriminants. This ensures that we ignore erroneous discriminators and gradually
// refine the target set without eliminating every constituent (which would lead to `never`).
let matched = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might call this excludeIndexes

for (let i = 0; i < types.length; i++) {
if (include[i]) {
const targetType = getTypeOfPropertyOfType(types[i], propertyName);
if (targetType && related(getDiscriminatingType(), targetType)) {
matched = true;
}
else {
include[i] = Ternary.Maybe;
}
}
else {
discriminable[i] = false;
}
// Turn each Ternary.Maybe into Ternary.False if there was a match. Otherwise, revert to Ternary.True.
for (let i = 0; i < types.length; i++) {
if (include[i] === Ternary.Maybe) {
include[i] = matched ? Ternary.False : Ternary.True;
}
i++;
}
}
const match = discriminable.indexOf(/*searchElement*/ true);
if (match === -1) {
return defaultValue;
}
return getUnionType(target.types.filter((_, index) => discriminable[index]));
const filtered = contains(include, Ternary.False) ? getUnionType(types.filter((_, i) => include[i])) : target;
return filtered.flags & TypeFlags.Never ? target : filtered;
}

/**
Expand Down Expand Up @@ -29276,8 +29276,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
s => [() => undefinedType, s.escapedName] as [() => Type, __String]
)
),
isTypeAssignableTo,
contextualType
isTypeAssignableTo
);
}

Expand All @@ -29293,8 +29292,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
s => [() => undefinedType, s.escapedName] as [() => Type, __String]
)
),
isTypeAssignableTo,
contextualType
isTypeAssignableTo
);
}

Expand Down Expand Up @@ -48739,7 +48737,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
MariaSolOs marked this conversation as resolved.
Show resolved Hide resolved

// Keep this up-to-date with the same logic within `getApparentTypeOfContextualType`, since they should behave similarly
function findMatchingDiscriminantType(source: Type, target: Type, isRelatedTo: (source: Type, target: Type) => Ternary, skipPartial?: boolean) {
function findMatchingDiscriminantType(source: Type, target: Type, isRelatedTo: (source: Type, target: Type) => Ternary) {
if (target.flags & TypeFlags.Union && source.flags & (TypeFlags.Intersection | TypeFlags.Object)) {
const match = getMatchingUnionConstituentForType(target as UnionType, source);
if (match) {
Expand All @@ -48749,7 +48747,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (sourceProperties) {
const sourcePropertiesFiltered = findDiscriminantProperties(sourceProperties, target);
if (sourcePropertiesFiltered) {
return discriminateTypeByDiscriminableItems(target as UnionType, map(sourcePropertiesFiltered, p => ([() => getTypeOfSymbol(p), p.escapedName] as [() => Type, __String])), isRelatedTo, /*defaultValue*/ undefined, skipPartial);
const discriminated = discriminateTypeByDiscriminableItems(target as UnionType, map(sourcePropertiesFiltered, p => ([() => getTypeOfSymbol(p), p.escapedName] as [() => Type, __String])), isRelatedTo);
if (discriminated !== target) {
return discriminated;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignmentCompatWithDiscriminatedUnion.ts(44,5): error TS2322: Type 'S' is not assignable to type 'T'.
Type 'S' is not assignable to type '{ a: 2; b: 3; }'.
Types of property 'a' are incompatible.
Type '0 | 2' is not assignable to type '2'.
Type '0' is not assignable to type '2'.
Type 'S' is not assignable to type '{ a: 0; b: 4 | 1; } | { a: 1; b: 2 | 4; }'.
Type 'S' is not assignable to type '{ a: 1; b: 2 | 4; }'.
Types of property 'a' are incompatible.
Type '0 | 2' is not assignable to type '1'.
Type '0' is not assignable to type '1'.
tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignmentCompatWithDiscriminatedUnion.ts(58,5): error TS2322: Type 'S' is not assignable to type 'T'.
Property 'c' is missing in type 'S' but required in type '{ a: 2; b: 4 | 3; c: string; }'.
Type 'S' is not assignable to type '{ a: 0; b: 4 | 1; } | { a: 2; b: 4 | 3; c: string; }'.
Property 'c' is missing in type 'S' but required in type '{ a: 2; b: 4 | 3; c: string; }'.
tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignmentCompatWithDiscriminatedUnion.ts(82,5): error TS2322: Type 'S' is not assignable to type 'T'.
Type 'S' is not assignable to type '{ a: N; b: N; c: 2; }'.
MariaSolOs marked this conversation as resolved.
Show resolved Hide resolved
Types of property 'c' are incompatible.
Type 'N' is not assignable to type '2'.
Type '0' is not assignable to type '2'.


==== tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignmentCompatWithDiscriminatedUnion.ts (3 errors) ====
Expand Down Expand Up @@ -59,10 +57,11 @@ tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignme
t = s;
~
!!! error TS2322: Type 'S' is not assignable to type 'T'.
!!! error TS2322: Type 'S' is not assignable to type '{ a: 2; b: 3; }'.
!!! error TS2322: Types of property 'a' are incompatible.
!!! error TS2322: Type '0 | 2' is not assignable to type '2'.
!!! error TS2322: Type '0' is not assignable to type '2'.
!!! error TS2322: Type 'S' is not assignable to type '{ a: 0; b: 4 | 1; } | { a: 1; b: 2 | 4; }'.
!!! error TS2322: Type 'S' is not assignable to type '{ a: 1; b: 2 | 4; }'.
!!! error TS2322: Types of property 'a' are incompatible.
!!! error TS2322: Type '0 | 2' is not assignable to type '1'.
!!! error TS2322: Type '0' is not assignable to type '1'.
}

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

Expand Down Expand Up @@ -107,10 +107,6 @@ tests/cases/conformance/types/typeRelationships/assignmentCompatibility/assignme
t = s;
~
!!! error TS2322: Type 'S' is not assignable to type 'T'.
!!! error TS2322: Type 'S' is not assignable to type '{ a: N; b: N; c: 2; }'.
!!! error TS2322: Types of property 'c' are incompatible.
!!! error TS2322: Type 'N' is not assignable to type '2'.
!!! error TS2322: Type '0' is not assignable to type '2'.
}

// https://github.com/Microsoft/TypeScript/issues/14865
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(83,5): erro
Object literal may only specify known properties, and 'b' does not exist in type 'Common | (Common & A)'.
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(93,5): error TS2322: Type '{ type: "A"; n: number; a: number; b: number; }' is not assignable to type 'CommonWithDisjointOverlappingOptionals'.
Object literal may only specify known properties, and 'b' does not exist in type 'Common | A'.
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(130,5): error TS2322: Type '"string"' is not assignable to type '"number"'.
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(136,5): error TS2322: Type '"string"' is not assignable to type '"number"'.
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(131,5): error TS2322: Type '{ type: "string"; autoIncrement: boolean; required: true; }' is not assignable to type 'Attribute'.
Object literal may only specify known properties, and 'autoIncrement' does not exist in type 'StringAttribute | OneToOneAttribute'.
tests/cases/compiler/excessPropertyCheckWithMultipleDiscriminants.ts(137,5): error TS2322: Type '{ type: "string"; autoIncrement: boolean; required: true; }' is not assignable to type 'Attribute2'.
Object literal may only specify known properties, and 'autoIncrement' does not exist in type 'StringAttribute'.
MariaSolOs marked this conversation as resolved.
Show resolved Hide resolved


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

const attributes2: Attribute2 = {
type: 'string',
~~~~
!!! error TS2322: Type '"string"' is not assignable to type '"number"'.
autoIncrement: true,
~~~~~~~~~~~~~
!!! error TS2322: Type '{ type: "string"; autoIncrement: boolean; required: true; }' is not assignable to type 'Attribute2'.
!!! error TS2322: Object literal may only specify known properties, and 'autoIncrement' does not exist in type 'StringAttribute'.
required: true,
};

6 changes: 4 additions & 2 deletions tests/baselines/reference/jsxComponentTypeErrors.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ tests/cases/compiler/jsxComponentTypeErrors.tsx(27,16): error TS2786: 'ClassComp
tests/cases/compiler/jsxComponentTypeErrors.tsx(28,16): error TS2786: 'MixedComponent' cannot be used as a JSX component.
Its element type 'ClassComponent | { type: string | undefined; }' is not a valid JSX element.
Type 'ClassComponent' is not assignable to type 'Element | ElementClass | null'.
Type 'ClassComponent' is not assignable to type 'ElementClass'.
Type 'ClassComponent' is not assignable to type 'Element | ElementClass'.
Type 'ClassComponent' is not assignable to type 'ElementClass'.
tests/cases/compiler/jsxComponentTypeErrors.tsx(37,16): error TS2786: 'obj.MemberFunctionComponent' cannot be used as a JSX component.
Its return type '{}' is not a valid JSX element.
Property 'type' is missing in type '{}' but required in type 'Element'.
Expand Down Expand Up @@ -79,7 +80,8 @@ tests/cases/compiler/jsxComponentTypeErrors.tsx(38,16): error TS2786: 'obj. Memb
!!! error TS2786: 'MixedComponent' cannot be used as a JSX component.
!!! error TS2786: Its element type 'ClassComponent | { type: string | undefined; }' is not a valid JSX element.
!!! error TS2786: Type 'ClassComponent' is not assignable to type 'Element | ElementClass | null'.
!!! error TS2786: Type 'ClassComponent' is not assignable to type 'ElementClass'.
!!! error TS2786: Type 'ClassComponent' is not assignable to type 'Element | ElementClass'.
!!! error TS2786: Type 'ClassComponent' is not assignable to type 'ElementClass'.

const obj = {
MemberFunctionComponent() {
Expand Down
10 changes: 4 additions & 6 deletions tests/baselines/reference/objectLiteralNormalization.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ tests/cases/conformance/expressions/objectLiterals/objectLiteralNormalization.ts
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; }'.
Type '{ c: true; }' is missing the following properties from type '{ a: number; b: string; c: boolean; }': a, b
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; }'.
Type '{ a: string; b: number; }' is not assignable to type '{ a?: undefined; b?: undefined; }'.
Types of property 'a' are incompatible.
Type 'string' is not assignable to type 'undefined'.
Types of property 'b' are incompatible.
Type 'number' is not assignable to type 'undefined'.
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; }'.
Property 'b' is missing in type '{ a: number; }' but required in type '{ a: number; b: number; }'.
tests/cases/conformance/expressions/objectLiterals/objectLiteralNormalization.ts(48,20): error TS2322: Type '2' is not assignable to type '1'.
Expand Down Expand Up @@ -44,9 +43,8 @@ tests/cases/conformance/expressions/objectLiterals/objectLiteralNormalization.ts
a2 = { a: "def", b: 20 }; // Error
~~
!!! error TS2322: Type '{ a: string; b: number; }' is not assignable to type '{ a: number; b: number; } | { a: string; b?: undefined; } | { a?: undefined; b?: undefined; }'.
!!! error TS2322: Type '{ a: string; b: number; }' is not assignable to type '{ a?: undefined; b?: undefined; }'.
!!! error TS2322: Types of property 'a' are incompatible.
!!! error TS2322: Type 'string' is not assignable to type 'undefined'.
!!! error TS2322: Types of property 'b' are incompatible.
!!! error TS2322: Type 'number' is not assignable to type 'undefined'.
a2 = { a: 1 }; // Error
~~
!!! error TS2322: Type '{ a: number; }' is not assignable to type '{ a: number; b: number; } | { a: string; b?: undefined; } | { a?: undefined; b?: undefined; }'.
Expand Down
19 changes: 19 additions & 0 deletions tests/cases/fourslash/completionsUnionStringLiteralProperty.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/// <reference path="fourslash.ts" />

//// type Foo = { a: 0, b: 'x' } | { a: 0, b: 'y' } | { a: 1, b: 'z' };
//// const foo: Foo = { a: 0, b: '/*1*/' }
////
//// type Bar = { a: 0, b: 'fx' } | { a: 0, b: 'fy' } | { a: 1, b: 'fz' };
//// const bar: Bar = { a: 0, b: 'f/*2*/' }
////
//// 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' };
//// const baz1: Baz = { z: '/*3*/' };
//// const baz2: Baz = { x: 0, z: '/*4*/' };
//// const baz3: Baz = { x: 0, y: 1, z: '/*5*/' };
//// const baz4: Baz = { x: 2, y: 1, z: '/*6*/' };
verify.completions({ marker: "1", triggerCharacter: "'", includes: [ "x", "y" ], excludes: [ "z" ] });
verify.completions({ marker: "2", triggerCharacter: "'", includes: [ "fx", "fy" ], excludes: [ "fz" ] });
verify.completions({ marker: "3", triggerCharacter: "'", includes: [ "a", "b", "c", "d" ] });
verify.completions({ marker: "4", triggerCharacter: "'", includes: [ "a", "b" ], excludes: [ "c", "d" ] });
verify.completions({ marker: "5", triggerCharacter: "'", includes: [ "b" ], excludes: [ "a", "c", "d" ] });
verify.completions({ marker: "6", triggerCharacter: "'", includes: [ "b", "d" ], excludes: [ "a", "c" ] });