Add feature to allow converting from a tuple to a named-struct. #28257

CyrusNajmabadi · 2018-07-03T08:13:24Z

Note: this is a WIP. There are several parts to complete. But i wanted to send this out for early review.

This is a sibling PR to Convert Anonymous Type to Class and Convert Anonymous Type to Tuple.

it should probably go in once those go in. This feature is most similar to 'Convert Anonymous Type to Class'. However, it has a lot more complexity in it due to the nature of Tuples versus Anonymous-Types. Specifically, Anonymous Types are simpler to manage because their types cannot 'leak' out of the containing method. Indeed, their types cannot actually be uttered in source. Tuples are far different. Their types can spread all over a codebase.

Before continuing on that issue, first let me show a picture of what this looks like:

--

First: note the concept of 'scopes' that the refactoring will apply to. Specifically, when you invoke it, we ask you to let us know how broadly you want us to search for matching types to convert to the new type you're introducing. The scopes are:

Method/Member level. For when you've just used a tuple in a single member (like an anonymous type) and you decide you want a real type before leaking it out elsewhere.
Type-level. For when you've used a tuple inside the impl of a type, and realize you wnt a named type before you expose it.
Project level. When you want to update all usages of that tuple type in the current project you're in.
Dependent projects. And when you want all downstream projects to be updated as well.

This approach gives you control of how wide you want the update to go. This is necessary because it's quite possible to use the same tuple type in several places in a project and want (or not want) to update all those places. For example, one part of a codebase may use it's own (int x, int y) tuple in some code. And that type may show up far elsewhere. It's not necessarily the case that when you are updating the first, that you want the other area to be affected.

--

Second: Note that the struct we generate has far more parts than the standard anonymous-type-to-class conversion. Specifically, on top of all the standard constructor/Equals/GetHashCode that we generate. We also generate conversion to/from tuples, as well as a Deconstruct method. This allows this type to seamlessly be used even when working with external APIs (or other APIs in your own code) that have not been updated. For example, say you have:

void Foo()
{
    var t = (x: 0, y: 1);
    Bar(t);
}

void Bar((int, int) tuple) { ... }

Here we have two distinct tuple types. The one with the names 'x' and 'y' in it, and the anonymous one. If you introduce a type for (x: 0, y: 1) this will produce:

void Foo()
{
    var t = new NewStruct(x: 0, y: 1);
    Bar(t);
}

void Bar((int, int) tuple) { ... }

And this code will continue to work because NewStruct is implicitly convertible to (int, int).

--

Third: When replacing, we only replace tuple types that exactly match the starting type. So, if you rename and reassign tuple members in code, we will consdier that a different type that we will not fixup. This is necessary so that we don't end up deciding to replace all 2-tuples in the world any time you're just trying to fixup your particular 2-tuple with your specific names.

--

Finally, there are some caveats here we can't really do anything about (Esp. as analyzers can't do global analysis). For example, if a tuple is used with a == expression, that has special meaning in the language that only tuples can support. This means that such code will break when converting to tuples.

--

TODO:

Actually implement hte Project/Dependent-project scopes. The stubs are there, but i need to fill them in.
Update SyntaxTreeIndex to contain information about what tuple shapes are found inside. We can use that to help speed up the global fixup operations.
Tests. Lots of tests needed.
Properly support cross-language scenarios. Right now i'm doing direct Type-Equality checks. Those won't work cross language. We need to use the IDE equality comparers.

CyrusNajmabadi · 2018-07-03T08:14:44Z

Tagging @jcouv @sharwell @jinujoseph @kuhlenh @dotnet/roslyn-ide

Note: not quite ready for review. I will ping when it is.

CyrusNajmabadi · 2018-07-03T08:15:32Z

src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs

+                    {
+                        refactoringSuggestedActions.Add(new CodeRefactoringSuggestedAction(
+                            _owner, workspace, _subjectBuffer, refactoring.Provider, action));
+                    }


needed to support refactorings with nested options. Up till now we've never had any of those.

CyrusNajmabadi · 2018-07-03T08:19:06Z

I would not review this until #28066 goes in. This PR depends on a lot of the changes in that PR. So there would be a lot of duplication currently.

CyrusNajmabadi · 2018-07-06T06:11:29Z

Tagging @jcouv @sharwell @jinujoseph @kuhlenh @dotnet/roslyn-ide

This is ready for review.

jcouv · 2018-07-11T22:53:27Z

...es/Core/Portable/ConvertTupleToStruct/AbstractConvertTupleToStructCodeRefactoringProvider.cs

+            }
+        }
+
+        private bool InfoProbablyContainsTupleFieldNames(SyntaxTreeIndex info, ImmutableArray<string> tupleFieldNames)


InfoProbablyContainsTupleFieldNames [](start = 21, length = 35)

static?
Then maybe callers might be made static too?

jcouv · 2018-07-11T23:02:26Z

...es/Core/Portable/ConvertTupleToStruct/AbstractConvertTupleToStructCodeRefactoringProvider.cs

+            });
+        }
+
+        private async Task<Solution> ApplyChangesAsync(


static here as well? #Closed

jcouv · 2018-07-11T23:05:16Z

...es/Core/Portable/ConvertTupleToStruct/AbstractConvertTupleToStructCodeRefactoringProvider.cs

+                });
+        }
+
+        private SeparatedSyntaxList<TArgumentSyntax> ConvertArguments(ISyntaxFactsService syntaxFacts, SyntaxGenerator generator, SeparatedSyntaxList<TArgumentSyntax> arguments)


ConvertArguments [](start = 53, length = 16)

~~One of the two ConvertArguments overloads may be unused.~~ Hum, it's good to double-check, but I think they are both used after all. #Closed

both are used. :)

the second is called by the first.

jcouv

LGTM Thanks (iteration 34).

CyrusNajmabadi · 2018-07-11T23:18:45Z

@jcouv Thanks. I will ping when ready to go in!

CyrusNajmabadi · 2018-07-12T20:28:57Z

test windows_debug_vs-integration_prtest

CyrusNajmabadi · 2018-07-12T22:03:04Z

@jcouv @ivanbasov this can be merged in now. Thanks!

jcouv · 2018-07-12T22:30:41Z

<data name="Convert_to_tuple" xml:space="preserve">

This shouldn't be here

Refers to: src/Features/Core/Portable/FeaturesResources.resx:1379 in e00a3fe. [](commit_id = e00a3fe, deletion_comment = False)

jcouv · 2018-07-12T22:33:01Z

        public const string CodeActionsConvertAnonymousTypeToTuple = "CodeActions.ConvertAnonymousTypeToTuple";

This shouldn't be here #Closed

Refers to: src/Test/Utilities/Portable/Traits/Traits.cs:52 in e00a3fe. [](commit_id = e00a3fe, deletion_comment = False)

jcouv

Done with review pass (iteration 38)

CyrusNajmabadi · 2018-07-12T22:42:45Z

@jcouv I don't know what your Traits.cs comment refers to...

CyrusNajmabadi · 2018-07-12T22:45:26Z

@jcouv i also do not understand the Featuresresources.resx comment...

jcouv · 2018-07-12T22:46:06Z

I did a diff in CodeFlow between iterations 34 and 38, and saw an addition of:
public const string CodeActionsConvertAnonymousTypeToTuple = "CodeActions.ConvertAnonymousTypeToTuple";

And in the resx, it shows an addition of <data name="Convert_to_tuple" xml:space="preserve">.

Not sure what's going on.

jcouv · 2018-07-12T22:47:28Z

Hum, if I just look at iteration 38, then those additions do not appear. CodeFlow must be confused by the merge from master...
Sorry about that.

LGTM (iteration 38)

CyrusNajmabadi · 2018-07-12T22:52:30Z

Thanks!

Add feature to allow converting from a tuple to a named-struct.

e519f41

CyrusNajmabadi requested a review from a team as a code owner July 3, 2018 08:13

CyrusNajmabadi commented Jul 3, 2018

View reviewed changes

CyrusNajmabadi added 3 commits July 3, 2018 01:16

Remove commented code.

7bd3b8d

Remove code.

f19e76e

Cleanup.

2a665cf

jcouv added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Jul 3, 2018

CyrusNajmabadi mentioned this pull request Jul 5, 2018

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

Merged

CyrusNajmabadi added 7 commits July 4, 2018 17:17

Merge branch 'tupleIndex' into convertTupleToStruct

e86e671

Add configureawait

b942e49

Only do global updates if we have names to search for.

a34f3ad

Add support for updating the current project.

4c5c748

Add support for updating dependent projects.

c9e43cb

Add comment.

5789fc4

Properly update dependent projects.

91d09a4

jinujoseph added this to the 16.0 milestone Jul 5, 2018

CyrusNajmabadi added 10 commits July 5, 2018 16:10

Handle captured type parameters.

6740c2f

Get tests building.

1a55dfe

Remove named args when not necessary.

900e76a

Don't include named args unless the expr was a literal.

696d54c

Add tests.

c484518

Update tests.

0b6f33e

Add more tests.

1fd069b

Add VB tests.

c167059

Add VB tests.

7fbe12d

Fix tests.

dfbb577

PR feedback.

e355099

jcouv reviewed Jul 11, 2018

View reviewed changes

CyrusNajmabadi added 2 commits July 11, 2018 15:56

Update comment.

9ddd130

Make members static.

6f4617d

jcouv reviewed Jul 11, 2018

View reviewed changes

make static.

c966d8f

jcouv approved these changes Jul 11, 2018

View reviewed changes

CyrusNajmabadi added 4 commits July 11, 2018 18:45

Fix assert.

d3efa1f

Merge remote-tracking branch 'upstream/master' into convertTupleToStruct

134daea

Update strings.

24d2a3a

only process VB/C# projects.

e00a3fe

jcouv reviewed Jul 12, 2018

View reviewed changes

jcouv merged commit 569124e into dotnet:master Jul 12, 2018

CyrusNajmabadi deleted the convertTupleToStruct branch July 12, 2018 22:52

CyrusNajmabadi mentioned this pull request Jul 13, 2018

A command to generate a struct from a tuple #26998

Closed

jinujoseph added the Feature Request label Feb 15, 2019

CyrusNajmabadi mentioned this pull request Mar 2, 2020

Feature Request: Quick action to generate struct/class from tuple declarations / parameter lists #16851

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add feature to allow converting from a tuple to a named-struct. #28257

Add feature to allow converting from a tuple to a named-struct. #28257

CyrusNajmabadi commented Jul 3, 2018 •

edited

Loading

CyrusNajmabadi commented Jul 3, 2018

CyrusNajmabadi Jul 3, 2018

CyrusNajmabadi commented Jul 3, 2018

CyrusNajmabadi commented Jul 6, 2018

jcouv Jul 11, 2018

CyrusNajmabadi Jul 11, 2018

jcouv Jul 11, 2018 •

edited

Loading

CyrusNajmabadi Jul 11, 2018

jcouv Jul 11, 2018 •

edited

Loading

CyrusNajmabadi Jul 11, 2018

CyrusNajmabadi Jul 11, 2018

jcouv left a comment

CyrusNajmabadi commented Jul 11, 2018

CyrusNajmabadi commented Jul 12, 2018

CyrusNajmabadi commented Jul 12, 2018

jcouv commented Jul 12, 2018 •

edited

Loading

jcouv commented Jul 12, 2018 •

edited

Loading

jcouv left a comment

CyrusNajmabadi commented Jul 12, 2018

CyrusNajmabadi commented Jul 12, 2018

jcouv commented Jul 12, 2018

jcouv commented Jul 12, 2018

CyrusNajmabadi commented Jul 12, 2018

Add feature to allow converting from a tuple to a named-struct. #28257

Add feature to allow converting from a tuple to a named-struct. #28257

Conversation

CyrusNajmabadi commented Jul 3, 2018 • edited Loading

CyrusNajmabadi commented Jul 3, 2018

CyrusNajmabadi Jul 3, 2018

Choose a reason for hiding this comment

CyrusNajmabadi commented Jul 3, 2018

CyrusNajmabadi commented Jul 6, 2018

jcouv Jul 11, 2018

Choose a reason for hiding this comment

CyrusNajmabadi Jul 11, 2018

Choose a reason for hiding this comment

jcouv Jul 11, 2018 • edited Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jul 11, 2018

Choose a reason for hiding this comment

jcouv Jul 11, 2018 • edited Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jul 11, 2018

Choose a reason for hiding this comment

CyrusNajmabadi Jul 11, 2018

Choose a reason for hiding this comment

jcouv left a comment

Choose a reason for hiding this comment

CyrusNajmabadi commented Jul 11, 2018

CyrusNajmabadi commented Jul 12, 2018

CyrusNajmabadi commented Jul 12, 2018

jcouv commented Jul 12, 2018 • edited Loading

jcouv commented Jul 12, 2018 • edited Loading

jcouv left a comment

Choose a reason for hiding this comment

CyrusNajmabadi commented Jul 12, 2018

CyrusNajmabadi commented Jul 12, 2018

jcouv commented Jul 12, 2018

jcouv commented Jul 12, 2018

CyrusNajmabadi commented Jul 12, 2018

CyrusNajmabadi commented Jul 3, 2018 •

edited

Loading

jcouv Jul 11, 2018 •

edited

Loading

jcouv Jul 11, 2018 •

edited

Loading

jcouv commented Jul 12, 2018 •

edited

Loading

jcouv commented Jul 12, 2018 •

edited

Loading