Skip to content

Conversation

@wonderful-panda
Copy link
Contributor

@wonderful-panda wonderful-panda commented Nov 28, 2016

Fixes #12536

@msftclas
Copy link

Hi @wonderful-panda, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

}

function addStringLiteralCompletionsFromType(type: Type, result: CompletionEntry[]): void {
if (type && type.flags & TypeFlags.TypeParameter) {
Copy link
Member

Choose a reason for hiding this comment

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

If you get the type constraint and the type is a keyof type, where is that handled below?


function addStringLiteralCompletionsFromType(type: Type, result: CompletionEntry[]): void {
if (type && type.flags & TypeFlags.TypeParameter) {
type = (<TypeParameter>type).constraint;
Copy link
Contributor

Choose a reason for hiding this comment

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

you should use checker::getApparentType here instead. it does some more than just checking the constraint.

Copy link
Contributor

Choose a reason for hiding this comment

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

to do that, you will need to expose this method on TypeChecker interface.

Copy link
Contributor

@mhegazy mhegazy left a comment

Choose a reason for hiding this comment

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

comments listed erlier

@wonderful-panda
Copy link
Contributor Author

Thank you for the review. I've pushed new version.

@mhegazy mhegazy merged commit 6418c2f into microsoft:master Dec 30, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants