Skip to content
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

Fix speculation of AsNewClause in Rename Rewriter #37844

Merged
merged 1 commit into from Aug 13, 2019

Conversation

@JoeRobich
Copy link
Member

commented Aug 8, 2019

Another case of building a SpeculativeModel from an AsNewClause and the tree being speculating on being generated from an EqualsValue node. Causing the nodes being speculated on to not exist in the tree.

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/941271

@JoeRobich JoeRobich added the Area-IDE label Aug 8, 2019

@JoeRobich JoeRobich requested a review from dotnet/roslyn-ide as a code owner Aug 8, 2019

@JoeRobich JoeRobich merged commit 7b4881f into dotnet:master Aug 13, 2019

20 checks passed

WIP Ready for review
Details
license/cla All CLA requirements met.
Details
roslyn-CI Build #20190808.58 succeeded
Details
roslyn-CI (Linux_Test coreclr) Linux_Test coreclr succeeded
Details
roslyn-CI (Linux_Test mono) Linux_Test mono succeeded
Details
roslyn-CI (MacOs_Test) MacOs_Test succeeded
Details
roslyn-CI (Windows_CoreClr_Unit_Tests debug) Windows_CoreClr_Unit_Tests debug succeeded
Details
roslyn-CI (Windows_CoreClr_Unit_Tests release) Windows_CoreClr_Unit_Tests release succeeded
Details
roslyn-CI (Windows_Correctness_Test) Windows_Correctness_Test succeeded
Details
roslyn-CI (Windows_Desktop_Spanish_Unit_Tests) Windows_Desktop_Spanish_Unit_Tests succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests debug_32) Windows_Desktop_Unit_Tests debug_32 succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests debug_64) Windows_Desktop_Unit_Tests debug_64 succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests release_32) Windows_Desktop_Unit_Tests release_32 succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests release_64) Windows_Desktop_Unit_Tests release_64 succeeded
Details
roslyn-CI (Windows_Determinism_Test) Windows_Determinism_Test succeeded
Details
roslyn-integration-CI Build #20190808.58 succeeded
Details
roslyn-integration-CI (VS_Integration debug_async) VS_Integration debug_async succeeded
Details
roslyn-integration-CI (VS_Integration debug_legacy) VS_Integration debug_legacy succeeded
Details
roslyn-integration-CI (VS_Integration release_async) VS_Integration release_async succeeded
Details
roslyn-integration-CI (VS_Integration release_legacy) VS_Integration release_legacy succeeded
Details
' There are cases when we change the type of node to make speculation work (e.g.,
' for AsNewClauseSyntax), so getting the newNode from the _speculativeModel
' ensures the final node replacing the original node is found.
probableRenameNode = Me._speculativeModel.SyntaxTree.GetRoot(_cancellationToken).GetAnnotatedNodes(Of SyntaxNode)(annotation).First()

This comment has been minimized.

Copy link
@mavasani

mavasani Aug 14, 2019

Contributor

Can we delete the code above? Seems that is a redundant assigment...

Dim probableRenameNode = speculativeTree.GetAnnotatedNodes(Of SyntaxNode)(annotation).First()

This comment has been minimized.

Copy link
@mavasani

mavasani Aug 14, 2019

Contributor

And the newly added assignment to speculativeNewNode below also seems redundant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.