microsoft / microsoft/TypeScript

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

Open
#57,057 6 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Awaiting More Feedback Suggestion
Dominant language
Go
Stars
111k
Forks
14.4k
Avg merge
1d 19h
Merged PRs (30d)
117

Description

🔍 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
⭐ 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

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by comparing this proposal with the referenced issue #57004 and the overload behavior shown in the motivating examples. Define the compiler-option behavior for source overload declarations versus imported declaration files, and treat support for the additional throws and utility-type proposals as separate scope.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.