update contextual discrimination to include omitted members (#43633)

This diff extends the types checked by discriminateContextualTypeByObjectMembers and discriminateContextualTypeByJSXAttributes to also include any optional components in the type union. fixes #41759 although it doesn't address the better error reporting for their last repro, which I'm not sure how to address.
microsoft · Apr 24, 2021 · 4d4ea66 · 4d4ea66
1 parent 0ad158e
commit 4d4ea66
Show file tree

Hide file tree

Showing 9 changed files with 588 additions and 6 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -25322,9 +25322,15 @@ namespace ts {
 
         function discriminateContextualTypeByObjectMembers(node: ObjectLiteralExpression, contextualType: UnionType) {
             return getMatchingUnionConstituentForObjectLiteral(contextualType, node) || discriminateTypeByDiscriminableItems(contextualType,
-                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])
+                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])
+                    ),
+                    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
@@ -25333,9 +25339,15 @@ namespace ts {
 
         function discriminateContextualTypeByJSXAttributes(node: JsxAttributes, contextualType: UnionType) {
             return discriminateTypeByDiscriminableItems(contextualType,
-                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) : (() => checkExpression((prop as JsxAttribute).initializer!)), prop.symbol.escapedName] as [() => Type, __String])
+                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) : (() => checkExpression((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]
+                    )
                 ),
                 isTypeAssignableTo,
                 contextualType

diff --git a/tests/baselines/reference/discriminantPropertyInference.js b/tests/baselines/reference/discriminantPropertyInference.js
@@ -0,0 +1,62 @@
+//// [discriminantPropertyInference.ts]
+// Repro from #41759
+
+type DiscriminatorTrue = {
+    disc: true;
+    cb: (x: string) => void;
+}
+
+type DiscriminatorFalse = {
+    disc?: false;
+    cb: (x: number) => void;
+}
+
+type Props = DiscriminatorTrue | DiscriminatorFalse;
+
+declare function f(options: DiscriminatorTrue | DiscriminatorFalse): any;
+
+// simple inference
+f({
+    disc: true,
+    cb: s => parseInt(s)
+});
+
+// simple inference
+f({
+    disc: false,
+    cb: n => n.toFixed()
+});
+
+// simple inference when strict-null-checks are enabled
+f({
+    disc: undefined,
+    cb: n => n.toFixed()
+});
+
+// requires checking type information since discriminator is missing from object
+f({
+    cb: n => n.toFixed()
+});
+
+
+//// [discriminantPropertyInference.js]
+// Repro from #41759
+// simple inference
+f({
+    disc: true,
+    cb: function (s) { return parseInt(s); }
+});
+// simple inference
+f({
+    disc: false,
+    cb: function (n) { return n.toFixed(); }
+});
+// simple inference when strict-null-checks are enabled
+f({
+    disc: undefined,
+    cb: function (n) { return n.toFixed(); }
+});
+// requires checking type information since discriminator is missing from object
+f({
+    cb: function (n) { return n.toFixed(); }
+});
diff --git a/tests/baselines/reference/discriminantPropertyInference.symbols b/tests/baselines/reference/discriminantPropertyInference.symbols
@@ -0,0 +1,97 @@
+=== tests/cases/compiler/discriminantPropertyInference.ts ===
+// Repro from #41759
+
+type DiscriminatorTrue = {
+>DiscriminatorTrue : Symbol(DiscriminatorTrue, Decl(discriminantPropertyInference.ts, 0, 0))
+
+    disc: true;
+>disc : Symbol(disc, Decl(discriminantPropertyInference.ts, 2, 26))
+
+    cb: (x: string) => void;
+>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 3, 15))
+>x : Symbol(x, Decl(discriminantPropertyInference.ts, 4, 9))
+}
+
+type DiscriminatorFalse = {
+>DiscriminatorFalse : Symbol(DiscriminatorFalse, Decl(discriminantPropertyInference.ts, 5, 1))
+
+    disc?: false;
+>disc : Symbol(disc, Decl(discriminantPropertyInference.ts, 7, 27))
+
+    cb: (x: number) => void;
+>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 8, 17))
+>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))
+
+declare function f(options: DiscriminatorTrue | DiscriminatorFalse): any;
+>f : Symbol(f, Decl(discriminantPropertyInference.ts, 12, 52))
+>options : Symbol(options, Decl(discriminantPropertyInference.ts, 14, 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))
+
+    disc: true,
+>disc : Symbol(disc, Decl(discriminantPropertyInference.ts, 17, 3))
+
+    cb: s => parseInt(s)
+>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 18, 15))
+>s : Symbol(s, Decl(discriminantPropertyInference.ts, 19, 7))
+>parseInt : Symbol(parseInt, Decl(lib.es5.d.ts, --, --))
+>s : Symbol(s, Decl(discriminantPropertyInference.ts, 19, 7))
+
+});
+
+// simple inference
+f({
+>f : Symbol(f, Decl(discriminantPropertyInference.ts, 12, 52))
+
+    disc: false,
+>disc : Symbol(disc, Decl(discriminantPropertyInference.ts, 23, 3))
+
+    cb: n => n.toFixed()
+>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 24, 16))
+>n : Symbol(n, Decl(discriminantPropertyInference.ts, 25, 7))
+>n.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
+>n : Symbol(n, Decl(discriminantPropertyInference.ts, 25, 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))
+
+    disc: undefined,
+>disc : Symbol(disc, Decl(discriminantPropertyInference.ts, 29, 3))
+>undefined : Symbol(undefined)
+
+    cb: n => n.toFixed()
+>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 30, 20))
+>n : Symbol(n, Decl(discriminantPropertyInference.ts, 31, 7))
+>n.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
+>n : Symbol(n, Decl(discriminantPropertyInference.ts, 31, 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))
+
+    cb: n => n.toFixed()
+>cb : Symbol(cb, Decl(discriminantPropertyInference.ts, 35, 3))
+>n : Symbol(n, Decl(discriminantPropertyInference.ts, 36, 7))
+>n.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
+>n : Symbol(n, Decl(discriminantPropertyInference.ts, 36, 7))
+>toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
+
+});
+
diff --git a/tests/baselines/reference/discriminantPropertyInference.types b/tests/baselines/reference/discriminantPropertyInference.types
@@ -0,0 +1,113 @@
+=== tests/cases/compiler/discriminantPropertyInference.ts ===
+// Repro from #41759
+
+type DiscriminatorTrue = {
+>DiscriminatorTrue : DiscriminatorTrue
+
+    disc: true;
+>disc : true
+>true : true
+
+    cb: (x: string) => void;
+>cb : (x: string) => void
+>x : string
+}
+
+type DiscriminatorFalse = {
+>DiscriminatorFalse : DiscriminatorFalse
+
+    disc?: false;
+>disc : false | undefined
+>false : false
+
+    cb: (x: number) => void;
+>cb : (x: number) => void
+>x : number
+}
+
+type Props = DiscriminatorTrue | DiscriminatorFalse;
+>Props : Props
+
+declare function f(options: DiscriminatorTrue | DiscriminatorFalse): any;
+>f : (options: DiscriminatorTrue | DiscriminatorFalse) => any
+>options : DiscriminatorTrue | DiscriminatorFalse
+
+// simple inference
+f({
+>f({    disc: true,    cb: s => parseInt(s)}) : any
+>f : (options: DiscriminatorTrue | DiscriminatorFalse) => any
+>{    disc: true,    cb: s => parseInt(s)} : { disc: true; cb: (s: string) => number; }
+
+    disc: true,
+>disc : true
+>true : true
+
+    cb: s => parseInt(s)
+>cb : (s: string) => number
+>s => parseInt(s) : (s: string) => number
+>s : string
+>parseInt(s) : number
+>parseInt : (string: string, radix?: number | undefined) => number
+>s : string
+
+});
+
+// simple inference
+f({
+>f({    disc: false,    cb: n => n.toFixed()}) : any
+>f : (options: DiscriminatorTrue | DiscriminatorFalse) => any
+>{    disc: false,    cb: n => n.toFixed()} : { disc: false; cb: (n: number) => string; }
+
+    disc: false,
+>disc : false
+>false : false
+
+    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
+
+});
+
+// simple inference when strict-null-checks are enabled
+f({
+>f({    disc: undefined,    cb: n => n.toFixed()}) : any
+>f : (options: DiscriminatorTrue | DiscriminatorFalse) => any
+>{    disc: undefined,    cb: n => n.toFixed()} : { disc: undefined; cb: (n: number) => string; }
+
+    disc: undefined,
+>disc : undefined
+>undefined : undefined
+
+    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
+
+});
+
+// requires checking type information since discriminator is missing from object
+f({
+>f({    cb: n => n.toFixed()}) : any
+>f : (options: DiscriminatorTrue | DiscriminatorFalse) => 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
+
+});
+
diff --git a/tests/baselines/reference/tsxDiscriminantPropertyInference.js b/tests/baselines/reference/tsxDiscriminantPropertyInference.js
@@ -0,0 +1,42 @@
+//// [tsxDiscriminantPropertyInference.tsx]
+// Repro from #41759
+namespace JSX {
+    export interface Element {}
+}
+
+type DiscriminatorTrue = {
+    disc: true;
+    cb: (x: string) => void;
+}
+
+type DiscriminatorFalse = {
+    disc?: false;
+    cb: (x: number) => void;
+}
+
+type Props = DiscriminatorTrue | DiscriminatorFalse;
+
+declare function Comp(props: DiscriminatorTrue | DiscriminatorFalse): JSX.Element;
+
+// simple inference
+void (<Comp disc cb={s => parseInt(s)} />);
+
+// simple inference
+void (<Comp disc={false} cb={n => n.toFixed()} />);
+
+// simple inference when strict-null-checks are enabled
+void (<Comp disc={undefined} cb={n => n.toFixed()} />);
+
+// requires checking type information since discriminator is missing from object
+void (<Comp cb={n => n.toFixed()} />);
+
+
+//// [tsxDiscriminantPropertyInference.jsx]
+// simple inference
+void (<Comp disc cb={function (s) { return parseInt(s); }}/>);
+// simple inference
+void (<Comp disc={false} cb={function (n) { return n.toFixed(); }}/>);
+// simple inference when strict-null-checks are enabled
+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(); }}/>);