microsoft / microsoft/TypeScript

Suggestion for improving generator and async function type checking

Open
#6,686 14 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

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 https://github.com/Microsoft/TypeScript/commit/a6af98e10025492e0afda315ea37cfebe0b2bbfb)

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>.

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 by reproducing the four generator and async return-type examples with tsc for target >= ES6, then compare the behavior with the referenced gen-async-func-return-types branch. Read the type-checking paths involved in generator and async functions; done means the suggested errors and accepted annotations match the examples and the two-step rules.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.