From cad9e200b2ea5e9a8354217210760796490f7305 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Sun, 5 Feb 2017 14:12:53 -0800 Subject: [PATCH 1/3] Offer nested code actions --- .../Extensions/ReflectionExtensions.cs | 20 +++--- .../Refactoring/V2/BaseCodeActionService.cs | 70 ++++++++++++++++--- .../Refactoring/V2/CodeActionAndParent.cs | 29 ++++++++ .../Refactoring/V2/GetCodeActionsService.cs | 7 +- .../Refactoring/V2/RunCodeActionService.cs | 10 +-- 5 files changed, 111 insertions(+), 25 deletions(-) create mode 100644 src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionAndParent.cs diff --git a/src/OmniSharp.Abstractions/Extensions/ReflectionExtensions.cs b/src/OmniSharp.Abstractions/Extensions/ReflectionExtensions.cs index 64228a3e34..19bcad3c12 100644 --- a/src/OmniSharp.Abstractions/Extensions/ReflectionExtensions.cs +++ b/src/OmniSharp.Abstractions/Extensions/ReflectionExtensions.cs @@ -34,11 +34,12 @@ public static Lazy LazyGetMethod(this Lazy lazyType, string me return new Lazy(() => { - var methodInfo = lazyType.Value.GetMethod(methodName); + var type = lazyType.Value; + var methodInfo = type.GetMethod(methodName); if (methodInfo == null) { - throw new InvalidOperationException($"Could not get type '{methodName}'"); + throw new InvalidOperationException($"Could not get method '{methodName}' on type '{type.FullName}'"); } return methodInfo; @@ -54,11 +55,12 @@ public static Lazy LazyGetMethod(this Lazy lazyType, string me return new Lazy(() => { - var methodInfo = lazyType.Value.GetMethod(methodName, bindingFlags); + var type = lazyType.Value; + var methodInfo = type.GetMethod(methodName, bindingFlags); if (methodInfo == null) { - throw new InvalidOperationException($"Could not get type '{methodName}'"); + throw new InvalidOperationException($"Could not get method '{methodName}' on type '{type.FullName}'"); } return methodInfo; @@ -72,11 +74,12 @@ public static MethodInfo GetMethod(this Lazy lazyType, string methodName) throw new ArgumentNullException(nameof(lazyType)); } - var methodInfo = lazyType.Value.GetMethod(methodName); + var type = lazyType.Value; + var methodInfo = type.GetMethod(methodName); if (methodInfo == null) { - throw new InvalidOperationException($"Could not get type '{methodName}'"); + throw new InvalidOperationException($"Could not get method '{methodName}' on type '{type.FullName}'"); } return methodInfo; @@ -89,11 +92,12 @@ public static MethodInfo GetMethod(this Lazy lazyType, string methodName, throw new ArgumentNullException(nameof(lazyType)); } - var methodInfo = lazyType.Value.GetMethod(methodName, bindingFlags); + var type = lazyType.Value; + var methodInfo = type.GetMethod(methodName, bindingFlags); if (methodInfo == null) { - throw new InvalidOperationException($"Could not get type '{methodName}'"); + throw new InvalidOperationException($"Could not get method '{methodName}' on type '{type.FullName}'"); } return methodInfo; diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index c56325050a..35d41b4433 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Reflection; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; @@ -11,9 +12,10 @@ using Microsoft.CodeAnalysis.Text; using Microsoft.Extensions.Logging; using OmniSharp.Models.V2; +using OmniSharp.Roslyn.CSharp.Services.CodeActions; using OmniSharp.Services; -namespace OmniSharp.Roslyn.CSharp.Services.CodeActions +namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2 { public abstract class BaseCodeActionService : RequestHandler { @@ -23,31 +25,40 @@ public abstract class BaseCodeActionService : RequestHandle private readonly CodeActionHelper _helper; + private readonly MethodInfo _getNestedCodeActions; + protected BaseCodeActionService(OmniSharpWorkspace workspace, CodeActionHelper helper, IEnumerable providers, ILogger logger) { this.Workspace = workspace; this.Providers = providers; this.Logger = logger; this._helper = helper; + + // Sadly, the CodeAction.NestedCodeActions property is still internal. + var nestedCodeActionsProperty = typeof(CodeAction).GetProperty("NestedCodeActions", BindingFlags.NonPublic | BindingFlags.Instance); + if (nestedCodeActionsProperty == null) + { + throw new InvalidOperationException("Could not find CodeAction.NestedCodeActions property."); + } + + this._getNestedCodeActions = nestedCodeActionsProperty.GetGetMethod(nonPublic: true); + if (this._getNestedCodeActions == null) + { + throw new InvalidOperationException("Could not retrieve 'get' method for CodeAction.NestedCodeActions property."); + } } public abstract Task Handle(TRequest request); - - protected async Task> GetActionsAsync(ICodeActionRequest request) + protected async Task> GetActionsAsync(ICodeActionRequest request) { - var actions = new List(); var originalDocument = this.Workspace.GetDocument(request.FileName); if (originalDocument == null) { - return actions; + return Array.Empty(); } - var refactoringContext = await GetRefactoringContext(originalDocument, request, actions); - if (refactoringContext != null) - { - await CollectRefactoringActions(refactoringContext.Value); - } + var actions = new List(); var codeFixContext = await GetCodeFixContext(originalDocument, request, actions); if (codeFixContext != null) @@ -55,9 +66,16 @@ protected async Task> GetActionsAsync(ICodeActionRequest await CollectCodeFixActions(codeFixContext.Value); } + var refactoringContext = await GetRefactoringContext(originalDocument, request, actions); + if (refactoringContext != null) + { + await CollectRefactoringActions(refactoringContext.Value); + } + + // TODO: Determine good way to order code actions. actions.Reverse(); - return actions; + return ConvertToCodeActionAndParents(actions); } private async Task GetRefactoringContext(Document originalDocument, ICodeActionRequest request, List actionsDestination) @@ -166,5 +184,35 @@ private async Task CollectRefactoringActions(CodeRefactoringContext context) } } } + + private IEnumerable ConvertToCodeActionAndParents(IEnumerable actions) + { + var codeActions = new List(); + + foreach (var action in actions) + { + var handledNestedActions = false; + + // Roslyn supports "nested" code actions in order to allow submenus in the VS light bulb menu. + // For now, we'll just expand nested code actions in place. + var nestedActions = this._getNestedCodeActions.Invoke>(action, null); + if (nestedActions.Length > 0) + { + foreach (var nestedAction in nestedActions) + { + codeActions.Add(new CodeActionAndParent(nestedAction, action)); + } + + handledNestedActions = true; + } + + if (!handledNestedActions) + { + codeActions.Add(new CodeActionAndParent(action)); + } + } + + return codeActions; + } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionAndParent.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionAndParent.cs new file mode 100644 index 0000000000..abbdfaefd6 --- /dev/null +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionAndParent.cs @@ -0,0 +1,29 @@ +using System; +using Microsoft.CodeAnalysis.CodeActions; + +namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2 +{ + public class CodeActionAndParent + { + public CodeAction CodeAction { get; } + public CodeAction ParentCodeAction { get; } + + public CodeActionAndParent(CodeAction codeAction, CodeAction parentCodeAction = null) + { + if (codeAction == null) + { + throw new ArgumentNullException(nameof(codeAction)); + } + + this.CodeAction = codeAction; + this.ParentCodeAction = parentCodeAction; + } + + public string GetTitle() + { + return ParentCodeAction != null + ? $"{ParentCodeAction.Title} -> {CodeAction.Title}" + : CodeAction.Title; + } + } +} diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/GetCodeActionsService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/GetCodeActionsService.cs index 32635bac1e..630fdcf803 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/GetCodeActionsService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/GetCodeActionsService.cs @@ -31,8 +31,13 @@ public override async Task Handle(GetCodeActionsRequest return new GetCodeActionsResponse { - CodeActions = actions.Select(a => new OmniSharpCodeAction(a.GetIdentifier(), a.Title)) + CodeActions = actions.Select(ConvertToOmniSharpCodeAction) }; } + + private static OmniSharpCodeAction ConvertToOmniSharpCodeAction(CodeActionAndParent codeActionAndParent) + { + return new OmniSharpCodeAction(codeActionAndParent.CodeAction.GetIdentifier(), codeActionAndParent.GetTitle()); + } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/RunCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/RunCodeActionService.cs index 222df5fa89..315447017c 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/RunCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/RunCodeActionService.cs @@ -33,16 +33,16 @@ public RunCodeActionService( public override async Task Handle(RunCodeActionRequest request) { - var actions = await GetActionsAsync(request); - var action = actions.FirstOrDefault(a => a.GetIdentifier().Equals(request.Identifier)); - if (action == null) + var codeActionAndParents = await GetActionsAsync(request); + var codeActionAndParent = codeActionAndParents.FirstOrDefault(a => a.CodeAction.GetIdentifier().Equals(request.Identifier)); + if (codeActionAndParent == null) { return new RunCodeActionResponse(); } - Logger.LogInformation($"Applying code action: {action.Title}"); + Logger.LogInformation($"Applying code action: {codeActionAndParent.GetTitle()}"); - var operations = await action.GetOperationsAsync(CancellationToken.None); + var operations = await codeActionAndParent.CodeAction.GetOperationsAsync(CancellationToken.None); var solution = this.Workspace.CurrentSolution; var changes = new List(); From a25c1a19871eefaaacb0b58c10bf6d93edade437 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Sun, 5 Feb 2017 14:20:05 -0800 Subject: [PATCH 2/3] Clean up nested action support a bit --- .../Extensions/CodeActionExtensions.cs | 12 ------------ ...ctionAndParent.cs => AvailableCodeAction.cs} | 17 +++++++++++++++-- .../Refactoring/V2/BaseCodeActionService.cs | 12 ++++++------ .../Refactoring/V2/GetCodeActionsService.cs | 9 ++++----- .../Refactoring/V2/RunCodeActionService.cs | 11 +++++------ 5 files changed, 30 insertions(+), 31 deletions(-) delete mode 100644 src/OmniSharp.Roslyn.CSharp/Extensions/CodeActionExtensions.cs rename src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/{CodeActionAndParent.cs => AvailableCodeAction.cs} (58%) diff --git a/src/OmniSharp.Roslyn.CSharp/Extensions/CodeActionExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Extensions/CodeActionExtensions.cs deleted file mode 100644 index ae69b7b490..0000000000 --- a/src/OmniSharp.Roslyn.CSharp/Extensions/CodeActionExtensions.cs +++ /dev/null @@ -1,12 +0,0 @@ -using Microsoft.CodeAnalysis.CodeActions; - -namespace OmniSharp.Roslyn.CSharp.Extensions -{ - public static class CodeActionExtensions - { - public static string GetIdentifier(this CodeAction codeAction) - { - return codeAction.EquivalenceKey ?? codeAction.Title; - } - } -} diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionAndParent.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/AvailableCodeAction.cs similarity index 58% rename from src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionAndParent.cs rename to src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/AvailableCodeAction.cs index abbdfaefd6..c29e975cf8 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CodeActionAndParent.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/AvailableCodeAction.cs @@ -1,14 +1,17 @@ using System; +using System.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2 { - public class CodeActionAndParent + public class AvailableCodeAction { public CodeAction CodeAction { get; } public CodeAction ParentCodeAction { get; } - public CodeActionAndParent(CodeAction codeAction, CodeAction parentCodeAction = null) + public AvailableCodeAction(CodeAction codeAction, CodeAction parentCodeAction = null) { if (codeAction == null) { @@ -19,11 +22,21 @@ public CodeActionAndParent(CodeAction codeAction, CodeAction parentCodeAction = this.ParentCodeAction = parentCodeAction; } + public string GetIdentifier() + { + return CodeAction.EquivalenceKey ?? GetTitle(); + } + public string GetTitle() { return ParentCodeAction != null ? $"{ParentCodeAction.Title} -> {CodeAction.Title}" : CodeAction.Title; } + + public Task> GetOperationsAsync(CancellationToken cancellationToken) + { + return CodeAction.GetOperationsAsync(cancellationToken); + } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index 35d41b4433..6e8bf17953 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -50,12 +50,12 @@ protected BaseCodeActionService(OmniSharpWorkspace workspace, CodeActionHelper h public abstract Task Handle(TRequest request); - protected async Task> GetActionsAsync(ICodeActionRequest request) + protected async Task> GetAvailableCodeActions(ICodeActionRequest request) { var originalDocument = this.Workspace.GetDocument(request.FileName); if (originalDocument == null) { - return Array.Empty(); + return Array.Empty(); } var actions = new List(); @@ -185,9 +185,9 @@ private async Task CollectRefactoringActions(CodeRefactoringContext context) } } - private IEnumerable ConvertToCodeActionAndParents(IEnumerable actions) + private IEnumerable ConvertToCodeActionAndParents(IEnumerable actions) { - var codeActions = new List(); + var codeActions = new List(); foreach (var action in actions) { @@ -200,7 +200,7 @@ private IEnumerable ConvertToCodeActionAndParents(IEnumerab { foreach (var nestedAction in nestedActions) { - codeActions.Add(new CodeActionAndParent(nestedAction, action)); + codeActions.Add(new AvailableCodeAction(nestedAction, action)); } handledNestedActions = true; @@ -208,7 +208,7 @@ private IEnumerable ConvertToCodeActionAndParents(IEnumerab if (!handledNestedActions) { - codeActions.Add(new CodeActionAndParent(action)); + codeActions.Add(new AvailableCodeAction(action)); } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/GetCodeActionsService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/GetCodeActionsService.cs index 630fdcf803..fd8eb8213e 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/GetCodeActionsService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/GetCodeActionsService.cs @@ -6,7 +6,6 @@ using Microsoft.Extensions.Logging; using OmniSharp.Mef; using OmniSharp.Models.V2; -using OmniSharp.Roslyn.CSharp.Extensions; using OmniSharp.Roslyn.CSharp.Services.CodeActions; using OmniSharp.Services; @@ -27,17 +26,17 @@ public GetCodeActionsService( public override async Task Handle(GetCodeActionsRequest request) { - var actions = await GetActionsAsync(request); + var availableActions = await GetAvailableCodeActions(request); return new GetCodeActionsResponse { - CodeActions = actions.Select(ConvertToOmniSharpCodeAction) + CodeActions = availableActions.Select(ConvertToOmniSharpCodeAction) }; } - private static OmniSharpCodeAction ConvertToOmniSharpCodeAction(CodeActionAndParent codeActionAndParent) + private static OmniSharpCodeAction ConvertToOmniSharpCodeAction(AvailableCodeAction availableAction) { - return new OmniSharpCodeAction(codeActionAndParent.CodeAction.GetIdentifier(), codeActionAndParent.GetTitle()); + return new OmniSharpCodeAction(availableAction.GetIdentifier(), availableAction.GetTitle()); } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/RunCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/RunCodeActionService.cs index 315447017c..73971bed59 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/RunCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/RunCodeActionService.cs @@ -9,7 +9,6 @@ using Microsoft.Extensions.Logging; using OmniSharp.Mef; using OmniSharp.Models; -using OmniSharp.Roslyn.CSharp.Extensions; using OmniSharp.Roslyn.CSharp.Services.CodeActions; using OmniSharp.Services; @@ -33,16 +32,16 @@ public RunCodeActionService( public override async Task Handle(RunCodeActionRequest request) { - var codeActionAndParents = await GetActionsAsync(request); - var codeActionAndParent = codeActionAndParents.FirstOrDefault(a => a.CodeAction.GetIdentifier().Equals(request.Identifier)); - if (codeActionAndParent == null) + var availableActions = await GetAvailableCodeActions(request); + var availableAction = availableActions.FirstOrDefault(a => a.GetIdentifier().Equals(request.Identifier)); + if (availableAction == null) { return new RunCodeActionResponse(); } - Logger.LogInformation($"Applying code action: {codeActionAndParent.GetTitle()}"); + Logger.LogInformation($"Applying code action: {availableAction.GetTitle()}"); - var operations = await codeActionAndParent.CodeAction.GetOperationsAsync(CancellationToken.None); + var operations = await availableAction.GetOperationsAsync(CancellationToken.None); var solution = this.Workspace.CurrentSolution; var changes = new List(); From 8396cee1591d78732679c27302c3b776e6991ee1 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Sun, 5 Feb 2017 14:41:22 -0800 Subject: [PATCH 3/3] Filter out code actions that are subclasses of CodeActionWithOptions --- .../Services/Refactoring/V2/BaseCodeActionService.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index 6e8bf17953..1d24b60799 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -75,7 +75,13 @@ protected async Task> GetAvailableCodeActions(I // TODO: Determine good way to order code actions. actions.Reverse(); - return ConvertToCodeActionAndParents(actions); + // 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. + var availableActions = ConvertToAvailableCodeAction(actions) + .Where(a => !a.CodeAction.GetType().GetTypeInfo().IsSubclassOf(typeof(CodeActionWithOptions))); + + return availableActions; } private async Task GetRefactoringContext(Document originalDocument, ICodeActionRequest request, List actionsDestination) @@ -142,7 +148,7 @@ private async Task CollectCodeFixActions(CodeFixContext context) // TODO: This is a horrible hack! However, remove unnecessary usings only // responds for diagnostics that are produced by its diagnostic analyzer. // We need to provide a *real* diagnostic engine to address this. - if (codeFix.ToString() != CodeActionHelper.RemoveUnnecessaryUsingsProviderName) + if (codeFix.GetType().FullName != CodeActionHelper.RemoveUnnecessaryUsingsProviderName) { if (!diagnosticIds.Any(id => codeFix.FixableDiagnosticIds.Contains(id))) { @@ -185,7 +191,7 @@ private async Task CollectRefactoringActions(CodeRefactoringContext context) } } - private IEnumerable ConvertToCodeActionAndParents(IEnumerable actions) + private IEnumerable ConvertToAvailableCodeAction(IEnumerable actions) { var codeActions = new List();