interfaceI<T>{}declarefunctionf1<T1,T2>(values:[I<T1>,I<T2>]):T1;declarefunctionf2<T1,T2>(values:readonly[I<T1>,I<T2>]):T1;declareconstvalues:[I<1>,I<number>];letone:1;one=f1(values);// ✓ 1one=f2(values);// ✗ number

Currentlyf2(values) isnumber whilef1(values) is1.

There are three cases ininferFromProperties()

Source and target are both tuples: infers types element-wise
Target is an array: callsinferFromIndexTypes()
Otherwise: iterates over properties

But theninferFromObjectTypesWorker() callsinferFromIndexTypes() 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 frominferFromObjectTypesWorker() 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, sof2(values) is1 likef1(values)

FYIf1(values) is currently1 unlikef2(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 argumentsinferFromTypeArguments(getTypeArguments(<TypeReference>source),getTypeArguments(<TypeReference>target),getVariances((<TypeReference>source).target));}

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

Copy link

ContributorAuthor

jablko commentedOct 15, 2019

@typescript-bot user test this

Copy link

Member

RyanCavanaugh commentedOct 15, 2019

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

Copy link

Collaborator

typescript-bot commentedOct 15, 2019•
edited
Loading

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

Copy link

Collaborator

typescript-bot commentedOct 15, 2019•
edited
Loading

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

typescript-bot mentioned this pull request

Oct 15, 2019

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

Open

Copy link

Collaborator

typescript-bot commentedOct 15, 2019

The user suite test run you requested has finished andfailed. I've opened aPR with the baseline diff from master.

jablko force-pushed thepatch-19 branch 2 times, most recently fromdea9d72 tod045c4cCompare

October 17, 2019 16:59

RyanCavanaugh assignedahejlsberg

Oct 17, 2019

AviVahl mentioned this pull request

Oct 18, 2019

TS3.7: No overload matches this call#34554

Closed

jablko force-pushed thepatch-19 branch fromd045c4c to70f90d9Compare

October 19, 2019 18:33

jablko force-pushed thepatch-19 branch from70f90d9 to0d9a3bcCompare

October 27, 2019 19:15

jablko force-pushed thepatch-19 branch from0d9a3bc to5399ce0Compare

November 4, 2019 15:21

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 thepatch-19 branch 3 times, most recently fromd37394a to82d6ad7Compare

November 18, 2019 14:18

jablko force-pushed thepatch-19 branch 2 times, most recently from4765642 to8af1dcaCompare

November 25, 2019 20:24

Copy link

Member

weswigham commentedNov 26, 2019

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

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

frominferFromProperties? That way the single unconditionalinferFromIndexTypes is still easy to track.

Copy link

ContributorAuthor

@weswigham Thanks a lot for taking a look at this! What you describe would callinferFromIndexTypes() 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 theninferFromObjectTypesWorker() callsinferFromIndexTypes() which re-infers an array type. ❌

jablko force-pushed thepatch-19 branch 2 times, most recently from1a56348 to84baa5cCompare

November 27, 2019 19:40

Copy link

Member

weswigham commentedNov 27, 2019

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 theinferFromObjectTypesWorker instead of shiftinginferFromIndexSignatures intoinferFromProperties? I don't like the implicit linkage of index inference to property inference caused by the current layering.

jablko force-pushed thepatch-19 branch from84baa5c to730aa56Compare

November 27, 2019 20:40

Copy link

ContributorAuthor

jablko commentedNov 27, 2019

✔️ Yup, that works. Done.

jablko force-pushed thepatch-19 branch 2 times, most recently from09e96b8 to6be718cCompare

November 29, 2019 22:25

jablko force-pushed thepatch-19 branch from6be718c tob3f48a6Compare

December 28, 2019 17:09

jablko force-pushed thepatch-19 branch fromb3f48a6 to19f513eCompare

January 21, 2020 18:18

jablko force-pushed thepatch-19 branch 3 times, most recently from9c6fe7d to2cfb902Compare

January 29, 2020 01:06

sandersn added the For Milestone BugPRs that fix a bug with a specific milestone label

Feb 1, 2020

jablko force-pushed thepatch-19 branch 5 times, most recently fromc248e7f tof85c83fCompare

February 9, 2020 17:19

jablko force-pushed thepatch-19 branch 5 times, most recently from012cbda to3f15d40Compare

February 13, 2020 15:42

jablko force-pushed thepatch-19 branch from3f15d40 to5d3d184Compare

February 21, 2020 00:06

jablko added2 commits

February 21, 2020 00:11

Don't inferFromIndexTypes() twice

4c503a3

Add tests

31d6f32

jablko force-pushed thepatch-19 branch from5d3d184 to31d6f32Compare

February 22, 2020 22:41

Copy link

Member

sandersn commentedMar 4, 2020

@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 approved these changes

Mar 4, 2020

View reviewed changes

Copy link

Member

weswigham left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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.