Order code actions

src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs

@@ -69,9 +69,6 @@ protected async Task<IEnumerable<AvailableCodeAction>> GetAvailableCodeActions(I
             await CollectCodeFixesActions(document, span, codeActions);
             await CollectRefactoringActions(document, span, codeActions);
 
-            // TODO: Determine good way to order code actions.
-            codeActions.Reverse();
-
             // Be sure to filter out any code actions that inherit from CodeActionWithOptions.
             // This isn't a great solution and might need changing later, but every Roslyn code action
             // derived from this type tries to display a dialog. For now, this is a reasonable solution.
@@ -128,26 +125,54 @@ private async Task CollectCodeFixesActions(Document document, TextSpan span, Lis
 
         private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerable<Diagnostic> diagnostics, List<CodeAction> codeActions)
         {
-            foreach (var provider in this.Providers)
+            IEnumerable<CodeFixProvider> sortedProviders = GetSortedCodeFixProviders();
+            foreach (var codeFixProvider in sortedProviders)
             {
-                foreach (var codeFixProvider in provider.CodeFixProviders)
+                var fixableDiagnostics = diagnostics.Where(d => HasFix(codeFixProvider, d.Id)).ToImmutableArray();
+                if (fixableDiagnostics.Length > 0)
                 {
-                    var fixableDiagnostics = diagnostics.Where(d => HasFix(codeFixProvider, d.Id)).ToImmutableArray();
-                    if (fixableDiagnostics.Length > 0)
+                    var context = new CodeFixContext(document, span, fixableDiagnostics, (a, _) => codeActions.Add(a), CancellationToken.None);
+
+                    try
                     {
-                        var context = new CodeFixContext(document, span, fixableDiagnostics, (a, _) => codeActions.Add(a), CancellationToken.None);
-
-                        try
-                        {
-                            await codeFixProvider.RegisterCodeFixesAsync(context);
-                        }
-                        catch (Exception ex)
-                        {
-                            this.Logger.LogError(ex, $"Error registering code fixes for {codeFixProvider.GetType().FullName}");
-                        }
+                        await codeFixProvider.RegisterCodeFixesAsync(context);
                     }
+                    catch (Exception ex)
+                    {
+                        this.Logger.LogError(ex, $"Error registering code fixes for {codeFixProvider.GetType().FullName}");
+                    }
+                }
+            }
+        }
+
+        private IEnumerable<CodeFixProvider> GetSortedCodeFixProviders()
+        {
+            List<ProviderNode<CodeFixProvider>> nodesList = new List<ProviderNode<CodeFixProvider>>();
+            foreach (var provider in this.Providers)
+            {
+                foreach (var codeFixProvider in provider.CodeFixProviders)
+                {
+                    nodesList.Add(ProviderNode<CodeFixProvider>.From(codeFixProvider));
                 }
             }
+
+            var graph = Graph<CodeFixProvider>.GetGraph(nodesList);
+            return graph.TopologicalSort();
+        }
+
+        private IEnumerable<CodeRefactoringProvider> GetSortedCodeRefactoringProviders()
+        {
+            List<ProviderNode<CodeRefactoringProvider>> nodesList = new List<ProviderNode<CodeRefactoringProvider>>();
+            foreach (var provider in this.Providers)
+            {
+                foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders)
+                {
+                    var node = ProviderNode<CodeRefactoringProvider>.From(codeRefactoringProvider);
+                }
+            }
+
+            var graph = Graph<CodeRefactoringProvider>.GetGraph(nodesList);
+            return graph.TopologicalSort();
         }
 
         private bool HasFix(CodeFixProvider codeFixProvider, string diagnosticId)
@@ -179,25 +204,23 @@ private bool HasFix(CodeFixProvider codeFixProvider, string diagnosticId)
 
         private async Task CollectRefactoringActions(Document document, TextSpan span, List<CodeAction> codeActions)
         {
-            foreach (var provider in this.Providers)
+            IEnumerable<CodeRefactoringProvider> sortedProviders = GetSortedCodeRefactoringProviders();
+            foreach (var codeRefactoringProvider in sortedProviders)
             {
-                foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders)
+                if (_helper.IsDisallowed(codeRefactoringProvider))
                 {
-                    if (_helper.IsDisallowed(codeRefactoringProvider))
-                    {
-                        continue;
-                    }
+                    continue;
+                }
 
-                    var context = new CodeRefactoringContext(document, span, a => codeActions.Add(a), CancellationToken.None);
+                var context = new CodeRefactoringContext(document, span, a => codeActions.Add(a), CancellationToken.None);
 
-                    try
-                    {
-                        await codeRefactoringProvider.ComputeRefactoringsAsync(context);
-                    }
-                    catch (Exception ex)
-                    {
-                        this.Logger.LogError(ex, $"Error computing refactorings for {codeRefactoringProvider.GetType().FullName}");
-                    }
+                try
+                {
+                    await codeRefactoringProvider.ComputeRefactoringsAsync(context);
+                }
+                catch (Exception ex)
+                {
+                    this.Logger.LogError(ex, $"Error computing refactorings for {codeRefactoringProvider.GetType().FullName}");
                 }
             }
         }

src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs

@@ -0,0 +1,75 @@
+using System;
+using System.Collections.Generic;
+
+namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2
+{
+    internal class Graph<T>
+    {
+        //Dictionary to map between nodes and the names
+        private Dictionary<string, ProviderNode<T>> Nodes { get; }
+        private List<ProviderNode<T>> AllNodes { get; }
+        private Graph(List<ProviderNode<T>> nodesList)
+        {
+            Nodes = new Dictionary<string, ProviderNode<T>>();
+            AllNodes = nodesList;
+        }
+        internal static Graph<T> GetGraph(List<ProviderNode<T>> nodesList)
+        {
+            var graph = new Graph<T>(nodesList);
+
+            foreach (ProviderNode<T> node in graph.AllNodes)
+            {
+                    graph.Nodes[node.ProviderName] = node;
+            }
+
+            foreach (ProviderNode<T> node in graph.AllNodes)
+            {
+                foreach (var before in node.Before)
+                {
+                    if (graph.Nodes.ContainsKey(before))
+                    {
+                        var beforeNode = graph.Nodes[before];
+                        beforeNode.NodesBeforeMeSet.Add(node);
+                    }
+                }
+
+                foreach (var after in node.After)
+                {
+                    if (graph.Nodes.ContainsKey(after))
+                    {
+                        var afterNode = graph.Nodes[after];
+                        node.NodesBeforeMeSet.Add(afterNode);
+                    }
+                }
+            }
+
+            return graph;
+        }
+
+        public List<T> TopologicalSort()
+        {
+            List<T> result = new List<T>();
+            var seenNodes = new HashSet<ProviderNode<T>>();
+
+            foreach (var node in AllNodes)
+            {
+                Visit(node, result, seenNodes);
+            }
+
+            return result;
+        }
+
+        private void Visit(ProviderNode<T> node, List<T> result, HashSet<ProviderNode<T>> seenNodes)
+        {
+            if (seenNodes.Add(node))
+            {
+                foreach (var before in node.NodesBeforeMeSet)
+                {
+                    Visit(before, result, seenNodes);
+                }
+
+                result.Add(node.Provider);
+            }
+        }
+    }
+}

src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs

@@ -0,0 +1,50 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CodeRefactorings;
+
+namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2
+{
+    internal class ProviderNode<T>
+    {
+        public string ProviderName { get; set; }
+        public List<string> Before { get; set; }
+        public List<string> After { get; set; }
+        public T Provider { get; set; }
+        public HashSet<ProviderNode<T>> NodesBeforeMeSet { get; set; }
+
+        public static ProviderNode<T> From(T provider)
+        {
+            var exportAttribute = provider.GetType().GetCustomAttribute(typeof(ExportCodeFixProviderAttribute));
+            string providerName = "";
+            if (exportAttribute is ExportCodeFixProviderAttribute && ((ExportCodeFixProviderAttribute)exportAttribute).Name != null)
+            {
+                providerName = ((ExportCodeFixProviderAttribute)exportAttribute).Name;
+            }
+
+            var orderAttributes = provider.GetType().GetCustomAttributes(typeof(ExtensionOrderAttribute), true).Select(attr => (ExtensionOrderAttribute)attr).ToList();
+            return new ProviderNode<T>(provider, providerName, orderAttributes);
+        }
+
+        private ProviderNode(T provider, string providerName, List<ExtensionOrderAttribute> orderAttributes)
+        {
+            Provider = provider;
+            ProviderName = providerName;
+            Before = new List<string>();
+            After = new List<string>();
+            NodesBeforeMeSet = new HashSet<ProviderNode<T>>();
+            orderAttributes.ForEach(attr => AddAttribute(attr));
+        }
+
+        private void AddAttribute(ExtensionOrderAttribute attribute)
+        {
+            if (attribute.Before != null)
+                Before.Add(attribute.Before);
+            if (attribute.After != null)
+                After.Add(attribute.After);
+        }
+    }
+}