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

Overload resolution: Choosing the wrong overload when arguments involve generic wrappers #13570

Closed
krryan opened this issue Jan 18, 2017 · 11 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@krryan
Copy link

krryan commented Jan 18, 2017

TypeScript Version: 2.2.0-dev.20161221

Code

function filter<P extends Q[], Q, R extends Q>(values: P, isR: (value: Q) => value is R): R[];
function filter<R>(values: R[], isValid: (value: R) => boolean): R[];
function filter<P extends Q[], Q, R extends Q>(values: P, isR: (value: Q) => value is R): R[] {
    const result: R[] = [];
    values.forEach(q => {
        if (isR(q)) {
            result.push(q);
        }
    });
    return result;
}

interface Foo<T> {
    foo: T;
}
interface Bar<T> {
    bar: T;
}
type FooBar<T> = Foo<T> | Bar<T>;

const foobars: FooBar<{}>[] = [];

function isFoo<T>(foobar: FooBar<T>): foobar is Foo<T> {
    return 'foo' in foobar;
}
filter(foobars, isFoo).map(foo => foo.foo);

filter<FooBar<{}>[], FooBar<{}>, Foo<{}>>(foobars, isFoo).map(foo => foo.foo);

function isFooObject(foobar: FooBar<{}>): foobar is Foo<{}> {
    return isFoo(foobar);
}
filter(foobars, isFooObject).map(foo => foo.foo);

(also on the Playground)

Expected behavior:
Compiles without error.

Actual behavior:
The first invocation of filter, filter(foobars, isFoo).map(foo => foo.foo);, has an error Property 'foo' does not exist on type 'FooBar<{}>'. The second/less-specific overload of filter is being selected, even though (as the second invocation shows) the first/more-specific overload is valid, and also despite the fact (as shown by the third invocation) the correct overload is chosen when the inferred type does not involve a generic.

@lucasray
Copy link

I believe this example also gets to the crux of the issue:

interface A<T> {
    _a: T;
}
interface B<T> {
    _b: T;
}
type AorB<T> = A<T> | B<T>;

declare function isA1<T>(input: AorB<T>): input is A<T>;
declare function isA2(input: AorB<{}>): input is A<{}>;

declare function overload<T>(fn: (input: AorB<T>) => input is A<T>);
declare function overload<T>(fn: (input: AorB<T>) => boolean);

overload(isA1); // uses 2nd overload
overload(isA2); // uses 1st

If only one overload is declared, both calls still typecheck correctly (so, both declarations are valid for both isA* functions). If both are present, they use the overloads as commented. I'm not sure what causes them to choose different overloads when both are valid.

@krryan
Copy link
Author

krryan commented Jan 25, 2017

Yes, exactly that. There should be some way to control which overload is "preferred" when both are valid, and I had thought that was order (first matching overload is chosen), but it seems like TypeScript is... I dunno, shortcutting and/or getting lazy too early on these evaluations when you don't hint them.

@mhegazy
Copy link
Contributor

mhegazy commented May 18, 2017

related to #9366

@Erikvv
Copy link

Erikvv commented Jun 18, 2018

(this seems a different issue namely #10957)

I have a similar issue while writing definitions for ohanhi/hyperscript-helpers

After passing a function through a generic wrapper it either choses the last overload or it loses type safety completely, depending on how I write the overloads.

Minimal example:

declare function overload(a: number);
declare function overload(b: string);

interface MyFunction<T> {
    (t: T): any
}

declare function wrap<T>(f: MyFunction<T>): MyFunction<T>

const myFunction = wrap(overload);

myFunction('qwerty'); // OK
myFunction(9000); // error TS2345: Argument of type '9000' is not assignable to parameter of type 'string'

This can be fixed by changing the wrap function to below.

declare function wrap<F extends MyFunction<T>, T>(f: F): F

But the issue returns when the return type is more complicated: it will then again only look to the last overload.

Also I suspect if you expand this solution to more parameters you will "squash" the overloads into one, lowering type safety. In my real-world code not all parameters are supposed to be specified in the interface MyFunction, I want to forward the tail arguments as-is. Like a curry/bind function.

@krryan
Copy link
Author

krryan commented Jun 18, 2018

Actually, that looks more like a question of higher-kinded types, which is the long-standing issue #1213.

@Erikvv
Copy link

Erikvv commented Jun 18, 2018

@krryan I'm no expert but it seems to me the problem is in the overloads being ignored.

@krryan
Copy link
Author

krryan commented Jun 18, 2018

They are, but more fundamentally, TS has no way of passing around something that can go multiple ways like that. When you pass overload to wrap, TS needs to pick one of the overloads—in this case, it seems, the first. Higher-kinded types might help that situation. And my question, ultimately, doesn't have to do with trying to get TS to maintain that flexibility, it's just about choosing the right one in a situation where I want TS to choose one.

@benneq
Copy link

benneq commented Aug 13, 2018

Is this the same issue?

interface Foo {

}

interface Bar extends Foo {
    someFn: (arg: string) => void
}

function myFn(obj: Foo);
function myFn(obj: Bar, arg: string);
function myFn(obj: Foo | Bar, arg?: string) {
    if (arg) {
        obj.someFn(arg);  // ERROR
                          // Property 'someFn' does not exist on type 'Foo'
    }
}

Can't even use if(obj.someFn !== undefined) to guard the access. Is there any known workaround?

Playground Link: https://www.typescriptlang.org/play/#src=interface%20Foo%20%7B%0D%0A%0D%0A%7D%0D%0A%0D%0Ainterface%20Bar%20extends%20Foo%20%7B%0D%0A%20%20%20%20someFn%3A%20(arg%3A%20string)%20%3D%3E%20void%0D%0A%7D%0D%0A%0D%0Afunction%20myFn(obj%3A%20Foo)%3B%0D%0Afunction%20myFn(obj%3A%20Bar%2C%20arg%3A%20string)%3B%0D%0Afunction%20myFn(obj%3A%20Foo%20%7C%20Bar%2C%20arg%3F%3A%20string)%20%7B%0D%0A%20%20%20%20if%20(arg)%20%7B%0D%0A%20%20%20%20%20%20%20%20obj.someFn(arg)%3B%0D%0A%20%20%20%20%7D%0D%0A%7D

@Erikvv
Copy link

Erikvv commented Aug 14, 2018

@benneq no this is not the issue. See https://basarat.gitbooks.io/typescript/docs/types/typeGuard.html on how to fix.

@krryan
Copy link
Author

krryan commented Aug 14, 2018

@Erikvv I don't think typeguards are going to help benneq. Note that he's testing arg but wants TS to infer something about obj. A typeguard is there answer to the second question regarding a workaround, though.

Anyway, @benneq, your first sniper doesn't work for a few reasons, but none of them are directly related to this. First, TS doesn't support the notion of “entangled” variables, so a test against one is not going to lead to inferences about another. Second, function overloads do very minimal type checking—the overloads have to be at least theoretically compatible with the primary signature, but they are never checked against the function body to see if that is hope they are being used.

tlancina added a commit to ionic-team/native-run that referenced this issue Feb 7, 2019
Both `open` and `openReadStream` have overloads, so `promisify`
can't know which one to choose and chooses the last one (which
doesn't include options).  Additionally, once we specify those two
overloads, optional args become `arg | undefined` which is subtly
different than not providing an arg (which is what we want to be
able to do), so we specify onEntry's `openReadStream` callback
explicitly as well (to have options be optional).

I think this last point is also why we can't just update `yauzl`'s
types to have both `options` and `callback` be optional, because
`options` when `promisified` will be unioned with `undefined`, not
optional (meaning you can't just leave it off, you'd have to
actually pass `undefined` as the value if you want to use it).

microsoft/TypeScript#13570
microsoft/TypeScript#13195
imhoffd pushed a commit to ionic-team/native-run that referenced this issue Feb 7, 2019
Both `open` and `openReadStream` have overloads, so `promisify`
can't know which one to choose and chooses the last one (which
doesn't include options).  Additionally, once we specify those two
overloads, optional args become `arg | undefined` which is subtly
different than not providing an arg (which is what we want to be
able to do), so we specify onEntry's `openReadStream` callback
explicitly as well (to have options be optional).

I think this last point is also why we can't just update `yauzl`'s
types to have both `options` and `callback` be optional, because
`options` when `promisified` will be unioned with `undefined`, not
optional (meaning you can't just leave it off, you'd have to
actually pass `undefined` as the value if you want to use it).

microsoft/TypeScript#13570
microsoft/TypeScript#13195
@RyanCavanaugh RyanCavanaugh added Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Needs Investigation This issue needs a team member to investigate its status. labels Jul 23, 2019
@RyanCavanaugh
Copy link
Member

Appears to be due to generic inference not using all overloads, which is a known limitation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

6 participants