Suggestion for improving generator and async function type checking

[yortus]

This is a suggestion to improve both the consistency and the type-safety of return type checking for
generator functions (GFs) and async functions (AFs).

NB: this issue refers to tsc behaviour for target >=ES6 with typescript@next as of 2016-01-28 (ie since commit a6af98e)

Current Behaviour

Consider the following examples of current behaviour for checking return type annotations:

// Current behaviour - generator functions
class MyIterator implements Iterator<any> {next}
function* gf1(): any {}             // OK
function* gf2(): MyIterator {}      // OK (but really is an error)

// Current behaviour - async functions
class MyPromise extends Promise<any> {}
async function af1(): any {}        // ERROR (but is really not an error)
async function af2(): MyPromise {}  // ERROR

Problems with Current Behaviour

Firstly, type checking is not consistent across the two kinds of functions. In the examples the GF checking is too loose and the AF checking is too strict. The inconsistency is due to the different approach to checking the return type. The two approaches may be summarised like this:

• Generator functions: accept any return type annotation that is assignable from IterableIterator<T>.
• Async functions: reject all return type annotations other than references to the global Promise<T>. This is a recent change, the rationale
  for it can be followed from here.

Secondly, the type checker only gets 2 out of 4 of the above checks right (gf1 and af2). Explanation:

• GOOD: gf1's return type annotation is not super helpful but is 100% consistent with the type system. No sense erroring here, so the implementation is good.
• BAD: gf2's return type annotation passes type checking because it passes the assignability check.
  However gf2 definitely does not return an instance of MyIterator. All generator functions
  return a generator object, so at runtime gf2() instanceof MyIterator is false. A compile error would have been helpful.
• BAD: af1's return type annotation is just like gf1: not super helpful but 100% consistent with the type system. The compiler errors here even though nothing is wrong (reason for the error is here).
• GOOD: af2's return type annotation fails type checking because it's not Promise<T>. The return type definitely won't be an instance of any class other than Promise, so the implementation is good.

Suggested Improvement

Since GFs and AFs always return instances of known instrinsic types, we can rule out any type annotation that asserts they will return an instance of some other class.

Both generator and async functions could therefore be checked with the same two steps:

1. Is Assignable: Ensure the return type of the GF/AF is assignable to the annotated return type. This is the basic check for all kinds of function return types. If not assignable, type checking fails. Otherwise, continue to step 2.
2. Not a Class Type: Ensure the return type annotation is not a class type (except Promise<T> which is allowed for AFs). For example if the return type annotation is Foo, ensure it does not refer to class Foo {...} or another class-like value.

These rules have the following effects:

• GF and AF type checking are mutually consistent.
• This fixes gf2 by ruling out class types like MyIterator in addition to checking assignability. GF type checking is made safer in general by catching a class of errors that currently slip through.
• This fixes af1, because it's no longer necessary to rule out all annotations other than Promise<T>, but just those that are assignment-compatible class types like MyPromise. This approach will catch the breaking change from 1.7 as a compile error (as desired for reason here), but allow harmless (and correct) things like any and PromiseLike<T>.

Working Implementation

This is a small code change. I implemented this in a branch as best I could (but I may have made errors since I'm still getting my head around the codebase). The diff can be seen here.

With this version of tsc the above code works as follows:

// Suggested behaviour - generator functions
class MyIterator implements Iterator<any> {next}
function* gf1(): any {}             // OK
function* gf2(): MyIterator {}      // ERROR: A generator cannot have a return type annotation of a class type.

// Suggested behaviour - async functions
class MyPromise extends Promise<any> {}
async function af1(): any {}        // OK
async function af2(): MyPromise {}  // ERROR: An async function cannot have a return type annotation of a class type other than Promise<T>.

[yortus]

TL;DR: Generator functions and async functions always return instances of intrinsic types, so they can never return instances of other class types. This observation could be used to improve consistency and correctness of return type checking for both generator functions (currently over-loose) and async functions (currently over-strict).

[mhegazy]

I asked this question before, and i do not think any thing has changed since #6068

How is the compiler expected to emit this:

async function af1(): any {}     

if as Promise, then either type it as Promise or write it as function af1(): MyPromise<someType>

[yortus]

@mhegazy things have changed since #6068. For ES6+, the emit always uses the global Promise, and the checker reserves the identifier Promise. The return type annotation is no longer used for emit, so now is conceptually no different to that of a generator function.

[yortus]

@mhegazy I just checked the emit for ES6, so now regardless of the return type annotation the emit is:

function asyncFunction() {
    return __awaiter(this, void 0, void 0, function* () {/* async function body */});
}

and the __awaiter helper now starts with:

    return new (P || (P = Promise))(function (resolve, reject) {
        ...

So basically for ES6, P is always passed as undefined and then set to Promise, regardless of how the async function was annotated.

[mhegazy]

so what is the scenario we are trying to do accomplish here? you can not have a function marked as async that returns MyPromise. what value is there to annotate it as any and keep async.

[yortus]

The value is in consistency in the type system. Annotations would ideally work consistently right across the language. I can do all of the following:

function f(): any[] { return [1,2,3]; }                        // OK
function p(): PromiseLike<any> { return Promise.resolve(42); } // OK
function* g(): Iterator<any> { yield 1; yield 2; yield 3; }    // OK

Note that none of these return type annotations is an exact match to what is actually returned, they are simply assignment-compatible. So now I have my nice consistent mental model, and I try out the following expecting it to work in the same way as the examples above:

async function a(): PromiseLike<any> { return 42; } // ERROR

...but it in fact raises a compiler error. The inconsistency is confusing (#5911 raised this very example). And as of #6631, there's no longer a need to keep this inconsistency.

[jods4]

@yortus suggested I copy my comment from #6631:

The fact that the return type must be strictly equal to Promise might become a hindrance in scenarios involving interfaces and/or methods overriding.

You can work around it by adding a layer of indirection (like all CS problems 😉) but it doesn't feel nice.

Here's one example: in frameworks such as Aurelia, many methods can return a value or a Promise for a value. Not mandating a Promise is both convenient when implementing trivial methods (like canSave() { return true; } ) and good for perf.

Say I have a base class that exposes such a function and is meant to be overriden. Assume that the base default implementation is async.

The natural way is this:

class BaseViewModel {
  async isValid() : boolean | Promise<boolean> { 
    /* some async implementation, maybe server call */  
    return await fetch("/valid");
  }
}

class DerivedViewModel extends BaseViewModel {
  isValid() : boolean | Promise<boolean> {
    return true;  // Non-async implementation
  }
}

but this wouldn't work :(

[mhegazy]

@jods4 async is not applicable in this context. marking a function as async means that the compiler will wrap it in a new Promise call all the time. if the function is not guaranteed to return a Promise it should not be marked as async, it should rather return T | Promise<T>.

This goes back to the issue that an async function will be rewritten by the compiler, there is no way to change that unless it is not marked as async. if it is marked as async, the only possible type that can come out of it is the Promise object, any thing else we allow here will be a lie.

The only proposal i see here is making a generator function returning something other than the generator interface an error to match the Promise implementation.

[yortus]

@mhegazy @jods4's example is valid both in terms of its type assertions and its runtime behaviour.

if the function is not guaranteed to return a Promise it should not be marked as async

isValid in the base class is guaranteed to return a promise so is correctly marked as async. It is annotated with boolean | Promise<boolean> so that derived classes can override it with either a sync or an async implementation, without TypeScript complaining about incompatibility.

The following workaround adds pointless boilerplate that is only neded to satisfy TypeScript (pointless since @jods4's version works fine at runtime as is currently is written, just removing the annotations) (actually it's much worse than @jods4's original because it creates a new closure instance on every call):

class BaseViewModel {
  isValid() : boolean | Promise<boolean> { 
      return (async () => {
          /* some async implementation, maybe server call */
      })();
  }
}

class DerivedViewModel extends BaseViewModel {
  isValid() : boolean | Promise<boolean> {
    return true;  // Non-async implementation
  }
}

Another workaround just to satisfy the compiler is this (which is also worse than @jods4's original because it instantiates a new implementation of isValid for every class instance, rather than adding the method once to the prototype):

class BaseViewModel {
    isValid: () => boolean | Promise<boolean> = async () => {
          /* some async implementation, maybe server call */
    };
}

[yortus]

The only proposal i see here is making a generator function returning something other than the generator interface an error to match the Promise implementation.

Then please look differently :)

Using annotations the way @jods4 does is a great example of the Liskov substitution principle. This surely is part of any good programmer's toolkit.

I'd much rather allow LSP to work on async functions, than deliberately stopping it from working on generator functions (where it currently works just fine).

---

the only possible type that can come out of it is the Promise object, any thing else we allow here will be a lie

It is categorically not a lie to describe a Promise instance as a union type that includes Promise. It's useful for applying LSP as mentioned above. More generally, for some value t whose real type is T, it's correct to say var t: T|U whatever U is.

function foo(): string|number { return 42; }

That's guaranteed to return a number. It's certainly not a lie to annotate the return type as string|number.

These are also not lies and may actually be useful annotations for keeping interfaces separate from implementations:

var p: PromiseLike<number> = Promise.resolve(42);
var q: Thenable = Promise.reject(new Error('fail!'));

The compiler allows these useful 'abstracting' annotations basically everywhere except on async function return types.