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

strictFunctionTypes has different behavior with parameter types and return types #18963

Closed
falsandtru opened this issue Oct 5, 2017 · 3 comments · Fixed by #18976
Closed
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@falsandtru
Copy link
Contributor

falsandtru commented Oct 5, 2017

cc @ahejlsberg

TypeScript Version: master

Code

declare class C {
  static a(f: (a: C) => C): void;
  static b(): (a: C) => C;
}
declare class D extends C {
  private p: void;
  static a(f: (a: D) => D): void;
  static b(): (a: D) => D;
}

Expected behavior:

no error or both a and b make an error.

Actual behavior:

$ node built/local/tsc.js index.d.ts --noEmit --strictFunctionTypes
index.d.ts(5,15): error TS2417: Class static side 'typeof D' incorrectly extends base class static side 'typeof C'.
  Types of property 'b' are incompatible.
    Type '() => (a: D) => D' is not assignable to type '() => (a: C) => C'.
      Type '(a: D) => D' is not assignable to type '(a: C) => C'.
        Types of parameters 'a' and 'a' are incompatible.
          Type 'C' is not assignable to type 'D'.
            Property 'p' is missing in type 'C'.
@falsandtru falsandtru changed the title strictFunctionTypes has different behavior with parameter types and return types strictFunctionTypes doesn't check parameter types of static methods Oct 5, 2017
@falsandtru falsandtru changed the title strictFunctionTypes doesn't check parameter types of static methods strictFunctionTypes doesn't work with parameter types of static methods Oct 5, 2017
@falsandtru falsandtru changed the title strictFunctionTypes doesn't work with parameter types of static methods strictFunctionTypes has different behavior with parameter types and return types Oct 5, 2017
@falsandtru
Copy link
Contributor Author

I'm not sure this difference is intended or not.

@ahejlsberg
Copy link
Member

ahejlsberg commented Oct 5, 2017

I think you're right, the following should be an error with --strictFunctionTypes:

declare class C {
    static a(f: (x: C) => C): void;
}
declare class D extends C {
    private p: void;
    static a(f: (x: D) => D): void;
}

We currently don't report an error because of overlap in functionality between strict function type checking and covariant checking for callback parameters (#15104). This causes us to always check the return type of a callback parameter bivariantly when we should actually check it contravariantly in strict function types mode.

Definitely a corner case, but one that we should get right.

@ahejlsberg ahejlsberg self-assigned this Oct 5, 2017
@ahejlsberg ahejlsberg added the Bug A bug in TypeScript label Oct 5, 2017
@ahejlsberg ahejlsberg added this to the TypeScript 2.6 milestone Oct 5, 2017
@ahejlsberg
Copy link
Member

So, it turns out we can't tighten the rules for the original example. If we did we would break covariance for a number of core types, such as Array<T>, which simply isn't an option. The issue is that many types rely on covariance for the result position of a callback parameter of a method. For example, the reduce method in Array<T>:

interface Array<T> {
    // ...
    reduce(callbackfn: (previousValue: T, currentValue: T, currentIndex: number, array: T[]) => T, initialValue?: T): T;
    // ...
}

If we were to strictly (i.e. contravariantly) check the return position of that callback in --strictFunctionTypes mode, Array<T> would become invariant.

However... It is the case that we aren't correctly checking callback parameters of regular function types in strict function types mode. For example, we should error on both assignments below, but we only error on the second one:

declare class C {
    private c: void;
}
declare class D extends C {
    private d: void;
}
declare let f1: (f: (x: C) => C) => void;
declare let f2: (f: (x: D) => D) => void;
f1 = f2;  // Should be an error
f2 = f1;  // Error

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants