Skip to content

Attempt to fix #9919 - Intersecting discriminated union breaks type narrowing #9939

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Attempt to fix #9919 - Intersecting discriminated union breaks type narrowing #9939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
10ca5e4
Fixed type narrowing when discrimited union is intersected
gcnew Jul 25, 2016
5eec3c7
Added tests for discriminated intersection types
gcnew Jul 25, 2016
32f0d5e
Correctly narrow any blending of discriminated unions and intersections
gcnew Jul 26, 2016
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
34 changes: 27 additions & 7 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7800,9 +7800,23 @@ namespace ts {
}

function filterType(type: Type, f: (t: Type) => boolean): Type {
return type.flags & TypeFlags.Union ?
getUnionType(filter((<UnionType>type).types, f)) :
f(type) ? type : neverType;
if (type.flags & TypeFlags.Intersection) {
const filteredSubtypes: Type[] = [];
for (const subType of (<IntersectionType> type).types) {
const filtered = filterType(subType, f);
if (filtered === neverType) {
return neverType;
}
filteredSubtypes.push(filtered);
}
return getIntersectionType(filteredSubtypes);
}
if (type.flags & TypeFlags.Union) {
return getUnionType(
map((<UnionType>type).types, t => filterType(t, f))
);
}
return f(type) ? type : neverType;
}

function getFlowTypeOfReference(reference: Node, declaredType: Type, assumeInitialized: boolean, includeOuterFunctions: boolean) {
Expand Down Expand Up @@ -8095,7 +8109,7 @@ namespace ts {
}
Copy link
Member

Choose a reason for hiding this comment

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

You don't want to narrow if the original type was a union? That doesn't sound right.

Copy link
Contributor Author

@gcnew gcnew Jul 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, maybe the better way to write it is:

if (!propType || (!isStringLiteralUnionType(propType) && (type.flags & TypeFlags.UnionOrIntersection) === 0))

i.e. if neither the discrimination property is a string-literal-union, nor the parent type is a union or intersection. I guess I made it unintelligible by applying De Morgan's law.

const propName = propAccess.name.text;
const propType = getTypeOfPropertyOfType(type, propName);
if (!propType || !isStringLiteralUnionType(propType)) {
if (!propType || !(isStringLiteralUnionType(propType) || (type.flags & TypeFlags.UnionOrIntersection))) {
return type;
}
const discriminantType = value.kind === SyntaxKind.StringLiteral ? getStringLiteralTypeForText((<StringLiteral>value).text) : checkExpression(value);
Expand All @@ -8106,7 +8120,10 @@ namespace ts {
assumeTrue = !assumeTrue;
}
if (assumeTrue) {
return filterType(type, t => areTypesComparable(getTypeOfPropertyOfType(t, propName), discriminantType));
return filterType(type, t => {
const propType = getTypeOfPropertyOfType(t, propName);
return !propType || areTypesComparable(propType, discriminantType);
});
}
if (discriminantType.flags & TypeFlags.StringLiteral) {
return filterType(type, t => getTypeOfPropertyOfType(t, propName) !== discriminantType);
Expand All @@ -8121,7 +8138,7 @@ namespace ts {
}
const propName = (<PropertyAccessExpression>switchStatement.expression).name.text;
const propType = getTypeOfPropertyOfType(type, propName);
if (!propType || !isStringLiteralUnionType(propType)) {
if (!propType || !(isStringLiteralUnionType(propType) || (type.flags & TypeFlags.UnionOrIntersection))) {
return type;
}
const switchTypes = getSwitchClauseTypes(switchStatement);
Expand All @@ -8132,7 +8149,10 @@ namespace ts {
const hasDefaultClause = clauseStart === clauseEnd || contains(clauseTypes, undefined);
const caseTypes = hasDefaultClause ? filter(clauseTypes, t => !!t) : clauseTypes;
const discriminantType = caseTypes.length ? getUnionType(caseTypes) : undefined;
const caseType = discriminantType && filterType(type, t => isTypeComparableTo(discriminantType, getTypeOfPropertyOfType(t, propName)));
const caseType = discriminantType && filterType(type, t => {
const propType = getTypeOfPropertyOfType(t, propName);
return !propType || isTypeComparableTo(discriminantType, propType);
});
if (!hasDefaultClause) {
return caseType;
}
Expand Down
3 changes: 2 additions & 1 deletion src/services/formatting/smartIndenter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,8 @@ namespace ts.formatting {

return Value.Unknown;

function getStartingExpression(node: PropertyAccessExpression | CallExpression | ElementAccessExpression) {
// inferred type is (correctly) `never` as all known node kinds are handled, thus resulting in an infinite loop
function getStartingExpression(node: PropertyAccessExpression | CallExpression | ElementAccessExpression): LeftHandSideExpression {
while (true) {
switch (node.kind) {
case SyntaxKind.CallExpression:
Expand Down
161 changes: 161 additions & 0 deletions tests/baselines/reference/discriminatedUnionTypesWithIntersection.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
tests/cases/conformance/types/union/discriminatedUnionTypesWithIntersection.ts(64,18): error TS2339: Property 'str' does not exist on type 'never'.
tests/cases/conformance/types/union/discriminatedUnionTypesWithIntersection.ts(75,26): error TS2339: Property 'str' does not exist on type 'never'.
tests/cases/conformance/types/union/discriminatedUnionTypesWithIntersection.ts(82,27): error TS2339: Property 'num' does not exist on type 'never'.
tests/cases/conformance/types/union/discriminatedUnionTypesWithIntersection.ts(93,31): error TS2339: Property 'num' does not exist on type 'never'.
tests/cases/conformance/types/union/discriminatedUnionTypesWithIntersection.ts(97,26): error TS2339: Property 'str' does not exist on type 'never'.


==== tests/cases/conformance/types/union/discriminatedUnionTypesWithIntersection.ts (5 errors) ====
type WithX = {
x: string;
}

type BoxedValue = { kind: 'int', num: number }
| { kind: 'string', str: string }

type BoxIntersection = BoxedValue & WithX

type BoxInline = { kind: 'int', num: number } & WithX
| { kind: 'string', str: string } & WithX

function getValueAsString_inline_if(value: BoxInline): string {
if (value.kind === 'int') {
value; // { kind: "int", num: number } & { x: string }
return '' + value.num;
}

value; // { kind: "string", str: string } & { x: string }
return value.str;
}

function getValueAsString_inline_switch(value: BoxInline): string {
switch (value.kind) {
case 'int':
value; // { kind: "int", num: number } & { x: string }
return '' + value.num;

case 'string':
value; // { kind: "string", str: string } & { x: string }
return value.str;
}
}

function getValueAsString_if(value: BoxIntersection): string {
if (value.kind === 'int') {
value; // { kind: "int", num: number } & { x: string }
return '' + value.num;
}

value; // { kind: "string", str: string } & { x: string }
return value.str;
}

function getValueAsString_switch(value: BoxIntersection): string {
switch (value.kind) {
case 'int':
value; // { kind: "int", num: number } & { x: string }
return '' + value.num;

case 'string':
value; // { kind: "string", str: string } & { x: string }
return value.str;
}
}

function getValueAsString_if_fixed(value: BoxIntersection & { kind: 'int' }): string {
if (value.kind === 'int') {
value; // { kind: "int", num: number } & { x: string } & { kind: "number" }
return '' + value.num;
}

value; // never
return value.str;
~~~
!!! error TS2339: Property 'str' does not exist on type 'never'.
}

function getValueAsString_switch_fixed(value: BoxIntersection & { kind: 'int' }): string {
switch (value.kind) {
case 'int':
value; // { kind: "int", num: number } & { x: string } & { kind: "number" }
return '' + value.num;

case 'string':
value; // never
return value.str;
~~~
!!! error TS2339: Property 'str' does not exist on type 'never'.
}
}

function getValueAsString_if_never(value: BoxIntersection & { kind: number }): string {
if (value.kind === 'int') {
value; // never
return '' + value.num;
~~~
!!! error TS2339: Property 'num' does not exist on type 'never'.
}

value; // { kind: "string", str: string } & { x: string } & { kind: "number" }
return value.str;
}

function getValueAsString_switch_never(value: BoxIntersection & { kind: number }): string {
switch (value.kind) {
case 'int':
value; // never
return '' + value.num;
~~~
!!! error TS2339: Property 'num' does not exist on type 'never'.

case 'string':
value; // never
return value.str;
~~~
!!! error TS2339: Property 'str' does not exist on type 'never'.
}
}

type Ext = { strVal: string } & { kind: 'int' }
| { intVal: string } & { kind: 'string' }

type Boxed2 = { kind: 'null' }
| BoxIntersection & Ext;

function arbitraryNesting_if(value: Boxed2): void {
if (value.kind === 'null') {
return;
}

if (value.kind === 'int') {
value; // { kind: 'int', num: number } & { x: string; }
value.x;
value.num;
value.strVal;
return;
}

value; // { kind: 'string', str: string } & { x: string } & { intVal: string } & { kind: 'string' }
value.x;
value.str;
value.intVal;
}

function arbitraryNesting_switch(value: Boxed2): void {
switch (value.kind) {
case 'null':
return;

case 'int':
value.x;
value.num;
value.strVal;
return;

case 'string':
value.x;
value.str;
value.intVal;
}
}

Loading