Fix narrowing by typeof applied to discriminant property (microsoft…

…#51720) * Fix narrowing by typeof applied to discriminant property * Include effects of getReferenceCandidate * Add tests
frmhd · Dec 5, 2022 · c07f512 · c07f512
1 parent 048029e
commit c07f512
Show file tree

Hide file tree

Showing 5 changed files with 362 additions and 4 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -26484,13 +26484,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             }
             const target = getReferenceCandidate(typeOfExpr.expression);
             if (!isMatchingReference(reference, target)) {
-                const propertyAccess = getDiscriminantPropertyAccess(typeOfExpr.expression, type);
+                if (strictNullChecks && optionalChainContainsReference(target, reference) && assumeTrue === (literal.text !== "undefined")) {
+                    type = getAdjustedTypeWithFacts(type, TypeFacts.NEUndefinedOrNull);
+                }
+                const propertyAccess = getDiscriminantPropertyAccess(target, type);
                 if (propertyAccess) {
                     return narrowTypeByDiscriminant(type, propertyAccess, t => narrowTypeByLiteralExpression(t, literal, assumeTrue));
                 }
-                if (strictNullChecks && optionalChainContainsReference(target, reference) && assumeTrue === (literal.text !== "undefined")) {
-                    return getAdjustedTypeWithFacts(type, TypeFacts.NEUndefinedOrNull);
-                }
                 return type;
             }
             return narrowTypeByLiteralExpression(type, literal, assumeTrue);

diff --git a/tests/baselines/reference/narrowingTypeofDiscriminant.js b/tests/baselines/reference/narrowingTypeofDiscriminant.js
@@ -0,0 +1,81 @@
+//// [narrowingTypeofDiscriminant.ts]
+function f1(obj: { kind: 'a', data: string } | { kind: 1, data: number }) {
+    if (typeof obj.kind === "string") {
+        obj;  // { kind: 'a', data: string }
+    }
+    else {
+        obj;  // { kind: 1, data: number }
+    }
+}
+
+function f2(obj: { kind: 'a', data: string } | { kind: 1, data: number } | undefined) {
+    if (typeof obj?.kind === "string") {
+        obj;  // { kind: 'a', data: string }
+    }
+    else {
+        obj;  // { kind: 1, data: number } | undefined
+    }
+}
+
+// Repro from #51700
+
+type WrappedStringOr<T> = { value?: string } | { value?: T };
+
+function numberOk(wrapped: WrappedStringOr<number> | null) {
+    if (typeof wrapped?.value !== 'string') {
+        return null;
+    }
+    return wrapped.value;
+}
+
+function booleanBad(wrapped: WrappedStringOr<boolean> | null) {
+    if (typeof wrapped?.value !== 'string') {
+        return null;
+    }
+    return wrapped.value;
+}
+
+function booleanFixed(wrapped: WrappedStringOr<boolean> | null) {
+    if (typeof (wrapped?.value) !== 'string') {
+        return null;
+    }
+    return wrapped.value;
+}
+
+
+//// [narrowingTypeofDiscriminant.js]
+"use strict";
+function f1(obj) {
+    if (typeof obj.kind === "string") {
+        obj; // { kind: 'a', data: string }
+    }
+    else {
+        obj; // { kind: 1, data: number }
+    }
+}
+function f2(obj) {
+    if (typeof (obj === null || obj === void 0 ? void 0 : obj.kind) === "string") {
+        obj; // { kind: 'a', data: string }
+    }
+    else {
+        obj; // { kind: 1, data: number } | undefined
+    }
+}
+function numberOk(wrapped) {
+    if (typeof (wrapped === null || wrapped === void 0 ? void 0 : wrapped.value) !== 'string') {
+        return null;
+    }
+    return wrapped.value;
+}
+function booleanBad(wrapped) {
+    if (typeof (wrapped === null || wrapped === void 0 ? void 0 : wrapped.value) !== 'string') {
+        return null;
+    }
+    return wrapped.value;
+}
+function booleanFixed(wrapped) {
+    if (typeof (wrapped === null || wrapped === void 0 ? void 0 : wrapped.value) !== 'string') {
+        return null;
+    }
+    return wrapped.value;
+}
diff --git a/tests/baselines/reference/narrowingTypeofDiscriminant.symbols b/tests/baselines/reference/narrowingTypeofDiscriminant.symbols
@@ -0,0 +1,108 @@
+=== tests/cases/compiler/narrowingTypeofDiscriminant.ts ===
+function f1(obj: { kind: 'a', data: string } | { kind: 1, data: number }) {
+>f1 : Symbol(f1, Decl(narrowingTypeofDiscriminant.ts, 0, 0))
+>obj : Symbol(obj, Decl(narrowingTypeofDiscriminant.ts, 0, 12))
+>kind : Symbol(kind, Decl(narrowingTypeofDiscriminant.ts, 0, 18))
+>data : Symbol(data, Decl(narrowingTypeofDiscriminant.ts, 0, 29))
+>kind : Symbol(kind, Decl(narrowingTypeofDiscriminant.ts, 0, 48))
+>data : Symbol(data, Decl(narrowingTypeofDiscriminant.ts, 0, 57))
+
+    if (typeof obj.kind === "string") {
+>obj.kind : Symbol(kind, Decl(narrowingTypeofDiscriminant.ts, 0, 18), Decl(narrowingTypeofDiscriminant.ts, 0, 48))
+>obj : Symbol(obj, Decl(narrowingTypeofDiscriminant.ts, 0, 12))
+>kind : Symbol(kind, Decl(narrowingTypeofDiscriminant.ts, 0, 18), Decl(narrowingTypeofDiscriminant.ts, 0, 48))
+
+        obj;  // { kind: 'a', data: string }
+>obj : Symbol(obj, Decl(narrowingTypeofDiscriminant.ts, 0, 12))
+    }
+    else {
+        obj;  // { kind: 1, data: number }
+>obj : Symbol(obj, Decl(narrowingTypeofDiscriminant.ts, 0, 12))
+    }
+}
+
+function f2(obj: { kind: 'a', data: string } | { kind: 1, data: number } | undefined) {
+>f2 : Symbol(f2, Decl(narrowingTypeofDiscriminant.ts, 7, 1))
+>obj : Symbol(obj, Decl(narrowingTypeofDiscriminant.ts, 9, 12))
+>kind : Symbol(kind, Decl(narrowingTypeofDiscriminant.ts, 9, 18))
+>data : Symbol(data, Decl(narrowingTypeofDiscriminant.ts, 9, 29))
+>kind : Symbol(kind, Decl(narrowingTypeofDiscriminant.ts, 9, 48))
+>data : Symbol(data, Decl(narrowingTypeofDiscriminant.ts, 9, 57))
+
+    if (typeof obj?.kind === "string") {
+>obj?.kind : Symbol(kind, Decl(narrowingTypeofDiscriminant.ts, 9, 18), Decl(narrowingTypeofDiscriminant.ts, 9, 48))
+>obj : Symbol(obj, Decl(narrowingTypeofDiscriminant.ts, 9, 12))
+>kind : Symbol(kind, Decl(narrowingTypeofDiscriminant.ts, 9, 18), Decl(narrowingTypeofDiscriminant.ts, 9, 48))
+
+        obj;  // { kind: 'a', data: string }
+>obj : Symbol(obj, Decl(narrowingTypeofDiscriminant.ts, 9, 12))
+    }
+    else {
+        obj;  // { kind: 1, data: number } | undefined
+>obj : Symbol(obj, Decl(narrowingTypeofDiscriminant.ts, 9, 12))
+    }
+}
+
+// Repro from #51700
+
+type WrappedStringOr<T> = { value?: string } | { value?: T };
+>WrappedStringOr : Symbol(WrappedStringOr, Decl(narrowingTypeofDiscriminant.ts, 16, 1))
+>T : Symbol(T, Decl(narrowingTypeofDiscriminant.ts, 20, 21))
+>value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27))
+>value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 48))
+>T : Symbol(T, Decl(narrowingTypeofDiscriminant.ts, 20, 21))
+
+function numberOk(wrapped: WrappedStringOr<number> | null) {
+>numberOk : Symbol(numberOk, Decl(narrowingTypeofDiscriminant.ts, 20, 61))
+>wrapped : Symbol(wrapped, Decl(narrowingTypeofDiscriminant.ts, 22, 18))
+>WrappedStringOr : Symbol(WrappedStringOr, Decl(narrowingTypeofDiscriminant.ts, 16, 1))
+
+    if (typeof wrapped?.value !== 'string') {
+>wrapped?.value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27), Decl(narrowingTypeofDiscriminant.ts, 20, 48))
+>wrapped : Symbol(wrapped, Decl(narrowingTypeofDiscriminant.ts, 22, 18))
+>value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27), Decl(narrowingTypeofDiscriminant.ts, 20, 48))
+
+        return null;
+    }
+    return wrapped.value;
+>wrapped.value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27), Decl(narrowingTypeofDiscriminant.ts, 20, 48))
+>wrapped : Symbol(wrapped, Decl(narrowingTypeofDiscriminant.ts, 22, 18))
+>value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27), Decl(narrowingTypeofDiscriminant.ts, 20, 48))
+}
+
+function booleanBad(wrapped: WrappedStringOr<boolean> | null) {
+>booleanBad : Symbol(booleanBad, Decl(narrowingTypeofDiscriminant.ts, 27, 1))
+>wrapped : Symbol(wrapped, Decl(narrowingTypeofDiscriminant.ts, 29, 20))
+>WrappedStringOr : Symbol(WrappedStringOr, Decl(narrowingTypeofDiscriminant.ts, 16, 1))
+
+    if (typeof wrapped?.value !== 'string') {
+>wrapped?.value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27), Decl(narrowingTypeofDiscriminant.ts, 20, 48))
+>wrapped : Symbol(wrapped, Decl(narrowingTypeofDiscriminant.ts, 29, 20))
+>value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27), Decl(narrowingTypeofDiscriminant.ts, 20, 48))
+
+        return null;
+    }
+    return wrapped.value;
+>wrapped.value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27))
+>wrapped : Symbol(wrapped, Decl(narrowingTypeofDiscriminant.ts, 29, 20))
+>value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27))
+}
+
+function booleanFixed(wrapped: WrappedStringOr<boolean> | null) {
+>booleanFixed : Symbol(booleanFixed, Decl(narrowingTypeofDiscriminant.ts, 34, 1))
+>wrapped : Symbol(wrapped, Decl(narrowingTypeofDiscriminant.ts, 36, 22))
+>WrappedStringOr : Symbol(WrappedStringOr, Decl(narrowingTypeofDiscriminant.ts, 16, 1))
+
+    if (typeof (wrapped?.value) !== 'string') {
+>wrapped?.value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27), Decl(narrowingTypeofDiscriminant.ts, 20, 48))
+>wrapped : Symbol(wrapped, Decl(narrowingTypeofDiscriminant.ts, 36, 22))
+>value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27), Decl(narrowingTypeofDiscriminant.ts, 20, 48))
+
+        return null;
+    }
+    return wrapped.value;
+>wrapped.value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27))
+>wrapped : Symbol(wrapped, Decl(narrowingTypeofDiscriminant.ts, 36, 22))
+>value : Symbol(value, Decl(narrowingTypeofDiscriminant.ts, 20, 27))
+}
+
diff --git a/tests/baselines/reference/narrowingTypeofDiscriminant.types b/tests/baselines/reference/narrowingTypeofDiscriminant.types
@@ -0,0 +1,125 @@
+=== tests/cases/compiler/narrowingTypeofDiscriminant.ts ===
+function f1(obj: { kind: 'a', data: string } | { kind: 1, data: number }) {
+>f1 : (obj: {    kind: 'a';    data: string;} | {    kind: 1;    data: number;}) => void
+>obj : { kind: 'a'; data: string; } | { kind: 1; data: number; }
+>kind : "a"
+>data : string
+>kind : 1
+>data : number
+
+    if (typeof obj.kind === "string") {
+>typeof obj.kind === "string" : boolean
+>typeof obj.kind : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
+>obj.kind : "a" | 1
+>obj : { kind: "a"; data: string; } | { kind: 1; data: number; }
+>kind : "a" | 1
+>"string" : "string"
+
+        obj;  // { kind: 'a', data: string }
+>obj : { kind: "a"; data: string; }
+    }
+    else {
+        obj;  // { kind: 1, data: number }
+>obj : { kind: 1; data: number; }
+    }
+}
+
+function f2(obj: { kind: 'a', data: string } | { kind: 1, data: number } | undefined) {
+>f2 : (obj: {    kind: 'a';    data: string;} | {    kind: 1;    data: number;} | undefined) => void
+>obj : { kind: 'a'; data: string; } | { kind: 1; data: number; } | undefined
+>kind : "a"
+>data : string
+>kind : 1
+>data : number
+
+    if (typeof obj?.kind === "string") {
+>typeof obj?.kind === "string" : boolean
+>typeof obj?.kind : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
+>obj?.kind : "a" | 1 | undefined
+>obj : { kind: "a"; data: string; } | { kind: 1; data: number; } | undefined
+>kind : "a" | 1 | undefined
+>"string" : "string"
+
+        obj;  // { kind: 'a', data: string }
+>obj : { kind: "a"; data: string; }
+    }
+    else {
+        obj;  // { kind: 1, data: number } | undefined
+>obj : { kind: 1; data: number; } | undefined
+    }
+}
+
+// Repro from #51700
+
+type WrappedStringOr<T> = { value?: string } | { value?: T };
+>WrappedStringOr : WrappedStringOr<T>
+>value : string | undefined
+>value : T | undefined
+
+function numberOk(wrapped: WrappedStringOr<number> | null) {
+>numberOk : (wrapped: WrappedStringOr<number> | null) => string | null
+>wrapped : WrappedStringOr<number> | null
+>null : null
+
+    if (typeof wrapped?.value !== 'string') {
+>typeof wrapped?.value !== 'string' : boolean
+>typeof wrapped?.value : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
+>wrapped?.value : string | number | undefined
+>wrapped : WrappedStringOr<number> | null
+>value : string | number | undefined
+>'string' : "string"
+
+        return null;
+>null : null
+    }
+    return wrapped.value;
+>wrapped.value : string
+>wrapped : WrappedStringOr<number>
+>value : string
+}
+
+function booleanBad(wrapped: WrappedStringOr<boolean> | null) {
+>booleanBad : (wrapped: WrappedStringOr<boolean> | null) => string | null
+>wrapped : WrappedStringOr<boolean> | null
+>null : null
+
+    if (typeof wrapped?.value !== 'string') {
+>typeof wrapped?.value !== 'string' : boolean
+>typeof wrapped?.value : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
+>wrapped?.value : string | boolean | undefined
+>wrapped : WrappedStringOr<boolean> | null
+>value : string | boolean | undefined
+>'string' : "string"
+
+        return null;
+>null : null
+    }
+    return wrapped.value;
+>wrapped.value : string
+>wrapped : { value?: string | undefined; }
+>value : string
+}
+
+function booleanFixed(wrapped: WrappedStringOr<boolean> | null) {
+>booleanFixed : (wrapped: WrappedStringOr<boolean> | null) => string | null
+>wrapped : WrappedStringOr<boolean> | null
+>null : null
+
+    if (typeof (wrapped?.value) !== 'string') {
+>typeof (wrapped?.value) !== 'string' : boolean
+>typeof (wrapped?.value) : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
+>(wrapped?.value) : string | boolean | undefined
+>wrapped?.value : string | boolean | undefined
+>wrapped : WrappedStringOr<boolean> | null
+>value : string | boolean | undefined
+>'string' : "string"
+
+        return null;
+>null : null
+    }
+    return wrapped.value;
+>wrapped.value : string
+>wrapped : { value?: string | undefined; }
+>value : string
+}
+
diff --git a/tests/cases/compiler/narrowingTypeofDiscriminant.ts b/tests/cases/compiler/narrowingTypeofDiscriminant.ts
@@ -0,0 +1,44 @@
+// @strict: true
+
+function f1(obj: { kind: 'a', data: string } | { kind: 1, data: number }) {
+    if (typeof obj.kind === "string") {
+        obj;  // { kind: 'a', data: string }
+    }
+    else {
+        obj;  // { kind: 1, data: number }
+    }
+}
+
+function f2(obj: { kind: 'a', data: string } | { kind: 1, data: number } | undefined) {
+    if (typeof obj?.kind === "string") {
+        obj;  // { kind: 'a', data: string }
+    }
+    else {
+        obj;  // { kind: 1, data: number } | undefined
+    }
+}
+
+// Repro from #51700
+
+type WrappedStringOr<T> = { value?: string } | { value?: T };
+
+function numberOk(wrapped: WrappedStringOr<number> | null) {
+    if (typeof wrapped?.value !== 'string') {
+        return null;
+    }
+    return wrapped.value;
+}
+
+function booleanBad(wrapped: WrappedStringOr<boolean> | null) {
+    if (typeof wrapped?.value !== 'string') {
+        return null;
+    }
+    return wrapped.value;
+}
+
+function booleanFixed(wrapped: WrappedStringOr<boolean> | null) {
+    if (typeof (wrapped?.value) !== 'string') {
+        return null;
+    }
+    return wrapped.value;
+}