Skip to content

GoToDef on deconstruction assignment and foreach#25984

Merged
jcouv merged 1 commit intodotnet:masterfrom
jcouv:decon-gotodef
Apr 6, 2018
Merged

GoToDef on deconstruction assignment and foreach#25984
jcouv merged 1 commit intodotnet:masterfrom
jcouv:decon-gotodef

Conversation

@jcouv
Copy link
Copy Markdown
Member

@jcouv jcouv commented Apr 6, 2018

This PR associates the symbols for various Deconstruct methods involved in a deconstruction with the = (in deconstruction assignment or deconstruction declaration) or the in syntax (in deconstruction foreach).

Note that GoToDef currently assumes that there is only one symbol that is "the definition". I think we should relax that, as a later change. If we did, we could not only list the Deconstruct methods, but also the GetEnumerator method, and even conversion operators involved in the assignment or foreach.

In the meantime, GoToDef only goes to the first Deconstruct method listed, which is the top-level one. That is good enough for most cases (nested deconstructions would be less common).

Fixes #16529
Fixes #24413
Tagging @CyrusNajmabadi FYI

image

@jcouv jcouv added the Area-IDE label Apr 6, 2018
@jcouv jcouv added this to the 15.8 milestone Apr 6, 2018
@jcouv jcouv self-assigned this Apr 6, 2018
@jcouv jcouv requested a review from a team as a code owner April 6, 2018 03:47
return GetSymbolInfo(semanticModel, node, token, cancellationToken).GetBestOrAllSymbols();
}

private SymbolInfo GetSymbolInfo(SemanticModel semanticModel, SyntaxNode node, SyntaxToken token, CancellationToken cancellationToken)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

📝 The constructors for SymbolInfo are internal to the compiler (inaccessible from this code), so I couldn't just add the new logic to the GetSymbolInfo method.

Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

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

Nice!

switch (node)
{
case AssignmentExpressionSyntax assignment when token.Kind() == SyntaxKind.EqualsToken:
return GetDeconstructionAssignmentMethods(semanticModel, node).As<ISymbol>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we be doing something similar in VB?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is no deconstruction in VB yet

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Apr 6, 2018

test windows_release_unit64_prtest please

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.

3 participants