Skip to content

Fixed type predicate inference for discriminated union parameters #57952

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

Merged
merged 2 commits into from
Apr 2, 2024
Merged

Fixed type predicate inference for discriminated union parameters #57952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b3def6
Fixed type predicate inference for discriminated union parameters
Andarist Mar 27, 2024
dae6a4c
Merge remote-tracking branch 'origin/main' into fix/type-predicate-in…
Andarist Mar 29, 2024
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
7 changes: 4 additions & 3 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37683,8 +37683,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
{ flags: FlowFlags.Start };
const trueCondition: FlowCondition = {
flags: FlowFlags.TrueCondition,
node: expr,
antecedent,
node: expr,
Copy link
Contributor

Choose a reason for hiding this comment

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

any particular reason for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to follow the structure used by the ones created in the binder to keep functions receiving this monomorphic. I'm not sure if my change prompted this further experiment but it certainly looked like it to me since it was open soon after I opened this one here ;p

TLDR: this was meant as an optimization for the engine. I didn't measure it at all since I don't have time for it but it's usually better to create structures in the same way (order of keys etc matters for the engine)

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Makes sense. The calls to getFlowTypeOfReference are so expensive compared to everything else in getTypePredicateFromBody that I doubt micro-optimizations matter (see #57465 (comment) and #57660). But I can't imagine they hurt!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

once u apply micro-optimizations at scale there are some good wins to be had :P #57977 (comment)

};

const trueType = getFlowTypeOfReference(param.name, initType, initType, func, trueCondition);
Expand All @@ -37693,10 +37693,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// "x is T" means that x is T if and only if it returns true. If it returns false then x is not T.
// This means that if the function is called with an argument of type trueType, there can't be anything left in the `else` branch. It must reduce to `never`.
const falseCondition: FlowCondition = {
...trueCondition,
flags: FlowFlags.FalseCondition,
antecedent,
node: expr,
};
const falseSubtype = getFlowTypeOfReference(param.name, trueType, trueType, func, falseCondition);
const falseSubtype = getFlowTypeOfReference(param.name, initType, trueType, func, falseCondition);
return falseSubtype.flags & TypeFlags.Never ? trueType : undefined;
}

Expand Down
10 changes: 10 additions & 0 deletions tests/baselines/reference/inferTypePredicates.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,4 +315,14 @@ inferTypePredicates.ts(205,7): error TS2741: Property 'z' is missing in type 'C1
function inferWithRest(x: string | null, ...f: ["a", "b"]) {
return typeof x === 'string';
}

// https://github.com/microsoft/TypeScript/issues/57947
declare const foobar:
| { type: "foo"; foo: number }
| { type: "bar"; bar: string };

const foobarPred = (fb: typeof foobar) => fb.type === "foo";
if (foobarPred(foobar)) {
foobar.foo;
}

25 changes: 25 additions & 0 deletions tests/baselines/reference/inferTypePredicates.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,16 @@ const noInferenceFromImpossibleRest = (...f: []) => typeof f === "undefined";
function inferWithRest(x: string | null, ...f: ["a", "b"]) {
return typeof x === 'string';
}

// https://github.com/microsoft/TypeScript/issues/57947
declare const foobar:
| { type: "foo"; foo: number }
| { type: "bar"; bar: string };

const foobarPred = (fb: typeof foobar) => fb.type === "foo";
if (foobarPred(foobar)) {
foobar.foo;
}


//// [inferTypePredicates.js]
Expand Down Expand Up @@ -524,6 +534,10 @@ function inferWithRest(x) {
}
return typeof x === 'string';
}
var foobarPred = function (fb) { return fb.type === "foo"; };
if (foobarPred(foobar)) {
foobar.foo;
}


//// [inferTypePredicates.d.ts]
Expand Down Expand Up @@ -605,3 +619,14 @@ declare function narrowFromAny(x: any): x is number;
declare const noInferenceFromRest: (f_0: "a" | "b") => boolean;
declare const noInferenceFromImpossibleRest: () => boolean;
declare function inferWithRest(x: string | null, ...f: ["a", "b"]): x is string;
declare const foobar: {
type: "foo";
foo: number;
} | {
type: "bar";
bar: string;
};
declare const foobarPred: (fb: typeof foobar) => fb is {
type: "foo";
foo: number;
};
30 changes: 30 additions & 0 deletions tests/baselines/reference/inferTypePredicates.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,3 +747,33 @@ function inferWithRest(x: string | null, ...f: ["a", "b"]) {
>x : Symbol(x, Decl(inferTypePredicates.ts, 265, 23))
}

// https://github.com/microsoft/TypeScript/issues/57947
declare const foobar:
>foobar : Symbol(foobar, Decl(inferTypePredicates.ts, 270, 13))

| { type: "foo"; foo: number }
>type : Symbol(type, Decl(inferTypePredicates.ts, 271, 5))
>foo : Symbol(foo, Decl(inferTypePredicates.ts, 271, 18))

| { type: "bar"; bar: string };
>type : Symbol(type, Decl(inferTypePredicates.ts, 272, 5))
>bar : Symbol(bar, Decl(inferTypePredicates.ts, 272, 18))

const foobarPred = (fb: typeof foobar) => fb.type === "foo";
>foobarPred : Symbol(foobarPred, Decl(inferTypePredicates.ts, 274, 5))
>fb : Symbol(fb, Decl(inferTypePredicates.ts, 274, 20))
>foobar : Symbol(foobar, Decl(inferTypePredicates.ts, 270, 13))
>fb.type : Symbol(type, Decl(inferTypePredicates.ts, 271, 5), Decl(inferTypePredicates.ts, 272, 5))
>fb : Symbol(fb, Decl(inferTypePredicates.ts, 274, 20))
>type : Symbol(type, Decl(inferTypePredicates.ts, 271, 5), Decl(inferTypePredicates.ts, 272, 5))

if (foobarPred(foobar)) {
>foobarPred : Symbol(foobarPred, Decl(inferTypePredicates.ts, 274, 5))
>foobar : Symbol(foobar, Decl(inferTypePredicates.ts, 270, 13))

foobar.foo;
>foobar.foo : Symbol(foo, Decl(inferTypePredicates.ts, 271, 18))
>foobar : Symbol(foobar, Decl(inferTypePredicates.ts, 270, 13))
>foo : Symbol(foo, Decl(inferTypePredicates.ts, 271, 18))
}

54 changes: 54 additions & 0 deletions tests/baselines/reference/inferTypePredicates.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1595,3 +1595,57 @@ function inferWithRest(x: string | null, ...f: ["a", "b"]) {
> : ^^^^^^^^
}

// https://github.com/microsoft/TypeScript/issues/57947
declare const foobar:
>foobar : { type: "foo"; foo: number; } | { type: "bar"; bar: string; }
> : ^^^^^^^^ ^^^^^^^ ^^^^^^^^^^^^^^ ^^^^^^^ ^^^

| { type: "foo"; foo: number }
>type : "foo"
> : ^^^^^
>foo : number
> : ^^^^^^

| { type: "bar"; bar: string };
>type : "bar"
> : ^^^^^
>bar : string
> : ^^^^^^

const foobarPred = (fb: typeof foobar) => fb.type === "foo";
>foobarPred : (fb: typeof foobar) => fb is { type: "foo"; foo: number; }
> : ^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>(fb: typeof foobar) => fb.type === "foo" : (fb: typeof foobar) => fb is { type: "foo"; foo: number; }
> : ^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>fb : { type: "foo"; foo: number; } | { type: "bar"; bar: string; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>foobar : { type: "foo"; foo: number; } | { type: "bar"; bar: string; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>fb.type === "foo" : boolean
> : ^^^^^^^
>fb.type : "bar" | "foo"
> : ^^^^^^^^^^^^^
>fb : { type: "foo"; foo: number; } | { type: "bar"; bar: string; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>type : "bar" | "foo"
> : ^^^^^^^^^^^^^
>"foo" : "foo"
> : ^^^^^

if (foobarPred(foobar)) {
>foobarPred(foobar) : boolean
> : ^^^^^^^
>foobarPred : (fb: { type: "foo"; foo: number; } | { type: "bar"; bar: string; }) => fb is { type: "foo"; foo: number; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>foobar : { type: "foo"; foo: number; } | { type: "bar"; bar: string; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

foobar.foo;
>foobar.foo : number
> : ^^^^^^
>foobar : { type: "foo"; foo: number; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>foo : number
> : ^^^^^^
}

10 changes: 10 additions & 0 deletions tests/cases/compiler/inferTypePredicates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,3 +269,13 @@ const noInferenceFromImpossibleRest = (...f: []) => typeof f === "undefined";
function inferWithRest(x: string | null, ...f: ["a", "b"]) {
return typeof x === 'string';
}

// https://github.com/microsoft/TypeScript/issues/57947
declare const foobar:
| { type: "foo"; foo: number }
| { type: "bar"; bar: string };

const foobarPred = (fb: typeof foobar) => fb.type === "foo";
if (foobarPred(foobar)) {
foobar.foo;
}