Strange behavior on coercing typeof T to generic constructor interface

[Artazor]

Can anyone shed a bit of light on the following behavior

interface Constructor<T> {
    new(...args): T;
    prototype: T;
}

class A<T> { a: T; }
class B<T> extends A<T> { b: T; }
var c:Constructor<B<boolean>> = B; // error here

class A1 { a: boolean; }
class B1 extends A1 { b: boolean; }
var c1:Constructor<B1> = B1; // no error here

The error is:

Type 'typeof B' is not assignable to type 'Constructor<B<boolean>>'.
  Type 'A<any>' is not assignable to type 'B<boolean>'.
    Property 'b' is missing in type 'A<any>'.

I suspect that it is a correct behavior in the current design of the type system, I just don't understand two things:

1. How the error is inferred
2. Can my sample be fixed somehow without killing actual types with <any>

@JsonFreeman, @ahejlsberg?

[RyanCavanaugh]

Playing with the repro, this really looks like a bug to me. I can't see any reason this should happen. I suspect the issue is we're not properly finding the anonymous constructor in B.

[RyanCavanaugh]

The example I gave @sandersn for why I consider this a bug

interface Constructor<T> {
    new(...args): T;
}

class A<T> { }
class B<T> extends A<T> {
    b: T;
    // This line shouldn't do anything, but its presence removes the error
    // constructor() {super(); }
}
var c:Constructor<B<boolean>> = B; // error here

[JsonFreeman]

I agree with Ryan. Looking at the error, I imagine that in the inherited constructor for typeof B, the return type A<any> fails to be replaced with B<any>

[sandersn]

Maybe. So far I've tracked it back to that the construct signature for the generic B<T> has the target A<T>, and when getErasedSignature is called to convert B<T> to B<any>, it follows the target chain back to A.

I don't understand what the meaning of Signature.target is -- the comment says "instantiation target", and I don't think that A<T> (base constructor) should be an instantation target of B<T> (derived constructor) for construct signatures. That's what I'm looking at now.

[JsonFreeman]

I think it is the instantiation target. Suppose it was A<U> and B<T> extends A<T>. Then B would have to inherit A's signatures, but replace all the U's with T's. In other words, it has to instantiate the signatures of A with a mapping U -> T, and the A signature becomes the target. Check out the function getDefaultConstructSignatures, in particular, the line that says

const sig = typeParamCount ? getSignatureInstantiation(baseSig, typeArguments) : cloneSignature(baseSig);

Calling getSignatureInstantiation will make A's signature the target.

I think I am right about this.

[sandersn]

Hmm. If I understand your hypothesis, it's that the signature that B inherits from A should have its return type replaced, but does not.

However, I can see that getDefaultConstructSignatures does this right after calling getSignatureInstantiation. A<U> gets replaced with B<V> as the return type. (I renamed the type variables to be unique -- see the renamed repro below.) So I think this hypothesis is false.

If getDefaultConstructSignatures is correct, is the answer for getErasedSignature not to follow the target chain back?

Renamed Repro

interface Constructor<T> {
    new(...args: any[]): T;
    prototype: T;
}

class A<U> { a: U; }
class B<V> extends A<V> { b: V; }
var c:Constructor<B<boolean>> = B; // error here

[JsonFreeman]

I have a feeling that the bug has to do with this line in getErasedSignature.

signature.erasedSignatureCache = instantiateSignature(getErasedSignature(signature.target), signature.mapper);

Because the signature in B has a target, it erases the target, but forgets about the changed return type. So you may be on the right track about not following the target chain. I think that signatures that get instantiated multiple times should be taken care of in instantiateSignature already (the block that creates freshTypeParameters). There are two reasons that a signature might be multiply instantiated: one of them is this generic inheritance case where the type parameters from the base type get overwritten by the derived type's type parameters. The other is when the signature references type parameters from outside. I think some of the core instantiation code fails to treat the former case properly.

One other thing to look at is in getReturnTypeOfSignature, where there is a similar phenomenon that may also cause an issue:

if (signature.target) {
    type = instantiateType(getReturnTypeOfSignature(signature.target), signature.mapper);
}

[sandersn]

So first I tried the nuclear option: I deleted the target-following code in getErasedSignature, which fixes this test and doesn't cause any other test to fail. But I don't trust that our test suite has this much detail, and I don't think the target-following code exists by mistake.

@ahejlsberg, can you take a look at this? @mhegazy says that he thinks you wrote this code. And I don't know enough about instantiation in general to understand single vs multiple instantiation, etc.

[ahejlsberg]

I took a look and the issue does indeed appear to be caused by the target-following code in getErasedSignature. It's been a while since I wrote that code and I think it was intended as an optimization. But, it obviously doesn't work and I think it is fine to remove it. The target and mapper properties are there to remember the original signature and mapper in cases where we instantiate a signature for which we haven't yet computed the return type. getReturnTypeOfSignature then ends up going back to the original signature, getting its return type, and instantiating that. That code all looks good. But the check for target in getErasedSignature doesn't make sense because erasing the signature has nothing to do with whether the return type is deferred.

So, summing up, I'm fine with removing the target-following code and I just confirmed that our tests all run fine.

[JsonFreeman]

I agree, I can't think of any reason why removing this would cause a problem.

[sandersn]

All right, I'll publish the PR that deletes the target-following code.

[sandersn]

Pull request is up