Skip to content

Mapped types fixes#17336

Closed
gcnew wants to merge 4 commits intomicrosoft:masterfrom
gcnew:mappedTypesFixes
Closed

Mapped types fixes#17336
gcnew wants to merge 4 commits intomicrosoft:masterfrom
gcnew:mappedTypesFixes

Conversation

@gcnew
Copy link
Contributor

@gcnew gcnew commented Jul 21, 2017 •
edited
Loading

Fixes #17238

This fix changes the rule from #12351 to:

  • The operation { [P in K]: T}[X] is equivalent to an instantiation of T where X is substituted for every occurrence of P, unless K is an indexing operation itself. For example, { [P in K]: Box<T[P]> }[X] is equivalent to Box<T[X]>, however { [P in T[X]]: Box<T[P]> }[X] is not.

This PR also fixes checking of operations done on the object of type indexing, e.g. the following is now correctly identified as error:

// Note that `extra` is not present in { a:'a', b:'a' }
type T5<S extends 'a'|'b'|'extra'> = {[key in { a:'a', b:'a' }[S]]: true}[S]

Examples:

type AB = {
    a: 'a'
    b: 'a'
}

type T1<K extends keyof AB> = { [key in AB[K]]: true }
type T2<K extends 'a'|'b'> = T1<K>[K] // BUG 1: should be error for K = 'b'

type R = AB[keyof AB]; // "a"
type T3 = { [key in R]: true }
type T4<K extends 'a'|'b'> = T3[K] // error as expected

// BUG 2: 'extra' not checked in AB[S]
type T5<S extends 'a'|'b'|'extra'> = {[key in AB[S]]: true}[S]

@gcnew gcnew mentioned this pull request Jul 21, 2017
Closed
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jul 26, 2017

@ahejlsberg can you take a look at this PR?

@ahejlsberg
Copy link
Member

ahejlsberg commented Jul 27, 2017

@gcnew Thanks for your contribution! You are definitely right about the missing checkSourceElement call call in checkIndexedAccessType. In fact, there should be a call for node.objectType as well.

Regarding the other change, the correct fix is to add a check in getIndexedAccessForMappedType that verifies that the index type is assignable to keyof T for the object type. This is supposed to be true for any indexed access type but ends up not being verified in the generic mapped type case.

I'm going to put up a new PR with these changes, but I will keep the tests you wrote. Thanks again!

@ahejlsberg ahejlsberg mentioned this pull request Jul 27, 2017
Merged
@gcnew
Copy link
Contributor Author

gcnew commented Jul 27, 2017 •
edited
Loading

Funny enough, I tried to fix checkIndexedAccessType at first, but due to my lack of knowledge and guided by the debugger, I went for getIndexedAccessType.

Now, I'm looking at another issue. Mapped types buried inside an intersection are eagerly resolved, leading to incorrect behaviour (see #17456 and gcnew@73c78e6). The problem is that if the indexing operation is postponed (as in my experiment), the index type is not properly checked.

@gcnew
Copy link
Contributor Author

gcnew commented Aug 2, 2017

Closing as the bugs have already been addressed by #17455.

@gcnew gcnew closed this Aug 2, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ahejlsberg ahejlsberg Awaiting requested review from ahejlsberg

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug with mapped types and indexing

4 participants

@gcnew @DanielRosenwasser @ahejlsberg @msftclas