From 30aef712cd60f5d38420edb5e7155caa85136c16 Mon Sep 17 00:00:00 2001 From: Dan Vanderkam Date: Thu, 11 Apr 2024 17:18:51 -0400 Subject: [PATCH 1/3] Infer type predicates with multiple returns --- src/compiler/checker.ts | 91 ++++++++++------ src/compiler/emitter.ts | 4 +- ...ameterAddsUndefinedWithStrictNullChecks.js | 2 +- ...terAddsUndefinedWithStrictNullChecks.types | 4 +- .../reference/inferTypePredicates.errors.txt | 28 +++++ .../reference/inferTypePredicates.js | 59 ++++++++++- .../reference/inferTypePredicates.symbols | 50 +++++++++ .../reference/inferTypePredicates.types | 100 +++++++++++++++++- .../reference/typeGuardsInIfStatement.types | 4 +- tests/cases/compiler/inferTypePredicates.ts | 28 +++++ 10 files changed, 329 insertions(+), 41 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 1360a884bc917..e3acebcde3cdb 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -37856,53 +37856,82 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const functionFlags = getFunctionFlags(func); if (functionFlags !== FunctionFlags.Normal) return undefined; - // Only attempt to infer a type predicate if there's exactly one return. - let singleReturn: Expression | undefined; + // Collect the returns; bail early if there's a non-boolean. + let returns: Expression[] = []; if (func.body && func.body.kind !== SyntaxKind.Block) { - singleReturn = func.body; // arrow function + returns = [func.body]; // arrow function } else { const bailedEarly = forEachReturnStatement(func.body as Block, returnStatement => { - if (singleReturn || !returnStatement.expression) return true; - singleReturn = returnStatement.expression; + if (!returnStatement.expression) return true; + const expr = skipParentheses(returnStatement.expression, /*excludeJSDocTypeAssertions*/ true); + const returnType = checkExpressionCached(expr); + if (!(returnType.flags & (TypeFlags.Boolean | TypeFlags.BooleanLiteral))) return true; + returns.push(expr); }); - if (bailedEarly || !singleReturn || functionHasImplicitReturn(func)) return undefined; + if (bailedEarly || !returns.length || functionHasImplicitReturn(func)) return undefined; } - return checkIfExpressionRefinesAnyParameter(func, singleReturn); - } - - function checkIfExpressionRefinesAnyParameter(func: FunctionLikeDeclaration, expr: Expression): TypePredicate | undefined { - expr = skipParentheses(expr, /*excludeJSDocTypeAssertions*/ true); - const returnType = checkExpressionCached(expr); - if (!(returnType.flags & TypeFlags.Boolean)) return undefined; + // We know it's all boolean returns. For each parameter, get the union of the true types. + // Then feed that union back through to make sure that a false return value corresponds to never. return forEach(func.parameters, (param, i) => { const initType = getTypeOfSymbol(param.symbol); if (!initType || initType.flags & TypeFlags.Boolean || !isIdentifier(param.name) || isSymbolAssigned(param.symbol) || isRestParameter(param)) { // Refining "x: boolean" to "x is true" or "x is false" isn't useful. return; } - const trueType = checkIfExpressionRefinesParameter(func, expr, param, initType); - if (trueType) { - return createTypePredicate(TypePredicateKind.Identifier, unescapeLeadingUnderscores(param.name.escapedText), i, trueType); - } - }); - } + let anyNonNarrowing = false; + const trueTypes: Type[] = []; + forEach(returns, expr => { + const antecedent = (expr as Expression & { flowNode?: FlowNode; }).flowNode || + expr.parent.kind === SyntaxKind.ReturnStatement && (expr.parent as ReturnStatement).flowNode || + createFlowNode(FlowFlags.Start, /*node*/ undefined, /*antecedent*/ undefined); + let localTrueType: Type; + if (expr.kind === SyntaxKind.TrueKeyword) { + localTrueType = getFlowTypeOfReference(param.name, initType, initType, func, antecedent); + } + else if (expr.kind === SyntaxKind.FalseKeyword) { + return; + } + else { + const trueCondition = createFlowNode(FlowFlags.TrueCondition, expr, antecedent); + localTrueType = getFlowTypeOfReference(param.name, initType, initType, func, trueCondition); + } - function checkIfExpressionRefinesParameter(func: FunctionLikeDeclaration, expr: Expression, param: ParameterDeclaration, initType: Type): Type | undefined { - const antecedent = (expr as Expression & { flowNode?: FlowNode; }).flowNode || - expr.parent.kind === SyntaxKind.ReturnStatement && (expr.parent as ReturnStatement).flowNode || - createFlowNode(FlowFlags.Start, /*node*/ undefined, /*antecedent*/ undefined); - const trueCondition = createFlowNode(FlowFlags.TrueCondition, expr, antecedent); + if (localTrueType === initType) { + anyNonNarrowing = true; + return true; + } - const trueType = getFlowTypeOfReference(param.name, initType, initType, func, trueCondition); - if (trueType === initType) return undefined; + trueTypes.push(localTrueType); + }); + if (anyNonNarrowing || !trueTypes.length) { + return; + } + const paramTrueType = getUnionType(trueTypes, UnionReduction.Subtype); + // Pass the trueType back into the function. The type should be narrowed to never whenever it returns false. + const anyFailed = forEach(returns, expr => { + const antecedent = (expr as Expression & { flowNode?: FlowNode; }).flowNode || + expr.parent.kind === SyntaxKind.ReturnStatement && (expr.parent as ReturnStatement).flowNode || + createFlowNode(FlowFlags.Start, /*node*/ undefined, /*antecedent*/ undefined); + let falseSubtype: Type; + if (expr.kind === SyntaxKind.TrueKeyword) { + return; + } + else if (expr.kind === SyntaxKind.FalseKeyword) { + falseSubtype = getFlowTypeOfReference(param.name, initType, paramTrueType, func, antecedent); + } + else { + const falseCondition = createFlowNode(FlowFlags.FalseCondition, expr, antecedent); + falseSubtype = getFlowTypeOfReference(param.name, initType, paramTrueType, func, falseCondition); + } - // "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 = createFlowNode(FlowFlags.FalseCondition, expr, antecedent); - const falseSubtype = getFlowTypeOfReference(param.name, initType, trueType, func, falseCondition); - return falseSubtype.flags & TypeFlags.Never ? trueType : undefined; + return (!(falseSubtype.flags & TypeFlags.Never)); + }); + if (!anyFailed) { + return createTypePredicate(TypePredicateKind.Identifier, unescapeLeadingUnderscores(param.name.escapedText), i, paramTrueType); + } + }); } /** diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 77a089585e6fc..cf334cadf4e93 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -5336,7 +5336,7 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri /** * Generate the text for a generated identifier. */ - function generateName(name: GeneratedIdentifier | GeneratedPrivateIdentifier) { + function generateName(name: GeneratedIdentifier | GeneratedPrivateIdentifier): string { const autoGenerate = name.emitNode.autoGenerate; if ((autoGenerate.flags & GeneratedIdentifierFlags.KindMask) === GeneratedIdentifierFlags.Node) { // Node names generate unique names based on their original node @@ -5351,7 +5351,7 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri } } - function generateNameCached(node: Node, privateName: boolean, flags?: GeneratedIdentifierFlags, prefix?: string | GeneratedNamePart, suffix?: string) { + function generateNameCached(node: Node, privateName: boolean, flags?: GeneratedIdentifierFlags, prefix?: string | GeneratedNamePart, suffix?: string): string { const nodeId = getNodeId(node); const cache = privateName ? nodeIdToGeneratedPrivateName : nodeIdToGeneratedName; return cache[nodeId] || (cache[nodeId] = generateNameForNode(node, privateName, flags ?? GeneratedIdentifierFlags.None, formatGeneratedNamePart(prefix, generateName), formatGeneratedNamePart(suffix))); diff --git a/tests/baselines/reference/defaultParameterAddsUndefinedWithStrictNullChecks.js b/tests/baselines/reference/defaultParameterAddsUndefinedWithStrictNullChecks.js index fc18b0fd2f0ef..c156085d2fda3 100644 --- a/tests/baselines/reference/defaultParameterAddsUndefinedWithStrictNullChecks.js +++ b/tests/baselines/reference/defaultParameterAddsUndefinedWithStrictNullChecks.js @@ -130,4 +130,4 @@ type OptionalNullableString = string | null | undefined; declare function allowsNull(val?: OptionalNullableString): void; declare function removeUndefinedButNotFalse(x?: boolean): false | undefined; declare const cond: boolean; -declare function removeNothing(y?: boolean | undefined): boolean; +declare function removeNothing(y?: boolean | undefined): y is true | undefined; diff --git a/tests/baselines/reference/defaultParameterAddsUndefinedWithStrictNullChecks.types b/tests/baselines/reference/defaultParameterAddsUndefinedWithStrictNullChecks.types index 2bac91a9434a4..a1f638d265399 100644 --- a/tests/baselines/reference/defaultParameterAddsUndefinedWithStrictNullChecks.types +++ b/tests/baselines/reference/defaultParameterAddsUndefinedWithStrictNullChecks.types @@ -302,8 +302,8 @@ declare const cond: boolean; > : ^^^^^^^ function removeNothing(y = cond ? true : undefined) { ->removeNothing : (y?: boolean | undefined) => boolean -> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +>removeNothing : (y?: boolean | undefined) => y is true | undefined +> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >y : boolean | undefined > : ^^^^^^^^^^^^^^^^^^^ >cond ? true : undefined : true | undefined diff --git a/tests/baselines/reference/inferTypePredicates.errors.txt b/tests/baselines/reference/inferTypePredicates.errors.txt index e61c1cb72beef..02865c11dbff8 100644 --- a/tests/baselines/reference/inferTypePredicates.errors.txt +++ b/tests/baselines/reference/inferTypePredicates.errors.txt @@ -325,4 +325,32 @@ inferTypePredicates.ts(205,7): error TS2741: Property 'z' is missing in type 'C1 if (foobarPred(foobar)) { foobar.foo; } + + // Returning true can result in a predicate if the function throws earlier. + function assertReturnTrue(x: string | number | Date) { + if (x instanceof Date) { + throw new Error(); + } + return true; + } + + function isStringForWhichWeHaveACaseHandler(anyString: string) { + switch (anyString) { + case 'a': + case 'b': + case 'c': + return true + default: + return false + } + } + + function ifElseIfPredicate(x: Date | string | number) { + if (x instanceof Date) { + return true; + } else if (typeof x === 'string') { + return true; + } + return false; + } \ No newline at end of file diff --git a/tests/baselines/reference/inferTypePredicates.js b/tests/baselines/reference/inferTypePredicates.js index b5802c0e50a99..5a5d5a952062c 100644 --- a/tests/baselines/reference/inferTypePredicates.js +++ b/tests/baselines/reference/inferTypePredicates.js @@ -279,6 +279,34 @@ const foobarPred = (fb: typeof foobar) => fb.type === "foo"; if (foobarPred(foobar)) { foobar.foo; } + +// Returning true can result in a predicate if the function throws earlier. +function assertReturnTrue(x: string | number | Date) { + if (x instanceof Date) { + throw new Error(); + } + return true; +} + +function isStringForWhichWeHaveACaseHandler(anyString: string) { + switch (anyString) { + case 'a': + case 'b': + case 'c': + return true + default: + return false + } +} + +function ifElseIfPredicate(x: Date | string | number) { + if (x instanceof Date) { + return true; + } else if (typeof x === 'string') { + return true; + } + return false; +} //// [inferTypePredicates.js] @@ -538,6 +566,32 @@ var foobarPred = function (fb) { return fb.type === "foo"; }; if (foobarPred(foobar)) { foobar.foo; } +// Returning true can result in a predicate if the function throws earlier. +function assertReturnTrue(x) { + if (x instanceof Date) { + throw new Error(); + } + return true; +} +function isStringForWhichWeHaveACaseHandler(anyString) { + switch (anyString) { + case 'a': + case 'b': + case 'c': + return true; + default: + return false; + } +} +function ifElseIfPredicate(x) { + if (x instanceof Date) { + return true; + } + else if (typeof x === 'string') { + return true; + } + return false; +} //// [inferTypePredicates.d.ts] @@ -583,7 +637,7 @@ declare let maybeDate: object; declare function irrelevantIsNumber(x: string | number): boolean; declare function irrelevantIsNumberDestructuring(x: string | number): boolean; declare function areBothNums(x: string | number, y: string | number): boolean; -declare function doubleReturn(x: string | number): boolean; +declare function doubleReturn(x: string | number): x is string; declare function guardsOneButNotOthers(a: string | number, b: string | number, c: string | number): b is string; declare function dunderguard(__x: number | string): __x is string; declare const booleanIdentity: (x: boolean) => boolean; @@ -630,3 +684,6 @@ declare const foobarPred: (fb: typeof foobar) => fb is { type: "foo"; foo: number; }; +declare function assertReturnTrue(x: string | number | Date): x is string | number; +declare function isStringForWhichWeHaveACaseHandler(anyString: string): anyString is "a" | "b" | "c"; +declare function ifElseIfPredicate(x: Date | string | number): x is string | Date; diff --git a/tests/baselines/reference/inferTypePredicates.symbols b/tests/baselines/reference/inferTypePredicates.symbols index 8fd879787c205..50f8b799c2153 100644 --- a/tests/baselines/reference/inferTypePredicates.symbols +++ b/tests/baselines/reference/inferTypePredicates.symbols @@ -777,3 +777,53 @@ if (foobarPred(foobar)) { >foo : Symbol(foo, Decl(inferTypePredicates.ts, 271, 18)) } +// Returning true can result in a predicate if the function throws earlier. +function assertReturnTrue(x: string | number | Date) { +>assertReturnTrue : Symbol(assertReturnTrue, Decl(inferTypePredicates.ts, 277, 1)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 280, 26)) +>Date : Symbol(Date, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.scripthost.d.ts, --, --)) + + if (x instanceof Date) { +>x : Symbol(x, Decl(inferTypePredicates.ts, 280, 26)) +>Date : Symbol(Date, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.scripthost.d.ts, --, --)) + + throw new Error(); +>Error : Symbol(Error, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) + } + return true; +} + +function isStringForWhichWeHaveACaseHandler(anyString: string) { +>isStringForWhichWeHaveACaseHandler : Symbol(isStringForWhichWeHaveACaseHandler, Decl(inferTypePredicates.ts, 285, 1)) +>anyString : Symbol(anyString, Decl(inferTypePredicates.ts, 287, 44)) + + switch (anyString) { +>anyString : Symbol(anyString, Decl(inferTypePredicates.ts, 287, 44)) + + case 'a': + case 'b': + case 'c': + return true + default: + return false + } +} + +function ifElseIfPredicate(x: Date | string | number) { +>ifElseIfPredicate : Symbol(ifElseIfPredicate, Decl(inferTypePredicates.ts, 296, 1)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 298, 27)) +>Date : Symbol(Date, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.scripthost.d.ts, --, --)) + + if (x instanceof Date) { +>x : Symbol(x, Decl(inferTypePredicates.ts, 298, 27)) +>Date : Symbol(Date, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.scripthost.d.ts, --, --)) + + return true; + } else if (typeof x === 'string') { +>x : Symbol(x, Decl(inferTypePredicates.ts, 298, 27)) + + return true; + } + return false; +} + diff --git a/tests/baselines/reference/inferTypePredicates.types b/tests/baselines/reference/inferTypePredicates.types index 691cb40881094..97b46065291ce 100644 --- a/tests/baselines/reference/inferTypePredicates.types +++ b/tests/baselines/reference/inferTypePredicates.types @@ -1059,8 +1059,8 @@ function areBothNums(x: string|number, y: string|number) { // Could potentially infer a type guard here but it would require more bookkeeping. function doubleReturn(x: string|number) { ->doubleReturn : (x: string | number) => boolean -> : ^ ^^ ^^^^^^^^^^^^ +>doubleReturn : (x: string | number) => x is string +> : ^ ^^ ^^^^^^^^^^^^^^^^ >x : string | number > : ^^^^^^^^^^^^^^^ @@ -1649,3 +1649,99 @@ if (foobarPred(foobar)) { > : ^^^^^^ } +// Returning true can result in a predicate if the function throws earlier. +function assertReturnTrue(x: string | number | Date) { +>assertReturnTrue : (x: string | number | Date) => x is string | number +> : ^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^ +>x : string | number | Date +> : ^^^^^^^^^^^^^^^^^^^^^^ + + if (x instanceof Date) { +>x instanceof Date : boolean +> : ^^^^^^^ +>x : string | number | Date +> : ^^^^^^^^^^^^^^^^^^^^^^ +>Date : DateConstructor +> : ^^^^^^^^^^^^^^^ + + throw new Error(); +>new Error() : Error +> : ^^^^^ +>Error : ErrorConstructor +> : ^^^^^^^^^^^^^^^^ + } + return true; +>true : true +> : ^^^^ +} + +function isStringForWhichWeHaveACaseHandler(anyString: string) { +>isStringForWhichWeHaveACaseHandler : (anyString: string) => anyString is "a" | "b" | "c" +> : ^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +>anyString : string +> : ^^^^^^ + + switch (anyString) { +>anyString : string +> : ^^^^^^ + + case 'a': +>'a' : "a" +> : ^^^ + + case 'b': +>'b' : "b" +> : ^^^ + + case 'c': +>'c' : "c" +> : ^^^ + + return true +>true : true +> : ^^^^ + + default: + return false +>false : false +> : ^^^^^ + } +} + +function ifElseIfPredicate(x: Date | string | number) { +>ifElseIfPredicate : (x: Date | string | number) => x is string | Date +> : ^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^ +>x : string | number | Date +> : ^^^^^^^^^^^^^^^^^^^^^^ + + if (x instanceof Date) { +>x instanceof Date : boolean +> : ^^^^^^^ +>x : string | number | Date +> : ^^^^^^^^^^^^^^^^^^^^^^ +>Date : DateConstructor +> : ^^^^^^^^^^^^^^^ + + return true; +>true : true +> : ^^^^ + + } else if (typeof x === 'string') { +>typeof x === 'string' : boolean +> : ^^^^^^^ +>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +>x : string | number +> : ^^^^^^^^^^^^^^^ +>'string' : "string" +> : ^^^^^^^^ + + return true; +>true : true +> : ^^^^ + } + return false; +>false : false +> : ^^^^^ +} + diff --git a/tests/baselines/reference/typeGuardsInIfStatement.types b/tests/baselines/reference/typeGuardsInIfStatement.types index 95a2b83831214..15c8610d1c689 100644 --- a/tests/baselines/reference/typeGuardsInIfStatement.types +++ b/tests/baselines/reference/typeGuardsInIfStatement.types @@ -320,8 +320,8 @@ function foo8(x: number | string | boolean) { } } function foo9(x: number | string) { ->foo9 : (x: number | string) => boolean -> : ^ ^^ ^^^^^^^^^^^^ +>foo9 : (x: number | string) => x is 10 | "hello" +> : ^ ^^ ^^^^^^^^^^^^^^^^^^^^^^ >x : string | number > : ^^^^^^^^^^^^^^^ diff --git a/tests/cases/compiler/inferTypePredicates.ts b/tests/cases/compiler/inferTypePredicates.ts index 9b996ee8c8414..22934301baa8d 100644 --- a/tests/cases/compiler/inferTypePredicates.ts +++ b/tests/cases/compiler/inferTypePredicates.ts @@ -279,3 +279,31 @@ const foobarPred = (fb: typeof foobar) => fb.type === "foo"; if (foobarPred(foobar)) { foobar.foo; } + +// Returning true can result in a predicate if the function throws earlier. +function assertReturnTrue(x: string | number | Date) { + if (x instanceof Date) { + throw new Error(); + } + return true; +} + +function isStringForWhichWeHaveACaseHandler(anyString: string) { + switch (anyString) { + case 'a': + case 'b': + case 'c': + return true + default: + return false + } +} + +function ifElseIfPredicate(x: Date | string | number) { + if (x instanceof Date) { + return true; + } else if (typeof x === 'string') { + return true; + } + return false; +} From 3311fd028b3e739315148f42944ad6278d141efd Mon Sep 17 00:00:00 2001 From: Dan Vanderkam Date: Thu, 11 Jul 2024 14:54:11 -0400 Subject: [PATCH 2/3] simplify and reduce diff --- src/compiler/checker.ts | 93 ++++++++++++++++++----------------------- src/compiler/emitter.ts | 4 +- 2 files changed, 43 insertions(+), 54 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 5f245b6477929..1785d3db4625a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -38359,73 +38359,62 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const bailedEarly = forEachReturnStatement(func.body as Block, returnStatement => { if (!returnStatement.expression) return true; const expr = skipParentheses(returnStatement.expression, /*excludeJSDocTypeAssertions*/ true); - const returnType = checkExpressionCached(expr); - if (!(returnType.flags & (TypeFlags.Boolean | TypeFlags.BooleanLiteral))) return true; returns.push(expr); }); if (bailedEarly || !returns.length || functionHasImplicitReturn(func)) return undefined; } + return checkIfExpressionsRefineAnyParameter(func, returns); + } + + function checkIfExpressionsRefineAnyParameter(func: FunctionLikeDeclaration, returns: readonly Expression[]): TypePredicate | undefined { + const allBoolean = every(returns, expr => { + const returnType = checkExpressionCached(expr); + return !!(returnType.flags & (TypeFlags.Boolean | TypeFlags.BooleanLiteral)); + }); + if (!allBoolean) return undefined; - // We know it's all boolean returns. For each parameter, get the union of the true types. - // Then feed that union back through to make sure that a false return value corresponds to never. return forEach(func.parameters, (param, i) => { const initType = getTypeOfSymbol(param.symbol); if (!initType || initType.flags & TypeFlags.Boolean || !isIdentifier(param.name) || isSymbolAssigned(param.symbol) || isRestParameter(param)) { // Refining "x: boolean" to "x is true" or "x is false" isn't useful. return; } - let anyNonNarrowing = false; - const trueTypes: Type[] = []; - forEach(returns, expr => { - const antecedent = (expr as Expression & { flowNode?: FlowNode; }).flowNode || - expr.parent.kind === SyntaxKind.ReturnStatement && (expr.parent as ReturnStatement).flowNode || - createFlowNode(FlowFlags.Start, /*node*/ undefined, /*antecedent*/ undefined); - let localTrueType: Type; - if (expr.kind === SyntaxKind.TrueKeyword) { - localTrueType = getFlowTypeOfReference(param.name, initType, initType, func, antecedent); - } - else if (expr.kind === SyntaxKind.FalseKeyword) { - return; - } - else { - const trueCondition = createFlowNode(FlowFlags.TrueCondition, expr, antecedent); - localTrueType = getFlowTypeOfReference(param.name, initType, initType, func, trueCondition); - } - - if (localTrueType === initType) { - anyNonNarrowing = true; - return true; - } + const trueType = checkIfExpressionsRefineParameter(func, returns, param, initType); - trueTypes.push(localTrueType); - }); - if (anyNonNarrowing || !trueTypes.length) { - return; + if (trueType) { + return createTypePredicate(TypePredicateKind.Identifier, unescapeLeadingUnderscores(param.name.escapedText), i, trueType); } - const paramTrueType = getUnionType(trueTypes, UnionReduction.Subtype); - // Pass the trueType back into the function. The type should be narrowed to never whenever it returns false. - const anyFailed = forEach(returns, expr => { - const antecedent = (expr as Expression & { flowNode?: FlowNode; }).flowNode || - expr.parent.kind === SyntaxKind.ReturnStatement && (expr.parent as ReturnStatement).flowNode || - createFlowNode(FlowFlags.Start, /*node*/ undefined, /*antecedent*/ undefined); - let falseSubtype: Type; - if (expr.kind === SyntaxKind.TrueKeyword) { - return; - } - else if (expr.kind === SyntaxKind.FalseKeyword) { - falseSubtype = getFlowTypeOfReference(param.name, initType, paramTrueType, func, antecedent); - } - else { - const falseCondition = createFlowNode(FlowFlags.FalseCondition, expr, antecedent); - falseSubtype = getFlowTypeOfReference(param.name, initType, paramTrueType, func, falseCondition); - } + }); + } - return (!(falseSubtype.flags & TypeFlags.Never)); - }); - if (!anyFailed) { - return createTypePredicate(TypePredicateKind.Identifier, unescapeLeadingUnderscores(param.name.escapedText), i, paramTrueType); - } + function checkIfExpressionsRefineParameter(func: FunctionLikeDeclaration, returns: readonly Expression[], param: ParameterDeclaration, initType: Type): Type | undefined { + const trueTypes: Type[] = []; + const anyNonNarrowing = forEach(returns, expr => { + if (expr.kind === SyntaxKind.FalseKeyword) return; + const antecedent = (expr as Expression & { flowNode?: FlowNode; }).flowNode || + expr.parent.kind === SyntaxKind.ReturnStatement && (expr.parent as ReturnStatement).flowNode || + createFlowNode(FlowFlags.Start, /*node*/ undefined, /*antecedent*/ undefined); + const trueCondition = expr.kind === SyntaxKind.TrueKeyword ? antecedent : createFlowNode(FlowFlags.TrueCondition, expr, antecedent); + const localTrueType = getFlowTypeOfReference(param.name, initType, initType, func, trueCondition); + + if (localTrueType === initType) return true; + trueTypes.push(localTrueType); + }); + if (anyNonNarrowing || !trueTypes.length) return; + const trueType = getUnionType(trueTypes, UnionReduction.Subtype); + + // "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, it should be narrowed to never whenever it returns false. + const reducesToNever = every(returns, expr => { + if (expr.kind === SyntaxKind.TrueKeyword) return true; + const antecedent = (expr as Expression & { flowNode?: FlowNode; }).flowNode || + expr.parent.kind === SyntaxKind.ReturnStatement && (expr.parent as ReturnStatement).flowNode || + createFlowNode(FlowFlags.Start, /*node*/ undefined, /*antecedent*/ undefined); + const falseCondition = expr.kind === SyntaxKind.FalseKeyword ? antecedent : createFlowNode(FlowFlags.FalseCondition, expr, antecedent); + const falseSubtype = getFlowTypeOfReference(param.name, initType, trueType, func, falseCondition); + return !!(falseSubtype.flags & TypeFlags.Never); }); + return reducesToNever ? trueType : undefined; } /** diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 567f8cfb03e73..c3b8babf5f954 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -5356,7 +5356,7 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri /** * Generate the text for a generated identifier. */ - function generateName(name: GeneratedIdentifier | GeneratedPrivateIdentifier): string { + function generateName(name: GeneratedIdentifier | GeneratedPrivateIdentifier) { const autoGenerate = name.emitNode.autoGenerate; if ((autoGenerate.flags & GeneratedIdentifierFlags.KindMask) === GeneratedIdentifierFlags.Node) { // Node names generate unique names based on their original node @@ -5371,7 +5371,7 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri } } - function generateNameCached(node: Node, privateName: boolean, flags?: GeneratedIdentifierFlags, prefix?: string | GeneratedNamePart, suffix?: string): string { + function generateNameCached(node: Node, privateName: boolean, flags?: GeneratedIdentifierFlags, prefix?: string | GeneratedNamePart, suffix?: string) { const nodeId = getNodeId(node); const cache = privateName ? nodeIdToGeneratedPrivateName : nodeIdToGeneratedName; return cache[nodeId] || (cache[nodeId] = generateNameForNode(node, privateName, flags ?? GeneratedIdentifierFlags.None, formatGeneratedNamePart(prefix, generateName), formatGeneratedNamePart(suffix))); From c64d7d04a8822aaa05a0db2d99ac42d6ad5650bd Mon Sep 17 00:00:00 2001 From: Dan Vanderkam Date: Thu, 11 Jul 2024 14:59:48 -0400 Subject: [PATCH 3/3] Add a few more tests --- .../reference/inferTypePredicates.errors.txt | 24 +++ .../reference/inferTypePredicates.js | 48 ++++++ .../reference/inferTypePredicates.symbols | 62 ++++++- .../reference/inferTypePredicates.types | 151 ++++++++++++++++++ tests/cases/compiler/inferTypePredicates.ts | 24 +++ 5 files changed, 305 insertions(+), 4 deletions(-) diff --git a/tests/baselines/reference/inferTypePredicates.errors.txt b/tests/baselines/reference/inferTypePredicates.errors.txt index 02865c11dbff8..eb8c61f9895e2 100644 --- a/tests/baselines/reference/inferTypePredicates.errors.txt +++ b/tests/baselines/reference/inferTypePredicates.errors.txt @@ -345,6 +345,24 @@ inferTypePredicates.ts(205,7): error TS2741: Property 'z' is missing in type 'C1 } } + function twoReturnExpressions(x: string | number) { + switch (typeof x) { + case 'string': + return x === 'a' || x === 'b'; + case 'number': + return x === 10 || x === 11; + } + } + + function oneNarrowingOneNot(x: string | number) { + switch (typeof x) { + case 'string': + return x === 'a' || x === 'b'; + case 'number': + return x >= 0 && x <= 10; + } + } + function ifElseIfPredicate(x: Date | string | number) { if (x instanceof Date) { return true; @@ -353,4 +371,10 @@ inferTypePredicates.ts(205,7): error TS2741: Property 'z' is missing in type 'C1 } return false; } + + function isArrayOfStrings(x: unknown) { + if (!(x instanceof Array)) return false; + + return x.every((y) => typeof y === 'string'); + } \ No newline at end of file diff --git a/tests/baselines/reference/inferTypePredicates.js b/tests/baselines/reference/inferTypePredicates.js index 5a5d5a952062c..525fa6d9058d3 100644 --- a/tests/baselines/reference/inferTypePredicates.js +++ b/tests/baselines/reference/inferTypePredicates.js @@ -299,6 +299,24 @@ function isStringForWhichWeHaveACaseHandler(anyString: string) { } } +function twoReturnExpressions(x: string | number) { + switch (typeof x) { + case 'string': + return x === 'a' || x === 'b'; + case 'number': + return x === 10 || x === 11; + } +} + +function oneNarrowingOneNot(x: string | number) { + switch (typeof x) { + case 'string': + return x === 'a' || x === 'b'; + case 'number': + return x >= 0 && x <= 10; + } +} + function ifElseIfPredicate(x: Date | string | number) { if (x instanceof Date) { return true; @@ -307,6 +325,12 @@ function ifElseIfPredicate(x: Date | string | number) { } return false; } + +function isArrayOfStrings(x: unknown) { + if (!(x instanceof Array)) return false; + + return x.every((y) => typeof y === 'string'); +} //// [inferTypePredicates.js] @@ -583,6 +607,22 @@ function isStringForWhichWeHaveACaseHandler(anyString) { return false; } } +function twoReturnExpressions(x) { + switch (typeof x) { + case 'string': + return x === 'a' || x === 'b'; + case 'number': + return x === 10 || x === 11; + } +} +function oneNarrowingOneNot(x) { + switch (typeof x) { + case 'string': + return x === 'a' || x === 'b'; + case 'number': + return x >= 0 && x <= 10; + } +} function ifElseIfPredicate(x) { if (x instanceof Date) { return true; @@ -592,6 +632,11 @@ function ifElseIfPredicate(x) { } return false; } +function isArrayOfStrings(x) { + if (!(x instanceof Array)) + return false; + return x.every(function (y) { return typeof y === 'string'; }); +} //// [inferTypePredicates.d.ts] @@ -686,4 +731,7 @@ declare const foobarPred: (fb: typeof foobar) => fb is { }; declare function assertReturnTrue(x: string | number | Date): x is string | number; declare function isStringForWhichWeHaveACaseHandler(anyString: string): anyString is "a" | "b" | "c"; +declare function twoReturnExpressions(x: string | number): x is 10 | "a" | "b" | 11; +declare function oneNarrowingOneNot(x: string | number): boolean; declare function ifElseIfPredicate(x: Date | string | number): x is string | Date; +declare function isArrayOfStrings(x: unknown): x is string[]; diff --git a/tests/baselines/reference/inferTypePredicates.symbols b/tests/baselines/reference/inferTypePredicates.symbols index 50f8b799c2153..a7722873f64eb 100644 --- a/tests/baselines/reference/inferTypePredicates.symbols +++ b/tests/baselines/reference/inferTypePredicates.symbols @@ -809,21 +809,75 @@ function isStringForWhichWeHaveACaseHandler(anyString: string) { } } +function twoReturnExpressions(x: string | number) { +>twoReturnExpressions : Symbol(twoReturnExpressions, Decl(inferTypePredicates.ts, 296, 1)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 298, 30)) + + switch (typeof x) { +>x : Symbol(x, Decl(inferTypePredicates.ts, 298, 30)) + + case 'string': + return x === 'a' || x === 'b'; +>x : Symbol(x, Decl(inferTypePredicates.ts, 298, 30)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 298, 30)) + + case 'number': + return x === 10 || x === 11; +>x : Symbol(x, Decl(inferTypePredicates.ts, 298, 30)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 298, 30)) + } +} + +function oneNarrowingOneNot(x: string | number) { +>oneNarrowingOneNot : Symbol(oneNarrowingOneNot, Decl(inferTypePredicates.ts, 305, 1)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 307, 28)) + + switch (typeof x) { +>x : Symbol(x, Decl(inferTypePredicates.ts, 307, 28)) + + case 'string': + return x === 'a' || x === 'b'; +>x : Symbol(x, Decl(inferTypePredicates.ts, 307, 28)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 307, 28)) + + case 'number': + return x >= 0 && x <= 10; +>x : Symbol(x, Decl(inferTypePredicates.ts, 307, 28)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 307, 28)) + } +} + function ifElseIfPredicate(x: Date | string | number) { ->ifElseIfPredicate : Symbol(ifElseIfPredicate, Decl(inferTypePredicates.ts, 296, 1)) ->x : Symbol(x, Decl(inferTypePredicates.ts, 298, 27)) +>ifElseIfPredicate : Symbol(ifElseIfPredicate, Decl(inferTypePredicates.ts, 314, 1)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 316, 27)) >Date : Symbol(Date, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.scripthost.d.ts, --, --)) if (x instanceof Date) { ->x : Symbol(x, Decl(inferTypePredicates.ts, 298, 27)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 316, 27)) >Date : Symbol(Date, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.scripthost.d.ts, --, --)) return true; } else if (typeof x === 'string') { ->x : Symbol(x, Decl(inferTypePredicates.ts, 298, 27)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 316, 27)) return true; } return false; } +function isArrayOfStrings(x: unknown) { +>isArrayOfStrings : Symbol(isArrayOfStrings, Decl(inferTypePredicates.ts, 323, 1)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 325, 26)) + + if (!(x instanceof Array)) return false; +>x : Symbol(x, Decl(inferTypePredicates.ts, 325, 26)) +>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) + + return x.every((y) => typeof y === 'string'); +>x.every : Symbol(Array.every, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(inferTypePredicates.ts, 325, 26)) +>every : Symbol(Array.every, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +>y : Symbol(y, Decl(inferTypePredicates.ts, 328, 18)) +>y : Symbol(y, Decl(inferTypePredicates.ts, 328, 18)) +} + diff --git a/tests/baselines/reference/inferTypePredicates.types b/tests/baselines/reference/inferTypePredicates.types index a1e54d89dd3e4..4b584131ec04b 100644 --- a/tests/baselines/reference/inferTypePredicates.types +++ b/tests/baselines/reference/inferTypePredicates.types @@ -1708,6 +1708,114 @@ function isStringForWhichWeHaveACaseHandler(anyString: string) { } } +function twoReturnExpressions(x: string | number) { +>twoReturnExpressions : (x: string | number) => x is 10 | "a" | "b" | 11 +> : ^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +>x : string | number +> : ^^^^^^^^^^^^^^^ + + switch (typeof x) { +>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +>x : string | number +> : ^^^^^^^^^^^^^^^ + + case 'string': +>'string' : "string" +> : ^^^^^^^^ + + return x === 'a' || x === 'b'; +>x === 'a' || x === 'b' : boolean +> : ^^^^^^^ +>x === 'a' : boolean +> : ^^^^^^^ +>x : string +> : ^^^^^^ +>'a' : "a" +> : ^^^ +>x === 'b' : boolean +> : ^^^^^^^ +>x : string +> : ^^^^^^ +>'b' : "b" +> : ^^^ + + case 'number': +>'number' : "number" +> : ^^^^^^^^ + + return x === 10 || x === 11; +>x === 10 || x === 11 : boolean +> : ^^^^^^^ +>x === 10 : boolean +> : ^^^^^^^ +>x : number +> : ^^^^^^ +>10 : 10 +> : ^^ +>x === 11 : boolean +> : ^^^^^^^ +>x : number +> : ^^^^^^ +>11 : 11 +> : ^^ + } +} + +function oneNarrowingOneNot(x: string | number) { +>oneNarrowingOneNot : (x: string | number) => boolean +> : ^ ^^ ^^^^^^^^^^^^ +>x : string | number +> : ^^^^^^^^^^^^^^^ + + switch (typeof x) { +>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +>x : string | number +> : ^^^^^^^^^^^^^^^ + + case 'string': +>'string' : "string" +> : ^^^^^^^^ + + return x === 'a' || x === 'b'; +>x === 'a' || x === 'b' : boolean +> : ^^^^^^^ +>x === 'a' : boolean +> : ^^^^^^^ +>x : string +> : ^^^^^^ +>'a' : "a" +> : ^^^ +>x === 'b' : boolean +> : ^^^^^^^ +>x : string +> : ^^^^^^ +>'b' : "b" +> : ^^^ + + case 'number': +>'number' : "number" +> : ^^^^^^^^ + + return x >= 0 && x <= 10; +>x >= 0 && x <= 10 : boolean +> : ^^^^^^^ +>x >= 0 : boolean +> : ^^^^^^^ +>x : number +> : ^^^^^^ +>0 : 0 +> : ^ +>x <= 10 : boolean +> : ^^^^^^^ +>x : number +> : ^^^^^^ +>10 : 10 +> : ^^ + } +} + function ifElseIfPredicate(x: Date | string | number) { >ifElseIfPredicate : (x: Date | string | number) => x is string | Date > : ^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^ @@ -1745,3 +1853,46 @@ function ifElseIfPredicate(x: Date | string | number) { > : ^^^^^ } +function isArrayOfStrings(x: unknown) { +>isArrayOfStrings : (x: unknown) => x is string[] +> : ^ ^^ ^^^^^^^^^^^^^^^^^^ +>x : unknown +> : ^^^^^^^ + + if (!(x instanceof Array)) return false; +>!(x instanceof Array) : boolean +> : ^^^^^^^ +>(x instanceof Array) : boolean +> : ^^^^^^^ +>x instanceof Array : boolean +> : ^^^^^^^ +>x : unknown +> : ^^^^^^^ +>Array : ArrayConstructor +> : ^^^^^^^^^^^^^^^^ +>false : false +> : ^^^^^ + + return x.every((y) => typeof y === 'string'); +>x.every((y) => typeof y === 'string') : boolean +> : ^^^^^^^ +>x.every : { (predicate: (value: any, index: number, array: any[]) => value is S, thisArg?: any): this is S[]; (predicate: (value: any, index: number, array: any[]) => unknown, thisArg?: any): boolean; } +> : ^^^ ^^^^^^^^^^^^^^ ^^^ ^^^^^^^ ^^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^ ^^^ ^^^ ^ ^^^ ^^^ ^^^^^^^ ^^ ^^ ^^^^^^^^^^^^ ^^ ^^^ ^^^ ^^^ +>x : any[] +> : ^^^^^ +>every : { (predicate: (value: any, index: number, array: any[]) => value is S, thisArg?: any): this is S[]; (predicate: (value: any, index: number, array: any[]) => unknown, thisArg?: any): boolean; } +> : ^^^ ^^^^^^^^^^^^^^ ^^^ ^^^^^^^ ^^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^ ^^^ ^^^ ^ ^^^ ^^^ ^^^^^^^ ^^ ^^ ^^^^^^^^^^^^ ^^ ^^^ ^^^ ^^^ +>(y) => typeof y === 'string' : (y: any) => y is string +> : ^ ^^^^^^^^^^^^^^^^^^^^^ +>y : any +> : ^^^ +>typeof y === 'string' : boolean +> : ^^^^^^^ +>typeof y : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" +> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +>y : any +> : ^^^ +>'string' : "string" +> : ^^^^^^^^ +} + diff --git a/tests/cases/compiler/inferTypePredicates.ts b/tests/cases/compiler/inferTypePredicates.ts index 22934301baa8d..3afdd27036b8d 100644 --- a/tests/cases/compiler/inferTypePredicates.ts +++ b/tests/cases/compiler/inferTypePredicates.ts @@ -299,6 +299,24 @@ function isStringForWhichWeHaveACaseHandler(anyString: string) { } } +function twoReturnExpressions(x: string | number) { + switch (typeof x) { + case 'string': + return x === 'a' || x === 'b'; + case 'number': + return x === 10 || x === 11; + } +} + +function oneNarrowingOneNot(x: string | number) { + switch (typeof x) { + case 'string': + return x === 'a' || x === 'b'; + case 'number': + return x >= 0 && x <= 10; + } +} + function ifElseIfPredicate(x: Date | string | number) { if (x instanceof Date) { return true; @@ -307,3 +325,9 @@ function ifElseIfPredicate(x: Date | string | number) { } return false; } + +function isArrayOfStrings(x: unknown) { + if (!(x instanceof Array)) return false; + + return x.every((y) => typeof y === 'string'); +}