From c2251635734a2307b25aa9d8e2930723b5393f7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Tue, 27 Jun 2023 10:04:28 +0200 Subject: [PATCH 1/2] Grab the left type when narrowing by in operator lazily --- src/compiler/checker.ts | 16 +-- .../reference/controlFlowInOperator.js | 71 ++++++++++ .../reference/controlFlowInOperator.symbols | 100 ++++++++++++++ .../reference/controlFlowInOperator.types | 130 ++++++++++++++++++ .../controlFlow/controlFlowInOperator.ts | 39 ++++++ 5 files changed, 347 insertions(+), 9 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d50ecd9b789d6..f8a0af7eb2cdb 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -27167,15 +27167,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { return narrowTypeByPrivateIdentifierInInExpression(type, expr, assumeTrue); } const target = getReferenceCandidate(expr.right); - const leftType = getTypeOfExpression(expr.left); - if (leftType.flags & TypeFlags.StringOrNumberLiteralOrUnique) { - if (containsMissingType(type) && isAccessExpression(reference) && isMatchingReference(reference.expression, target) && - getAccessedPropertyName(reference) === getPropertyNameFromType(leftType as StringLiteralType | NumberLiteralType | UniqueESSymbolType)) { - return getTypeWithFacts(type, assumeTrue ? TypeFacts.NEUndefined : TypeFacts.EQUndefined); - } - if (isMatchingReference(reference, target)) { - return narrowTypeByInKeyword(type, leftType as StringLiteralType | NumberLiteralType | UniqueESSymbolType, assumeTrue); - } + let leftType: Type; + if (containsMissingType(type) && isAccessExpression(reference) && isMatchingReference(reference.expression, target) && (leftType = getTypeOfExpression(expr.left)).flags & TypeFlags.StringOrNumberLiteralOrUnique && + getAccessedPropertyName(reference) === getPropertyNameFromType(leftType as StringLiteralType | NumberLiteralType | UniqueESSymbolType)) { + return getTypeWithFacts(type, assumeTrue ? TypeFacts.NEUndefined : TypeFacts.EQUndefined); + } + if (isMatchingReference(reference, target) && (leftType = getTypeOfExpression(expr.left)).flags & TypeFlags.StringOrNumberLiteralOrUnique) { + return narrowTypeByInKeyword(type, leftType as StringLiteralType | NumberLiteralType | UniqueESSymbolType, assumeTrue); } break; case SyntaxKind.CommaToken: diff --git a/tests/baselines/reference/controlFlowInOperator.js b/tests/baselines/reference/controlFlowInOperator.js index 771769710fae9..46c01de9b52b5 100644 --- a/tests/baselines/reference/controlFlowInOperator.js +++ b/tests/baselines/reference/controlFlowInOperator.js @@ -27,6 +27,45 @@ if (a in c) { if (d in c) { c; // never } + +// repro from https://github.com/microsoft/TypeScript/issues/54790 + +function uniqueID_54790( + id: string | undefined, + seenIDs: { [key: string]: string } +): string { + if (id === undefined) { + id = "1"; + } + if (!(id in seenIDs)) { + return id; + } + for (let i = 1; i < Number.MAX_VALUE; i++) { + const newID = `${id}-${i}`; + if (!(newID in seenIDs)) { + return newID; + } + } + throw Error("heat death of the universe"); +} + +function uniqueID_54790_2(id: string | number, seenIDs: object) { + id = "a"; + for (let i = 1; i < 3; i++) { + const newID = `${id}`; + if (newID in seenIDs) { + } + } +} + +function uniqueID_54790_3(id: string | number, seenIDs: object) { + id = "a"; + for (let i = 1; i < 3; i++) { + const newID = id; + if (newID in seenIDs) { + } + } +} //// [controlFlowInOperator.js] @@ -47,3 +86,35 @@ if (a in c) { if (d in c) { c; // never } +// repro from https://github.com/microsoft/TypeScript/issues/54790 +function uniqueID_54790(id, seenIDs) { + if (id === undefined) { + id = "1"; + } + if (!(id in seenIDs)) { + return id; + } + for (var i = 1; i < Number.MAX_VALUE; i++) { + var newID = "".concat(id, "-").concat(i); + if (!(newID in seenIDs)) { + return newID; + } + } + throw Error("heat death of the universe"); +} +function uniqueID_54790_2(id, seenIDs) { + id = "a"; + for (var i = 1; i < 3; i++) { + var newID = "".concat(id); + if (newID in seenIDs) { + } + } +} +function uniqueID_54790_3(id, seenIDs) { + id = "a"; + for (var i = 1; i < 3; i++) { + var newID = id; + if (newID in seenIDs) { + } + } +} diff --git a/tests/baselines/reference/controlFlowInOperator.symbols b/tests/baselines/reference/controlFlowInOperator.symbols index 2fcb56d00947a..9d66ad6cc316d 100644 --- a/tests/baselines/reference/controlFlowInOperator.symbols +++ b/tests/baselines/reference/controlFlowInOperator.symbols @@ -63,3 +63,103 @@ if (d in c) { >c : Symbol(c, Decl(controlFlowInOperator.ts, 7, 13)) } +// repro from https://github.com/microsoft/TypeScript/issues/54790 + +function uniqueID_54790( +>uniqueID_54790 : Symbol(uniqueID_54790, Decl(controlFlowInOperator.ts, 25, 1)) + + id: string | undefined, +>id : Symbol(id, Decl(controlFlowInOperator.ts, 29, 24)) + + seenIDs: { [key: string]: string } +>seenIDs : Symbol(seenIDs, Decl(controlFlowInOperator.ts, 30, 25)) +>key : Symbol(key, Decl(controlFlowInOperator.ts, 31, 14)) + +): string { + if (id === undefined) { +>id : Symbol(id, Decl(controlFlowInOperator.ts, 29, 24)) +>undefined : Symbol(undefined) + + id = "1"; +>id : Symbol(id, Decl(controlFlowInOperator.ts, 29, 24)) + } + if (!(id in seenIDs)) { +>id : Symbol(id, Decl(controlFlowInOperator.ts, 29, 24)) +>seenIDs : Symbol(seenIDs, Decl(controlFlowInOperator.ts, 30, 25)) + + return id; +>id : Symbol(id, Decl(controlFlowInOperator.ts, 29, 24)) + } + for (let i = 1; i < Number.MAX_VALUE; i++) { +>i : Symbol(i, Decl(controlFlowInOperator.ts, 39, 10)) +>i : Symbol(i, Decl(controlFlowInOperator.ts, 39, 10)) +>Number.MAX_VALUE : Symbol(NumberConstructor.MAX_VALUE, Decl(lib.es5.d.ts, --, --)) +>Number : Symbol(Number, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +>MAX_VALUE : Symbol(NumberConstructor.MAX_VALUE, Decl(lib.es5.d.ts, --, --)) +>i : Symbol(i, Decl(controlFlowInOperator.ts, 39, 10)) + + const newID = `${id}-${i}`; +>newID : Symbol(newID, Decl(controlFlowInOperator.ts, 40, 9)) +>id : Symbol(id, Decl(controlFlowInOperator.ts, 29, 24)) +>i : Symbol(i, Decl(controlFlowInOperator.ts, 39, 10)) + + if (!(newID in seenIDs)) { +>newID : Symbol(newID, Decl(controlFlowInOperator.ts, 40, 9)) +>seenIDs : Symbol(seenIDs, Decl(controlFlowInOperator.ts, 30, 25)) + + return newID; +>newID : Symbol(newID, Decl(controlFlowInOperator.ts, 40, 9)) + } + } + throw Error("heat death of the universe"); +>Error : Symbol(Error, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +} + +function uniqueID_54790_2(id: string | number, seenIDs: object) { +>uniqueID_54790_2 : Symbol(uniqueID_54790_2, Decl(controlFlowInOperator.ts, 46, 1)) +>id : Symbol(id, Decl(controlFlowInOperator.ts, 48, 26)) +>seenIDs : Symbol(seenIDs, Decl(controlFlowInOperator.ts, 48, 46)) + + id = "a"; +>id : Symbol(id, Decl(controlFlowInOperator.ts, 48, 26)) + + for (let i = 1; i < 3; i++) { +>i : Symbol(i, Decl(controlFlowInOperator.ts, 50, 10)) +>i : Symbol(i, Decl(controlFlowInOperator.ts, 50, 10)) +>i : Symbol(i, Decl(controlFlowInOperator.ts, 50, 10)) + + const newID = `${id}`; +>newID : Symbol(newID, Decl(controlFlowInOperator.ts, 51, 9)) +>id : Symbol(id, Decl(controlFlowInOperator.ts, 48, 26)) + + if (newID in seenIDs) { +>newID : Symbol(newID, Decl(controlFlowInOperator.ts, 51, 9)) +>seenIDs : Symbol(seenIDs, Decl(controlFlowInOperator.ts, 48, 46)) + } + } +} + +function uniqueID_54790_3(id: string | number, seenIDs: object) { +>uniqueID_54790_3 : Symbol(uniqueID_54790_3, Decl(controlFlowInOperator.ts, 55, 1)) +>id : Symbol(id, Decl(controlFlowInOperator.ts, 57, 26)) +>seenIDs : Symbol(seenIDs, Decl(controlFlowInOperator.ts, 57, 46)) + + id = "a"; +>id : Symbol(id, Decl(controlFlowInOperator.ts, 57, 26)) + + for (let i = 1; i < 3; i++) { +>i : Symbol(i, Decl(controlFlowInOperator.ts, 59, 10)) +>i : Symbol(i, Decl(controlFlowInOperator.ts, 59, 10)) +>i : Symbol(i, Decl(controlFlowInOperator.ts, 59, 10)) + + const newID = id; +>newID : Symbol(newID, Decl(controlFlowInOperator.ts, 60, 9)) +>id : Symbol(id, Decl(controlFlowInOperator.ts, 57, 26)) + + if (newID in seenIDs) { +>newID : Symbol(newID, Decl(controlFlowInOperator.ts, 60, 9)) +>seenIDs : Symbol(seenIDs, Decl(controlFlowInOperator.ts, 57, 46)) + } + } +} + diff --git a/tests/baselines/reference/controlFlowInOperator.types b/tests/baselines/reference/controlFlowInOperator.types index 1a5e013ff420f..7ef3c0fbd807e 100644 --- a/tests/baselines/reference/controlFlowInOperator.types +++ b/tests/baselines/reference/controlFlowInOperator.types @@ -72,3 +72,133 @@ if (d in c) { >c : (A | B) & Record<"d", unknown> } +// repro from https://github.com/microsoft/TypeScript/issues/54790 + +function uniqueID_54790( +>uniqueID_54790 : (id: string | undefined, seenIDs: { [key: string]: string; }) => string + + id: string | undefined, +>id : string + + seenIDs: { [key: string]: string } +>seenIDs : { [key: string]: string; } +>key : string + +): string { + if (id === undefined) { +>id === undefined : boolean +>id : string +>undefined : undefined + + id = "1"; +>id = "1" : "1" +>id : string +>"1" : "1" + } + if (!(id in seenIDs)) { +>!(id in seenIDs) : boolean +>(id in seenIDs) : boolean +>id in seenIDs : boolean +>id : string +>seenIDs : { [key: string]: string; } + + return id; +>id : string + } + for (let i = 1; i < Number.MAX_VALUE; i++) { +>i : number +>1 : 1 +>i < Number.MAX_VALUE : boolean +>i : number +>Number.MAX_VALUE : number +>Number : NumberConstructor +>MAX_VALUE : number +>i++ : number +>i : number + + const newID = `${id}-${i}`; +>newID : string +>`${id}-${i}` : string +>id : string +>i : number + + if (!(newID in seenIDs)) { +>!(newID in seenIDs) : boolean +>(newID in seenIDs) : boolean +>newID in seenIDs : boolean +>newID : string +>seenIDs : { [key: string]: string; } + + return newID; +>newID : string + } + } + throw Error("heat death of the universe"); +>Error("heat death of the universe") : Error +>Error : ErrorConstructor +>"heat death of the universe" : "heat death of the universe" +} + +function uniqueID_54790_2(id: string | number, seenIDs: object) { +>uniqueID_54790_2 : (id: string | number, seenIDs: object) => void +>id : string | number +>seenIDs : object + + id = "a"; +>id = "a" : "a" +>id : string | number +>"a" : "a" + + for (let i = 1; i < 3; i++) { +>i : number +>1 : 1 +>i < 3 : boolean +>i : number +>3 : 3 +>i++ : number +>i : number + + const newID = `${id}`; +>newID : string +>`${id}` : string +>id : string + + if (newID in seenIDs) { +>newID in seenIDs : boolean +>newID : string +>seenIDs : object + } + } +} + +function uniqueID_54790_3(id: string | number, seenIDs: object) { +>uniqueID_54790_3 : (id: string | number, seenIDs: object) => void +>id : string | number +>seenIDs : object + + id = "a"; +>id = "a" : "a" +>id : string | number +>"a" : "a" + + for (let i = 1; i < 3; i++) { +>i : number +>1 : 1 +>i < 3 : boolean +>i : number +>3 : 3 +>i++ : number +>i : number + + const newID = id; +>newID : string +>id : string + + if (newID in seenIDs) { +>newID in seenIDs : boolean +>newID : string +>seenIDs : object + } + } +} + diff --git a/tests/cases/conformance/controlFlow/controlFlowInOperator.ts b/tests/cases/conformance/controlFlow/controlFlowInOperator.ts index 5dc27c45e8692..ce4fbbff1ef15 100644 --- a/tests/cases/conformance/controlFlow/controlFlowInOperator.ts +++ b/tests/cases/conformance/controlFlow/controlFlowInOperator.ts @@ -24,3 +24,42 @@ if (a in c) { if (d in c) { c; // never } + +// repro from https://github.com/microsoft/TypeScript/issues/54790 + +function uniqueID_54790( + id: string | undefined, + seenIDs: { [key: string]: string } +): string { + if (id === undefined) { + id = "1"; + } + if (!(id in seenIDs)) { + return id; + } + for (let i = 1; i < Number.MAX_VALUE; i++) { + const newID = `${id}-${i}`; + if (!(newID in seenIDs)) { + return newID; + } + } + throw Error("heat death of the universe"); +} + +function uniqueID_54790_2(id: string | number, seenIDs: object) { + id = "a"; + for (let i = 1; i < 3; i++) { + const newID = `${id}`; + if (newID in seenIDs) { + } + } +} + +function uniqueID_54790_3(id: string | number, seenIDs: object) { + id = "a"; + for (let i = 1; i < 3; i++) { + const newID = id; + if (newID in seenIDs) { + } + } +} From fe2b83752b66c934209449eee4c285e06ca2b104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Wed, 19 Jul 2023 10:29:12 +0200 Subject: [PATCH 2/2] address review feedback --- src/compiler/checker.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index f8a0af7eb2cdb..e11a3bcd077d8 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -27167,13 +27167,17 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { return narrowTypeByPrivateIdentifierInInExpression(type, expr, assumeTrue); } const target = getReferenceCandidate(expr.right); - let leftType: Type; - if (containsMissingType(type) && isAccessExpression(reference) && isMatchingReference(reference.expression, target) && (leftType = getTypeOfExpression(expr.left)).flags & TypeFlags.StringOrNumberLiteralOrUnique && - getAccessedPropertyName(reference) === getPropertyNameFromType(leftType as StringLiteralType | NumberLiteralType | UniqueESSymbolType)) { - return getTypeWithFacts(type, assumeTrue ? TypeFacts.NEUndefined : TypeFacts.EQUndefined); + if (containsMissingType(type) && isAccessExpression(reference) && isMatchingReference(reference.expression, target)) { + const leftType = getTypeOfExpression(expr.left); + if (isTypeUsableAsPropertyName(leftType) && getAccessedPropertyName(reference) === getPropertyNameFromType(leftType)) { + return getTypeWithFacts(type, assumeTrue ? TypeFacts.NEUndefined : TypeFacts.EQUndefined); + } } - if (isMatchingReference(reference, target) && (leftType = getTypeOfExpression(expr.left)).flags & TypeFlags.StringOrNumberLiteralOrUnique) { - return narrowTypeByInKeyword(type, leftType as StringLiteralType | NumberLiteralType | UniqueESSymbolType, assumeTrue); + if (isMatchingReference(reference, target)) { + const leftType = getTypeOfExpression(expr.left); + if (isTypeUsableAsPropertyName(leftType)) { + return narrowTypeByInKeyword(type, leftType, assumeTrue); + } } break; case SyntaxKind.CommaToken: