From e91a566faeee510d34e0c3a6ff448f4b7606c0ad Mon Sep 17 00:00:00 2001
From: Allison Chou <allichou@microsoft.com>
Date: Mon, 6 Jul 2020 18:05:33 -0700
Subject: [PATCH] Code review feedback pt2

---
 .../CodeActions/CodeActionResolveHandler.cs   | 36 ++++++++++---------
 .../CodeActions/RunCodeActionsHandler.cs      |  4 ++-
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveHandler.cs b/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveHandler.cs
index dcdd56e2403..4360bc66889 100644
--- a/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveHandler.cs
+++ b/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveHandler.cs
@@ -76,6 +76,14 @@ internal class CodeActionResolveHandler : AbstractRequestHandler<LSP.VSCodeActio
                 return codeAction;
             }
 
+            // If we have all non-ApplyChangesOperations, set up to run as command on the server
+            // instead of using WorkspaceEdits.
+            if (operations.All(operation => !(operation is ApplyChangesOperation)))
+            {
+                codeAction.Command = SetCommand(codeAction.Title, data);
+                return codeAction;
+            }
+
             // TO-DO:
             // 1) We currently must execute code actions which add new documents on the server as commands,
             // since there is no LSP support for adding documents yet. In the future, we should move these actions
@@ -84,7 +92,6 @@ internal class CodeActionResolveHandler : AbstractRequestHandler<LSP.VSCodeActio
             // one where the code action was invoked from do not work. We must temporarily execute these as commands
             // as well.
             // https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1147293/
-            var runAsCommand = false;
 
             // Add workspace edits
             var applyChangesOperations = operations.OfType<ApplyChangesOperation>();
@@ -104,8 +111,8 @@ internal class CodeActionResolveHandler : AbstractRequestHandler<LSP.VSCodeActio
                         pc => pc.GetAddedDocuments().Concat(pc.GetAddedAdditionalDocuments().Concat(pc.GetAddedAnalyzerConfigDocuments())));
                     if (addedDocuments.Any())
                     {
-                        runAsCommand = true;
-                        break;
+                        codeAction.Command = SetCommand(codeAction.Title, data);
+                        return codeAction;
                     }
 
                     var changedDocuments = projectChanges.SelectMany(pc => pc.GetChangedDocuments());
@@ -120,8 +127,8 @@ internal class CodeActionResolveHandler : AbstractRequestHandler<LSP.VSCodeActio
                         changedAnalyzerConfigDocuments.Any() ||
                         changedAdditionalDocuments.Any())
                     {
-                        runAsCommand = true;
-                        break;
+                        codeAction.Command = SetCommand(codeAction.Title, data);
+                        return codeAction;
                     }
 
                     // Changed documents
@@ -150,21 +157,16 @@ internal class CodeActionResolveHandler : AbstractRequestHandler<LSP.VSCodeActio
                 codeAction.Edit = new LSP.WorkspaceEdit { DocumentChanges = textDocumentEdits.ToArray() };
             }
 
-            // Set up to run as command on the server instead of using WorkspaceEdits.
-            var commandOperations = operations.All(operation => !(operation is ApplyChangesOperation));
-            if (commandOperations || runAsCommand)
-            {
-                codeAction.Command = new LSP.Command
-                {
-                    CommandIdentifier = CodeActionsHandler.RunCodeActionCommandName,
-                    Title = codeAction.Title,
-                    Arguments = new object[] { data }
-                };
-            }
-
             return codeAction;
 
             // Local functions
+            static LSP.Command SetCommand(string title, CodeActionResolveData data) => new LSP.Command
+            {
+                CommandIdentifier = CodeActionsHandler.RunCodeActionCommandName,
+                Title = title,
+                Arguments = new object[] { data }
+            };
+
             static async Task AddTextDocumentEdits<T>(
                 ArrayBuilder<TextDocumentEdit> textDocumentEdits,
                 ApplyChangesOperation applyChangesOperation,
diff --git a/src/Features/LanguageServer/Protocol/Handler/CodeActions/RunCodeActionsHandler.cs b/src/Features/LanguageServer/Protocol/Handler/CodeActions/RunCodeActionsHandler.cs
index abcddc3cb6a..cfc44b8b273 100644
--- a/src/Features/LanguageServer/Protocol/Handler/CodeActions/RunCodeActionsHandler.cs
+++ b/src/Features/LanguageServer/Protocol/Handler/CodeActions/RunCodeActionsHandler.cs
@@ -66,11 +66,13 @@ internal class RunCodeActionsHandler : IExecuteWorkspaceCommandHandler
             var actionToRun = CodeActionHelpers.GetCodeActionToResolve(runRequest.UniqueIdentifier, codeActions.ToImmutableArray());
             Contract.ThrowIfNull(actionToRun);
 
+            var operations = await actionToRun.GetOperationsAsync(cancellationToken).ConfigureAwait(false);
+
             // TODO - This UI thread dependency should be removed.
             // https://github.com/dotnet/roslyn/projects/45#card-20619668
             await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
 
-            foreach (var operation in await actionToRun.GetOperationsAsync(cancellationToken).ConfigureAwait(false))
+            foreach (var operation in operations)
             {
                 operation.Apply(document.Project.Solution.Workspace, cancellationToken);
             }
-- 
GitLab