Skip to content

Fix contextual discrimination for omitted members #48448

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

Closed
wants to merge 1 commit into from
Closed
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
111 changes: 92 additions & 19 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26853,34 +26853,107 @@ namespace ts {
return false;
}

/**
* get discriminators for missing properties on node that are optional for any member of the union context type
*
* Because we know in advance which types each member can potentially
* discriminate against, we only have to choose one property among the
* set that discriminates between the same types in the union.
*
* @internal
*/
function getOptionalPropertyDiscriminators(node: ObjectLiteralExpression | JsxAttributes, contextualType: UnionType): [() => Type, __String][] | undefined {
const nodeSymbolMembers = node?.symbol?.members;
if (!nodeSymbolMembers) {
return undefined;
}

let discriminantNames = contextualType.discriminantOptionalPropertyNames;
if (!discriminantNames) {
const optionalNames = new Map<__String, number[]>();
let index = 0;
for (const memberType of contextualType.types) {
for (const s of getPropertiesOfType(memberType)) {
if (s.flags & SymbolFlags.Optional) {
const name = s.escapedName;
const inds = optionalNames.get(name);
inds ? inds.push(index) : optionalNames.set(name, [index]);
}
}
++index;
}

const grouped = new Map<string, __String[]>();
optionalNames.forEach((inds, escapedName) => {
const key = inds.join(" ");
const names = grouped.get(key);
names ? names.push(escapedName) : grouped.set(key, [escapedName]);
});
const names: __String[][] = [];
grouped.forEach(nameGroup => names.push(nameGroup));
discriminantNames = contextualType.discriminantOptionalPropertyNames = names;
}

return map(
filter(
map(discriminantNames, names => names.find(name => !nodeSymbolMembers.has(name))),
name => !!name // remove undefineds to select only missing members
),
name => [() => undefinedType, name] as [() => Type, __String],
);
}

function discriminateContextualTypeByObjectMembers(node: ObjectLiteralExpression, contextualType: UnionType) {
return getMatchingUnionConstituentForObjectLiteral(contextualType, node) || discriminateTypeByDiscriminableItems(contextualType,
concatenate(
map(
filter(node.properties, p => !!p.symbol && p.kind === SyntaxKind.PropertyAssignment && isPossiblyDiscriminantValue(p.initializer) && isDiscriminantProperty(contextualType, p.symbol.escapedName)),
prop => ([() => getContextFreeTypeOfExpression((prop as PropertyAssignment).initializer), prop.symbol.escapedName] as [() => Type, __String])
return (
getMatchingUnionConstituentForObjectLiteral(contextualType, node) ||
discriminateTypeByDiscriminableItems(
contextualType,
concatenate(
map(
filter(
node.properties,
p =>
!!p.symbol &&
p.kind === SyntaxKind.PropertyAssignment &&
isPossiblyDiscriminantValue(p.initializer) &&
isDiscriminantProperty(contextualType, p.symbol.escapedName)
),
prop =>
[
() => getContextFreeTypeOfExpression((prop as PropertyAssignment).initializer),
prop.symbol.escapedName,
] as [() => Type, __String]
),
getOptionalPropertyDiscriminators(node, contextualType),
),
map(
filter(getPropertiesOfType(contextualType), s => !!(s.flags & SymbolFlags.Optional) && !!node?.symbol?.members && !node.symbol.members.has(s.escapedName) && isDiscriminantProperty(contextualType, s.escapedName)),
s => [() => undefinedType, s.escapedName] as [() => Type, __String]
)
),
isTypeAssignableTo,
contextualType
isTypeAssignableTo,
contextualType
)
);
}

function discriminateContextualTypeByJSXAttributes(node: JsxAttributes, contextualType: UnionType) {
return discriminateTypeByDiscriminableItems(contextualType,
return discriminateTypeByDiscriminableItems(
contextualType,
concatenate(
map(
filter(node.properties, p => !!p.symbol && p.kind === SyntaxKind.JsxAttribute && isDiscriminantProperty(contextualType, p.symbol.escapedName) && (!p.initializer || isPossiblyDiscriminantValue(p.initializer))),
prop => ([!(prop as JsxAttribute).initializer ? (() => trueType) : (() => getContextFreeTypeOfExpression((prop as JsxAttribute).initializer!)), prop.symbol.escapedName] as [() => Type, __String])
filter(
node.properties,
p =>
!!p.symbol &&
p.kind === SyntaxKind.JsxAttribute &&
isDiscriminantProperty(contextualType, p.symbol.escapedName) &&
(!p.initializer || isPossiblyDiscriminantValue(p.initializer))
),
prop =>
[
!(prop as JsxAttribute).initializer
? () => trueType
: () => getContextFreeTypeOfExpression((prop as JsxAttribute).initializer!),
prop.symbol.escapedName,
] as [() => Type, __String]
),
map(
filter(getPropertiesOfType(contextualType), s => !!(s.flags & SymbolFlags.Optional) && !!node?.symbol?.members && !node.symbol.members.has(s.escapedName) && isDiscriminantProperty(contextualType, s.escapedName)),
s => [() => undefinedType, s.escapedName] as [() => Type, __String]
)
getOptionalPropertyDiscriminators(node, contextualType),
),
isTypeAssignableTo,
contextualType
Expand Down
7 changes: 7 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5532,6 +5532,13 @@ namespace ts {
keyPropertyName?: __String; // Property with unique unit type that exists in every object/intersection in union type
/* @internal */
constituentMap?: ESMap<TypeId, Type>; // Constituents keyed by unit type discriminants
/**
* names of discriminant properties that are optional on at least one type
*
* These are grouped by the set of types they discriminate to speed up checks
* @internal
*/
discriminantOptionalPropertyNames?: __String[][];
}

export interface IntersectionType extends UnionOrIntersectionType {
Expand Down
16 changes: 15 additions & 1 deletion tests/baselines/reference/discriminantPropertyInference.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ type DiscriminatorFalse = {
cb: (x: number) => void;
}

type Props = DiscriminatorTrue | DiscriminatorFalse;
type Unrelated = {
val: number;
}

declare function f(options: DiscriminatorTrue | DiscriminatorFalse): any;

Expand All @@ -37,6 +39,14 @@ f({
f({
cb: n => n.toFixed()
});


declare function g(options: DiscriminatorTrue | DiscriminatorFalse | Unrelated): any;

// requires checking properties of all types, rather than properties of just the union type (e.g. only intersection)
g({
cb: n => n.toFixed()
});


//// [discriminantPropertyInference.js]
Expand All @@ -60,3 +70,7 @@ f({
f({
cb: function (n) { return n.toFixed(); }
});
// requires checking properties of all types, rather than properties of just the union type (e.g. only intersection)
g({
cb: function (n) { return n.toFixed(); }
});
73 changes: 48 additions & 25 deletions tests/baselines/reference/discriminantPropertyInference.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -23,74 +23,97 @@ type DiscriminatorFalse = {
>x : Symbol(x, Decl(discriminantPropertyInference.ts, 9, 9))
}

type Props = DiscriminatorTrue | DiscriminatorFalse;
>Props : Symbol(Props, Decl(discriminantPropertyInference.ts, 10, 1))
>DiscriminatorTrue : Symbol(DiscriminatorTrue, Decl(discriminantPropertyInference.ts, 0, 0))
>DiscriminatorFalse : Symbol(DiscriminatorFalse, Decl(discriminantPropertyInference.ts, 5, 1))
type Unrelated = {
>Unrelated : Symbol(Unrelated, Decl(discriminantPropertyInference.ts, 10, 1))

val: number;
>val : Symbol(val, Decl(discriminantPropertyInference.ts, 12, 18))
}

declare function f(options: DiscriminatorTrue | DiscriminatorFalse): any;
>f : Symbol(f, Decl(discriminantPropertyInference.ts, 12, 52))
>options : Symbol(options, Decl(discriminantPropertyInference.ts, 14, 19))
>f : Symbol(f, Decl(discriminantPropertyInference.ts, 14, 1))
>options : Symbol(options, Decl(discriminantPropertyInference.ts, 16, 19))
>DiscriminatorTrue : Symbol(DiscriminatorTrue, Decl(discriminantPropertyInference.ts, 0, 0))
>DiscriminatorFalse : Symbol(DiscriminatorFalse, Decl(discriminantPropertyInference.ts, 5, 1))

// simple inference
f({
>f : Symbol(f, Decl(discriminantPropertyInference.ts, 12, 52))
>f : Symbol(f, Decl(discriminantPropertyInference.ts, 14, 1))

disc: true,
>disc : Symbol(disc, Decl(discriminantPropertyInference.ts, 17, 3))
>disc : Symbol(disc, Decl(discriminantPropertyInference.ts, 19, 3))

cb: s => parseInt(s)
>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 18, 15))
>s : Symbol(s, Decl(discriminantPropertyInference.ts, 19, 7))
>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 20, 15))
>s : Symbol(s, Decl(discriminantPropertyInference.ts, 21, 7))
>parseInt : Symbol(parseInt, Decl(lib.es5.d.ts, --, --))
>s : Symbol(s, Decl(discriminantPropertyInference.ts, 19, 7))
>s : Symbol(s, Decl(discriminantPropertyInference.ts, 21, 7))

});

// simple inference
f({
>f : Symbol(f, Decl(discriminantPropertyInference.ts, 12, 52))
>f : Symbol(f, Decl(discriminantPropertyInference.ts, 14, 1))

disc: false,
>disc : Symbol(disc, Decl(discriminantPropertyInference.ts, 23, 3))
>disc : Symbol(disc, Decl(discriminantPropertyInference.ts, 25, 3))

cb: n => n.toFixed()
>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 24, 16))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 25, 7))
>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 26, 16))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 27, 7))
>n.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 25, 7))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 27, 7))
>toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))

});

// simple inference when strict-null-checks are enabled
f({
>f : Symbol(f, Decl(discriminantPropertyInference.ts, 12, 52))
>f : Symbol(f, Decl(discriminantPropertyInference.ts, 14, 1))

disc: undefined,
>disc : Symbol(disc, Decl(discriminantPropertyInference.ts, 29, 3))
>disc : Symbol(disc, Decl(discriminantPropertyInference.ts, 31, 3))
>undefined : Symbol(undefined)

cb: n => n.toFixed()
>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 30, 20))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 31, 7))
>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 32, 20))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 33, 7))
>n.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 31, 7))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 33, 7))
>toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))

});

// requires checking type information since discriminator is missing from object
f({
>f : Symbol(f, Decl(discriminantPropertyInference.ts, 12, 52))
>f : Symbol(f, Decl(discriminantPropertyInference.ts, 14, 1))

cb: n => n.toFixed()
>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 37, 3))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 38, 7))
>n.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 38, 7))
>toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))

});


declare function g(options: DiscriminatorTrue | DiscriminatorFalse | Unrelated): any;
>g : Symbol(g, Decl(discriminantPropertyInference.ts, 39, 3))
>options : Symbol(options, Decl(discriminantPropertyInference.ts, 42, 19))
>DiscriminatorTrue : Symbol(DiscriminatorTrue, Decl(discriminantPropertyInference.ts, 0, 0))
>DiscriminatorFalse : Symbol(DiscriminatorFalse, Decl(discriminantPropertyInference.ts, 5, 1))
>Unrelated : Symbol(Unrelated, Decl(discriminantPropertyInference.ts, 10, 1))

// requires checking properties of all types, rather than properties of just the union type (e.g. only intersection)
g({
>g : Symbol(g, Decl(discriminantPropertyInference.ts, 39, 3))

cb: n => n.toFixed()
>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 35, 3))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 36, 7))
>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 45, 3))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 46, 7))
>n.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 36, 7))
>n : Symbol(n, Decl(discriminantPropertyInference.ts, 46, 7))
>toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))

});
Expand Down
30 changes: 28 additions & 2 deletions tests/baselines/reference/discriminantPropertyInference.types
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,12 @@ type DiscriminatorFalse = {
>x : number
}

type Props = DiscriminatorTrue | DiscriminatorFalse;
>Props : Props
type Unrelated = {
>Unrelated : Unrelated

val: number;
>val : number
}

declare function f(options: DiscriminatorTrue | DiscriminatorFalse): any;
>f : (options: DiscriminatorTrue | DiscriminatorFalse) => any
Expand Down Expand Up @@ -111,3 +115,25 @@ f({

});


declare function g(options: DiscriminatorTrue | DiscriminatorFalse | Unrelated): any;
>g : (options: DiscriminatorTrue | DiscriminatorFalse | Unrelated) => any
>options : DiscriminatorTrue | DiscriminatorFalse | Unrelated

// requires checking properties of all types, rather than properties of just the union type (e.g. only intersection)
g({
>g({ cb: n => n.toFixed()}) : any
>g : (options: DiscriminatorTrue | DiscriminatorFalse | Unrelated) => any
>{ cb: n => n.toFixed()} : { cb: (n: number) => string; }

cb: n => n.toFixed()
>cb : (n: number) => string
>n => n.toFixed() : (n: number) => string
>n : number
>n.toFixed() : string
>n.toFixed : (fractionDigits?: number | undefined) => string
>n : number
>toFixed : (fractionDigits?: number | undefined) => string

});

15 changes: 14 additions & 1 deletion tests/baselines/reference/tsxDiscriminantPropertyInference.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,15 @@ type DiscriminatorFalse = {
cb: (x: number) => void;
}

type Unrelated = {
val: number;
}

type Props = DiscriminatorTrue | DiscriminatorFalse;

declare function Comp(props: DiscriminatorTrue | DiscriminatorFalse): JSX.Element;
type UnrelatedProps = Props | Unrelated;

declare function Comp(props: Props): JSX.Element;

// simple inference
void (<Comp disc cb={s => parseInt(s)} />);
Expand All @@ -29,6 +35,11 @@ void (<Comp disc={undefined} cb={n => n.toFixed()} />);

// requires checking type information since discriminator is missing from object
void (<Comp cb={n => n.toFixed()} />);

declare function UnrelatedComp(props: UnrelatedProps): JSX.Element;

// requires checking properties of all types, rather than properties of just the union type (e.g. only intersection)
void (<Comp cb={n => n.toFixed()} />);


//// [tsxDiscriminantPropertyInference.jsx]
Expand All @@ -40,3 +51,5 @@ void (<Comp disc={false} cb={function (n) { return n.toFixed(); }}/>);
void (<Comp disc={undefined} cb={function (n) { return n.toFixed(); }}/>);
// requires checking type information since discriminator is missing from object
void (<Comp cb={function (n) { return n.toFixed(); }}/>);
// requires checking properties of all types, rather than properties of just the union type (e.g. only intersection)
void (<Comp cb={function (n) { return n.toFixed(); }}/>);
Loading