Strictly check callback parameters

[ahejlsberg]

With this PR we fix checking of the return position of callback parameters in --strictFunctionTypes mode. Previously we'd never check this position strictly (because of #15104) but now we check it strictly (i.e. contravariantly) for callback parameters of non-methods. For example:

// Compile with --strictFunctionTypes
declare let f1: (cb: (x: Animal) => Animal) => void;
declare let f2: (cb: (x: Dog) => Dog) => void;
f1 = f2;  // Error, but previously wasn't
f2 = f1;  // Error

The way the fix works is that once we've decided to check a parameter position strictly, we avoid the code path for callback parameters added in #15104. Basically, when we're checking a parameter strictly, we don't need to make exceptions for callbacks.

Fixes #18963 (at least the parts that we can fix).

[mhegazy]

@ahejlsberg last time we talked about disabling this code path for --strictFunctionTypes you mentioned we needed this to avoid excessive recursion for computing the variance digest, is that not an issue now?

[DanielRosenwasser]

It feels strange that the following examples undergo a regression and you lose checking under strictFunctionTypes:

interface Animal {a: any}
interface Dog extends Animal {d: any}


namespace n1 {
    class Foo {
        static f1(x: Animal): Animal { throw "wat"; }
        static f2(x: Dog): Animal { throw "wat"; };
    }
    
    declare let f1: (cb: typeof Foo.f1) => void;
    declare let f2: (cb: typeof Foo.f2) => void;
    f1 = f2;
    f2 = f1; // errors outside strictFunctionTypes, not with it.
}

namespace n2 {
    type BivariantHack<Input, Output> = { foo(x: Input): Output }["foo"];
    
    declare let f1: (cb: BivariantHack<Animal, Animal>) => void;
    declare let f2: (cb: BivariantHack<Dog, Animal>) => void;
    f1 = f2;
    f2 = f1; // errors outside strictFunctionTypes, not with it.
}

At the very least, we should have tests demonstrating the behavior, but I think we can still tighten this up.

[ahejlsberg]

@DanielRosenwasser It's because a callback parameter check (contravariant parameters, bivariant return type) sits halfway between a strict function type check (contravariant parameters, covariant return type) and a method type check (bivariant parameters, bivariant return type). I suppose we could say that a callback parameter check occurs only if the callback type isn't declared as method (which you really have to contort yourself to do, as your example demonstrates). I think it is largely immaterial though, it never really occurs in normal code.

[ahejlsberg]

last time we talked about disabling this code path for --strictFunctionTypes you mentioned we needed this to avoid excessive recursion for computing the variance digest, is that not an issue now?

Not sure what I was talking about there, it shouldn't matter.

[DanielRosenwasser]

While I think it wouldn't be super difficult to do that, we should acknowledge these cases and add them to our test suite, with and without strictFunctionTypes.

[ahejlsberg]

@DanielRosenwasser @mhegazy With the latest commits I've reverted back to using the callback parameter code path in --strictFunctionTypes mode as well, but with a tighter check on the return type of the callback. This means that in --strictFunctionTypes mode we'll check function types loosely only when they originate in method or constructor declarations and aren't in a callback parameter position.

[SlurpTheo]

Why !callbackCheck instead of CallbackCheck.None === callbackCheck?