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

Union types not working with old-style overloads #1805

Closed
nycdotnet opened this issue Jan 26, 2015 · 20 comments
Closed

Union types not working with old-style overloads #1805

nycdotnet opened this issue Jan 26, 2015 · 20 comments
Labels
Canonical This issue contains a lengthy and complete description of a particular problem, solution, or design Declined The issue was declined as something which matches the TypeScript vision Revisit An issue worth coming back to Suggestion An idea for TypeScript

Comments

@nycdotnet
Copy link

I was just working with Big.js and tried to pass a union type into its constructor function. It seems that union types don't play nice with old-style overloaded constructors. Please consider the code below that errors with TypeScript 1.4. I would have expected this to work.

interface BigJS_Constructors {
    new (value: number): BigJS;
    new (value: string): BigJS;
    new (value: BigJS): BigJS;
}

interface BigJS extends BigJS_Constructors {
    abs(): BigJS;
    /* more... */
}

var Big : BigJS;

class MyThing {
    public value : BigJS;
    constructor(value: number | string | BigJS) {
        this.value = new Big(value); //This is an error in 1.4.
    }
}

If I change the new signature under BigJS_Constructors to new (value: number | string | BigJS) : BigJS;, and eliminate the other two options, it works fine, but to me the two styles should be identical in this case.

My apologies if I'm doing something stupid here. Thanks!

@danquirk
Copy link
Member

Definitely reasonable to expect this to work. The main reason it doesn't work at the moment is we couldn't figure out any general solution to this problem and opted not to special case anything. It's possible we could special case this pattern (an overload set where all overloads have a single argument with the same return type) if it's reasonably common. The larger problem is in the cost/complexity when overload sets are more complicated. Consider:

function foo(x: string, y: string) {}
function foo(x: number, y: number) {}

var arg: string|number;
foo(arg, arg); // should be an error

@danquirk danquirk added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Jan 26, 2015
@nycdotnet
Copy link
Author

Interesting. I would even say that you could allow that because arg can't be both, though this would not be allowed:

function foo(x: string, y: string) {}
function foo(x: number, y: number) {}

var arg1: string|number;
var arg2: string|number;
foo(arg1, arg2); // should be an error because you don't know if arg1 is a string and arg2 is a number, for example.

Maybe that's what you meant...

@danquirk
Copy link
Member

Yes, that's the problem. Essentially the overload set was used to create information tying the type of argument 1 to the type of argument 2 (and can be used likewise to tie this to a return type). With a union type that information is lost and any combination of argument types becomes allowed.

@nycdotnet
Copy link
Author

I agree that this is hard to solve and might be a black hole for special cases. Since this scenario isn't supported anyway, and if converting the definition to use union types would fix the problem (it would in this case), perhaps a better thing to do is to just have TypeScript raise a different (new) error when passing a union type (of any variety) to an overloaded function that is not written with union types. Something like 'Union types are not assignable to overloaded function parameters - consider rewriting the function definition to accept a union type'. Does that make any sense?

Thanks, Dan.

@danielearwicker
Copy link

It seems to me that the only case where this can be allowed is when there is a single parameter for which the argument is of a union type whose members are a subset of those allowed by the overloads. All other argument types must match in the usual way.

If there are two or more parameters involved in resolution (union type -> overload) then as noted above, nothing can be done and the user ought not to expect it to work. Overloads specify what combinations of types are valid. A single function declaration with union types allows all combinations. So they aren't equivalent.

To put it another way: overloads continue to be a uniquely useful feature in some situations, despite the overlap with union types in other situations.

For exactly the same reason, I don't think the error message should suggest to the user that they rewrite the overload to use union types in such situations - it is probably unsafe advice! Such advice would only be wise if the compiler knew it was safe advice, in which case the compiler could solve it anyway.

As I noted in 1883 (the issue, not the year), the reverse mapping from overloads to union type must occur for the return type (of which fortunately there is only one), as that may vary between overloads:

munge(p: string): Promise<string>;
munge(p: number): Promise<number>;

var a: string|number = ...
var r = munge(a);

So the type of r will be Promise<string> | Promise<number (or Promise<string|number> - is there any difference?)

@danielearwicker
Copy link

Generalising the description above: if there are arguments of union types, there must be overloads that allow every combination of those types.

For two parameters being passed arguments such as string|number and boolean|(() => void)|DownloadOptions there would need to be six overloads for all the combinations 2x3 before the call was allowed to compile.

In the case of a single such parameter it collapses to the simplistic description in my previous comment.

@nycdotnet
Copy link
Author

👍

@RyanCavanaugh
Copy link
Member

Generalising the description above: if there are arguments of union types, there must be overloads that allow every combination of those types.

This is the ideal, of course, but we really don't want to implement a combinatorially explosive algorithm in the compiler.

@danielearwicker
Copy link

@RyanCavanaugh but is this algorithm explosive? It seems to be bounded (time and memory) by the number of overloads actually declared by the user, not by the number of possible overloads. It's an AND requirement, so it can be short-circuited; the compiler can quit immediately on discovering a single overload is not present.

@RyanCavanaugh
Copy link
Member

Consider something like this:

var a: string|number;
var b: string|boolean;
var c: boolean|number;

function fn(a: string|boolean, b: boolean, c: number|string);
function fn(a: boolean, b: string, c: number|string);
function fn(a: number|boolean, b: boolean, c: string|number);
function fn(a: string|boolean, b: string, c: boolean|number);
function fn() {}

fn(a, b, c);

There are only 4 overloads, but if you try to puzzle this out manually you realize you have to enumerate through the 8 (2 * 2 * 2) possible combinations of types to find the invalid combination. There's no guarantee you find the bad one 'early' in that process.

@danielearwicker
Copy link

Ah, well, I'd count that as shorthand for 14 overloads! :) But I see what you mean.

@RyanCavanaugh
Copy link
Member

I am inclined to say we should just special-case the single-argument variant. Actual prevalence of the mutli-argument variants seems very low, and it is a real pain for wrapper functions.

@toothbrush7777777
Copy link

At least having single-argument overloads like @RyanCavanaugh said would cover most applications.

@masaeedu
Copy link
Contributor

masaeedu commented Oct 3, 2016

Too late to ask for special casing the single argument variant? 😄

@datokrat
Copy link

datokrat commented Oct 9, 2016

Sorry for posting in a closed issue...
I understand that you can rewrite many cases to union-typed-arguments style.
But what about this case?

enum Types { A, B, C }

interface TypeA { __a; }
interface TypeB { __b; }
interface TypeC { __c; }

function createInstance(type: Types.A): TypeA;
function createInstance(type: Types.B): TypeB;
function createInstance(type: Types.C): TypeC;
function createInstance(x: Types): TypeA | TypeB | TypeC {/***/}

var c: Types.A | Types.B;
createInstance(c); // ERROR: Argument type 'Types.A | Types.B' is not assignable to parameter type 'Types.C'
// c should have type TypeA | TypeB

This is useful whenever we have to parse a data structure with a "type" property and map the type of this property to the parsed object type. How about this special case? We could avoid another combinatorically explosive case ;o)

function createInstance(type: Types.A): TypeA;
function createInstance(type: Types.B): TypeB;
function createInstance(type: Types.C): TypeC;
function createInstance(type: Types.A | Types.B): TypeA | TypeB;
function createInstance(type: Types.A | Types.C): TypeA | TypeC;
function createInstance(type: Types.B | Types.C): TypeB | TypeC;
function createInstance(type: Types.A | Types.B | Types.C): TypeA | TypeB | TypeC;
function createInstance(x: Types): TypeA | TypeB | TypeC {/***/}

@RyanCavanaugh RyanCavanaugh added the Revisit An issue worth coming back to label Oct 10, 2016
@masaeedu
Copy link
Contributor

@RyanCavanaugh I think the "special case" of single argument functions is actually sufficient to formulate a solution to the problem in the general case. At least at the type checker level, all functions of multiple arguments can be modeled as curried functions of at most one argument (returning other functions).

Exploiting this, you can soundly deal with algebraic data types involving functions:

  • For the purposes of overload resolution, a function of arity > 1 is modeled as a curried function of arity 1
  • A union of functions can be invoked with an argument assignable to an intersection of their (respective) parameter types. The result is a union of their return types.
  • An intersection of functions can be invoked with an argument assignable to a union of their parameter types. The result is an appropriately narrowed union of their return types

I've set up a toy implementation of this here: https://github.com/masaeedu/overloadresolution, would appreciate your thoughts on any cases this doesn't cover.

@huan
Copy link

huan commented Jun 28, 2017

This is an old issue. So it's very strange that I just ran into this issue recently(this week). The same code work without any problem before(with tsc and tslint, in the past 12 months).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Canonical This issue contains a lengthy and complete description of a particular problem, solution, or design Declined The issue was declined as something which matches the TypeScript vision Revisit An issue worth coming back to Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests