Skip to content

Conversation

@auduchinok
Copy link
Member

@auduchinok auduchinok commented Nov 24, 2018

Currently AstTraversal.Traverse goes into expr parts of type test expressions only and ignores type parts. This PR adds diving into type parts.

In the example below System.String will now be visited by VisitType.

"" :? System.String

@TIHan TIHan added the Area-FCS label Dec 6, 2018
@TIHan
Copy link
Contributor

TIHan commented Dec 6, 2018

Is it possible to add a test case for this?

@auduchinok auduchinok force-pushed the astVisitor-diveIntoTypes branch 2 times, most recently from 7428ef8 to 343ed6d Compare December 11, 2018 20:34
@auduchinok
Copy link
Member Author

@TIHan Thanks, I added a test.

@auduchinok auduchinok force-pushed the astVisitor-diveIntoTypes branch from 343ed6d to db7cf44 Compare December 11, 2018 20:51
@auduchinok auduchinok force-pushed the astVisitor-diveIntoTypes branch from db7cf44 to 1130a55 Compare December 11, 2018 20:52
Copy link
Contributor

@TIHan TIHan left a comment

Choose a reason for hiding this comment

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

Thank you!

Seems CI is still failing though. When that is resolved, we can merge.

@TIHan
Copy link
Contributor

TIHan commented Dec 11, 2018

Also, as a note, I am also lurking in changing some of this code to accomodate ISourceText, #6001. I have a feeling this will get merged in first so you may not have to worry about it.

@auduchinok auduchinok closed this Dec 12, 2018
@auduchinok auduchinok reopened this Dec 12, 2018
@auduchinok
Copy link
Member Author

It's green.

@cartermp
Copy link
Contributor

@auduchinok Is there an issue to link here? No worries if there's no, I just want to associate something with a milestone for traceability.

@auduchinok
Copy link
Member Author

auduchinok commented Dec 12, 2018

@cartermp No issue to link to, sorry. I pushed the fix right after discovering it.

@cartermp cartermp added this to the 16.0 milestone Dec 12, 2018
@TIHan TIHan merged commit 9b55ecc into dotnet:master Dec 22, 2018
@TIHan
Copy link
Contributor

TIHan commented Dec 22, 2018

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants