Roslyn: "Implement abstract class" reorders members

[OndrejPetrzilka]

Version Used:
Visual Studio 15, Preview 5

Steps to Reproduce:

1. Write abstract class with two abstract methods
2. Name first method 'XXX'
3. Name second method 'AAA'
4. Write class which inherits abstract class
5. Use 'Quick Actions': 'Implement abstract class'

Expected Behavior:
Member order would be preserved

    abstract class BaseClass
    {
        public abstract void XXX();
        public abstract void AAA();
    }

    class Implementation : BaseClass
    {
        public override void XXX()
        {
            throw new NotImplementedException();
        }

        public override void AAA()
        {
            throw new NotImplementedException();
        }
    }

Actual Behavior:
Members are sorted by name

    abstract class BaseClass
    {
        public abstract void XXX();
        public abstract void AAA();
    }

    class Implementation : BaseClass
    {
        public override void AAA()
        {
            throw new NotImplementedException();
        }

        public override void XXX()
        {
            throw new NotImplementedException();
        }
    }

Justification:
If I wanted members sorted by name, I would have done it in the base class. Member sorting should be optional (e.g. formatting options in TextEditor/C#), imho disabled by default. When somebody orders members somehow, it's usually for a reason. For our organization, it's typical to write Dispose method as first under constructors. Similarly methods like Initialize and Terminate are next to each other.
Reason for this is to 'see' both methods at the same time, which is convenient when making changes to the code. It's similar for abstract interface implementations.

[0xd4d]

Related to #4548

I agree. It's so bad I've even stopped using that quick action and just do it manually.

MS: Name one programmer who sorts all his/her methods and properties in alphabetical order.

[CyrusNajmabadi]

Seems reasonable to expose an option to disable this behavior.

Note: the goal is not to keep members alphabetized by default. The goal is to keep things grouped by default. So we'll generate properties into your existing group of properties. Constructors into your existing group of constructors. etc. etc. If your code happens to be already have a group sorted by name, then we simply attempt to preserve that.

So, if you have a normal class, and you add a new interface/base type, and choose the code-action, then we'll preserve the grouping that you already have indicated. If you don't have grouping/ordering, then we won't attempt to do this.

The problem you're seeing here occurs because the class you're starting in is empty. In this case we see no members, and our logic goes "well your zero members are sorted, so we'll preserve that as we add new stuff". This is pretty bogus reasoning as it's using the absence of information to infer something about how you want your code organized. We should only use the presence of members to infer anything. And even then we likely should rarely ever infer sorting alphabetically. The only name inference that seems reasonable is to keep same named items together. That at least fits the model where people keep overloads together.

I'll see if i can get this fixed for RTM.

[CyrusNajmabadi]

Ok. I've got a preliminary PR that i'm trying out to help fix this. :) I hope it will make it so you can use the feature again :)