Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange behavior in type argument inference with Promises #248

Closed
JsonFreeman opened this issue Jul 25, 2014 · 5 comments
Closed

Strange behavior in type argument inference with Promises #248

JsonFreeman opened this issue Jul 25, 2014 · 5 comments
Labels
Duplicate An existing issue was already created In Discussion Not yet reached consensus Spec Issues related to the TypeScript language specification Suggestion An idea for TypeScript

Comments

@JsonFreeman
Copy link
Contributor

JsonFreeman commented Jul 25, 2014

interface Promise<T> {
    then<U>(cb: (x: T) => Promise<U>): Promise<U>;
}

// Consider both orderings of the following overloads
declare function testFunction(n: number): Promise<number>;
declare function testFunction(s: string): Promise<string>;

var numPromise: Promise<number>;
var newPromise = numPromise.then(testFunction);

In the old compiler, with either ordering of the overloads, newPromise had type Promise<{}>.
In the new compiler, if the number signature comes first, we get an error. If the string signature comes first, newPromise is of type Promise<number>! This is very inconsistent.
We have based this on the notion that the last overload will always be more general than the preceding overloads. However, with disjoint overloads, we are essentially giving the last one undue priority. The rule of thumb is that in a call, the first overload has priority, so giving the last one priority when passing a callback seems really unintuitive.

@JsonFreeman
Copy link
Contributor Author

Two proposals here:

  1. Fold type argument inference into assignment compatibility. Right now, we perform type argument inference first, and then check assignment compatibility using the inferred types in the target. We could perform type argument inference during assignment compatibility checking!
  2. A more crude, but simple proposal: For type argument inference, relate signatures pairwise quadratically (as in 1.0), but erase generic type parameters to undefined/null instead of any. Erasing to a bottom type would allow a more meaningful type to squash the bogus type when type argument inference is done.

@mhegazy mhegazy added this to the TypeScript 1.2 milestone Jul 25, 2014
@JsonFreeman
Copy link
Contributor Author

Proposal:
In Section 3.8.6 of the spec (Type Inference), we would change signature relation in the following way:
Define inferential erasure on a signature S to be the following process:
Replace with any all of S's own type parameters. In addition, all type parameters currently being inferred for get replaced. Unfixed type parameters get replaced with any, and fixed type parameters get replaced with their corresponding inferred types.

Now the new second and third sub-bullet in section 3.8.6 would look like this:

  • If M is a call/construct signature then let M' be the inferentially erased M. Then for the last call/construct signature N in S, for which N (once all N's own type parameters are instantiated to any) is assignable to M', inferences are made from parameter types in N to the corresponding parameter types in M for positions that are present in both signatures, and from the return type of N to the return type of M.

@JsonFreeman
Copy link
Contributor Author

The reason we pick the last signature instead of the first one is methods like addEventListener, which have lots of string literal overloads, followed by a string overload. We want to pick the most general overload that fits, or at least one that is not more specific than another one.

@JsonFreeman
Copy link
Contributor Author

This bug is observable in the following type baselines:

  • tests/cases/conformance/types/typeRelationships/typeInference/genericCallWithFunctionTypedArguments4.ts (var r = foo4(a);)

@JsonFreeman
Copy link
Contributor Author

This is a duplicate of #1729

@JsonFreeman JsonFreeman added the Duplicate An existing issue was already created label Jan 30, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created In Discussion Not yet reached consensus Spec Issues related to the TypeScript language specification Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants