microsoft / microsoft/TypeScript

Don't allow falsely discarded parameters when determining overloaded signature compatibility

Open
#598 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Help Wanted Suggestion
Dominant language
Go
Stars
111k
Forks
14.3k
Avg merge
2d 4h
Merged PRs (30d)
132

Description

Motivation

Libraries such as Knockout use overloaded functions, for example:

var x = new MyKnockoutModel();
console.log(x.customerName()); // string
x.customerName('bob'); // OK
x.customerName(42); // Error, 42 is not a string

We can model these in TypeScript:

interface KnockoutElement<T> {
  // Getter
  (): T;
  // Setter
  (value: T): void;
}

However, these overloads turn out to not be safe when used in callbacks. For example:

function readFile(finished: (contents: string) => void) { /*... */ }
var ko: KnockoutElement<number>;
readFile(ko); // No error, but passes a string to a number-expecting setter!

This is because we look at all the candidate source signatures, reject the (x :number) => void signature for having an incompatible type, and accept the () => T signature because it has fewer parameters (an acceptable variance).

This behavior does not map to any actual runtime behavior. The invoker's side might check the .length property of the function to determine what kind of arguments to pass, and the invokee's side might check arguments.length to determine which overload was intended, but the invoker cannot tell what the expected parameter type of the function is, and the invokee reasonably expects to be called with the correct type given a certain arity.

Proposal

As an example, if a callback declaration says it’s going to invoke f with two parameters, we should not consider overloads of f which take zero arguments if an overload with one or two arguments exists.

In 3.8.3 / 3.8.4:

M is a non-specialized call or construct signature and S’ contains a call or construct signature N where […]

Change to

M is a non-specialized call or construct signature and S’ contains a call or construct signature N where no other call or construct signature in S’ has more parameters than N but not more than M, and […]

Analysis

A sample implementation of this rule breaks no existing tests. Check stopParameterNeglect branch for code/tests.

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 with the proposal and inspect the stopParameterNeglect branch, including its code and tests. Run the existing tests first, then compare the signature-compatibility behavior with the proposed parameter-count rule. Done means overloads that accept fewer parameters are not selected when a compatible overload accepts more parameters within the callback's arity, without breaking existing tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.