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.

Sign up for GitHub

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

Jump to bottom

Track if a file contains a tuple expression or tuple type in our index. #28287

Merged
merged 1 commit into from
Jul 11, 2018
Merged

Track if a file contains a tuple expression or tuple type in our index. #28287

merged 1 commit into from
Jul 11, 2018

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented Jul 5, 2018 •
edited
Loading

This will be used by the "convert tuple to struct" feature (#28257). We want to track this information to so that we can limit the number of documents we need to look at when choosing "... and update matches in current project" or "... and update matches in dependent projects".

By using this index we can search for files containing tuples which also contain the names of the tuple fields. This should help greatly limit our search space.

I made this into its own PR to help extract out bits of "convert tuple to struct" that could go in independently.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner July 5, 2018 00:11
@CyrusNajmabadi
Copy link
Member Author

Tagging @dotnet/roslyn-ide
Also @jcouv as you have updated this index yourself for features you care about.

CyrusNajmabadi
CyrusNajmabadi commented Jul 5, 2018
View reviewed changes
src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex_Persistence.cs
@@ -13,7 +13,7 @@ namespace Microsoft.CodeAnalysis.FindSymbols
internal sealed partial class SyntaxTreeIndex : IObjectWritable
{
private const string PersistenceName = "<SyntaxTreeIndex>";
private const string SerializationFormat = "12";
private const string SerializationFormat = "15";
Copy link
Member Author

Choose a reason for hiding this comment

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

Specifically not incrementing by one. I think there's another PR going in now that is changing this value, and i want them to conflict.

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 mean #28230, it's been merged.

jcouv
jcouv reviewed Jul 5, 2018
View reviewed changes
src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex.ContextInfo.cs
@@ -151,6 +158,7 @@ private enum ContainingNodes
ContainsIndexerMemberCref = 1 << 7,
ContainsDeconstruction = 1 << 8,
ContainsAwait = 1 << 9,
ContainsTupleExpressionOrTupleType = 1 << 10,
Copy link
Member

@jcouv jcouv Jul 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This list is out-of-date (I've just changed it in #28230, which was merged earlier this week).

Copy link
Member

Choose a reason for hiding this comment

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

Never mind, I'd added the ContainsAwait. I'm distracted... :-S

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah ok :) Thanks!

jcouv
jcouv approved these changes Jul 5, 2018
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM

@jcouv jcouv added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Jul 5, 2018
@jcouv jcouv added this to the 16.0 milestone Jul 5, 2018
ivanbasov
ivanbasov reviewed Jul 5, 2018
View reviewed changes
src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs
SyntaxNode GetOperandOfPrefixUnaryExpression(SyntaxNode node);
SyntaxToken GetOperatorTokenOfPrefixUnaryExpression(SyntaxNode node);


// Left side of = assignment.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: extra line

ivanbasov
ivanbasov approved these changes Jul 5, 2018
View reviewed changes
Copy link
Contributor

@ivanbasov ivanbasov left a comment

Choose a reason for hiding this comment

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

:shipit:

@CyrusNajmabadi
Copy link
Member Author

@jinujoseph Can we merge this in?

Also, what's an appropriate brnach to target for feature-work?

@jinujoseph
Copy link
Contributor

Hold on merging this.
feature-work branch is master.
Should not be merged into it till we snap for 15.8 release. (expected to be done by next week)

@CyrusNajmabadi
Copy link
Member Author

@jinujoseph Can't we just make a branch off of master, and pull into that? It's unclear why it would take a week just to have someplace to check into... Can you clarify? Thanks!

@jinujoseph
Copy link
Contributor

Thats the plan , we will be branching of master. holiday week plus we have been having some infrastructure issue. thanks for your patience.

@CyrusNajmabadi
Copy link
Member Author

test this please

@CyrusNajmabadi
Copy link
Member Author

@jinujoseph This has been reviewed and is ready to go into 16.0 Thanks!

@jinujoseph jinujoseph merged commit 30cda01 into dotnet:master Jul 11, 2018
@CyrusNajmabadi
Copy link
Member Author

Thanks!

@CyrusNajmabadi CyrusNajmabadi deleted the tupleIndex branch July 11, 2018 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivanbasov ivanbasov ivanbasov approved these changes

@jcouv jcouv jcouv approved these changes

Assignees

@ivanbasov ivanbasov

Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
4 participants
@CyrusNajmabadi @jinujoseph @ivanbasov @jcouv