use default type parameter rather than unknown when show quick help

[ShuiRuTian]

Fixes #46774

[andrewbranch]

In what cases does this loop still run?

[ShuiRuTian]

like this(the example is from instantiateGenericClassWithWrongNumberOfTypeArguments.ts):

class D<T, U> {
    x: T
    y: U
}

var d = new D<number>();
// when hover on `d`, it shows `var d: D<number, unknown>`
//                                               ^^^^^^ here is added by this loop, and if `U` has a constraint, it would be the constraint.

fillMissingTypeArguments only fills typeArgument when there are valid default type parameter, which means the number of typeArgument is between minTypeParameterCount and typeParameterCount. It does not always fill the type argument to full.

[ShuiRuTian]

And, we would better to mention this case:

interface A { a: number }
interface B extends A { b: string }

class D<T, U, V extends A = B> {
    x: T
    y: U
    z: V
}

var d = new D<number>();
// when hover on `d`, it shows `var d: D<number, unknown, A>`
// `A` is its constraint rather than the default type parameter `B`, this PR does not change this behavior now

However, it is a little different: there is an error on the number of type arguments. So I feel that both methods are OK? Or the default type parameter is better than constraint?

[andrewbranch]

Hm, I feel like using the default here makes a little more sense, but I don’t know that it’s super critical since it’s an error anyway.

[ShuiRuTian]

Now prefer default type parameter than constraint for quick info. Correspond tests are added.

[andrewbranch]

It should be noted that this change doesn’t just affect quick info; it actually affects the type instantiated in these kinds of error scenarios. I’m surprised there weren’t any existing baselines that needed updating. In your example above, you can check the type of d.z and see that in main, the type is A, and this branch, the type is B.

[andrewbranch]

The new behavior is more intuitive to me, but others should weigh in. The most conservative thing would be to leave it as it was, against my earlier advice.

[ShuiRuTian]

Thanks for pointing it out, I did not notice it! This does make sense. And, I am surprised too :)
But do we really need to revert to last commit? For:

1. we are doing things in the right way
2. anyway, it is just an error condition which finally need be fixed by user

Again, I feel both methods are OK. The final decision is up to you~

[andrewbranch]

@typescript-bot pack this

[typescript-bot]

Heya @andrewbranch, I've started to run the tarball bundle task on this PR at a0aa803. You can monitor the build here.

[andrewbranch]

npm died 😦

@typescript-bot pack this

[typescript-bot]

Heya @andrewbranch, I've started to run the tarball bundle task on this PR at a0aa803. You can monitor the build here.

[andrewbranch]

@typescript-bot pack this please

[typescript-bot]

Heya @andrewbranch, I've started to run the tarball bundle task on this PR at a0aa803. You can monitor the build here.

[typescript-bot]

Hey @andrewbranch, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/115173/artifacts?artifactName=tgz&fileId=27B511EEB46E38603FFE7E443A46392F30DBBDE2CAAFDB59058D0EFF3B9B5B4402&fileName=/typescript-4.6.0-insiders.20211116.tgz"
    }
}

and then running npm install.

---

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;