Compiler option to require final "catch-all" case in overload declarations

[craigphicks]

🔍 Search Terms

Most permissive catch-all case, cover, overload, gap, signature call error, require overload catch-all

This proposal overlaps #57004, but does not include a change from overload single matching to overload multiple matching. That makes it a much smaller change. Notably this proposal satisfies the viability checklist while 57004 did not.

✅ Viability Checklist

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
• This isn't a request to add a new utility type: https://github.com/microsoft/TypeScript/wiki/No-New-Utility-Types
• This feature would agree with the rest of our Design Goals: https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals

⭐ Suggestion

In current TypeScript, when the compiler is faced with an overload that does not include a "most permissive catch-all case" the compiler assumes that the user has declared by omission that the "most permissive catch-all case" could result in an unexpected error, and so emits a compile error to prevent that. (See Examples below.)

Note: Herein the "most permissive catch-all case" is called the Cover).

This proposal adds a new compiler flag:

--requireOverloadCatchAll

When this flag is set, the compiler will check that the cover is declared as for overload declarations, or overload type declaration in the users source code, but not for overloads written in a declaration file (i.e., imported).

For back compatibility the default value of the flag is false.

📃 Motivating Example

Example 1, Unecessary signature call errors

Example 1: Current behavior - toy case of unnecessary signature call error

function stringOrNum(x: string): number;
function stringOrNum(x: number): string;
function stringOrNum(x: string|number): string|number {
    return x;
}

stringOrNum(Math.random()<0.5 ? "" : 0); // error
//          ~~~~~~~~~~~~~~~~~~~~~~~~~~~
// No overload matches this call.
//   Overload 1 of 2, '(x: string): number', gave the following error.
//     Argument of type 'string | number' is not assignable to parameter of type 'string'.
//       Type 'number' is not assignable to type 'string'.
//   Overload 2 of 2, '(x: number): string', gave the following error.
//     Argument of type 'string | number' is not assignable to parameter of type 'number'.
//       Type 'string' is not assignable to type 'number'.(2769)

With the --requireOverloadCatchAll flag set, the user would be required to write the cover case,

function stringOrNum(x: string): number;
function stringOrNum(x: number): string;
function stringOrNum(x: string|number): string|number;
function stringOrNum(x: string|number): string|number {
    return x;
}

and the error would not be emitted.

Example 2: Current behavior - hard to understand errors can be solved by writing cover case

A simple mapping overload function

function f(x:string):1;
function f(x:number):2;
function f(x:string|number): 1|2 {
    if (typeof x==="string") return 1;
    if (typeof x==="number") return 2;
    throw "impossible";
}

results in an difficult to understand error when applied to the array map function:

let arr: number[] | string[] = [];
arr = arr.map(f); // error
//      ~
// Argument of type '{ (x: string): 1; (x: number): 2; }' is not assignable to parameter of type '((value: number, index: number, array: number[]) => 2) & ((value: string, index: number, array: string[]) => 2)'.
//   Type '{ (x: string): 1; (x: number): 2; }' is not assignable to type '(value: string, index: number, array: string[]) => 2'.
//     Type '1' is not assignable to type '2'.(2345)
// function f(x: string): 1 (+1 overload)

With the --requireOverloadCatchAll flag set, the user would have been required to write the cover case,

function f(x:string):1;
function f(x:number):2;
function f(x:string|number):1|2;
function f( // implementation

and the error would not have occurred.

Example 3: Current behavior - No cover case is not good practice for a library writer

Example 3a:
A simple mapping function. The library writer has declared the cover case in the documentation, but not in the implementation.

// Library writer
/**
 * @throws "rangeError" if (x,y) is not in the input range described
 * by the overload declaration.
*/
function g0(x:string,y:number):1;
function g0(x:number,y:boolean):2;
function g0(x:string|number,y:number|boolean):1|2 {
    if (typeof x === "string" && typeof y === "number") return 1;
    if (typeof x === "number" && typeof y === "boolean") return 2;
    throw "rangeError"; // from library writer's POV, not  an unexpected error
}

The library client gets a hard to understand signature call error message as shown:
Example 3b:

declare const arr2: [string, number][]|[number, boolean][];
let mappped = arr2.map(g0);
// error               ~~~
// Argument of type '{ (x: string, y: number): 1; (x: number, y: boolean): 2; }' is not assignable to parameter of type '((value: [string, number], index: number, array: [string, number][]) => 2) & ((value: [number, boolean], index: number, array: [number, boolean][]) => 2)'.
//   Type '{ (x: string, y: number): 1; (x: number, y: boolean): 2; }' is not assignable to type '(value: [string, number], index: number, array: [string, number][]) => 2'.
//     Types of parameters 'x' and 'value' are incompatible.
//       Type '[string, number]' is not assignable to type 'string'.(2345)

The error is a nuisance to the library client, because the library user has already declared the cover case in the documentation so there is no danger of a mistake.

With the --requireOverloadCatchAll flag set, the library writer would be required to write the cover case,

declare function g0(x:string,y:number):1;
declare function g0(x:number,y:boolean):2;
declare function g0(x:number|string,y:boolean):1|2; // throws "rangeError" (library client needs to know this)

and the client would not have encountered the error.

A counter argument is that the library writer might want to save a few characters and write the implementation without the catch all case, e.g.

function g(x:string|number,y:number|boolean):1|2 {
    if (typeof x === "string" && typeof y === "number") return 1;
    else return 2;
}

and have the compiler emit an error, even if it troublesome for the user. That's possible, but it's not good practice for a library writer, so that is not a good argument.

Example 4: No cover case hobbles inheritance, and is not good practice for a library writer

Adapted from issue #56829.

class PDate {
  /**
   * makeData @throws "rangeError" if the date is not in the input range described
   * by the overload declaration.
  */
  makeDate(timestamp: number);
  makeDate(m: number, d: number, y: number): Date;
  //makeDate(mOrTimestamp: number, d?: number, y?: number): Date; // cover case omitted
  makeDate(mOrTimestamp: number, d?: number, y?: number): Date {
    if (d !== undefined && y !== undefined) {
      return new Date(y, mOrTimestamp, d);
    } else if (!d) {
      return new Date(mOrTimestamp);
    }
    throw "rangeError";
  }
}
class CDate extends PDate {
  constructor() {
    super();
  }
  override makeDate(...args: Parameters<PDate['makeDate']>) {

// Library client
class CDate extends PDate {
  constructor() {
    super();
  }
  override makeDate(...args: Parameters<PDate['makeDate']>) {
// error   ~~~~~~~~
// Property 'makeDate' in type 'CDate' is not assignable to the same property in base type 'PDate'.
//   Type '(m: number, d: number, y: number) => Date' is not assignable to type '{ (timestamp: number): any; (m: number, d: number, y: number): Date; }'.
//     Target signature provides too few arguments. Expected 3 or more, but got 1.(2416)
//    (method) CDate.makeDate(m: number, d: number, y: number): Date
    return super.makeDate(...args);
  }
}
    return super.makeDate(...args);
  }
}

With the --requireOverloadCatchAll flag set, the library writer would be required to write the cover case,

  makeDate(timestamp: number);
  makeDate(m: number, d: number, y: number): Date;
  makeDate(mOrTimestamp: number, d?: number, y?: number): Date; // throws "rangeError" (library client needs to know this)
  makeDate( // implementation

which would enable the library client to successfully inherit.

💻 Use Cases

As show in the examples, enabling --requireOverloadCatchAll flag would

1. save users from encountering some hard to understand errors,
2. would be useful for library writers who want to ensure that their clients don't get unnecessary signature call errors, and can successfully inherit from their classes.

---

Additional Proposals that would be user friendly additions to the --requireOverloadCatchAll proposal.

These addition proposals not abolute necessities, but would be user-friendly. It makes sense to attach them as an addendum to this proposal, because they are strongly related.

Additional Proposals Part 1:

#13219 (closed) is a superset of this (this doesn't include throw flow tracking). However, the decision to close 13219 t didn't consider it's overwhelming advantage in the context of overloads and their covers (that was part of the issue). The "throws" declaration provides the library user critical information about actions taken in the cover "catch-all" case.

Proposal: Add the ability to declare the thrown errors:

function g2(x:string,y:number):1;
function g2(x:number,y:boolean):2;
function g2(x:string|number,y:number|boolean):1|2|throws "rangeError"
function g2(x:string|number,y:number|boolean):1|2|throws "rangeError" {
    if (typeof x === "string" && typeof y === "number") return 1;
    if (typeof x === "number" && typeof y === "boolean") return 2;
    throw "rangeError";
}

This would be visible to the library client in type display.

This proposal does NOT inlcude full throw flow tracking. It just help to ensure that the library clients known the library writers intention.

Additional Proposals Part 2

• [Y] This WOULD BE a request to add a new utility type: https://github.com/microsoft/TypeScript/wiki/No-New-Utility-Types

It's troublesome to write out the cover type for the last case, so an intrinsic could be added to assist:

SetupOverload(
    functionSymbol: TypeScript Symbol // e.g. overloadFunction,
    // the extra type in addition to the explicit overloads cover type
    gapReturnType: throws | never | any = never,
    thrownType: any = undefined
): void;

where

• functionSymbol is the symbol for the overload function declaration,
• gapReturnType is a type that is either throws, never, or any other type,
  • Here throws a keyword but not a new type. To be specific: throws is to never as void is to undefined.
  • The gapReturnType is added to the cover of the explicit overloads.
• thrownType indicates the type that should be thrown when gapReturnType is throws, otherwise ignored.

Instead of writing out the cover type for the last case like this:

function g2(x:string,y:number):1;
function g2(x:number,y:boolean):2;
function g2(x:string|number,y:number|boolean):1|2|throws "rangeError"
function g2(... // implementation

the library writer could write:

function g2(x:string,y:number):1;
function g2(x:number,y:boolean):2;
SetupOverload(g2, throws "rangeError");
function g2(... // implementation

Similarly to defined an overload type without a declaration:

type CreateOverload<
    TupleOfFuncs extends [... ((...args:any[])=>any)[]],
    GapReturnType extends throws | never | any = never,
    ThrownType extends any = undefined
>; // intrinsic

SetupOverload and CreateOverload were also included in proposal #57004. Even though the --requireOverloadCatchAll proposal does not include multiple matching, these intrinsics would be useful friendly for the current overload single matching algorithm as well.

---

Update 1/26/2024 - Give GapReturn an additional choice: compilerError, which corresponds to the current behavior when no catch-all case is included. Consider this scenario:

• The user has a separate UI line corresponding to each overload case so they intend to always call with parameters narrowed to a single overload - they just want to make sure at compile time that the 1-to-1 UI-to-overload mapping is correct. It is a priori known that the overloads will never be called in any other way.

[fatcerberus]

So one issue with this I can see is: even if it’s opt-in, basically all library writers would have to adopt it, even if it doesn’t accurately model their API, because there’s no telling whether an end-user will have it enabled for their project.

There’s no concept of per-module compiler options in TypeScript.

[craigphicks]

@fatcerebus
From the Suggestion section

When this flag is set, the compiler will check that the cover is declared for overload declarations, or overload type declarations in the users source code, but not for overload declarations/types written in a declaration file (i.e., imported).

I think that's possible.

The --requireOverloadsCatchAll does not suppress the existing compile errors when an overload doesn't have a cover. (Nor for intersections of functions, when they don't include a cover - that's another issue.)

It's just that with --requireOverloadsCatchAll set, users are nagged to (1) properly handle their implementation catch-all cases (not much "extra" work), and (2) add that case to the declaration, so that (3) they and their clients don't encounter a certain class of compiler errors.

even if it doesn’t accurately model their API

So they should still be OK because their intended compile errors should still be triggered in their clients code.

[fatcerberus]

Ah, I missed the part where it doesn't apply to .d.ts files, thanks. That actually makes sense since unless the compiler sees the implementation, it doesn't even know what the cover case should look like.

Now there is still the issue that the required implementation signature might in fact be too wide that the library writer doesn't want to expose it. I wonder how often it happens in practice that the implementation signature of a set of overloads is something a user would be comfortable making public...

[craigphicks]

Could you please clarify with an example what you mean by the implementation signature of a set of overloads ?. My mental model is that the overloads are the signature of an implementation.

Maybe the problem is with my statement

with --requireOverloadsCatchAll set, users are nagged to (1) properly handle their implementation catch-all cases (not much "extra" work), and (2) add that case to the declaration, so that (3) they and their clients don't encounter a certain class of compiler errors.

where (1) and (2) should be reversed. It should be:

• (1) nag to add the catch-all case to the declaration,
• (2) doing so encourages users to make sure that the catch all case is accurately implemented

Certainly it would be ideal for the compiler to type check, as far as possible, that the implementation and overload signatures, correspond correctly. There is an open issue for doing exactly that, at least in limited cases. Of source the catch-all case should also be checked if possible. (Looking for it ....).

But this proposal does not extend to type checking the implementation against the overloads. Simple because of need to keep proposals simple.

[fatcerberus]

"Implementation signature" in reference to TypeScript overloads almost invariably refers to the last one that isn't visible to callers, containing the implementation of the function. If I've read your proposal correctly, you're proposing a new flag that forces the user to expose that signature publicly as a "cover case", but part of the reason it's not publicly exposed by default is that, generally, it ends up being too permissive for callers with no practical means of narrowing it down.

For example say I write

function add(x: number, y: number): number;
function add(x: string, y: string): string;
function add(x: string | number, y: string | number): string | number {
    return x + y;
}

I really, really don't want callers to be doing add("foo", 42) but enabling this flag would force me to allow it.

[craigphicks]

Without catch-all:

// lib
function add(x: number, y: number): number;
function add(x: string, y: string): string;
function add(x: string | number, y: string | number): string | number {
    return x as any + y;  // to suppress error, lib writer has the privilege
}

// client
declare const x: string | number;
declare const y: string | number;
// add(x,y); // error
if (typeof x === "string" && typeof y ==="string"){
    add(x,y); // string
} else if (typeof x === "number" && typeof y ==="number") {
    add(x,y); // number
} else {
}

If you wrote the the lib with the proposal + additional proposals 1 & 2:

/**
 * Throws "rangeError" if typeof x !== typeof y
 */
function add(x: number, y: number): number;
function add(x: string, y: string): string;
function add(....): ... {
    if (typeof x === typeof y)  return x as any + y;
    throw "rangeError";
}
SetupOverload(add, throws "rangeError")

the the client may write

declare const x: string | number;
declare const y: string | number;
try { 
    add(x,y); // string | number
}
catch (e) {
}