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

Type inference regression with overloads #27972

Closed
ghost opened this issue Oct 18, 2018 · 3 comments
Closed

Type inference regression with overloads #27972

ghost opened this issue Oct 18, 2018 · 3 comments
Assignees
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@ghost
Copy link

ghost commented Oct 18, 2018

TypeScript Version: 3.2.0-dev.20181018

Search Terms:

Code

declare function f<T>(cf: (() => T) | ((x: T) => boolean)): void;

declare function myFn(): Date;
declare function myFn(n: number): void;

f<Date>(myFn); // works
f(myFn); // error

Expected behavior:

No error, as in typescript@3.1.

Actual behavior:

src/a.ts:7:3 - error TS2345: Argument of type '{ (): Date; (n: number): void; }' is not assignable to parameter of type '(() => number) | ((x: number) => boolean)'.
  Type '{ (): Date; (n: number): void; }' is not assignable to type '() => number'.
    Type 'Date' is not assignable to type 'number'.

7 f(myFn); // error

Looks like this was introduced by #27028. Discovered in bluebird on DefinitelyTyped -- in that case it had to do with type CatchFilter<E> = (new (...args: any[]) => E) | ((error: E) => boolean) | (object & E); which unions a construct and call signature which more clearly should be separate.

@ghost ghost added the Bug A bug in TypeScript label Oct 18, 2018
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.2 milestone Oct 25, 2018
@ahejlsberg
Copy link
Member

This is working as intended. First, some clarifications. This is a change introduced in 3.1, not a new change in 3.2 as implied above, and the issue surfaces only in --strictFunctionTypes mode.

The change in 3.1 is that when, in --strictFunctionTypes mode, we have both co- and contra-variant inferences for a type parameter, we prefer the contra-variant inference unless the co-variant inference is a subtype and not never. That causes us to infer number instead of void in the example (which, for those two candidates, is the correct choice).

Now, the real issue is that we only make inferences from the last overload because we're inferring from a type with two signatures to a type with only one signature. We've always had the rule that we match signatures pair-wise from the bottom when doing inference, so nothing new there. However, it means that nothing is or was ever inferred from the first overload, and it just so happened to work previously because we made a bad inference from the second overload.

@ahejlsberg ahejlsberg added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript labels Nov 4, 2018
@ahejlsberg ahejlsberg removed this from the TypeScript 3.2 milestone Nov 4, 2018
@demurgos
Copy link

demurgos commented Dec 2, 2018

Hi,

I think I have a related bug that is failing due to this issue:

Playground

export interface DocumentType<T> {
    hasKey(key: keyof T): boolean;
}

export interface DocumentTypeConstructor {
  new<T>(value: T): DocumentType<T>;
}

export const DocumentType: DocumentTypeConstructor = class <T> {
    private readonly value: T;

    constructor(value: T) {
        this.value = value;
    }

    hasKey(key: keyof T): boolean {
        return key in this.value;
    }
};

I get error TS2394: Overload signature is not compatible with function implementation. associated to hasKey in the first interface (see playground).
This code was compiling in 3.0 but started to fail in 3.1.

I found that the following code fixes the issue, but I don't understand why:

export interface DocumentType<T> {
    hasKey(key: keyof T): boolean;
}

export interface DocumentTypeConstructor {
  new<T>(value: T): DocumentType<T>;
}

export const DocumentType: DocumentTypeConstructor = class <T> {
    private readonly value: T;

    constructor(value: T) {
        this.value = value;
    }

    hasKey<TT extends keyof T>(key: TT): boolean {
        return key in this.value;
    }
};

The only difference is that I modified the signature of the implementation from hasKey(key: keyof T): boolean to hasKey<TT extends keyof T>(key: TT): boolean but in my opinion both should be equivalent. What am I missing?

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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

4 participants