Don't inferFromIndexTypes() twice #34501

jablko · 2019-10-15T21:33:35Z

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()

Source and target are both tuples: infers types element-wise
Target is an array: calls inferFromIndexTypes()
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

inferFromIndexTypes() isn't called twice in the second (array) case
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 · 2019-10-15T22:03:17Z

@typescript-bot user test this

RyanCavanaugh · 2019-10-15T22:49:33Z

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

typescript-bot · 2019-10-15T22:50:34Z

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 · 2019-10-15T22:50:34Z

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 · 2019-10-15T23:19:18Z

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

weswigham · 2019-11-26T00:05:39Z

@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 · 2019-11-26T14:40:28Z

@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 · 2019-11-27T19:46:37Z

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 · 2019-11-27T21:19:00Z

✔️ Yup, that works. Done.


        Don't inferFromIndexTypes() twice


        Add tests

sandersn · 2020-03-04T19:47:56Z

@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.

typescript-bot mentioned this pull request Oct 15, 2019

🤖 User test baselines have changed for patch-19 #1

Open

jablko force-pushed the jablko:patch-19 branch 2 times, most recently from dea9d72 to d045c4c Oct 16, 2019

RyanCavanaugh assigned ahejlsberg Oct 17, 2019

AviVahl mentioned this pull request Oct 18, 2019

TS3.7: No overload matches this call #34554

Closed

jablko force-pushed the jablko:patch-19 branch from d045c4c to 70f90d9 Oct 19, 2019

jablko force-pushed the jablko:patch-19 branch from 70f90d9 to 0d9a3bc Oct 27, 2019

jablko force-pushed the jablko:patch-19 branch from 0d9a3bc to 5399ce0 Nov 4, 2019

jablko mentioned this pull request Nov 12, 2019

3.7.2 Promise.all infers base type of interfaces rather than actual interface #34937

Closed

jablko force-pushed the jablko:patch-19 branch 3 times, most recently from d37394a to 82d6ad7 Nov 13, 2019

jablko force-pushed the jablko:patch-19 branch 2 times, most recently from 4765642 to 8af1dca Nov 22, 2019

jablko force-pushed the jablko:patch-19 branch 2 times, most recently from 1a56348 to 84baa5c Nov 26, 2019

jablko force-pushed the jablko:patch-19 branch from 84baa5c to 730aa56 Nov 27, 2019

jablko force-pushed the jablko:patch-19 branch 2 times, most recently from 09e96b8 to 6be718c Nov 28, 2019

jablko force-pushed the jablko:patch-19 branch from 6be718c to b3f48a6 Dec 28, 2019

jablko force-pushed the jablko:patch-19 branch from b3f48a6 to 19f513e Jan 21, 2020

jablko force-pushed the jablko:patch-19 branch 3 times, most recently from 9c6fe7d to 2cfb902 Jan 25, 2020

sandersn added this to Gallery in Pall Mall Jan 30, 2020

sandersn added the For Milestone Bug label Feb 1, 2020

jablko force-pushed the jablko:patch-19 branch 5 times, most recently from c248e7f to f85c83f Feb 3, 2020

jablko force-pushed the jablko:patch-19 branch 5 times, most recently from 012cbda to 3f15d40 Feb 10, 2020

jablko force-pushed the jablko:patch-19 branch from 3f15d40 to 5d3d184 Feb 21, 2020

jablko added 2 commits Oct 15, 2019

Don't inferFromIndexTypes() twice

4c503a3

Add tests

Loading status checks…

31d6f32

jablko force-pushed the jablko:patch-19 branch from 5d3d184 to 31d6f32 Feb 22, 2020

sandersn added this to Needs review in PR Backlog Mar 4, 2020

PR Backlog automation moved this from Needs review to Needs merge Mar 4, 2020

weswigham approved these changes Mar 4, 2020

View changes

ahejlsberg approved these changes Mar 4, 2020

View changes

PR Backlog automation moved this from Needs merge to Done Mar 4, 2020

sandersn removed this from Gallery in Pall Mall Mar 4, 2020

strelga mentioned this pull request May 7, 2020

Better typings for Promise, like #31117 #33707

Open

Apr	MAY	Jun
	28
2019	2020	2021

microsoft / TypeScript

Don't inferFromIndexTypes() twice #34501

Don't inferFromIndexTypes() twice #34501

jablko commented Oct 15, 2019 •

edited

jablko commented Oct 15, 2019

RyanCavanaugh commented Oct 15, 2019

typescript-bot commented Oct 15, 2019 •

edited

typescript-bot commented Oct 15, 2019 •

edited

typescript-bot commented Oct 15, 2019

weswigham commented Nov 26, 2019

jablko commented Nov 26, 2019

weswigham commented Nov 27, 2019

jablko commented Nov 27, 2019

sandersn commented Mar 4, 2020

weswigham left a comment •

edited

microsoft / TypeScript

Join GitHub today

Don't inferFromIndexTypes() twice #34501

Don't inferFromIndexTypes() twice #34501

Conversation

jablko commented Oct 15, 2019 • edited

jablko commented Oct 15, 2019

RyanCavanaugh commented Oct 15, 2019

typescript-bot commented Oct 15, 2019 • edited

typescript-bot commented Oct 15, 2019 • edited

typescript-bot commented Oct 15, 2019

weswigham commented Nov 26, 2019

jablko commented Nov 26, 2019

weswigham commented Nov 27, 2019

jablko commented Nov 27, 2019

sandersn commented Mar 4, 2020

weswigham left a comment • edited

jablko commented Oct 15, 2019 •

edited

typescript-bot commented Oct 15, 2019 •

edited

typescript-bot commented Oct 15, 2019 •

edited

weswigham left a comment •

edited