strictFunctionTypes disallows overloads of static methods using namespaces

[falsandtru]

cc @ahejlsberg

TypeScript Version: master

Code

class C {
  m(a: C) {
  }
  static m(a: C) {
  }
}
namespace C {
  export function s(a: C) {
  }
}
class D extends C {
  p;
  m(a: D) {
  }
  static m(a: D) {
  }
}
namespace D {
  export function s(a: D) {
  }
}

Expected behavior:

no error

Actual behavior:

$ node built/local/tsc.js index.ts --noEmit --strictFunctionTypes
index.ts(11,7): error TS2417: Class static side 'typeof D' incorrectly extends base class static side 'typeof C'.
  Types of property 's' are incompatible.
    Type '(a: D) => void' is not assignable to type '(a: C) => void'.
      Types of parameters 'a' and 'a' are incompatible.
        Type 'C' is not assignable to type 'D'.
          Property 'p' is missing in type 'C'.

[ahejlsberg]

In --strictFunctionTypes mode, only members that are declared as methods or constructors are checked bivariantly. Since s is declared as a function in both namespaces, it is subject to a stricter check, and you get an error because the a parameters don't relate contravariantly as required.

I think this behavior is pretty reasonable and it isn't clear what a better rule would be. Is there an actual real world scenario that was broken by this? Any reason the functions couldn't just be declared as static members?

[falsandtru]

That error message says Class static side. So the compiler has checked that function as a static method. If compiler checks that function as a function, shouldn't regard that as a class member (static method). And then compiler throw no error because those functions declared in different namespace.

Actual cases are here: https://github.com/falsandtru/spica/blob/master/index.d.ts

I got errors with mplus functions.

[falsandtru]

Namespaced functions are useful for declaration of semantics they don't depend its context. For example, static methods could throw an error because of missing this variable, but namespaced functions are not because they don't have this as their namespace.

As another actual case, console.log and sibling methods are changed from static methods to just functions by the spec. So they improved to be able to be directly passed as a parameter like addEventListener(console.log). They shouldn't be typed as static methods.

Another point, this problem is a breaking change under the --strict flag.

[ahejlsberg]

It's important to understand that the pattern you're using isn't type safe:

let x: typeof C = D;
x.s(new C());  // D.s expects a D but here we're passing a C

We are knowingly permitting this unsafe pattern for members declared as methods, but I would hate to further extend it to some functions. The whole point of --strictFunctionTypes is to catch situations like the one above to the greatest practical extent.

Another point, this problem is a breaking change under the --strict flag.

Yes, but that's to be expected with --strict. With that flag you're saying "please opt me in to the latest and greatest checks, even if it means reporting new errors." There are new errors in about 3% of the DefinitelyTyped packages and we're working towards fixing them. If want assurance of no new errors when you upgrade to a new version of the compiler, don't use --strict but instead explicitly list the flags you want (e.g. --noImplicitAny --strictNullChecks).

As another actual case, console.log and sibling methods are changed from static methods to just functions by the spec. So they improved to be able to be directly passed as a parameter like addEventListener(console.log). They shouldn't be typed as static methods.

Not sure I understand what you mean by this.

[falsandtru]

It's important to understand that the pattern you're using isn't type safe:

I got the following correct error with your code. So it is type safe.

index.ts(22,5): error TS2322: Type 'typeof D' is not assignable to type 'typeof C'.
  Types of property 's' are incompatible.
    Type '(a: D) => void' is not assignable to type '(a: C) => void'.

Not sure I understand what you mean by this.

It is the answer for your question Any reason the functions couldn't just be declared as static members?. I mean the console functions should be defined using namespace and functions as follows: Sorry, console is not a correct case.

My proposal is, like class methods, excluding strictFunctionTypes checks of functions in namespaces extending classes. You said #18654 (comment) but my reported case is an exceptional case of overloaded function declarations. So that comment should be ...non-overloaded function declarations, and overloaded function declarations not declared in namespaces.

Or disabling checks of overloaded methods with functions in namespaces. Those functions are defined as functions, not static methods.

[ahejlsberg]

I got the following correct error with your code. So it is type safe.

The pattern of overriding a static method with one that takes a parameter of a more specific type isn't type safe. That's why we're reporting an error in --strictFunctionTypes mode. With the changes you're proposing you wouldn't get an error. I disagree with that proposal. With our current implementation you have the simple rule that members declared as methods or constructors have bivariant parameters whereas all other function types have contravariant parameters. With your proposal we'd have to make exceptions for functions declared in namespaces that are merged with classes. I don't think there's a compelling case for that, particularly since you can just declare the members as static methods if you really want the (unsafe) behavior.

[RyanCavanaugh]

See also #4628 - there have been past discussions about whether or not to enforce the static-side assignability of derived classes.

[falsandtru]

@ahejlsberg I agree your basics of type system. But I believe you don't need to check those functions as overloaded methods (compiler regards them as static methods, don't you?) because they are not written by (static) class method syntax...

@RyanCavanaugh That issue targets static methods, but I'm saying about functions. Static methods have no problem in this case. My proposals is Don't include any functions in the static-side of classes, or completely behave as static methods. Compiler regards some functions as static methods, but compiler doesn't apply some checks of static methods to them. It is unfair.

Thanks.

[mhegazy]

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.