Fix crash in JS declaration emit

[rbuckton]

Our binder has a heuristic that detects X.prototype.y assignment in a JS file and treats X as a class, even if X isn't constructible. That results in a crash in JS declaration emit. This adds a check to ensure that the symbol for X actually has valid signatures.

Fixes #36273

[weswigham]

Hm, thinking about it, is there a way to adjust the binder to only apply the Class flag if the thing is actually constructable?

Alternatively, should we still emit this as a class, but with a private constructor?

[rbuckton]

Binder has no access to type information, so there's no way to be sure because of augmentations, merging globals, etc.

[weswigham]

What about the private constructor suggestion?

[rbuckton]

I can look into it.

[weswigham]

For the sake of cataloging our behavior, adding

class B extends A {}

(new B()).b;

to the test might be worthwhile, too. I honestly don't know how we type check it offhand, but checking that the declarations are somewhat sensible (or at least in line with any errors we may produce) would seem to make sense.

[rbuckton]

Hmm. Seems like there's more to do here:

// .js
let A;
A = {};
A.prototype.b = {};
class B extends A {}
const x = (new B()).b;

// .d.ts
declare class A {
    private constructor();
    b: {};
}
declare class B {
}
declare const x: any;

The private constructor heuristic is only used in declaration emit. We should probably extend it to type checking class B extends A to make it an error since the constructor is "private".

For the equivalent TS, the output is this:

// .ts
class A {
    private constructor() { }
    b: "x" = "x";
}
// @ts-ignore
class B extends A { }
// @ts-ignore
const x = (new B()).b;

// .d.ts
declare class A {
    private constructor();
    b: "x";
}
declare class B extends A {
}
declare const x: any;

So we're consistent on getting any for (new B()).b, but not in the emit and we don't report errors in JS at the two locations where we do report errors in the TS (labeled above with // @ts-ignore comments.

[weswigham]

Hm, the emitting errors part is probably important (declaration emit in the presence thereof a little less so). I agree, we should probably add the error. It's certainly odd that we allow it as a base class when it's not actually constructable... The js does error at runtime, too, right?

[rbuckton]

Yes, the JS would error at runtime. The JS decl emit differs because the original declaration of A isn't actually a class and a lot of our code in the checker for resolving base constructor types depends on it being a class, regardless of what the binder says. Its probably fine to have the decl emit be wrong here if we report an error when you subclass.