Skip to content

Let primitive types discriminate a union of objects #10

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

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ interface ActiveLabel {
export function getModuleInstanceState(node: ModuleDeclaration, visited?: Map<number, ModuleInstanceState | undefined>): ModuleInstanceState {
if (node.body && !node.body.parent) {
// getModuleInstanceStateForAliasTarget needs to walk up the parent chain, so parent pointers must be set on this tree already
setParent(node.body, node);
setParent(node.body as ModuleDeclaration["parent"], node);
setParentRecursive(node.body, /*incremental*/ false);
}
return node.body ? getModuleInstanceStateCached(node.body, visited) : ModuleInstanceState.Instantiated;
Expand Down
28 changes: 22 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15050,6 +15050,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (isLiteralType(type) || isPatternLiteralType(type)) {
checkFlags |= CheckFlags.HasLiteralType;
}
if (type.flags & TypeFlags.Primitive) {
checkFlags |= CheckFlags.HasPrimitiveType;
}
if (type.flags & TypeFlags.Never && type !== uniqueLiteralType) {
checkFlags |= CheckFlags.HasNeverType;
}
Expand Down Expand Up @@ -27281,16 +27284,29 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return false;
}

function isDiscriminantProperty(type: Type | undefined, name: __String) {
// The flag `considerNonUniformPrimitivePropDiscriminant`, when enabled, introduces a new criterion for a property to be considered discriminant:
// 1 The property must be non-uniform
// 2 The property must include at least one primitive type as a possible type
function isDiscriminantProperty(type: Type | undefined, name: __String, considerNonUniformPrimitivePropDiscriminant: boolean = false) {
if (type && type.flags & TypeFlags.Union) {
const prop = getUnionOrIntersectionProperty(type as UnionType, name);
if (prop && getCheckFlags(prop) & CheckFlags.SyntheticProperty) {
const propType = getTypeOfSymbol(prop);
// NOTE: cast to TransientSymbol should be safe because only TransientSymbols can have CheckFlags.SyntheticProperty
if ((prop as TransientSymbol).links.isDiscriminantProperty === undefined) {
(prop as TransientSymbol).links.isDiscriminantProperty = ((prop as TransientSymbol).links.checkFlags & CheckFlags.Discriminant) === CheckFlags.Discriminant &&
!isGenericType(getTypeOfSymbol(prop));
const transientSymbol = prop as TransientSymbol;
transientSymbol.links.isDiscriminantProperty ??= new Map();

let isDiscriminant = transientSymbol.links.isDiscriminantProperty.get(considerNonUniformPrimitivePropDiscriminant);

if (isDiscriminant === undefined) {
isDiscriminant = (((transientSymbol.links.checkFlags & CheckFlags.Discriminant) === CheckFlags.Discriminant)
|| !!(considerNonUniformPrimitivePropDiscriminant && (transientSymbol.links.checkFlags & (CheckFlags.HasNonUniformType | CheckFlags.HasPrimitiveType))))
&& !isGenericType(propType);

transientSymbol.links.isDiscriminantProperty.set(considerNonUniformPrimitivePropDiscriminant, isDiscriminant);
}
return !!(prop as TransientSymbol).links.isDiscriminantProperty;

return isDiscriminant;
}
}
return false;
Expand Down Expand Up @@ -28858,7 +28874,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const name = getAccessedPropertyName(access);
if (name) {
const type = declaredType.flags & TypeFlags.Union && isTypeSubsetOf(computedType, declaredType) ? declaredType : computedType;
if (isDiscriminantProperty(type, name)) {
if (isDiscriminantProperty(type, name, /*considerNonUniformPrimitivePropDiscriminant*/ true)) {
return access;
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2756,7 +2756,7 @@ export interface ArrowFunction extends Expression, FunctionLikeDeclarationBase,
readonly modifiers?: NodeArray<Modifier>;
readonly equalsGreaterThanToken: EqualsGreaterThanToken;
readonly body: ConciseBody;
readonly name: never;
readonly name?: never;
}

// The text property of a LiteralExpression stores the interpreted value of the literal in text form. For a StringLiteral,
Expand Down Expand Up @@ -6020,7 +6020,7 @@ export interface SymbolLinks {
leftSpread?: Symbol; // Left source for synthetic spread property
rightSpread?: Symbol; // Right source for synthetic spread property
syntheticOrigin?: Symbol; // For a property on a mapped or spread type, points back to the original property
isDiscriminantProperty?: boolean; // True if discriminant synthetic property
isDiscriminantProperty?: Map<boolean, boolean | undefined> // Key is a flag, value is true if discriminant synthetic property
resolvedExports?: SymbolTable; // Resolved exports of module or combined early- and late-bound static members of a class.
resolvedMembers?: SymbolTable; // Combined early- and late-bound members of a symbol
exportsChecked?: boolean; // True if exports of external module have been checked
Expand Down Expand Up @@ -6071,6 +6071,7 @@ export const enum CheckFlags {
Mapped = 1 << 18, // Property of mapped type
StripOptional = 1 << 19, // Strip optionality in mapped property
Unresolved = 1 << 20, // Unresolved type alias symbol
HasPrimitiveType = 1 << 21, // Synthetic property with at least one primitive type in constituents
Synthetic = SyntheticProperty | SyntheticMethod,
Discriminant = HasNonUniformType | HasLiteralType,
Partial = ReadPartial | WritePartial,
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4989,7 +4989,7 @@ declare namespace ts {
readonly modifiers?: NodeArray<Modifier>;
readonly equalsGreaterThanToken: EqualsGreaterThanToken;
readonly body: ConciseBody;
readonly name: never;
readonly name?: never;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is basically name?: undefined without exactOptionalPropertyTypes (and TS codebase doesnt use this flag). Is this property truly optional at runtime anyway? or is it always set to undefined?

Copy link
Owner Author

Choose a reason for hiding this comment

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

it seems that createArrowFunction do not set it at all

Copy link
Owner Author

Choose a reason for hiding this comment

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

idk if it is set to undefined somewhere else, I'd say it isn't. Even updateArrowFunction ignores it

}
interface LiteralLikeNode extends Node {
text: string;
Expand Down
32 changes: 16 additions & 16 deletions tests/baselines/reference/destructuringControlFlow.types
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ function f2(obj: [number, string] | null[]) {
> : ^^^^^^^^^^^^^^^^^^^^^^^^^
>0 : 0
> : ^
>obj[1] : string | null
> : ^^^^^^^^^^^^^
>obj : [number, string] | null[]
> : ^^^^^^^^^^^^^^^^^^^^^^^^^
>obj[1] : string
> : ^^^^^^
>obj : [number, string]
> : ^^^^^^^^^^^^^^^^
>1 : 1
> : ^

Expand All @@ -116,8 +116,8 @@ function f2(obj: [number, string] | null[]) {
> : ^^^^^^
>obj[0] : number
> : ^^^^^^
>obj : [number, string] | null[]
> : ^^^^^^^^^^^^^^^^^^^^^^^^^
>obj : [number, string]
> : ^^^^^^^^^^^^^^^^
>0 : 0
> : ^

Expand All @@ -126,8 +126,8 @@ function f2(obj: [number, string] | null[]) {
> : ^^^^^^
>obj[1] : string
> : ^^^^^^
>obj : [number, string] | null[]
> : ^^^^^^^^^^^^^^^^^^^^^^^^^
>obj : [number, string]
> : ^^^^^^^^^^^^^^^^
>1 : 1
> : ^

Expand All @@ -136,22 +136,22 @@ function f2(obj: [number, string] | null[]) {
> : ^^^^^^
>d1 : string
> : ^^^^^^
>obj : [number, string] | null[]
> : ^^^^^^^^^^^^^^^^^^^^^^^^^
>obj : [number, string]
> : ^^^^^^^^^^^^^^^^

([c0, c1] = obj);
>([c0, c1] = obj) : [number, string] | null[]
> : ^^^^^^^^^^^^^^^^^^^^^^^^^
>[c0, c1] = obj : [number, string] | null[]
> : ^^^^^^^^^^^^^^^^^^^^^^^^^
>([c0, c1] = obj) : [number, string]
> : ^^^^^^^^^^^^^^^^
>[c0, c1] = obj : [number, string]
> : ^^^^^^^^^^^^^^^^
>[c0, c1] : [number, string]
> : ^^^^^^^^^^^^^^^^
>c0 : number
> : ^^^^^^
>c1 : string
> : ^^^^^^
>obj : [number, string] | null[]
> : ^^^^^^^^^^^^^^^^^^^^^^^^^
>obj : [number, string]
> : ^^^^^^^^^^^^^^^^
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
discriminatedUnionTypes2.ts(27,30): error TS2353: Object literal may only specify known properties, and 'c' does not exist in type '{ a: null; b: string; }'.
discriminatedUnionTypes2.ts(32,11): error TS2339: Property 'b' does not exist on type '{ a: 0; b: string; } | { a: T; c: number; }'.
Property 'b' does not exist on type '{ a: T; c: number; }'.
discriminatedUnionTypes2.ts(132,11): error TS2339: Property 'value' does not exist on type 'never'.


==== discriminatedUnionTypes2.ts (2 errors) ====
==== discriminatedUnionTypes2.ts (3 errors) ====
function f10(x : { kind: false, a: string } | { kind: true, b: string } | { kind: string, c: string }) {
if (x.kind === false) {
x.a;
Expand Down Expand Up @@ -141,6 +142,8 @@ discriminatedUnionTypes2.ts(32,11): error TS2339: Property 'b' does not exist on
}
else {
x.value; // number
~~~~~
!!! error TS2339: Property 'value' does not exist on type 'never'.
}
}

Expand Down
2 changes: 0 additions & 2 deletions tests/baselines/reference/discriminatedUnionTypes2.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,7 @@ function foo1(x: RuntimeValue & { type: 'number' }) {
}
else {
x.value; // number
>x.value : Symbol(value, Decl(discriminatedUnionTypes2.ts, 122, 23))
>x : Symbol(x, Decl(discriminatedUnionTypes2.ts, 126, 14))
>value : Symbol(value, Decl(discriminatedUnionTypes2.ts, 122, 23))
}
}

Expand Down
12 changes: 6 additions & 6 deletions tests/baselines/reference/discriminatedUnionTypes2.types
Original file line number Diff line number Diff line change
Expand Up @@ -578,12 +578,12 @@ function foo1(x: RuntimeValue & { type: 'number' }) {
}
else {
x.value; // number
>x.value : number
> : ^^^^^^
>x : { type: "number"; value: number; } & { type: "number"; }
> : ^^^^^^^^ ^^^^^^^^^ ^^^^^^^^^^^^^^ ^^^
>value : number
> : ^^^^^^
>x.value : any
> : ^^^
>x : never
> : ^^^^^
>value : any
> : ^^^
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
narrowUnionOfObjectsByPrimitiveProperty.ts(62,1): error TS2322: Type '{ prop: string | number; other: never; }' is not assignable to type '{ prop: string; other: string; } | { prop: number; other: number; }'.
Type '{ prop: string | number; other: never; }' is not assignable to type '{ prop: number; other: number; }'.
Types of property 'prop' are incompatible.
Type 'string | number' is not assignable to type 'number'.
Type 'string' is not assignable to type 'number'.
narrowUnionOfObjectsByPrimitiveProperty.ts(70,1): error TS2322: Type '{ prop: string | number; other: never; }' is not assignable to type '{ prop: string; other: string; } | { prop: number; other: number; }'.
Type '{ prop: string | number; other: never; }' is not assignable to type '{ prop: number; other: number; }'.
Types of property 'prop' are incompatible.
Type 'string | number' is not assignable to type 'number'.
Type 'string' is not assignable to type 'number'.
narrowUnionOfObjectsByPrimitiveProperty.ts(96,32): error TS2339: Property 'label' does not exist on type 'never'.


==== narrowUnionOfObjectsByPrimitiveProperty.ts (3 errors) ====
export {}

interface State<Type> {
state: Type;
}

interface UserName {
first: string;
last?: string;
}

const nameState = {} as {
value: string;
state: State<string>;
} | {
value: UserName;
state: State<UserName>;
}

if (typeof nameState.value === "string") {
nameState.state satisfies State<string>;
} else {
nameState.state satisfies State<UserName>;
}


declare const arr: [string, number] | [number, string];
if (typeof arr[0] === "string") {
arr[1] satisfies number;
} else {
arr[1] satisfies string;
}


function aStringOrANumber<T extends { a: string } | { a: number }>(param: T): T extends { a: string } ? string : T extends { a: number } ? number : never {
if (typeof param.a === "string") {
return param.a.repeat(3);
}
if (typeof param.a === "number") {
return Math.exp(param.a);
}
throw new Error()
}

aStringOrANumber({ a: "string" })
aStringOrANumber({ a: 42 })


// The following two tests ensure that the discriminativeness of property 'prop'
// is treated differently in assignability and narrowing, and that the discriminativeness is properly cached.
declare let obj: { prop: string, other: string } | { prop: number, other: number }

// Here, we first perform narrowing, but the subsequent assignability should not be affected.
// We expect an error there because of an incorrect value assigned to 'prop'.
// See contextualTypeWithUnionTypeObjectLiteral.ts
if(typeof obj.prop === "string") {
obj.other.repeat(3);
} else {
Math.exp(obj.other);
}

obj = { prop: Math.random() > 0.5 ? "whatever" : 42, other: "irrelevant" as never }
~~~
!!! error TS2322: Type '{ prop: string | number; other: never; }' is not assignable to type '{ prop: string; other: string; } | { prop: number; other: number; }'.
!!! error TS2322: Type '{ prop: string | number; other: never; }' is not assignable to type '{ prop: number; other: number; }'.
!!! error TS2322: Types of property 'prop' are incompatible.
!!! error TS2322: Type 'string | number' is not assignable to type 'number'.
!!! error TS2322: Type 'string' is not assignable to type 'number'.


declare let obj2: { prop: string, other: string } | { prop: number, other: number }

// Here, we first assign a value to 'obj2' and then perform narrowing.
// We expect an error here because of an incorrect value assigned to 'prop', like above,
// but the subsequent narrowing should not be affected by the assignability.
obj2 = { prop: Math.random() > 0.5 ? "whatever" : 42, other: "irrelevant" as never }
~~~~
!!! error TS2322: Type '{ prop: string | number; other: never; }' is not assignable to type '{ prop: string; other: string; } | { prop: number; other: number; }'.
!!! error TS2322: Type '{ prop: string | number; other: never; }' is not assignable to type '{ prop: number; other: number; }'.
!!! error TS2322: Types of property 'prop' are incompatible.
!!! error TS2322: Type 'string | number' is not assignable to type 'number'.
!!! error TS2322: Type 'string' is not assignable to type 'number'.

if(typeof obj2.prop === "string") {
obj2.other.repeat(3);
} else {
Math.exp(obj2.other);
}


interface ILocalizedString {
original: string;
value: string;
}

type Opt = ({
label: ILocalizedString;
alias?: string;
} | {
label: string;
alias: string;
})

declare const opt: Opt

if (typeof opt.label === 'string') {
const l = opt.label;
const a = opt.alias ?? opt.label;
~~~~~
!!! error TS2339: Property 'label' does not exist on type 'never'.
} else {
const l = opt.label;
const a = opt.alias ?? opt.label.original;
}
Loading
Loading