Strictly check callback parameters#18976
Conversation
|
@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? |
|
It feels strange that the following examples undergo a regression and you lose checking under 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. |
|
@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. |
Not sure what I was talking about there, it shouldn't matter. |
|
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 |
|
@DanielRosenwasser @mhegazy With the latest commits I've reverted back to using the callback parameter code path in |
SlurpTheo
left a comment
There was a problem hiding this comment.
Why !callbackCheck instead of CallbackCheck.None === callbackCheck?
With this PR we fix checking of the return position of callback parameters in
--strictFunctionTypesmode. 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: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).