Use different relation when checking overload vs implementation signature compatibility

[RyanCavanaugh]

Consider some code:

/* ... disjoint classes Giraffe, Elephant which extend Animal ... */
function f(n: string): Giraffe;
function f(n: number): Elephant;
function f(n: any): Animal {
  if(typeof n === 'string') {
    return new Giraffe(n);
  } else if(typeof n === 'number') {
    return new Elephant(n);
  }
}

Perhaps surprisingly, this yields errors on the overload signatures because the function implementation signature is not assignable to the overload signatures. This is unfortunate for implementors as they must use :any when implementing a function with disjoint parameter types in its overloads. Now that union types are possible, the implementor of f could even write Giraffe|Elephant to get more specific type checking of their return expressions.

One option is to change the relation in 6.1 from "assignable to" to "assignable to or from". This solves the issue above, but has a negative side effect of allowing an implementation signature to have more required parameters than any of the overloads. This might be a desirable trade-off compared to not allowing implementations to have good return expression checking. Alternatively, we could create some new kind of relation that is bivariant for return types but maintains the existing requirements of parameter arity.

Note that we do want to use the opposite assignability relation here as the implementation signature may be of an inconsequentially more-specific type.

[RyanCavanaugh]

Was looking at wrong issue

[RyanCavanaugh]

Come up with some reasonable behavior specification

[adidahiya]

+1, this is unfortunate because, as you said, we trade type safety in our implementation in favor of better type safety for consumers of our library. related: palantir/plottable#3010 (comment)

[DanielRosenwasser]

I have an implementation of "assignable to or from" on return types working, but @JsonFreeman suggested an approach where we take the union of overload return types and check if the implementation is compatible with the produced union.

So with

function f(x: number): Dog;
function f(y: string): Cat;
function f(x: number | string): /*????*/ { return undefined; }

That way, Cat | Dog could be used in place of /*????*/, but not Animal.

Of course, this might get a little unwieldy when you might return 20 different types depending on the overload, and your implementation has to name them all again.

[JsonFreeman]

If the user has lots of overloads returning different specific animals, they could add one overload at the end, which returns Animal. That way they don't have to write out the union of 20 different types. This is pretty similar to the string literal overloads in lib.d.ts, which all have a catch-all overload that returns string. To me, that feels more conservative, while at the same time loosening the current behavior.

[JsonFreeman]

I'll also add that if the user does not want to write the long union type, nor do they want to provide a catch-all overload, return type inference can infer the return type for the implementation signature.

[DanielRosenwasser]

@JsonFreeman not quite, you'll get something like "No common supertype exists between returned expressions" or we'd infer Animal which is the issue.

[JsonFreeman]

That's true, but it might make sense to lift the common supertype restriction in the presence of overloads. If you are going to check the implementation return type against all the overload return types, I'm not sure what would be the point of enforcing that there exists a common supertype. It's a bit like how you infer the union type of the return expressions (instead of the common supertype) when the function expression is contextually typed.

[DanielRosenwasser]

We discussed this a bit offline.

There are two scenarios that the union of all returns doesn't answer:

1. The Animal case as mentioned above.
2. The case where each type is a type predicate.

The second one has certain non-trivialities associated with it. Consider the following:

function hasKind(x: Node, kind: "StringLiteral"): x is LiteralNode;
function hasKind(x: Node, kind: "PropertyAccess"): x is PropertyAccess;
function hasKind(x: Node, kind: "StringLiteral" | "PropertyAccess"): /*????*/ {
    x.kind === kind;
}

Specific issues in figuring out what to write for /*????*? are:

1. The boolean type is currently not assignable to a parameter predicate type, so boolean would not suffice.
   • If it were allowed, then parameter type predication would no longer be opt-in per definition site.
2. Predicate types are currently disallowed within a union (i.e. (x is LiteralNode) | (x is PropertyAccess) is not allowed).
   • Even if they were allowed, writing out more than a few predicates doesn't scale, since these types couldn't be reused in a type alias.
   • Writing a predicate for a union of predicated constituents would not be compatible either (x is (LiteralNode | PropertyAccess) would not be assignable even to an internally constructed (x is LiteralNode) | (x is PropertyAccess).

For this reason, I think we are leaning towards checking for assignability to/from.

[JsonFreeman]

@DanielRosenwasser

1. For the type predicate case, is boolean really not assignable to a type predicate? I'm looking at line https://github.com/Microsoft/TypeScript/blob/master/src/compiler/checker.ts#L5070, and it seems like it will be assignable. If boolean is assignable, then I think boolean is what you would put in that /*????*/ slot. Though it's possible I missed something.
2. I'm not so sure the Animal case should succeed. If the Animal case succeeds, then you can do something like this:

function func(name: "Cat"): Cat;
function func(name: string): Animal {
     return new Dog();
}

So you are effectively assigning a Dog-returning function to a Cat-returning function, which ideally should not be allowed. The key is that this bivariance rule would not require users to be exhaustive in their list of overloads. They would not have to account for everything that the function could return.

If you indeed want the Animal case in the original bug description to succeed, and you don't mind letting through examples like the one above, then bivariance will get you that. But that also means you don't want to require the overload set to be exhaustive. Am I correct then that you don't care about checking whether the overload return types are exhaustive with respect to the implementation?

[weswigham]

@JsonFreeman
Signatures are special-cased so boolean return types are not assignable to predicate return types here to preserve existing behavior. Though I'd be all for removing it.

[DanielRosenwasser]

@JsonFreeman that's correct, that's the general feeling we had offline. If @ahejlsberg or @RyanCavanaugh would like to weigh in, it'd be welcome.

All of you are encouraged to leave feedback on #6075. 😃

[DanielRosenwasser]

This should be fixed for TypeScript 1.8.