From 20b6302400e8cee170b848f0719d82a3e1e88cd4 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 10 Jun 2020 12:37:14 -0700 Subject: [PATCH 1/5] Adjust TypeFact calculation for intersections to omit negative typeof facts when an equivalent positive fact is present --- src/compiler/checker.ts | 16 +++++++--- .../controlFlowTypeofFunctionElseNarrowing.js | 26 ++++++++++++++++ ...rolFlowTypeofFunctionElseNarrowing.symbols | 29 ++++++++++++++++++ ...ntrolFlowTypeofFunctionElseNarrowing.types | 30 +++++++++++++++++++ .../controlFlowTypeofFunctionElseNarrowing.ts | 15 ++++++++++ 5 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.js create mode 100644 tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.symbols create mode 100644 tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.types create mode 100644 tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 87aa1e346265d..a13f86ed1979d 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -73,7 +73,7 @@ namespace ts { TypeofNEString = 1 << 8, // typeof x !== "string" TypeofNENumber = 1 << 9, // typeof x !== "number" TypeofNEBigInt = 1 << 10, // typeof x !== "bigint" - TypeofNEBoolean = 1 << 11, // typeof x !== "boolean" + TypeofNEBoolean = 1 << 11, // typeof x !== "boolean" TypeofNESymbol = 1 << 12, // typeof x !== "symbol" TypeofNEObject = 1 << 13, // typeof x !== "object" TypeofNEFunction = 1 << 14, // typeof x !== "function" @@ -90,6 +90,7 @@ namespace ts { // The following members encode facts about particular kinds of types for use in the getTypeFacts function. // The presence of a particular fact means that the given test is true for some (and possibly all) values // of that kind of type. + NegativeTypeofFacts = TypeofNEString | TypeofNENumber | TypeofNEBigInt | TypeofNEBoolean | TypeofNESymbol | TypeofNEObject | TypeofNEFunction | TypeofNEHostObject, BaseStringStrictFacts = TypeofEQString | TypeofNENumber | TypeofNEBigInt | TypeofNEBoolean | TypeofNESymbol | TypeofNEObject | TypeofNEFunction | TypeofNEHostObject | NEUndefined | NENull | NEUndefinedOrNull, BaseStringFacts = BaseStringStrictFacts | EQUndefined | EQNull | EQUndefinedOrNull | Falsy, StringStrictFacts = BaseStringStrictFacts | Truthy | Falsy, @@ -19643,10 +19644,17 @@ namespace ts { return declaredType; } - function getTypeFactsOfTypes(types: Type[]): TypeFacts { + function getTypeFactsOfTypes(types: Type[], isUnion: boolean): TypeFacts { let result: TypeFacts = TypeFacts.None; for (const t of types) { - result |= getTypeFacts(t); + const facts = getTypeFacts(t); + result |= facts; + } + if (!isUnion) { + // Get the set of postive facts for the intersection by masking with the negative set shifted left, then shift those present positive facts into the negative fact + // value range, and unset any of those bits (by negating that mask and then intersecting it with the original value) + const positiveFacts = result & (TypeFacts.NegativeTypeofFacts << 7); + result &= ~(positiveFacts >> 7); } return result; } @@ -19722,7 +19730,7 @@ namespace ts { return getTypeFacts(getBaseConstraintOfType(type) || unknownType); } if (flags & TypeFlags.UnionOrIntersection) { - return getTypeFactsOfTypes((type).types); + return getTypeFactsOfTypes((type).types, !!(flags & TypeFlags.Union)); } return TypeFacts.All; } diff --git a/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.js b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.js new file mode 100644 index 0000000000000..7b7072e9014e6 --- /dev/null +++ b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.js @@ -0,0 +1,26 @@ +//// [controlFlowTypeofFunctionElseNarrowing.ts] +// regression for https://github.com/microsoft/TypeScript/issues/32928 +// Callable +type F = (...args: any[]) => any; +// Callable but intersected +type F2 = F & { inject?: string[] } + +declare const a: string | F2 + +if (typeof a == 'function') { + // only F2 + a +} else { + // Should be only a string + a +} + +//// [controlFlowTypeofFunctionElseNarrowing.js] +if (typeof a == 'function') { + // only F2 + a; +} +else { + // Should be only a string + a; +} diff --git a/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.symbols b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.symbols new file mode 100644 index 0000000000000..a7b12a46b0f47 --- /dev/null +++ b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.symbols @@ -0,0 +1,29 @@ +=== tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts === +// regression for https://github.com/microsoft/TypeScript/issues/32928 +// Callable +type F = (...args: any[]) => any; +>F : Symbol(F, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 0, 0)) +>args : Symbol(args, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 2, 10)) + +// Callable but intersected +type F2 = F & { inject?: string[] } +>F2 : Symbol(F2, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 2, 33)) +>F : Symbol(F, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 0, 0)) +>inject : Symbol(inject, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 4, 15)) + +declare const a: string | F2 +>a : Symbol(a, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 6, 13)) +>F2 : Symbol(F2, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 2, 33)) + +if (typeof a == 'function') { +>a : Symbol(a, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 6, 13)) + + // only F2 + a +>a : Symbol(a, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 6, 13)) + +} else { + // Should be only a string + a +>a : Symbol(a, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 6, 13)) +} diff --git a/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.types b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.types new file mode 100644 index 0000000000000..3bc0a3bd3963c --- /dev/null +++ b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.types @@ -0,0 +1,30 @@ +=== tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts === +// regression for https://github.com/microsoft/TypeScript/issues/32928 +// Callable +type F = (...args: any[]) => any; +>F : F +>args : any[] + +// Callable but intersected +type F2 = F & { inject?: string[] } +>F2 : F2 +>inject : string[] + +declare const a: string | F2 +>a : string | F2 + +if (typeof a == 'function') { +>typeof a == 'function' : boolean +>typeof a : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>a : string | F2 +>'function' : "function" + + // only F2 + a +>a : F2 + +} else { + // Should be only a string + a +>a : string +} diff --git a/tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts b/tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts new file mode 100644 index 0000000000000..2e730020ff7d4 --- /dev/null +++ b/tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts @@ -0,0 +1,15 @@ +// regression for https://github.com/microsoft/TypeScript/issues/32928 +// Callable +type F = (...args: any[]) => any; +// Callable but intersected +type F2 = F & { inject?: string[] } + +declare const a: string | F2 + +if (typeof a == 'function') { + // only F2 + a +} else { + // Should be only a string + a +} \ No newline at end of file From 269f5ba5914d2e865da255c5b10c79d259e03b9a Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Fri, 12 Jun 2020 11:53:47 -0700 Subject: [PATCH 2/5] PR feedback --- src/compiler/checker.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index dfb819d82aef8..86a1b198492ec 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -19705,14 +19705,14 @@ namespace ts { function getTypeFactsOfTypes(types: Type[], isUnion: boolean): TypeFacts { let result: TypeFacts = TypeFacts.None; for (const t of types) { - const facts = getTypeFacts(t); - result |= facts; + result |= getTypeFacts(t); } if (!isUnion) { - // Get the set of postive facts for the intersection by masking with the negative set shifted left, then shift those present positive facts into the negative fact - // value range, and unset any of those bits (by negating that mask and then intersecting it with the original value) - const positiveFacts = result & (TypeFacts.NegativeTypeofFacts << 7); - result &= ~(positiveFacts >> 7); + // Get the set of positive facts for the intersection by masking with the negative set shifted left, + // then shift those present positive facts into the negative fact value range, and unset any of those + // bits (by negating that mask and then intersecting it with the original value) + const positiveFacts = result & (TypeFacts.NegativeTypeofFacts >> 8); + result &= ~(positiveFacts << 8); } return result; } From 3de676ea02e73c4dd8f399d6c29fce549c8043c1 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 10 Jun 2020 12:37:14 -0700 Subject: [PATCH 3/5] Adjust TypeFact calculation for intersections to omit negative typeof facts when an equivalent positive fact is present --- src/compiler/checker.ts | 16 +++++++--- .../controlFlowTypeofFunctionElseNarrowing.js | 26 ++++++++++++++++ ...rolFlowTypeofFunctionElseNarrowing.symbols | 29 ++++++++++++++++++ ...ntrolFlowTypeofFunctionElseNarrowing.types | 30 +++++++++++++++++++ .../controlFlowTypeofFunctionElseNarrowing.ts | 15 ++++++++++ 5 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.js create mode 100644 tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.symbols create mode 100644 tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.types create mode 100644 tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index fb32f9b482424..110ea9560bff3 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -73,7 +73,7 @@ namespace ts { TypeofNEString = 1 << 8, // typeof x !== "string" TypeofNENumber = 1 << 9, // typeof x !== "number" TypeofNEBigInt = 1 << 10, // typeof x !== "bigint" - TypeofNEBoolean = 1 << 11, // typeof x !== "boolean" + TypeofNEBoolean = 1 << 11, // typeof x !== "boolean" TypeofNESymbol = 1 << 12, // typeof x !== "symbol" TypeofNEObject = 1 << 13, // typeof x !== "object" TypeofNEFunction = 1 << 14, // typeof x !== "function" @@ -90,6 +90,7 @@ namespace ts { // The following members encode facts about particular kinds of types for use in the getTypeFacts function. // The presence of a particular fact means that the given test is true for some (and possibly all) values // of that kind of type. + NegativeTypeofFacts = TypeofNEString | TypeofNENumber | TypeofNEBigInt | TypeofNEBoolean | TypeofNESymbol | TypeofNEObject | TypeofNEFunction | TypeofNEHostObject, BaseStringStrictFacts = TypeofEQString | TypeofNENumber | TypeofNEBigInt | TypeofNEBoolean | TypeofNESymbol | TypeofNEObject | TypeofNEFunction | TypeofNEHostObject | NEUndefined | NENull | NEUndefinedOrNull, BaseStringFacts = BaseStringStrictFacts | EQUndefined | EQNull | EQUndefinedOrNull | Falsy, StringStrictFacts = BaseStringStrictFacts | Truthy | Falsy, @@ -20287,10 +20288,17 @@ namespace ts { return declaredType; } - function getTypeFactsOfTypes(types: Type[]): TypeFacts { + function getTypeFactsOfTypes(types: Type[], isUnion: boolean): TypeFacts { let result: TypeFacts = TypeFacts.None; for (const t of types) { - result |= getTypeFacts(t); + const facts = getTypeFacts(t); + result |= facts; + } + if (!isUnion) { + // Get the set of postive facts for the intersection by masking with the negative set shifted left, then shift those present positive facts into the negative fact + // value range, and unset any of those bits (by negating that mask and then intersecting it with the original value) + const positiveFacts = result & (TypeFacts.NegativeTypeofFacts << 7); + result &= ~(positiveFacts >> 7); } return result; } @@ -20366,7 +20374,7 @@ namespace ts { return getTypeFacts(getBaseConstraintOfType(type) || unknownType); } if (flags & TypeFlags.UnionOrIntersection) { - return getTypeFactsOfTypes((type).types); + return getTypeFactsOfTypes((type).types, !!(flags & TypeFlags.Union)); } return TypeFacts.All; } diff --git a/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.js b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.js new file mode 100644 index 0000000000000..7b7072e9014e6 --- /dev/null +++ b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.js @@ -0,0 +1,26 @@ +//// [controlFlowTypeofFunctionElseNarrowing.ts] +// regression for https://github.com/microsoft/TypeScript/issues/32928 +// Callable +type F = (...args: any[]) => any; +// Callable but intersected +type F2 = F & { inject?: string[] } + +declare const a: string | F2 + +if (typeof a == 'function') { + // only F2 + a +} else { + // Should be only a string + a +} + +//// [controlFlowTypeofFunctionElseNarrowing.js] +if (typeof a == 'function') { + // only F2 + a; +} +else { + // Should be only a string + a; +} diff --git a/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.symbols b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.symbols new file mode 100644 index 0000000000000..a7b12a46b0f47 --- /dev/null +++ b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.symbols @@ -0,0 +1,29 @@ +=== tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts === +// regression for https://github.com/microsoft/TypeScript/issues/32928 +// Callable +type F = (...args: any[]) => any; +>F : Symbol(F, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 0, 0)) +>args : Symbol(args, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 2, 10)) + +// Callable but intersected +type F2 = F & { inject?: string[] } +>F2 : Symbol(F2, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 2, 33)) +>F : Symbol(F, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 0, 0)) +>inject : Symbol(inject, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 4, 15)) + +declare const a: string | F2 +>a : Symbol(a, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 6, 13)) +>F2 : Symbol(F2, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 2, 33)) + +if (typeof a == 'function') { +>a : Symbol(a, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 6, 13)) + + // only F2 + a +>a : Symbol(a, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 6, 13)) + +} else { + // Should be only a string + a +>a : Symbol(a, Decl(controlFlowTypeofFunctionElseNarrowing.ts, 6, 13)) +} diff --git a/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.types b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.types new file mode 100644 index 0000000000000..3bc0a3bd3963c --- /dev/null +++ b/tests/baselines/reference/controlFlowTypeofFunctionElseNarrowing.types @@ -0,0 +1,30 @@ +=== tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts === +// regression for https://github.com/microsoft/TypeScript/issues/32928 +// Callable +type F = (...args: any[]) => any; +>F : F +>args : any[] + +// Callable but intersected +type F2 = F & { inject?: string[] } +>F2 : F2 +>inject : string[] + +declare const a: string | F2 +>a : string | F2 + +if (typeof a == 'function') { +>typeof a == 'function' : boolean +>typeof a : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +>a : string | F2 +>'function' : "function" + + // only F2 + a +>a : F2 + +} else { + // Should be only a string + a +>a : string +} diff --git a/tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts b/tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts new file mode 100644 index 0000000000000..2e730020ff7d4 --- /dev/null +++ b/tests/cases/conformance/controlFlow/controlFlowTypeofFunctionElseNarrowing.ts @@ -0,0 +1,15 @@ +// regression for https://github.com/microsoft/TypeScript/issues/32928 +// Callable +type F = (...args: any[]) => any; +// Callable but intersected +type F2 = F & { inject?: string[] } + +declare const a: string | F2 + +if (typeof a == 'function') { + // only F2 + a +} else { + // Should be only a string + a +} \ No newline at end of file From 161dda35659cd343f5f99585e5fd04ef3c68a093 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Fri, 12 Jun 2020 11:53:47 -0700 Subject: [PATCH 4/5] PR feedback --- src/compiler/checker.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 110ea9560bff3..214094fcc7b23 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -20291,14 +20291,14 @@ namespace ts { function getTypeFactsOfTypes(types: Type[], isUnion: boolean): TypeFacts { let result: TypeFacts = TypeFacts.None; for (const t of types) { - const facts = getTypeFacts(t); - result |= facts; + result |= getTypeFacts(t); } if (!isUnion) { - // Get the set of postive facts for the intersection by masking with the negative set shifted left, then shift those present positive facts into the negative fact - // value range, and unset any of those bits (by negating that mask and then intersecting it with the original value) - const positiveFacts = result & (TypeFacts.NegativeTypeofFacts << 7); - result &= ~(positiveFacts >> 7); + // Get the set of positive facts for the intersection by masking with the negative set shifted left, + // then shift those present positive facts into the negative fact value range, and unset any of those + // bits (by negating that mask and then intersecting it with the original value) + const positiveFacts = result & (TypeFacts.NegativeTypeofFacts >> 8); + result &= ~(positiveFacts << 8); } return result; } From 5425753bd51c5c3a9a8c5ba6265d32c1eab6ba9c Mon Sep 17 00:00:00 2001 From: Orta Date: Thu, 17 Sep 2020 12:05:52 -0400 Subject: [PATCH 5/5] Adds a unit test for the bitmasking logic --- src/compiler/checker.ts | 3 +- src/testRunner/tsconfig.json | 1 + .../unittests/tsc/getTypeFactsOfTypes.ts | 40 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 src/testRunner/unittests/tsc/getTypeFactsOfTypes.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 214094fcc7b23..22eafbdeb1f9c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -60,7 +60,8 @@ namespace ts { GeneratorYield, } - const enum TypeFacts { + /** @internal */ + export const enum TypeFacts { None = 0, TypeofEQString = 1 << 0, // typeof x === "string" TypeofEQNumber = 1 << 1, // typeof x === "number" diff --git a/src/testRunner/tsconfig.json b/src/testRunner/tsconfig.json index 8b0123e1ef9be..1c4b45a9dae95 100644 --- a/src/testRunner/tsconfig.json +++ b/src/testRunner/tsconfig.json @@ -130,6 +130,7 @@ "unittests/tsbuild/watchMode.ts", "unittests/tsc/composite.ts", "unittests/tsc/declarationEmit.ts", + "unittests/tsc/getTypeFactsOfTypes.ts", "unittests/tsc/incremental.ts", "unittests/tsc/listFilesOnly.ts", "unittests/tsc/projectReferences.ts", diff --git a/src/testRunner/unittests/tsc/getTypeFactsOfTypes.ts b/src/testRunner/unittests/tsc/getTypeFactsOfTypes.ts new file mode 100644 index 0000000000000..263d3ee0bed6a --- /dev/null +++ b/src/testRunner/unittests/tsc/getTypeFactsOfTypes.ts @@ -0,0 +1,40 @@ +namespace ts { + describe("unittests:: tsc:: getTypeFactsOfTypes::", () => { + // See: https://github.com/microsoft/TypeScript/pull/39016 + it("correctly can strip NegativeTypeofFacts when there are PositiveTypeofFacts of the same type via bitmask logic", () => { + const TypeFacts = (ts as any).TypeFacts; + + // For a set of facts, which include both the positive and negative of each other + const positiveFacts = [ + TypeFacts.TypeofEQString, + TypeFacts.TypeofEQNumber, + TypeFacts.TypeofEQBigInt, + TypeFacts.TypeofEQBoolean, + TypeFacts.TypeofEQSymbol, + TypeFacts.TypeofEQObject, + TypeFacts.TypeofEQFunction, + TypeFacts.TypeofEQHostObject, + ]; + + const negativeFacts = [ + TypeFacts.TypeofNEString, + TypeFacts.TypeofNENumber, + TypeFacts.TypeofNEBigInt, + TypeFacts.TypeofNEBoolean, + TypeFacts.TypeofNESymbol, + TypeFacts.TypeofNEObject, + TypeFacts.TypeofNEFunction, + TypeFacts.TypeofNEHostObject, + ]; + + // Using this line of code (with the 8 which effectively represents the number of EQ types) + positiveFacts.forEach((pf, i) => { + const nf = pf << 8; + assert.equal(nf, negativeFacts[i]); + }); + + // If this test has failed, you _probably_ need to adjust the 8 here, and in the + // getTypeFactsOfTypes function in checker.ts. + }); + }); +}