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 array types do not infer common array prototype methods #10620

Closed
IntermittentlyRupert opened this issue Aug 31, 2016 · 15 comments
Closed
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@IntermittentlyRupert
Copy link

IntermittentlyRupert commented Aug 31, 2016

TypeScript Version: 2.0.2rc

Code

const foo = (): number[] | string[] => Math.random() > 0.5 ? [0, 1, 2] : ["0", "1", "2"];
const bar = foo().filter((x) => Number(x) % 2 === 1);

Expected behavior:
No errors.

Actual behavior:
Compiler gives: error TS2349: Cannot invoke an expression whose type lacks a call signature.

This issue seems to be array-specific. The following code compiles as expected:

const foo = (): { a } | { b } => Math.random() > 0.5 ? { a: true } : { b: true }
const bar = foo().hasOwnProperty('a');
@azz
Copy link

azz commented Aug 31, 2016

I think this would be because there is no way to infer a common type between Array<number> and Array<string>. The definition of Array#filter is scoped to the T in Array<T>. I'm guessing a compiler exception would have to be made to infer T = Foo | Bar when calling (Foo[] | Bar[]).filter (and other methods).

@jwbay
Copy link
Contributor

jwbay commented Aug 31, 2016

Workaround:

const foo = (): (number | string)[] => ...

(Note this unveils an expected second error in the snippet because number % number yields number and .filter expects boolean)

@IntermittentlyRupert
Copy link
Author

@jwbay That works, but it's semantically different - [0, "1", 2] matches that type declaration, but not the one in my original example.

(Also, you're right - it compiles but the reliance on implicit type coercion was lazy. I've amended my example.)

@mhegazy
Copy link
Contributor

mhegazy commented Aug 31, 2016

This is a bit subtle, but number[] | string[] !=== (number | string)[]. The first one has an assert on homogeneity, while the second does not. just as in the example noted above by @kaotik4266, [0, "1", 2] is not of type number[] | string[].

There is not a meaningful way to describe the general case for merging unmatching signatures; while filter might be arguably safe, push is not. So merging the signatures of (number[]|string[]).push(...) th same proposed for filter would result in push(a: number | string); which means that we are treating the original type as (number | string)[], which is not type safe.

So the general case, is there is no common signatures between the two types, and thus the resulting union type does have a filter property but it does not have a signature, and hence the error message Cannot invoke an expression whose type lacks a call signature.

@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Aug 31, 2016
@gcnew
Copy link
Contributor

gcnew commented Sep 23, 2016

I think the more natural intuition is not to try to merge the signatures into one at all. Instead, they should be tried one after the other for conformance with the given arguments and if a call is not possible, then that's a type error according to the existing rules. This way an appropriate massage can be displayed as an error (i.e. the offending signature) and the return type is a union of all return types.

With the above example in mind:

const foo: number[] | string[] = null as any;

The signature for filter which the compiler suggests (and is 100% correct) is:

filter: ((calbackFn: (value: string, index: number, array: string[]) => any) => string[])
      | ((calbackFn: (value: number, index: number, array: number[]) => any) => number[])

And the provided function is compatible with both overloads, thus this call should be valid.

foo.filter((x: string|number): boolean => Number(x) % 2 === 1)
// => string[] | number[] (union of the return types of both overloads)

On the other hand, the signature for push is:

push: ((...items: string[]) => number) 
    | ((...items: number[]) => number)

Clearly foo.push('hello'); is a type violation, because the second overload is not met. The expected error should be something like:

Type '((...items: string[]) => number) | ((...items: number[]) => number)' is not invokable with argument of type 'string'.
    Type '((...items: number[]) => number)' is not invokable with argument of type 'string'.
        Argument of type 'string' is not assignable to parameter of type 'number'

I suspect a signature unifying solution might be also possible, but it would be much harder to implement, will have to take into account variances and deriving a digestible error messages would be much harder.

@masaeedu
Copy link
Contributor

masaeedu commented Oct 3, 2016

@mhegazy I don't know if there is already an issue to track this, but in general, given a union of two function signatures:

let f: ((foo: Foo) => void) | ((foo: Bar) => void);

It should be possible to invoke to invoke the function with arguments that are assignable to an intersection of each parameter type:

let x: Foo & Bar;
f(x); // This is sound, and should be allowed

In the case given above, Foo and Bar are Predicate<string> and Predicate<number>, respectively, and Predicate<any> satisfies Predicate<string> & Predicate<number>, so this should be allowed.

I feel like this is a special case of a more general structural subtyping rule, but can't quite formulate how.

@RyanCavanaugh
Copy link
Member

@masaeedu there are simple answers for one-off cases but your proposed fix doesn't address the case where there are multiple signatures in each union constituent

@masaeedu
Copy link
Contributor

masaeedu commented Oct 3, 2016

@RyanCavanaugh By multiple signatures, do you mean multiple arguments, or are you referring to overloads?

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Oct 3, 2016

Overloads

@masaeedu
Copy link
Contributor

masaeedu commented Oct 3, 2016

Okay, lets see. Here's an overloaded function signature:

type ThisOrThat<This, That> = {
    (arg: This): void;
    (arg: That): void;
}
let f: ThisOrThat<Foo, Bar> | ThisOrThat<Baz, Quux>;

In this case, continuing the suggested pattern of allowing intersections and recalling that the implementation of an overloaded function must be written to deal with a union of each argument's types in every overload:

let x: (Foo | Bar) & (Baz | Quux);
f(x); // This should be allowed

This at least handwavily makes sense; if the argument implements the required interface for at least one overload from both possible function signatures, then it is acceptable to pass it to f regardless of which of the function signatures actually inhabits f.

@masaeedu
Copy link
Contributor

masaeedu commented Oct 3, 2016

@RyanCavanaugh Not to derail the discussion, but this does bring up the tangentially related issue of why does this not compile?

function flipflop(arg: number): string
function flipflop(arg: string): number
function flipflop(arg: number | string): number | string {
    if (typeof arg === 'string') {
        return arg.length;
    } else {
        return arg.toString();
    }
 }

let x: number | string;
let result = flipflop(x);

I would expect it to compile and for result to have type number | string.

@dead-claudia
Copy link

Overloads are intersections, not unions. It's closer to impl1 & impl2 semantically than impl1 | impl2, although I'm proposing merging overloads with intersections.

@masaeedu
Copy link
Contributor

Overloads are intersections, not unions.

@isiahmeadows That may be so, but when you're invoking an overloaded function, each positional argument must be assignable to a union of the corresponding positional parameter types of the constituent functions, not an intersection. This is why I said, I would expect

let x: number | string;
let result = flipflop(x);

to compile. That said, I do think removing "overloads" as a distinct concept in favor of type algebra on functions is a good idea. You may be interested in this: #1805 (comment)

@masaeedu
Copy link
Contributor

@isiahmeadows Sorry, I think I misunderstood which comment you were responding to. I think you might have been responding to:

I don't know if there is already an issue to track this, but in general, given a union of two function signatures ...

In that case, yes; overloads are not unions. However it should still be possible to construct a union of two functions and invoke it (with an intersection of their argument types). Put another way, overloads are merely a special case of the ability to do type algebra with function types.

@RyanCavanaugh
Copy link
Member

See also #7294

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

8 participants