Skip to content

Resuscitate the SymbolWalker API #17844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Aug 23, 2017
Merged

Conversation

amcasey
Copy link
Member

@amcasey amcasey commented Aug 16, 2017

All the good bits are from @weswigham (PR #9847). I just tweaked the API and made everything internal.

@amcasey amcasey requested review from weswigham and rbuckton August 16, 2017 21:21
@msftclas
Copy link

@amcasey,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@amcasey
Copy link
Member Author

amcasey commented Aug 16, 2017

I must have missed my test cleanup when I rebased. Just a minute...

function visitObjectType(type: ObjectType): void {
const stringIndexType = getIndexTypeOfStructuredType(type, IndexKind.String);
visitType(stringIndexType);
const numberIndexType = getIndexTypeOfStructuredType(type, IndexKind.String);
Copy link
Member Author

Choose a reason for hiding this comment

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

@weswigham Is that supposed to be IndexKind.Number?

Copy link
Member

Choose a reason for hiding this comment

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

Yep.

@@ -0,0 +1,173 @@
/** @internal */
namespace ts {
/** @internal */
Copy link

Choose a reason for hiding this comment

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

Don't need to do this twice

Copy link
Member Author

Choose a reason for hiding this comment

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

I wondered. Presumably, the inner one is redundant?

Copy link

Choose a reason for hiding this comment

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

Yeah.

Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

I think visitType needs clauses for visiting the related types for index types, index access types, and mapped types - those are new since its original writing, so it doesn't handle them yet, from the look of it.

@amcasey
Copy link
Member Author

amcasey commented Aug 17, 2017

The travis failure is unrelated. The PR is here: ivogabe/gulp-typescript#536.

}
}

function leftmostSymbol(expr: QualifiedName | Identifier): Symbol {
Copy link
Member

Choose a reason for hiding this comment

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

This can probably just be implemented as getResolvedSymbol(getFirstIdentifier(expr)), if you're willing to have getFirstIdentifier from the checker as a parameter.

@amcasey
Copy link
Member Author

amcasey commented Aug 22, 2017

Closing and re-opening to re-queue tests.

@amcasey amcasey closed this Aug 22, 2017
@amcasey amcasey reopened this Aug 22, 2017
@msftclas
Copy link

@amcasey,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@amcasey amcasey merged commit 49676c5 into microsoft:master Aug 23, 2017
@amcasey amcasey deleted the SymbolWalker branch August 23, 2017 20:32
amcasey added a commit to amcasey/TypeScript that referenced this pull request Sep 15, 2017
@ghost ghost mentioned this pull request Sep 18, 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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants