Show errors from all overloads if no overload was matched

[aj-r]

TypeScript Version: 2.7.0-dev.20171118

Code

interface Test {
    foo<T extends object>(obj: T, prop: keyof T): void;
    foo(obj: any[], index: number): void;
}

const test: Test = /* something */;
const obj = { a: 1, b: "b" };
test.foo(obj, "c");

Expected behavior: Typescript should fail with the error:

Argument of type '"c"' is not assignable to parameter of type '"b" | "a"'. (2345)

Actual behavior: Typescript fails with the error:

Argument of type '{ a: number; b: string; }' is not assignable to parameter of type 'any[]'.
  Property 'length' is missing in type '{ a: number; b: string; }'. (2345)

This is unhelpful. I was clearly going for the first overload, but I made a typo in the property name (or maybe the "c" property used to exist but was deleted). I think if typescript fails to find a match for any overload, it should display errors for all overloads with the same number of parameters. Something like:

Failed to match any overload of 'foo'.
  Argument of type '"c"' is not assignable to parameter of type '"b" | "a"'. (2345)
  Argument of type '{ a: number; b: string; }' is not assignable to parameter of type 'any[]'.
    Property 'length' is missing in type '{ a: number; b: string; }'. (2345)

If that's not possible for some reason, or if it seems too verbose, then we should at least display the error for the closest match, instead of always showing the last overload.

[mhegazy]

If that's not possible for some reason, or if it seems too verbose, then we should at least display the error for the closest match, instead of always showing the last overload.

That is the crux of the issue.. knowing what is a "closest" match. It is easy for a human to do that, but a bit hard to encode in an algorithm. we are open to suggestions for improving the user experience, if you would like to take a stab at enhancing the error reporting would be happy to take a PR.

I should note that adding all errors is not helpful. first it is confusing by itself adding unrelated errors, so there has to be some context, e.g. trying to resolve to "foo<T>(obj: T, prop: keyof T): void;" then errors, then trying to resolve to "foo(obj: any[], index: number): void;" then errors... this gets too verbose in the general case, and makes it hard to find any actionable information. The current behavior attempts to get you a single error, it is not the best all the time, but it works relatively well most of the time

Duplicate of #16793

[RyanCavanaugh]

Worth noting that some declaration files describe literally dozens of overloads for certain functions, so the worst-case behavior here would be completely inscrutable

[aj-r]

I should note that adding all errors is not helpful. first it is confusing by itself adding unrelated errors

I would argue that it's less confusing than what we sometimes do right now, which is to show an error message relating to an overload we weren't trying to use.

Worth noting that some declaration files describe literally dozens of overloads for certain functions, so the worst-case behavior here would be completely inscrutable

True, but in the case of dozens of overloads, our current strategy of choosing the last overload is extremely unlikely to output a useful error message.

I can see how printing errors for all overloads could be annoying in some cases, so maybe we can at least attempt to find a "best match". I'm thinking we could add some logic that works like this, in this order:

1. If a function has multiple overloads, choose the one with the same number of parameters. (I believe this is the current behavior?)
2. If there are multiple overloads with the same number of parameters, choose the one with the highest number of matching parameters
   • so in my example above, it would choose the first overload, because it has 1 matching parameter (obj: T), but the second overload has 0 matching parameters.
3. If multiple overloads are tied for matching parameters, then we just pick one (probably the last one to be consistent with the current behaviour).

It's not perfect, but it should at least be better than the current behavior.

[Ghabriel]

I don't think displaying all possible overloads gets too messy if the error messages are properly formatted (possibly with colors and/or markers). As an example, in the C++ world both gcc and clang show all alternatives (don't know about other compilers), e.g:

#include <vector>

template<typename T>
void foo(const T&, int) {};

template<typename T>
void foo(const std::vector<T>&, int) {};

template<typename T>
void foo(const T&, char) {};

int main(int, char**) {
    foo(123, "abc");
}

Results in:

A possible diagnostics pattern for TS can be something like:

Failed to match any overload of 'foo'.
  Candidate: foo<T extends object>(obj: T, prop: keyof T): void
      Argument of type '"c"' is not assignable to parameter of type '"b" | "a"'. (2345)

  Candidate: foo(obj: any[], index: number): void
      Argument of type '{ a: number; b: string; }' is not assignable to parameter of type 'any[]'.
        Property 'length' is missing in type '{ a: number; b: string; }'. (2345)

The empty lines between each overload can really reduce the "bloated feeling" when there are many overloads.

Edit: maybe make it clear the used generic types in the diagnostics like clang does. For example:

Candidate: foo<T extends object>(obj: T, prop: keyof T): void [with T = { a: number; b: string; }]
    Argument of type...

or

Candidate: foo<T extends object>(obj: T, prop: keyof T): void
    Note: using T = { a: number; b: string; }
    Argument of type...

[typescript-bot]

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.