Typescript: Unsafe function overload resolution when applied with generic arguments

[Igorbek]

TypeScript Version: 2.4.1

Code

Case 1

// overloaded functions
declare function f(a: number): number;
declare function f(a: string): string;

function g<T extends number | string>(a: T) {
    return f(a); // either of arguments is acceptable
//           ~
// Argument of type 'T' is not assignable to parameter of type 'string'.
//  Type 'string | number' is not assignable to type 'string'.
//    Type 'number' is not assignable to type 'string'.
}

Case 2

declare function f2<T>(a: Promise<T>): number;
declare function f2<T>(a: T): string;

function g2<T>(a: T) {
    const result: string = f2(a); // first overload is ignored, but the argument can be a Promise
    return result;
}

Expected behavior:

Case 1: compiles with no errors
Case 2: error that type string | number is not assignable to type string

Actual behavior:

Case 1: error

Argument of type 'T' is not assignable to parameter of type 'string'.
  Type 'string | number' is not assignable to type 'string'.
    Type 'number' is not assignable to type 'string'

Case 2: compiles without errors

Note

I remember here were some discussions about that, so that verifying all possible paths may result in N*M complexity (N overloads, M constituent types in unions). I could not find it.

The second case seems unsafe at all, because skips a possibly valid overload which may effect on return type. I expect that f2(a) would be of type number | string because either of these two overloads can play. It actually has the same result with a: any.

[kitsonk]

If you say a generic without any constraints, like <T> then anything can be assigned to it, therefore the first overload pattern is ignored and the second one is selected. In the first example, to compare apples to apples you would be better to write:

declare function f(a: Promise<any>): number;
declare function f(a: any): string;

[Igorbek]

Why is first overload ignored? If T can accept anything, including Promise<X> that means, in run time, the first overload would also possibly be selected.

declare function f<T>(a: Promise<T>): number;
declare function f<T>(a: T): string;

const g = <T>(a: T) => f(a); // g is a simple proxy to f

declare const p: Promise<number>;

f(p); // first overload, number in compile-time, number in run-time
g(p); // g uses second overload of f in compile-time, so the result is of type string in compile-time
      // in fact, it is number in run-time

[kitsonk]

What is f2 and g doesn't have any overloads. Your code isn't matching what you are trying to explain.

[Igorbek]

@kitsonk sorry, it was a typo. I've updated the sample code and added more explanation.

[DanielRosenwasser]

I remember here were some discussions about that, so that verifying all possible paths may result in N*M complexity (N overloads, M constituent types in unions). I could not find it.

Here you go! #1805.

[DanielRosenwasser]

I was talking to @ahejlsberg about something like this a few days ago. Maybe we can bring it up again.

[Igorbek]

@DanielRosenwasser that would be great. I've walked through referenced #1805, and it seemed to only cover mostly case 1, and completely missing case 2 where you cannot really control function application.
When case 1 is just unfortunate, case 2 is really a source of compile-time and run-time inconsistency.

[simonbuchan]

Current real world example: the current typings for react mean that React.createElement() cannot take a ReactType = keyof JSX.IntrinsicElements | ComponentType or ComponentType<P = any> = StatelessComponent<P> | ComponentClass<P>, even though it can take keyof JSX.IntrinsicElements or StatelessComponent<P> or ComponentClass<P>. This is pretty confusing!

[Igorbek]

@tycho01 pointed that the issue was identified before in https://github.com/Microsoft/TypeScript/issues/12424#issuecomment-320511965 by @jcalz when discussing #6606

[simonbuchan]

That is delaying overload resolution until generic parameters are bound, this is synthesizing a union of possible result types when overload resolution would otherwise fail, they don't seem that similar?

[simonbuchan]

Oh, reread OP, maybe it's worth opening what I thought it was as a new issue?

[simonbuchan]

Sorry for the spam: nope, it's just #1805

[Igorbek]

@simonbuchan my point for claiming they're related is that they have the same overlooked case where run-time behavior that expressed by overloads is not reflected in the type system respectfully. However I agree that it may be worth to distinguish due to different implementation paths.

[tycho01]

The generics at the repro in 12424 were unnecessary:

interface Match {
  (o: object): 0;
  (o: any): 1;
}
type Wrap = <T>(v: T) => Match(T);
type A = Wrap(RegExp);
// falls thru to 1, `object` checked not with generic val `RegExp` but with its constraint (generic `any`)

(I'm using #17961 though that wasn't needed)

If I comment A it no longer evaluates the overloads, so at least it doesn't do it that early.

It appears the Wrap evaluation gets called inside of the A evaluation's resolveCallExpression though, which does in fact have access to that RegExp. Now to figure out how to propagate that such that it won't just default to the implicit any constraint on T in the checker's checkApplicableSignature...

Edit: the type parameter's type seems to propagate through the SymbolTable. Wonder if that could help.

Edit:

So our call stack, from deep to shallow, goes like this:

chooseOverload
resolveCall
resolveCallExpression
resolveSignature
getResolvedSignature
checkCallExpression
checkExpressionWorker
checkExpression
getTypeFromTypeCallNode
getTypeFromTypeNode
getSignatureReturnTypeFromDeclaration
getSignatureFromDeclaration
getSignaturesOfSymbol
resolveAnonymousTypeMembers
resolveStructuredTypeMembers
getSignaturesOfStructuredType
getSignaturesOfType
resolveCallExpression

The point to note there is how it loops from the resolveCallExpression of the outer call (bottom of the stack, Wrap(RegExp)) to the same function for the inner call (near the top of the stack, Match(T)). So the checkApplicableSignature is where our signature gets misjudged, because it hasn't taken into account the argument types and type arguments of our outer call, which influence the outer bound (~> constraint) of the type parameters (here just <T>) for outer function Wrap.

So the question is how to pass this info in from the top resolveCallExpression where it's available to where it's needed, 18 stack layers away.

For type arguments passed in from the top (here none, as T is implicitly inferred), the best I can think of would be to like pass an altered function node in such that the type parameters (or rather, their constraints) would be pre-filled from the type args explicitly passed in from the top call. Not sure if it'd work.

The bigger question seems regular argument's types. I'd want to just similarly inject them, but it's no longer a 1:1 mapping to type parameters like it is for type arguments. Would anyone know how the type parameters are normally filled out in a call signature? In checkApplicableSignature for one I don't even see mention of them.

[gcnew]

For me the problem is the way Wrap type-checks. There is an obvious information loss there. I see two solutions:

• either allow the type parameter T to stay unconstrained as is now, but resolve the call to Match as a union of all possible returns
• or, somehow propagate the overload to the caller. This means that Wrap itself should become an overloaded function.

The first option is not very useful for type level programming. The second option seems hard to implement and not in line with the current design.

Edit:
Looking at OPs case 2 it looks like both options should work in conjunction. Resolving the result of an overloaded function call as a union of all possible returns is crucial for maintaining safety, while propagating the overloaded signature will alleviate some of the false-negatives and will be more precise.

[tycho01]

@gcnew:

This means that Wrap itself should become an overloaded function.

Hm. The way I looked at it Match(T) shouldn't actually be resolved until the T is known, just like we van do T[K] and have it resolve when it knows what's what. Or at least tricking it into evaluating a version with the info available.
I imagine overload propagation could grow fast.

I mean, resolveCallExpression knows its argument types; I think that should be sufficient to resolve the type parameters' types before having to calculate the return type.

I hope that means applying that type parameter info for the purpose of calculating the return type, by e.g. pre-filling them to use those rather than just the constraints, should do the trick.

I may not seeing the full picture though, so any insights from those who do should help a lot.

[gcnew]

Hm. The way I looked at it Match(T) shouldn't actually be resolved until the T is known

I think delaying the resolution is basically the same as propagating the overload. In both cases you'd have to compute the constraints to guarantee safety. In the propagation case you'd have the "overload" type at the top level, while in the other, you'd have to follow several indirections. I'm fine with either, as long as "all possible outcomes" (i.e. constraints) are properly computed and checked.

Note: the lazy approach would need bounds on the type variable as is the case with T[K].

[tycho01]

I think delaying the resolution is basically the same as propagating the overload.

Oh, my interpretation had been that propagating the overload would mean storing the different possible outcomes, which may sound manageable in this toy example, but perhaps less so if we'd be nesting calls with respectively n, i, j overloads, making for n * i * j potential overloads throughout the evaluation from the top level.

I'm thinking we might not necessarily need much more complexity.
Knowing:

• we just passed RegExp to v
• inferring from this that T would be RegExp as well

... then if rather than <T>(v: T) => Match(T) we'd be evaluating the return type of signature <T>(v: T extends RegExp) => Match(T) instead, I'm thinking that might already suffice to evaluate to Match(RegExp) -> 0.