Proposing two new refactoring options for tuple assignments in constructors

[IEvangelist]

Version Used: 3.3.0-beta3-19407-03+d961190a1b4b2a81ac178b567840dfeff048b037

Steps to Reproduce:

There are certain language features that could use a bit more tooling support, this is one of those situations. I would like to see a styling option for refactoring that enabled code that looks like this:

class Program
{
    class FooBar 
    {
        readonly IServer _server;
        readonly IActor _actor;

        public FooBar(IServer server, IActor actor) 
        {
            _server = server;
            _actor = actor;
        }
    }

    interface IServer { }
    interface IActor { }
}

To be converted to this:

class Program
{
    class FooBar 
    {
        readonly IServer _server;
        readonly IActor _actor;

        public FooBar(IServer server, IActor actor) 
        {
            (_server, _actor) = (server, actor);
        }
    }

    interface IServer { }
    interface IActor { }
}

Taking it a step further, if the user has already defined that their styling preference is to prefer constructor expressions when possible -- the result of the refactoring would be as follows:

class Program
{
    class FooBar 
    {
        readonly IServer _server;
        readonly IActor _actor;

        public FooBar(IServer server, IActor actor) 
            => (_server, _actor) = (server, actor);
    }

    interface IServer { }
    interface IActor { }
}

These refactoring would need to work in the opposite manner too, going from tuple to separate statements. This proposal comes from a conversation with @BillWagner, @mikadumont and I were having in the twitter verse.

• From statements to tuple expression.
• From tuple to separate statements.

I'm assuming this should be labeled as "Area-IDE" and "IDE-CodeStyle"

[CyrusNajmabadi]

@IEvangelist i would be happy to walk you through contributing such a refactoring to the roslyn codebase.

[BillWagner]

@CyrusNajmabadi Awesome. I'd be happy to work with @IEvangelist on this.

[CyrusNajmabadi]

@IEvangelist @BillWagner Awesome! The first thing we need to decide on is "is this an analyzer or an refactoring". Analyzers are for cases where you think:

1. it's really important to get diagnostic messages when people don't use this pattern.
2. you want to be able to configure the severity of this sort of thing (i.e. making it an error to not use this pattern).
3. you may want to bulk update tons of these cases across the codebase.

Refactorings are for cases where this is more of a one-off type of operation a user might want to change in a particular code location. i.e. in some places this would be the preferred pattern, in others it would be find to just have a set of assignments.

IMO, this feels more like a refactoring. While i can see some arguments for this to be an analyzer, it feels a bit heavy-handed to be enforcing a style that mandated this in a codebase.

[CyrusNajmabadi]

So, first get up and running with roslyn: https://github.com/dotnet/roslyn/wiki/Building-Testing-and-Debugging

Once you've done this, open Roslyn.sln collapse everything in Solution Explorer and then find the 'Features' folder. In there there will be something like Microsoft.CodeAnalysis.Features.CSharp. We're going to add the new feature there because this is a C# only feature (i.e. VB doesn't have teh equivalent).

Once we're in there, it can be helpful to start with an existing refactoring, and copy it into a new location to help get an idea of how to build one of these guys.

Let's start with this one as it's fairly simple and can be a good starting point for your refactoring:

We'll copy that into a new sibling folder called something like "UseTupleAssignment". Update the namespace/classname to match this new name and then go to town.

I recommend just commenting out most of:

        public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context)

Then try to just get things building, and ensure you can hit 'f5' to debug roslyn-in-roslyn. This will then enable you to have a development loop where you can make changes, hit f5 to debug them, and rinse-repeat as you build at the refactoring.

Once you get to this point, you'll likely hit a wall trying to figure out all the roslyn APIs. Reach out again then and i can help you with this. :)

[IEvangelist]

Hi @CyrusNajmabadi,

I'll work on this tonight, thank you kind sir -- looking forward to a Roslyn contribution.

[IEvangelist]

Ok, @CyrusNajmabadi I've been spending a bit of time on this and you're right -- I hit the "roslyn API wall"... it's real. My break points are getting hit and I have it getting into my new method:

static async Task<Document> UpdateDocumentAsync(
            SyntaxNode root,
            Document document,
            ConstructorDeclarationSyntax constructor,
            CancellationToken cancellationToken)
{
    var generator = s_generator;
    var editor = new SyntaxEditor(root, generator);

    // Determine which .ctor parameters are assigning to class members
    var ctorAssignments =
        constructor.Body
                   .Statements
                   .Select(assignment =>
                   {
                       var simpleAssignment =
                           assignment.ChildNodesAndTokens()
                                     .FirstOrDefault(not => not.IsKind(SyntaxKind.SimpleAssignmentExpression));
                               return (assignment, simpleAssignment);
                   })
                   .ToList();

    foreach (var param in constructor.ParameterList.Parameters)
    {
        var correspondingAssignment =
            ctorAssignments.FirstOrDefault(ca =>
            {
                var (a, b) = ca;
                var node = b.AsNode();
                var token = b.AsToken();

                var isAssignment = a.Contains(param) || node.Contains(param);
                return isAssignment || token.ValueText == param.Identifier.ValueText;
            });
    }
}

Send help! Is there an API reference doc or example docs at all?

[CyrusNajmabadi]

@IEvangelist at this point i recommend a couple of things:

1. use the roslyn syntax visualizer. It's under View | Other Windows | Syntax Visualizer. This will help you get an understanding of the trees Roslyn has.
2. Read through the overview here to get an idea of what's going on: https://github.com/dotnet/roslyn/wiki/Roslyn-Overview
3. Join both/either https://gitter.im/dotnet/roslyn and/or the C# Discord server (#roslyn channel) and ask for help. I and others frequent there and can help you out :)

[BillWagner]

@IEvangelist I'd love to help. I realistically don't have time between now and .NET Conf. (Have I talked to you about C# 8.0 yet? 😄 )

After that week, I would be happy to dive in to help.

[IEvangelist]

@IEvangelist I'd love to help. I realistically don't have time between now and .NET Conf. (Have I talked to you about C# 8.0 yet? 😄 )

After that week, I would be happy to dive in to help.

Awesome, I will take you up on that. I have my working fork in place now with some of these changes, reach out to me when you're ready to dive in and we can collaborate -- I'd love that too!

[jinujoseph]

Design meeting Notes

We are not convinced that this code style will be easy for our users to read , but we have work going on C# 9.0 related to record types for scenarios where this will apply, that may be superior alternative and we want to focus on that instead. If you have questions on how to make your own analyzer, we are happy to help.