Don't inferFromIndexTypes() twice

[jablko]

Fixes #33559
Fixes #33752
Fixes #34924
Fixes #34925
Fixes #34937
Fixes #35136
Fixes #35258

interface I<T> {}
declare function f1<T1, T2>(values: [I<T1>, I<T2>]): T1;
declare function f2<T1, T2>(values: readonly [I<T1>, I<T2>]): T1;

declare const values: [I<1>, I<number>];
let one: 1;
one = f1(values); // ✓ 1
one = f2(values); // ✗ number

Currently f2(values) is number while f1(values) is 1.

There are three cases in inferFromProperties()

1. Source and target are both tuples: infers types element-wise
2. Target is an array: calls inferFromIndexTypes()
3. Otherwise: iterates over properties

But then inferFromObjectTypesWorker() calls inferFromIndexTypes() again, in all cases

                if (!typesDefinitelyUnrelated(source, target)) {
                    inferFromProperties(source, target);
                    inferFromSignatures(source, target, SignatureKind.Call);
                    inferFromSignatures(source, target, SignatureKind.Construct);
                    inferFromIndexTypes(source, target);
                }

This PR moves that call from inferFromObjectTypesWorker() to the third case. Consequently

1. inferFromIndexTypes() isn't called twice in the second (array) case
2. it isn't called at all in the first (tuple) case, so f2(values) is 1 like f1(values)

FYI f1(values) is currently 1 unlike f2(values) because parameter and argument are the same generic type (tuple) and handled by

                if (getObjectFlags(source) & ObjectFlags.Reference && getObjectFlags(target) & ObjectFlags.Reference && (
                    (<TypeReference>source).target === (<TypeReference>target).target || isArrayType(source) && isArrayType(target)) &&
                    !((<TypeReference>source).node && (<TypeReference>target).node)) {
                    // If source and target are references to the same generic type, infer from type arguments
                    inferFromTypeArguments(getTypeArguments(<TypeReference>source), getTypeArguments(<TypeReference>target), getVariances((<TypeReference>source).target));
                }

whereas f2(values) parameter (readonly tuple) and argument (tuple) aren't.

[jablko]

@typescript-bot user test this

[RyanCavanaugh]

@typescript-bot user test this
@typescript-bot test this

[typescript-bot]

Heya @RyanCavanaugh, I've started to run the parallelized community code test suite on this PR at e427f3f. You can monitor the build here. It should now contribute to this PR's status checks.

[typescript-bot]

Heya @RyanCavanaugh, I've started to run the extended test suite on this PR at e427f3f. You can monitor the build here. It should now contribute to this PR's status checks.

[typescript-bot]

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

[weswigham]

@jablko rather than the other changes in checker.ts, could you just delete

                    if (isArrayType(target)) {
                        inferFromIndexTypes(source, target);
                        return;
                    }

from inferFromProperties? That way the single unconditional inferFromIndexTypes is still easy to track.

[jablko]

@weswigham Thanks a lot for taking a look at this! What you describe would call inferFromIndexTypes() just once, however it would still call it in the tuple case, so I assume it wouldn't resolve the test case included in this issue: Inferring an array type instead of a tuple type.

inferFromProperties() infers a tuple type element-wise, but then inferFromObjectTypesWorker() calls inferFromIndexTypes() which re-infers an array type. ❌

[weswigham]

however it would still call it in the tuple case, so I assume it wouldn't resolve the test case included in this issue: Inferring an array type instead of a tuple type

Hm, if that's the case, can we copy the tuple check into the inferFromObjectTypesWorker instead of shifting inferFromIndexSignatures into inferFromProperties? I don't like the implicit linkage of index inference to property inference caused by the current layering.

[jablko]

✔️ Yup, that works. Done.

[sandersn]

@weswigham are you happy with this change now? It's been a while, but the inference code hasn't changed much in the last few months.

[weswigham]

Yeah, I think this is fine now (it just moves a chunk of code out of a call and into the (only) caller), I just wanted @ahejlsberg to look at it briefly before it got in, which is why he's assigned iirc.

[StefanXhunga]

Thank you for your assistance, please change the nesessery scedares!