Experiment: report functions with multiple returns that could be type predicates

src/compiler/checker.ts

@@ -37856,53 +37856,86 @@ 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;
 
-        return forEach(func.parameters, (param, i) => {
+        // 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.
+        const predicate = 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);
+            }
+        });
+        if (predicate && returns.length > 1) {
+            error(func.name ?? func, Diagnostics.Function_with_multiple_returns_is_implicitly_a_type_predicate);
+        }
+        return predicate;
     }
 
     /**

src/compiler/diagnosticMessages.json

@@ -1645,6 +1645,10 @@
         "category": "Error",
         "code": 1498
     },
+    "Function with multiple returns is implicitly a type predicate.": {
+        "category": "Error",
+        "code": 1499
+    },
 
     "The types of '{0}' are incompatible between these types.": {
         "category": "Error",

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)));

src/compiler/factory/nodeFactory.ts

@@ -6518,7 +6518,7 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
         return createGlobalMethodCall("Reflect", "set", receiver ? [target, propertyKey, value, receiver] : [target, propertyKey, value]);
     }
 
-    function tryAddPropertyAssignment(properties: PropertyAssignment[], propertyName: string, expression: Expression | undefined) {
+    function tryAddPropertyAssignment(properties: PropertyAssignment[], propertyName: string, expression: Expression | undefined): boolean {
         if (expression) {
             properties.push(createPropertyAssignment(propertyName, expression));
             return true;

src/services/patternMatcher.ts

@@ -528,7 +528,7 @@ function breakIntoSpans(identifier: string, word: boolean): TextSpan[] {
     return result;
 }
 
-function charIsPunctuation(ch: number) {
+function charIsPunctuation(ch: number): boolean {
     switch (ch) {
         case CharacterCodes.exclamation:
         case CharacterCodes.doubleQuote:

src/services/services.ts

@@ -2600,7 +2600,7 @@ export function createLanguageService(
         }
         else {
             // determines if the cursor is in an element tag
-            const tag = findAncestor(token.parent, n => {
+            const tag = findAncestor(token.parent, (n): boolean => {
                 if (isJsxOpeningElement(n) || isJsxClosingElement(n)) {
                     return true;
                 }

tests/baselines/reference/defaultParameterAddsUndefinedWithStrictNullChecks.errors.txt

@@ -0,0 +1,66 @@
+error TS-1: Pre-emit (0) and post-emit (1) diagnostic counts do not match! This can indicate that a semantic _error_ was added by the emit resolver - such an error may not be reflected on the command line or in the editor, but may be captured in a baseline here!
+
+
+!!! error TS-1: Pre-emit (0) and post-emit (1) diagnostic counts do not match! This can indicate that a semantic _error_ was added by the emit resolver - such an error may not be reflected on the command line or in the editor, but may be captured in a baseline here!
+!!! related TS-1: The excess diagnostics are:
+!!! related TS1499 defaultParameterAddsUndefinedWithStrictNullChecks.ts:51:10: Function with multiple returns is implicitly a type predicate.
+==== defaultParameterAddsUndefinedWithStrictNullChecks.ts (0 errors) ====
+    function f(addUndefined1 = "J", addUndefined2?: number) {
+        return addUndefined1.length + (addUndefined2 || 0);
+    }
+    function g(addUndefined = "J", addDefined: number) {
+        return addUndefined.length + addDefined;
+    }
+    let total = f() + f('a', 1) + f('b') + f(undefined, 2);
+    total = g('c', 3) + g(undefined, 4);
+
+    function foo1(x: string = "string", b: number) {
+        x.length;
+    }
+
+    function foo2(x = "string", b: number) {
+        x.length; // ok, should be string
+    }
+
+    function foo3(x: string | undefined = "string", b: number) {
+        x.length; // ok, should be string
+        x = undefined;
+    }
+
+    function foo4(x: string | undefined = undefined, b: number) {
+        x; // should be string | undefined
+        x = undefined;
+    }
+
+    type OptionalNullableString = string | null | undefined;
+    function allowsNull(val: OptionalNullableString = "") {
+        val = null;
+        val = 'string and null are both ok';
+    }
+    allowsNull(null); // still allows passing null
+
+
+
+    // .d.ts should have `string | undefined` for foo1, foo2, foo3 and foo4
+    foo1(undefined, 1);
+    foo2(undefined, 1);
+    foo3(undefined, 1);
+    foo4(undefined, 1);
+
+
+    function removeUndefinedButNotFalse(x = true) {
+        if (x === false) {
+            return x;
+        }
+    }
+
+    declare const cond: boolean;
+    function removeNothing(y = cond ? true : undefined) {
+        if (y !== undefined) {
+            if (y === false) {
+                return y;
+            }
+        }
+        return true;
+    }
+

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;

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

tests/baselines/reference/inferTypePredicates.errors.txt

@@ -1,3 +1,4 @@
+error TS-1: Pre-emit (11) and post-emit (14) diagnostic counts do not match! This can indicate that a semantic _error_ was added by the emit resolver - such an error may not be reflected on the command line or in the editor, but may be captured in a baseline here!
 inferTypePredicates.ts(4,7): error TS2322: Type '(number | null)[]' is not assignable to type 'number[]'.
   Type 'number | null' is not assignable to type 'number'.
     Type 'null' is not assignable to type 'number'.
@@ -17,6 +18,11 @@ inferTypePredicates.ts(133,7): error TS2740: Type '{}' is missing the following
 inferTypePredicates.ts(205,7): error TS2741: Property 'z' is missing in type 'C1' but required in type 'C2'.
 
 
+!!! error TS-1: Pre-emit (11) and post-emit (14) diagnostic counts do not match! This can indicate that a semantic _error_ was added by the emit resolver - such an error may not be reflected on the command line or in the editor, but may be captured in a baseline here!
+!!! related TS-1: The excess diagnostics are:
+!!! related TS1499 inferTypePredicates.ts:155:10: Function with multiple returns is implicitly a type predicate.
+!!! related TS1499 inferTypePredicates.ts:288:10: Function with multiple returns is implicitly a type predicate.
+!!! related TS1499 inferTypePredicates.ts:299:10: Function with multiple returns is implicitly a type predicate.
 ==== inferTypePredicates.ts (11 errors) ====
     // https://github.com/microsoft/TypeScript/issues/16069
 
@@ -325,4 +331,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;
+    }
 

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;