From 4dabba9f4352b27051799a939bd0056558587fb5 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Mon, 1 Jan 2018 13:46:21 -0800 Subject: [PATCH 01/12] Test --- OmniSharp.sln | 4 +- .../Refactoring/V2/BaseCodeActionService.cs | 52 +++++++++---- .../Refactoring/V2/CodeActionsOrder.Graph.cs | 74 +++++++++++++++++++ .../V2/CodeActionsOrder.ProviderNode.cs | 64 ++++++++++++++++ 4 files changed, 178 insertions(+), 16 deletions(-) create mode 100644 src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs create mode 100644 src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs diff --git a/OmniSharp.sln b/OmniSharp.sln index d07f37632d..5e7a0bb124 100644 --- a/OmniSharp.sln +++ b/OmniSharp.sln @@ -1,6 +1,6 @@ Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 15 -VisualStudioVersion = 15.0.27004.2000 +VisualStudioVersion = 15.0.27130.2010 MinimumVisualStudioVersion = 10.0.40219.1 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{2C348365-A9D8-459E-9276-56FC46AAEE31}" EndProject @@ -382,7 +382,7 @@ Global {1217C534-E8EB-454D-B6E1-12BD30E72F8E} = {35E025BF-BBB2-4FAC-9F4B-37CBA083EE47} {31580626-6D87-491D-A65E-F6B881164398} = {2C348365-A9D8-459E-9276-56FC46AAEE31} {BA0713A6-EF63-419D-B60D-7A84B134531F} = {2C348365-A9D8-459E-9276-56FC46AAEE31} - {17DAECA3-AE28-4B40-AA52-1EF618346A85} = {35E025BF-BBB2-4FAC-9F4B-37CBA083EE47} + {17DAECA3-AE28-4B40-AA52-1EF618346A85} = {07464F68-2D8E-45E5-B30A-768FCF4CC903} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {4DD725CE-B49A-4151-8B77-BB33FE88E46E} diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index 3dc6718078..a325550440 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -128,28 +128,52 @@ private async Task CollectCodeFixesActions(Document document, TextSpan span, Lis private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerable diagnostics, List codeActions) { - foreach (var provider in this.Providers) + IEnumerable 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 + { + await codeFixProvider.RegisterCodeFixesAsync(context); + } + catch (Exception ex) { - 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}"); - } + this.Logger.LogError(ex, $"Error registering code fixes for {codeFixProvider.GetType().FullName}"); } } } } + private IEnumerable GetSortedCodeFixProviders() + { + List nodesList = new List(); + foreach (var provider in this.Providers) + { + foreach (var codeFixProvider in provider.CodeFixProviders) + { + nodesList.Add(ProviderNode.From(codeFixProvider)); + } + } + var graph = Graph.GetGraph(nodesList); + return graph.TopologicalSort(); + } + + private IEnumerable GetSortedCodeRefactoringProviders() + { + foreach (var provider in this.Providers) + { + foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders) + { + var node = ProviderNode.From(codeRefactoringProvider); + } + } + throw new NotImplementedException(); + } + private bool HasFix(CodeFixProvider codeFixProvider, string diagnosticId) { var typeName = codeFixProvider.GetType().FullName; diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs new file mode 100644 index 0000000000..05f92caa87 --- /dev/null +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs @@ -0,0 +1,74 @@ +using System; +using System.Collections.Generic; + +namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2 +{ + internal class Graph + { + private Dictionary Nodes; + private Graph() + { + Nodes = new Dictionary(); + } + internal static Graph GetGraph(List nodesList) + { + var graph = new Graph(); + + foreach (ProviderNode node in nodesList) + { + if (!graph.Nodes.ContainsKey(node.ProviderName)) + graph.Nodes[node.ProviderName] = node; + } + + foreach (ProviderNode node in nodesList) + { + 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 TopologicalSort() + { + List result = new List(); + var seenNodes = new HashSet(); + + foreach(var nodes in Nodes.Values) + { + foreach(var node in nodes) + Visit(node, result, seenNodes); + } + + return result; + } + + private void Visit(CodeActionNode node, List result, HashSet seenNodes) + { + if(seenNodes.Add(node)) + { + foreach (var before in node.NodesBeforeMeSet) + { + Visit(before, result, seenNodes); + } + result.Add(node.CodeAction); + } + } + + } +} diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs new file mode 100644 index 0000000000..85a3c72618 --- /dev/null +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs @@ -0,0 +1,64 @@ +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 + { + public string ProviderName { get; set; } + public List Before { get; set; } + public List After { get; set; } + public CodeFixProvider FixProvider { get; set; } + public CodeRefactoringProvider RefactoringProvider { get; set; } + public HashSet> NodesBeforeMeSet { get; set; } + + public static ProviderNode From(T provider) + { + var exportAttribute = provider.GetType().GetCustomAttribute(typeof(ExportCodeFixProviderAttribute)); + var providerName = exportAttribute is ExportCodeFixProviderAttribute ? ((ExportCodeFixProviderAttribute)exportAttribute).Name : ""; + var orderAttributes = provider.GetType().GetCustomAttributes(typeof(ExtensionOrderAttribute), true).Select(attr => (ExtensionOrderAttribute)attr).ToList(); + return new ProviderNode(provider, providerName, orderAttributes); + } + + public static ProviderNode From(CodeRefactoringProvider codeRefactoringProvider) + { + var exportAttribute = codeRefactoringProvider.GetType().GetCustomAttribute(typeof(ExportCodeFixProviderAttribute)); + var providerName = exportAttribute is ExportCodeFixProviderAttribute ? ((ExportCodeFixProviderAttribute)exportAttribute).Name : ""; + var orderAttributes = codeRefactoringProvider.GetType().GetCustomAttributes(typeof(ExtensionOrderAttribute), true).Select(attr => (ExtensionOrderAttribute)attr).ToList(); + return new ProviderNode(codeRefactoringProvider, providerName, orderAttributes); + } + + private ProviderNode(CodeFixProvider provider, string providerName, List orderAttributes) + { + FixProvider = provider; + ProviderName = providerName; + Before = new List(); + After = new List(); + NodesBeforeMeSet = new HashSet(); + orderAttributes.ForEach(attr => AddAttribute(attr)); + } + + private ProviderNode(CodeRefactoringProvider provider, string providerName, List orderAttributes) + { + RefactoringProvider = provider; + ProviderName = providerName; + Before = new List(); + After = new List(); + NodesBeforeMeSet = new HashSet(); + 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); + } + } +} From 653e4bed2fd30dc6f7020420bd62f805cbabac27 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Mon, 1 Jan 2018 14:03:15 -0800 Subject: [PATCH 02/12] Order CodeProviders using Generics --- .../Refactoring/V2/BaseCodeActionService.cs | 40 +++++++++---------- .../Refactoring/V2/CodeActionsOrder.Graph.cs | 32 +++++++-------- .../V2/CodeActionsOrder.ProviderNode.cs | 29 +++----------- 3 files changed, 40 insertions(+), 61 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index a325550440..25a42714be 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -150,28 +150,30 @@ private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerabl private IEnumerable GetSortedCodeFixProviders() { - List nodesList = new List(); + List> nodesList = new List>(); foreach (var provider in this.Providers) { foreach (var codeFixProvider in provider.CodeFixProviders) { - nodesList.Add(ProviderNode.From(codeFixProvider)); + nodesList.Add(ProviderNode.From(codeFixProvider)); } } - var graph = Graph.GetGraph(nodesList); + var graph = Graph.GetGraph(nodesList); return graph.TopologicalSort(); } private IEnumerable GetSortedCodeRefactoringProviders() { + List> nodesList = new List>(); foreach (var provider in this.Providers) { foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders) { - var node = ProviderNode.From(codeRefactoringProvider); + var node = ProviderNode.From(codeRefactoringProvider); } } - throw new NotImplementedException(); + var graph = Graph.GetGraph(nodesList); + return graph.TopologicalSort(); } private bool HasFix(CodeFixProvider codeFixProvider, string diagnosticId) @@ -203,25 +205,23 @@ private bool HasFix(CodeFixProvider codeFixProvider, string diagnosticId) private async Task CollectRefactoringActions(Document document, TextSpan span, List codeActions) { - foreach (var provider in this.Providers) + IEnumerable 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}"); } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs index 05f92caa87..a647a87f51 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs @@ -3,24 +3,24 @@ namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2 { - internal class Graph + internal class Graph { - private Dictionary Nodes; + private Dictionary> Nodes; private Graph() { - Nodes = new Dictionary(); + Nodes = new Dictionary>(); } - internal static Graph GetGraph(List nodesList) + internal static Graph GetGraph(List> nodesList) { - var graph = new Graph(); + var graph = new Graph(); - foreach (ProviderNode node in nodesList) + foreach (ProviderNode node in nodesList) { if (!graph.Nodes.ContainsKey(node.ProviderName)) graph.Nodes[node.ProviderName] = node; } - foreach (ProviderNode node in nodesList) + foreach (ProviderNode node in nodesList) { foreach (var before in node.Before) { @@ -44,31 +44,29 @@ internal static Graph GetGraph(List nodesList) return graph; } - public List TopologicalSort() + public List TopologicalSort() { - List result = new List(); - var seenNodes = new HashSet(); + List result = new List(); + var seenNodes = new HashSet>(); - foreach(var nodes in Nodes.Values) + foreach (var node in Nodes.Values) { - foreach(var node in nodes) - Visit(node, result, seenNodes); + Visit(node, result, seenNodes); } return result; } - private void Visit(CodeActionNode node, List result, HashSet seenNodes) + private void Visit(ProviderNode node, List result, HashSet> seenNodes) { - if(seenNodes.Add(node)) + if (seenNodes.Add(node)) { foreach (var before in node.NodesBeforeMeSet) { Visit(before, result, seenNodes); } - result.Add(node.CodeAction); + result.Add(node.Provider); } } - } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs index 85a3c72618..21d0222ef3 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs @@ -13,8 +13,7 @@ internal class ProviderNode public string ProviderName { get; set; } public List Before { get; set; } public List After { get; set; } - public CodeFixProvider FixProvider { get; set; } - public CodeRefactoringProvider RefactoringProvider { get; set; } + public T Provider { get; set; } public HashSet> NodesBeforeMeSet { get; set; } public static ProviderNode From(T provider) @@ -22,34 +21,16 @@ public static ProviderNode From(T provider) var exportAttribute = provider.GetType().GetCustomAttribute(typeof(ExportCodeFixProviderAttribute)); var providerName = exportAttribute is ExportCodeFixProviderAttribute ? ((ExportCodeFixProviderAttribute)exportAttribute).Name : ""; var orderAttributes = provider.GetType().GetCustomAttributes(typeof(ExtensionOrderAttribute), true).Select(attr => (ExtensionOrderAttribute)attr).ToList(); - return new ProviderNode(provider, providerName, orderAttributes); + return new ProviderNode(provider, providerName, orderAttributes); } - public static ProviderNode From(CodeRefactoringProvider codeRefactoringProvider) + private ProviderNode(T provider, string providerName, List orderAttributes) { - var exportAttribute = codeRefactoringProvider.GetType().GetCustomAttribute(typeof(ExportCodeFixProviderAttribute)); - var providerName = exportAttribute is ExportCodeFixProviderAttribute ? ((ExportCodeFixProviderAttribute)exportAttribute).Name : ""; - var orderAttributes = codeRefactoringProvider.GetType().GetCustomAttributes(typeof(ExtensionOrderAttribute), true).Select(attr => (ExtensionOrderAttribute)attr).ToList(); - return new ProviderNode(codeRefactoringProvider, providerName, orderAttributes); - } - - private ProviderNode(CodeFixProvider provider, string providerName, List orderAttributes) - { - FixProvider = provider; - ProviderName = providerName; - Before = new List(); - After = new List(); - NodesBeforeMeSet = new HashSet(); - orderAttributes.ForEach(attr => AddAttribute(attr)); - } - - private ProviderNode(CodeRefactoringProvider provider, string providerName, List orderAttributes) - { - RefactoringProvider = provider; + Provider = provider; ProviderName = providerName; Before = new List(); After = new List(); - NodesBeforeMeSet = new HashSet(); + NodesBeforeMeSet = new HashSet>(); orderAttributes.ForEach(attr => AddAttribute(attr)); } From 82c3a2ca376d85df37e25120433a084e62ed248c Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Mon, 1 Jan 2018 18:54:14 -0800 Subject: [PATCH 03/12] Changes to incorporate providers with no name attribute --- .../Refactoring/V2/BaseCodeActionService.cs | 3 --- .../Refactoring/V2/CodeActionsOrder.Graph.cs | 16 +++++++++------- .../V2/CodeActionsOrder.ProviderNode.cs | 7 ++++++- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index 25a42714be..f6dbc235ec 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -69,9 +69,6 @@ protected async Task> 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. diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs index a647a87f51..d4ceef1b0c 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs @@ -5,22 +5,24 @@ namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2 { internal class Graph { - private Dictionary> Nodes; - private Graph() + //Dictionary to map between nodes and the names + private Dictionary> Nodes { get; } + private List> AllNodes { get; } + private Graph(List> nodesList) { Nodes = new Dictionary>(); + AllNodes = nodesList; } internal static Graph GetGraph(List> nodesList) { - var graph = new Graph(); + var graph = new Graph(nodesList); - foreach (ProviderNode node in nodesList) + foreach (ProviderNode node in graph.AllNodes) { - if (!graph.Nodes.ContainsKey(node.ProviderName)) graph.Nodes[node.ProviderName] = node; } - foreach (ProviderNode node in nodesList) + foreach (ProviderNode node in graph.AllNodes) { foreach (var before in node.Before) { @@ -49,7 +51,7 @@ public List TopologicalSort() List result = new List(); var seenNodes = new HashSet>(); - foreach (var node in Nodes.Values) + foreach (var node in AllNodes) { Visit(node, result, seenNodes); } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs index 21d0222ef3..5ed2023f45 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs @@ -19,7 +19,12 @@ internal class ProviderNode public static ProviderNode From(T provider) { var exportAttribute = provider.GetType().GetCustomAttribute(typeof(ExportCodeFixProviderAttribute)); - var providerName = exportAttribute is ExportCodeFixProviderAttribute ? ((ExportCodeFixProviderAttribute)exportAttribute).Name : ""; + var providerName = ""; + if(exportAttribute != null) + { + providerName = ((ExportCodeFixProviderAttribute)exportAttribute).Name != null ? ((ExportCodeFixProviderAttribute)exportAttribute).Name :""; + } + var orderAttributes = provider.GetType().GetCustomAttributes(typeof(ExtensionOrderAttribute), true).Select(attr => (ExtensionOrderAttribute)attr).ToList(); return new ProviderNode(provider, providerName, orderAttributes); } From 14b14996510528f6b5da60765ceb6a74fcd978e8 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Tue, 2 Jan 2018 11:52:48 -0800 Subject: [PATCH 04/12] Changes for cases where the provider doesn't have a name --- OmniSharp.sln | 4 ++-- .../Services/Refactoring/V2/BaseCodeActionService.cs | 2 ++ .../Services/Refactoring/V2/CodeActionsOrder.Graph.cs | 1 + .../Refactoring/V2/CodeActionsOrder.ProviderNode.cs | 8 ++++---- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/OmniSharp.sln b/OmniSharp.sln index 5e7a0bb124..d07f37632d 100644 --- a/OmniSharp.sln +++ b/OmniSharp.sln @@ -1,6 +1,6 @@ Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 15 -VisualStudioVersion = 15.0.27130.2010 +VisualStudioVersion = 15.0.27004.2000 MinimumVisualStudioVersion = 10.0.40219.1 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{2C348365-A9D8-459E-9276-56FC46AAEE31}" EndProject @@ -382,7 +382,7 @@ Global {1217C534-E8EB-454D-B6E1-12BD30E72F8E} = {35E025BF-BBB2-4FAC-9F4B-37CBA083EE47} {31580626-6D87-491D-A65E-F6B881164398} = {2C348365-A9D8-459E-9276-56FC46AAEE31} {BA0713A6-EF63-419D-B60D-7A84B134531F} = {2C348365-A9D8-459E-9276-56FC46AAEE31} - {17DAECA3-AE28-4B40-AA52-1EF618346A85} = {07464F68-2D8E-45E5-B30A-768FCF4CC903} + {17DAECA3-AE28-4B40-AA52-1EF618346A85} = {35E025BF-BBB2-4FAC-9F4B-37CBA083EE47} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {4DD725CE-B49A-4151-8B77-BB33FE88E46E} diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index f6dbc235ec..ae585687d9 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -155,6 +155,7 @@ private IEnumerable GetSortedCodeFixProviders() nodesList.Add(ProviderNode.From(codeFixProvider)); } } + var graph = Graph.GetGraph(nodesList); return graph.TopologicalSort(); } @@ -169,6 +170,7 @@ private IEnumerable GetSortedCodeRefactoringProviders() var node = ProviderNode.From(codeRefactoringProvider); } } + var graph = Graph.GetGraph(nodesList); return graph.TopologicalSort(); } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs index d4ceef1b0c..8d07d67f88 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs @@ -67,6 +67,7 @@ private void Visit(ProviderNode node, List result, HashSet { Visit(before, result, seenNodes); } + result.Add(node.Provider); } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs index 5ed2023f45..fbebbcf838 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs @@ -19,10 +19,10 @@ internal class ProviderNode public static ProviderNode From(T provider) { var exportAttribute = provider.GetType().GetCustomAttribute(typeof(ExportCodeFixProviderAttribute)); - var providerName = ""; - if(exportAttribute != null) + string providerName = ""; + if (exportAttribute is ExportCodeFixProviderAttribute && ((ExportCodeFixProviderAttribute)exportAttribute).Name != null) { - providerName = ((ExportCodeFixProviderAttribute)exportAttribute).Name != null ? ((ExportCodeFixProviderAttribute)exportAttribute).Name :""; + providerName = ((ExportCodeFixProviderAttribute)exportAttribute).Name; } var orderAttributes = provider.GetType().GetCustomAttributes(typeof(ExtensionOrderAttribute), true).Select(attr => (ExtensionOrderAttribute)attr).ToList(); @@ -41,7 +41,7 @@ private ProviderNode(T provider, string providerName, List Date: Tue, 2 Jan 2018 14:30:21 -0800 Subject: [PATCH 05/12] Code to check for cycles in providers --- .../Refactoring/V2/BaseCodeActionService.cs | 51 ++++++++++++++++--- .../Refactoring/V2/CodeActionsOrder.Graph.cs | 10 +++- .../V2/CodeActionsOrder.ProviderNode.cs | 21 ++++++++ 3 files changed, 75 insertions(+), 7 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index ae585687d9..07de0ab29e 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -125,8 +125,24 @@ private async Task CollectCodeFixesActions(Document document, TextSpan span, Lis private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerable diagnostics, List codeActions) { - IEnumerable sortedProviders = GetSortedCodeFixProviders(); - foreach (var codeFixProvider in sortedProviders) + List orderedProviders; + try + { + orderedProviders = GetSortedCodeFixProviders(); + } + catch + { + orderedProviders = new List(); + foreach (var provider in this.Providers) + { + foreach (var codeFixProvider in provider.CodeFixProviders) + { + orderedProviders.Add(codeFixProvider); + } + } + } + + foreach (var codeFixProvider in orderedProviders) { var fixableDiagnostics = diagnostics.Where(d => HasFix(codeFixProvider, d.Id)).ToImmutableArray(); if (fixableDiagnostics.Length > 0) @@ -145,33 +161,39 @@ private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerabl } } - private IEnumerable GetSortedCodeFixProviders() + private List GetSortedCodeFixProviders() { List> nodesList = new List>(); + List providerList = new List(); foreach (var provider in this.Providers) { foreach (var codeFixProvider in provider.CodeFixProviders) { + providerList.Add(codeFixProvider); nodesList.Add(ProviderNode.From(codeFixProvider)); } } var graph = Graph.GetGraph(nodesList); + graph.CheckForCycles(); return graph.TopologicalSort(); } - private IEnumerable GetSortedCodeRefactoringProviders() + private List GetSortedCodeRefactoringProviders() { List> nodesList = new List>(); + List providerList = new List(); foreach (var provider in this.Providers) { foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders) { + providerList.Add(codeRefactoringProvider); var node = ProviderNode.From(codeRefactoringProvider); } } var graph = Graph.GetGraph(nodesList); + graph.CheckForCycles(); return graph.TopologicalSort(); } @@ -204,8 +226,25 @@ private bool HasFix(CodeFixProvider codeFixProvider, string diagnosticId) private async Task CollectRefactoringActions(Document document, TextSpan span, List codeActions) { - IEnumerable sortedProviders = GetSortedCodeRefactoringProviders(); - foreach (var codeRefactoringProvider in sortedProviders) + List orderedProviders; + try + { + orderedProviders = GetSortedCodeRefactoringProviders(); + } + catch + { + //If we cannot get a sortedlist then just proceed with the available list + orderedProviders = new List(); + foreach (var provider in this.Providers) + { + foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders) + { + orderedProviders.Add(codeRefactoringProvider); + } + } + } + + foreach (var codeRefactoringProvider in orderedProviders) { if (_helper.IsDisallowed(codeRefactoringProvider)) { diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs index 8d07d67f88..0fdfa21d80 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs @@ -19,7 +19,7 @@ internal static Graph GetGraph(List> nodesList) foreach (ProviderNode node in graph.AllNodes) { - graph.Nodes[node.ProviderName] = node; + graph.Nodes[node.ProviderName] = node; } foreach (ProviderNode node in graph.AllNodes) @@ -46,6 +46,14 @@ internal static Graph GetGraph(List> nodesList) return graph; } + public void CheckForCycles() + { + foreach (var node in this.AllNodes) + { + node.CheckForCycles(); + } + } + public List TopologicalSort() { List result = new List(); diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs index fbebbcf838..c0af437d70 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs @@ -46,5 +46,26 @@ private void AddAttribute(ExtensionOrderAttribute attribute) if (attribute.After != null) After.Add(attribute.After); } + + internal void CheckForCycles() + { + CheckForCycles(new HashSet>()); + } + + private void CheckForCycles(HashSet> seenNodes) + { + if (!seenNodes.Add(this)) + { + //Cycle detected + throw new ArgumentException("Cycle detected"); + } + + foreach (var before in this.NodesBeforeMeSet) + { + before.CheckForCycles(seenNodes); + } + + seenNodes.Remove(this); + } } } From 2e2b5c70890ef41f314372234f79763163d7c814 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Tue, 2 Jan 2018 15:46:07 -0800 Subject: [PATCH 06/12] Created fields for storing the orderedProviders --- .../Refactoring/V2/BaseCodeActionService.cs | 82 +++++++++---------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index 07de0ab29e..b7cdf4b5c8 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -23,6 +23,8 @@ public abstract class BaseCodeActionService : IRequestHandl { protected readonly OmniSharpWorkspace Workspace; protected readonly IEnumerable Providers; + protected List OrderedCodeFixProviders; + protected List OrderedCodeRefactoringProviders; protected readonly ILogger Logger; private readonly CodeActionHelper _helper; @@ -37,6 +39,8 @@ protected BaseCodeActionService(OmniSharpWorkspace workspace, CodeActionHelper h this.Logger = logger; this._helper = helper; + SetOrderedProviders(); + // Sadly, the CodeAction.NestedCodeActions property is still internal. var nestedCodeActionsProperty = typeof(CodeAction).GetProperty("NestedCodeActions", BindingFlags.NonPublic | BindingFlags.Instance); if (nestedCodeActionsProperty == null) @@ -51,6 +55,41 @@ protected BaseCodeActionService(OmniSharpWorkspace workspace, CodeActionHelper h } } + private void SetOrderedProviders() + { + try + { + OrderedCodeFixProviders = GetSortedCodeFixProviders(); + } + catch + { + OrderedCodeFixProviders = new List(); + foreach (var provider in this.Providers) + { + foreach (var codeFixProvider in provider.CodeFixProviders) + { + OrderedCodeFixProviders.Add(codeFixProvider); + } + } + } + + try + { + OrderedCodeRefactoringProviders = GetSortedCodeRefactoringProviders(); + } + catch + { + OrderedCodeRefactoringProviders = new List(); + foreach (var provider in this.Providers) + { + foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders) + { + OrderedCodeRefactoringProviders.Add(codeRefactoringProvider); + } + } + } + } + public abstract Task Handle(TRequest request); protected async Task> GetAvailableCodeActions(ICodeActionRequest request) @@ -125,24 +164,7 @@ private async Task CollectCodeFixesActions(Document document, TextSpan span, Lis private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerable diagnostics, List codeActions) { - List orderedProviders; - try - { - orderedProviders = GetSortedCodeFixProviders(); - } - catch - { - orderedProviders = new List(); - foreach (var provider in this.Providers) - { - foreach (var codeFixProvider in provider.CodeFixProviders) - { - orderedProviders.Add(codeFixProvider); - } - } - } - - foreach (var codeFixProvider in orderedProviders) + foreach (var codeFixProvider in OrderedCodeFixProviders) { var fixableDiagnostics = diagnostics.Where(d => HasFix(codeFixProvider, d.Id)).ToImmutableArray(); if (fixableDiagnostics.Length > 0) @@ -164,12 +186,10 @@ private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerabl private List GetSortedCodeFixProviders() { List> nodesList = new List>(); - List providerList = new List(); foreach (var provider in this.Providers) { foreach (var codeFixProvider in provider.CodeFixProviders) { - providerList.Add(codeFixProvider); nodesList.Add(ProviderNode.From(codeFixProvider)); } } @@ -182,12 +202,10 @@ private List GetSortedCodeFixProviders() private List GetSortedCodeRefactoringProviders() { List> nodesList = new List>(); - List providerList = new List(); foreach (var provider in this.Providers) { foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders) { - providerList.Add(codeRefactoringProvider); var node = ProviderNode.From(codeRefactoringProvider); } } @@ -226,25 +244,7 @@ private bool HasFix(CodeFixProvider codeFixProvider, string diagnosticId) private async Task CollectRefactoringActions(Document document, TextSpan span, List codeActions) { - List orderedProviders; - try - { - orderedProviders = GetSortedCodeRefactoringProviders(); - } - catch - { - //If we cannot get a sortedlist then just proceed with the available list - orderedProviders = new List(); - foreach (var provider in this.Providers) - { - foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders) - { - orderedProviders.Add(codeRefactoringProvider); - } - } - } - - foreach (var codeRefactoringProvider in orderedProviders) + foreach (var codeRefactoringProvider in OrderedCodeRefactoringProviders) { if (_helper.IsDisallowed(codeRefactoringProvider)) { From b8eefcf71d4e5abbe4626841e47cfa3e7d0c6c2b Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Tue, 2 Jan 2018 15:48:44 -0800 Subject: [PATCH 07/12] Added comment for adaptation from roslyn --- .../Services/Refactoring/V2/CodeActionsOrder.Graph.cs | 2 +- .../Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs index 0fdfa21d80..16df0fbc45 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs @@ -1,4 +1,4 @@ -using System; +// Adapted from ExtensionOrderer in Roslyn using System.Collections.Generic; namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2 diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs index c0af437d70..95ec8c8586 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs @@ -1,10 +1,10 @@ -using System; +// Adapted from ExtensionOrderer in Roslyn +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 { From 751051c64b4b344df974743c19c44bffed5e5fb4 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Tue, 2 Jan 2018 17:03:51 -0800 Subject: [PATCH 08/12] Tests passing --- .../Services/Refactoring/V2/BaseCodeActionService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index b7cdf4b5c8..695e28afbd 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -206,7 +206,7 @@ private List GetSortedCodeRefactoringProviders() { foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders) { - var node = ProviderNode.From(codeRefactoringProvider); + nodesList.Add(ProviderNode.From(codeRefactoringProvider)); } } From 20d35afd97736d3dc814c1ff150446fc21c4372e Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Tue, 2 Jan 2018 17:25:28 -0800 Subject: [PATCH 09/12] Added test for ordered code action --- .../CodeActionsV2Facts.cs | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsV2Facts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsV2Facts.cs index 63c09cea6c..e7073a43a8 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsV2Facts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsV2Facts.cs @@ -99,6 +99,35 @@ public void Whatever() Assert.Contains("Extract Method", refactorings); } + [Fact] + public async Task Returns_ordered_code_actions() + { + const string code = + @"public class Class1 + { + public void Whatever() + { + [|Console.Write(""should be using System;"");|] + } + }"; + + var refactorings = await FindRefactoringNamesAsync(code); + List expected = new List + { + "using System;", + "System.Console", + "Generate variable 'Console' -> Generate property 'Class1.Console'", + "Generate variable 'Console' -> Generate field 'Class1.Console'", + "Generate variable 'Console' -> Generate read-only field 'Class1.Console'", + "Generate variable 'Console' -> Generate local 'Console'", + "Generate type 'Console' -> Generate class 'Console' in new file", + "Generate type 'Console' -> Generate class 'Console'", + "Generate type 'Console' -> Generate nested class 'Console'", + "Extract Method" + }; + Assert.Equal(expected, refactorings); + } + [Fact] public async Task Can_extract_method() { @@ -190,7 +219,7 @@ private async Task> FindRefactoringsAsync(strin { var testFile = new TestFile(BufferPath, code); - using (var host = CreateOmniSharpHost(new [] { testFile }, configurationData)) + using (var host = CreateOmniSharpHost(new[] { testFile }, configurationData)) { var requestHandler = host.GetRequestHandler(OmniSharpEndpoints.V2.GetCodeActions); From 0467d17d1d915d109bc5b1279915ecbc82fad85c Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Wed, 3 Jan 2018 16:51:00 -0800 Subject: [PATCH 10/12] Removed exception --- .../Refactoring/V2/BaseCodeActionService.cs | 57 ++++++------------- .../Refactoring/V2/CodeActionsOrder.Graph.cs | 6 +- .../V2/CodeActionsOrder.ProviderNode.cs | 12 ++-- 3 files changed, 28 insertions(+), 47 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index 695e28afbd..f1cf2e3045 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -23,8 +23,6 @@ public abstract class BaseCodeActionService : IRequestHandl { protected readonly OmniSharpWorkspace Workspace; protected readonly IEnumerable Providers; - protected List OrderedCodeFixProviders; - protected List OrderedCodeRefactoringProviders; protected readonly ILogger Logger; private readonly CodeActionHelper _helper; @@ -32,6 +30,9 @@ public abstract class BaseCodeActionService : IRequestHandl private static readonly Func> s_createDiagnosticList = _ => new List(); + protected List OrderedCodeFixProviders; + protected List OrderedCodeRefactoringProviders; + protected BaseCodeActionService(OmniSharpWorkspace workspace, CodeActionHelper helper, IEnumerable providers, ILogger logger) { this.Workspace = workspace; @@ -39,7 +40,8 @@ protected BaseCodeActionService(OmniSharpWorkspace workspace, CodeActionHelper h this.Logger = logger; this._helper = helper; - SetOrderedProviders(); + OrderedCodeFixProviders = GetSortedCodeFixProviders(); + OrderedCodeRefactoringProviders = GetSortedCodeRefactoringProviders(); // Sadly, the CodeAction.NestedCodeActions property is still internal. var nestedCodeActionsProperty = typeof(CodeAction).GetProperty("NestedCodeActions", BindingFlags.NonPublic | BindingFlags.Instance); @@ -55,41 +57,6 @@ protected BaseCodeActionService(OmniSharpWorkspace workspace, CodeActionHelper h } } - private void SetOrderedProviders() - { - try - { - OrderedCodeFixProviders = GetSortedCodeFixProviders(); - } - catch - { - OrderedCodeFixProviders = new List(); - foreach (var provider in this.Providers) - { - foreach (var codeFixProvider in provider.CodeFixProviders) - { - OrderedCodeFixProviders.Add(codeFixProvider); - } - } - } - - try - { - OrderedCodeRefactoringProviders = GetSortedCodeRefactoringProviders(); - } - catch - { - OrderedCodeRefactoringProviders = new List(); - foreach (var provider in this.Providers) - { - foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders) - { - OrderedCodeRefactoringProviders.Add(codeRefactoringProvider); - } - } - } - } - public abstract Task Handle(TRequest request); protected async Task> GetAvailableCodeActions(ICodeActionRequest request) @@ -186,32 +153,42 @@ private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerabl private List GetSortedCodeFixProviders() { List> nodesList = new List>(); + List providerList = new List(); foreach (var provider in this.Providers) { foreach (var codeFixProvider in provider.CodeFixProviders) { + providerList.Add(codeFixProvider); nodesList.Add(ProviderNode.From(codeFixProvider)); } } var graph = Graph.GetGraph(nodesList); - graph.CheckForCycles(); + if(graph.HasCycles()) + { + return providerList; + } return graph.TopologicalSort(); } private List GetSortedCodeRefactoringProviders() { List> nodesList = new List>(); + List providerList = new List(); foreach (var provider in this.Providers) { foreach (var codeRefactoringProvider in provider.CodeRefactoringProviders) { + providerList.Add(codeRefactoringProvider); nodesList.Add(ProviderNode.From(codeRefactoringProvider)); } } var graph = Graph.GetGraph(nodesList); - graph.CheckForCycles(); + if (graph.HasCycles()) + { + return providerList; + } return graph.TopologicalSort(); } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs index 16df0fbc45..79c8a007c5 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.Graph.cs @@ -46,12 +46,14 @@ internal static Graph GetGraph(List> nodesList) return graph; } - public void CheckForCycles() + public bool HasCycles() { foreach (var node in this.AllNodes) { - node.CheckForCycles(); + if (node.CheckForCycles()) + return true; } + return false; } public List TopologicalSort() diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs index 95ec8c8586..3263d893a5 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs @@ -47,25 +47,27 @@ private void AddAttribute(ExtensionOrderAttribute attribute) After.Add(attribute.After); } - internal void CheckForCycles() + internal bool CheckForCycles() { - CheckForCycles(new HashSet>()); + return CheckForCycles(new HashSet>()); } - private void CheckForCycles(HashSet> seenNodes) + private bool CheckForCycles(HashSet> seenNodes) { if (!seenNodes.Add(this)) { //Cycle detected - throw new ArgumentException("Cycle detected"); + return true; } foreach (var before in this.NodesBeforeMeSet) { - before.CheckForCycles(seenNodes); + if (before.CheckForCycles(seenNodes)) + return true; } seenNodes.Remove(this); + return false; } } } From 81b0935ed1ee53fc5ff379e89b57a788ffeda586 Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Thu, 4 Jan 2018 10:08:27 -0800 Subject: [PATCH 11/12] Implemented list using Lazy --- .../Refactoring/V2/BaseCodeActionService.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index f1cf2e3045..1945ee3d42 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -30,8 +30,8 @@ public abstract class BaseCodeActionService : IRequestHandl private static readonly Func> s_createDiagnosticList = _ => new List(); - protected List OrderedCodeFixProviders; - protected List OrderedCodeRefactoringProviders; + protected Lazy> OrderedCodeFixProviders; + protected Lazy> OrderedCodeRefactoringProviders; protected BaseCodeActionService(OmniSharpWorkspace workspace, CodeActionHelper helper, IEnumerable providers, ILogger logger) { @@ -40,8 +40,8 @@ protected BaseCodeActionService(OmniSharpWorkspace workspace, CodeActionHelper h this.Logger = logger; this._helper = helper; - OrderedCodeFixProviders = GetSortedCodeFixProviders(); - OrderedCodeRefactoringProviders = GetSortedCodeRefactoringProviders(); + OrderedCodeFixProviders = new Lazy>(() => GetSortedCodeFixProviders()); + OrderedCodeRefactoringProviders = new Lazy>(() => GetSortedCodeRefactoringProviders()); // Sadly, the CodeAction.NestedCodeActions property is still internal. var nestedCodeActionsProperty = typeof(CodeAction).GetProperty("NestedCodeActions", BindingFlags.NonPublic | BindingFlags.Instance); @@ -131,7 +131,7 @@ private async Task CollectCodeFixesActions(Document document, TextSpan span, Lis private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerable diagnostics, List codeActions) { - foreach (var codeFixProvider in OrderedCodeFixProviders) + foreach (var codeFixProvider in OrderedCodeFixProviders.Value) { var fixableDiagnostics = diagnostics.Where(d => HasFix(codeFixProvider, d.Id)).ToImmutableArray(); if (fixableDiagnostics.Length > 0) @@ -164,7 +164,7 @@ private List GetSortedCodeFixProviders() } var graph = Graph.GetGraph(nodesList); - if(graph.HasCycles()) + if (graph.HasCycles()) { return providerList; } @@ -221,7 +221,7 @@ private bool HasFix(CodeFixProvider codeFixProvider, string diagnosticId) private async Task CollectRefactoringActions(Document document, TextSpan span, List codeActions) { - foreach (var codeRefactoringProvider in OrderedCodeRefactoringProviders) + foreach (var codeRefactoringProvider in OrderedCodeRefactoringProviders.Value) { if (_helper.IsDisallowed(codeRefactoringProvider)) { From 9a7fb14516f7064269c1f99b8e1d5ac6499d627e Mon Sep 17 00:00:00 2001 From: Akshita Agarwal Date: Mon, 8 Jan 2018 17:00:19 -0800 Subject: [PATCH 12/12] Added code for exportCodeRefactoring attribute --- .../V2/CodeActionsOrder.ProviderNode.cs | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs index 3263d893a5..92be73ac8a 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionsOrder.ProviderNode.cs @@ -5,37 +5,49 @@ using System.Reflection; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CodeRefactorings; namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2 { - internal class ProviderNode + internal class ProviderNode { public string ProviderName { get; set; } public List Before { get; set; } public List After { get; set; } - public T Provider { get; set; } - public HashSet> NodesBeforeMeSet { get; set; } + public TProvider Provider { get; set; } + public HashSet> NodesBeforeMeSet { get; set; } - public static ProviderNode From(T provider) + public static ProviderNode From(TProvider provider) { - var exportAttribute = provider.GetType().GetCustomAttribute(typeof(ExportCodeFixProviderAttribute)); string providerName = ""; - if (exportAttribute is ExportCodeFixProviderAttribute && ((ExportCodeFixProviderAttribute)exportAttribute).Name != null) + if (provider is CodeFixProvider) { - providerName = ((ExportCodeFixProviderAttribute)exportAttribute).Name; + var exportAttribute = provider.GetType().GetCustomAttribute(typeof(ExportCodeFixProviderAttribute)); + if (exportAttribute is ExportCodeFixProviderAttribute fixAttribute && fixAttribute.Name != null) + { + providerName = fixAttribute.Name; + } + } + else + { + var exportAttribute = provider.GetType().GetCustomAttribute(typeof(ExportCodeRefactoringProviderAttribute)); + if (exportAttribute is ExportCodeRefactoringProviderAttribute refactoringAttribute && refactoringAttribute.Name != null) + { + providerName = refactoringAttribute.Name; + } } var orderAttributes = provider.GetType().GetCustomAttributes(typeof(ExtensionOrderAttribute), true).Select(attr => (ExtensionOrderAttribute)attr).ToList(); - return new ProviderNode(provider, providerName, orderAttributes); + return new ProviderNode(provider, providerName, orderAttributes); } - private ProviderNode(T provider, string providerName, List orderAttributes) + private ProviderNode(TProvider provider, string providerName, List orderAttributes) { Provider = provider; ProviderName = providerName; Before = new List(); After = new List(); - NodesBeforeMeSet = new HashSet>(); + NodesBeforeMeSet = new HashSet>(); orderAttributes.ForEach(attr => AddAttribute(attr)); } @@ -49,10 +61,10 @@ private void AddAttribute(ExtensionOrderAttribute attribute) internal bool CheckForCycles() { - return CheckForCycles(new HashSet>()); + return CheckForCycles(new HashSet>()); } - private bool CheckForCycles(HashSet> seenNodes) + private bool CheckForCycles(HashSet> seenNodes) { if (!seenNodes.Add(this)) {