LSP: fix performance of the textDocument/codeAction request (#1814)

* LSP: fix performance of the textDocument/codeAction request This improves performance of codeAction request on large codes base quite a bit by making the response from textDocument/codeAction to be more lightweight. This is accomplished by adding a separate command handler for "omnisharp/executeCodeAction" that actually sends the changes to client with "workspace/applyEdit". * Update CHANGELOG.md * Updated to use new execute command logic. Added unit tests to ensure funcitonality is working as expected Co-authored-by: David Driscoll <[email protected]>
OmniSharp · Aug 3, 2020 · 96600de · 96600de
1 parent 52621cb
commit 96600de
Show file tree

Hide file tree

Showing 20 changed files with 1,033 additions and 146 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@ All changes to the project will be documented in this file.
 * Fixed return type in LSP completion handler ([#1864](https://github.com/OmniSharp/omnisharp-roslyn/issues/1864), PR: [#1869](https://github.com/OmniSharp/omnisharp-roslyn/pull/1869))
 * Upgraded to the latest version of the csharp-language-server-protocol [#1815](https://github.com/OmniSharp/omnisharp-roslyn/pull/1815)
 * Added support for Roslyn `EmbeddedLanguageCompletionProvider` which enables completions for string literals for `DateTime` and `Regex` ([#1871](https://github.com/OmniSharp/omnisharp-roslyn/pull/1871))
+* Improve performance of the `textDocument/codeAction` request. (PR: [#1814](https://github.com/OmniSharp/omnisharp-roslyn/pull/1814))
 
 ## [1.35.4] - 2020-07-22
 * Update to Roslyn `3.8.0-1.20357.3` (PR: [#1849](https://github.com/OmniSharp/omnisharp-roslyn/pull/1849))
@@ -23,7 +24,6 @@ All changes to the project will be documented in this file.
 * Expose a custom LSP `omnisharp/client/findReferences` command via code lens (meant to be handled by LSP client). (PR: [#1807](https://github.com/OmniSharp/omnisharp-roslyn/pull/1807))
 * Added `DirectoryDelete` option to `FileChangeType` allowing clients to report deleted directories that need to be removed (along all the files) from the workspace (PR: [#1821](https://github.com/OmniSharp/omnisharp-roslyn/pull/1821))
 * Do not crash when plugin assembly cannot be loaded ([#1307](https://github.com/OmniSharp/omnisharp-roslyn/issues/1307), PR: [#1827](https://github.com/OmniSharp/omnisharp-roslyn/pull/1827))
-* Update to Roslyn `3.7.0-4.20311.4` (PR: [#1832](https://github.com/OmniSharp/omnisharp-roslyn/pull/1832))
 
 ## [1.35.2] - 2020-05-20
 * Added support for `WarningsAsErrors` in csproj files (PR: [#1779](https://github.com/OmniSharp/omnisharp-roslyn/pull/1779))

diff --git a/build/Packages.props b/build/Packages.props
@@ -62,8 +62,8 @@
     <PackageReference Update="Nuget.ProjectModel" Version="$(NuGetPackageVersion)" />
     <PackageReference Update="Nuget.Versioning" Version="$(NuGetPackageVersion)" />
 
-    <PackageReference Update="OmniSharp.Extensions.LanguageServer" Version="0.17.3" />
-    <PackageReference Update="OmniSharp.Extensions.LanguageProtocol.Testing" Version="0.17.3" />
+    <PackageReference Update="OmniSharp.Extensions.LanguageServer" Version="0.18.0-beta0003" />
+    <PackageReference Update="OmniSharp.Extensions.LanguageProtocol.Testing" Version="0.18.0-beta0003" />
 
     <PackageReference Update="SQLitePCLRaw.bundle_green" Version="1.1.2" />
     <PackageReference Update="System.Collections.Immutable" Version="1.4.0" />

diff --git a/src/OmniSharp.Host/CompositionHostBuilder.cs b/src/OmniSharp.Host/CompositionHostBuilder.cs
@@ -75,6 +75,7 @@ public CompositionHost Build()
                 .WithProvider(MefValueProvider.From(loggerFactory))
                 .WithProvider(MefValueProvider.From(environment))
                 .WithProvider(MefValueProvider.From(options.CurrentValue))
+                .WithProvider(MefValueProvider.From(options))
                 .WithProvider(MefValueProvider.From(options.CurrentValue.FormattingOptions))
                 .WithProvider(MefValueProvider.From(assemblyLoader))
                 .WithProvider(MefValueProvider.From(analyzerAssemblyLoader))

diff --git a/src/OmniSharp.Host/WorkspaceInitializer.cs b/src/OmniSharp.Host/WorkspaceInitializer.cs
@@ -30,6 +30,7 @@ public static void Initialize(IServiceProvider serviceProvider, CompositionHost
             var projectSystems = compositionHost.GetExports<IProjectSystem>();
 
             workspace.EditorConfigEnabled = options.CurrentValue.FormattingOptions.EnableEditorConfigSupport;
+            options.OnChange(x => workspace.EditorConfigEnabled = x.FormattingOptions.EnableEditorConfigSupport);
 
             foreach (var projectSystem in projectSystems)
             {

diff --git a/src/OmniSharp.LanguageServerProtocol/Eventing/LanguageServerEventEmitter.cs b/src/OmniSharp.LanguageServerProtocol/Eventing/LanguageServerEventEmitter.cs
@@ -1,10 +1,12 @@
 using System.Linq;
+using Microsoft.Extensions.DependencyInjection;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 using OmniSharp.Eventing;
 using OmniSharp.Extensions.LanguageServer.Protocol.Document;
 using OmniSharp.Extensions.LanguageServer.Protocol.Models;
 using OmniSharp.Extensions.LanguageServer.Protocol.Server;
+using OmniSharp.LanguageServerProtocol.Handlers;
 using OmniSharp.Models.Diagnostics;
 using OmniSharp.Models.Events;
 
@@ -13,10 +15,12 @@ namespace OmniSharp.LanguageServerProtocol.Eventing
     public class LanguageServerEventEmitter : IEventEmitter
     {
         private readonly ILanguageServer _server;
+        private readonly DocumentVersions _documentVersions;
 
         public LanguageServerEventEmitter(ILanguageServer server)
         {
             _server = server;
+            _documentVersions = server.Services.GetRequiredService<DocumentVersions>();
         }
 
         public void Emit(string kind, object args)
@@ -34,6 +38,7 @@ public void Emit(string kind, object args)
                             _server.TextDocument.PublishDiagnostics(new PublishDiagnosticsParams()
                             {
                                 Uri = group.Key,
+                                Version = _documentVersions.GetVersion(group.Key),
                                 Diagnostics = group
                                     .SelectMany(z => z.Select(v => v.ToDiagnostic()))
                                     .ToArray()

diff --git a/src/OmniSharp.LanguageServerProtocol/Handlers/DocumentVersions.cs b/src/OmniSharp.LanguageServerProtocol/Handlers/DocumentVersions.cs
@@ -0,0 +1,36 @@
+using System.Collections.Concurrent;
+using OmniSharp.Extensions.LanguageServer.Protocol;
+using OmniSharp.Extensions.LanguageServer.Protocol.Models;
+
+namespace OmniSharp.LanguageServerProtocol.Handlers
+{
+    public class DocumentVersions
+    {
+        private readonly ConcurrentDictionary<DocumentUri, int> _documentVersions = new ConcurrentDictionary<DocumentUri, int>();
+
+        public int? GetVersion(DocumentUri documentUri)
+        {
+            if (_documentVersions.TryGetValue(documentUri, out var version))
+            {
+                return version;
+            }
+
+            return null;
+        }
+
+        public void Update(VersionedTextDocumentIdentifier identifier)
+        {
+            _documentVersions.AddOrUpdate(identifier.Uri, identifier.Version ?? 0, (uri, i) => identifier.Version ?? 0);
+        }
+
+        public void Reset(TextDocumentIdentifier identifier)
+        {
+            _documentVersions.AddOrUpdate(identifier.Uri, 0, (uri, i) => 0);
+        }
+
+        public void Remove(TextDocumentIdentifier identifier)
+        {
+            _documentVersions.TryRemove(identifier.Uri, out _);
+        }
+    }
+}
diff --git a/src/OmniSharp.LanguageServerProtocol/Handlers/OmniSharpCodeActionHandler.cs b/src/OmniSharp.LanguageServerProtocol/Handlers/OmniSharpCodeActionHandler.cs
@@ -1,36 +1,55 @@
-using System;
 using System.Linq;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Threading;
 using System.Threading.Tasks;
+using MediatR;
+using Microsoft.CodeAnalysis;
+using Newtonsoft.Json.Linq;
 using OmniSharp.Extensions.JsonRpc;
+using OmniSharp.Extensions.LanguageServer.Protocol;
+using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities;
 using OmniSharp.Extensions.LanguageServer.Protocol.Models;
 using OmniSharp.Extensions.LanguageServer.Protocol.Server;
-using OmniSharp.Models;
 using OmniSharp.Models.V2.CodeActions;
 using OmniSharp.Extensions.LanguageServer.Protocol.Document;
+using OmniSharp.Extensions.LanguageServer.Protocol.Workspace;
+using OmniSharp.Models;
+using Diagnostic = OmniSharp.Extensions.LanguageServer.Protocol.Models.Diagnostic;
 
 namespace OmniSharp.LanguageServerProtocol.Handlers
 {
-    internal sealed class OmniSharpCodeActionHandler : CodeActionHandler
+    internal sealed class OmniSharpCodeActionHandler : CodeActionHandler, IExecuteCommandHandler
     {
-        public static IEnumerable<IJsonRpcHandler> Enumerate(RequestHandlers handlers)
+        public static IEnumerable<IJsonRpcHandler> Enumerate(
+            RequestHandlers handlers,
+            ISerializer serializer,
+            ILanguageServer mediator,
+            DocumentVersions versions)
         {
             foreach (var (selector, getActionsHandler, runActionHandler) in handlers
                      .OfType<Mef.IRequestHandler<GetCodeActionsRequest, GetCodeActionsResponse>,
                              Mef.IRequestHandler<RunCodeActionRequest, RunCodeActionResponse>>())
             {
-                yield return new OmniSharpCodeActionHandler(getActionsHandler, runActionHandler, selector);
+                yield return new OmniSharpCodeActionHandler(getActionsHandler, runActionHandler, selector, serializer, mediator, versions);
             }
         }
 
         private readonly Mef.IRequestHandler<GetCodeActionsRequest, GetCodeActionsResponse> _getActionsHandler;
-        private readonly Mef.IRequestHandler<RunCodeActionRequest, RunCodeActionResponse> _runActionHandler;
+        private readonly ExecuteCommandRegistrationOptions _executeCommandRegistrationOptions;
+        private ExecuteCommandCapability _executeCommandCapability;
+        private Mef.IRequestHandler<RunCodeActionRequest, RunCodeActionResponse> _runActionHandler;
+        private readonly ISerializer _serializer;
+        private readonly ILanguageServer _server;
+        private readonly DocumentVersions _documentVersions;
 
         public OmniSharpCodeActionHandler(
             Mef.IRequestHandler<GetCodeActionsRequest, GetCodeActionsResponse> getActionsHandler,
             Mef.IRequestHandler<RunCodeActionRequest, RunCodeActionResponse> runActionHandler,
-            DocumentSelector documentSelector)
+            DocumentSelector documentSelector,
+            ISerializer serializer,
+            ILanguageServer server,
+            DocumentVersions documentVersions)
             : base(new CodeActionRegistrationOptions()
             {
                 DocumentSelector = documentSelector,
@@ -42,15 +61,22 @@ public OmniSharpCodeActionHandler(
         {
             _getActionsHandler = getActionsHandler;
             _runActionHandler = runActionHandler;
+            _serializer = serializer;
+            _server = server;
+            _documentVersions = documentVersions;
+            _executeCommandRegistrationOptions = new ExecuteCommandRegistrationOptions()
+            {
+                Commands = new Container<string>("omnisharp/executeCodeAction"),
+            };
         }
 
-        public async override Task<CommandOrCodeActionContainer> Handle(CodeActionParams request, CancellationToken cancellationToken)
+        public override async Task<CommandOrCodeActionContainer> Handle(CodeActionParams request, CancellationToken cancellationToken)
         {
             var omnisharpRequest = new GetCodeActionsRequest
             {
                 FileName = Helpers.FromUri(request.TextDocument.Uri),
-                Column = (int)request.Range.Start.Character,
-                Line = (int)request.Range.Start.Line,
+                Column = request.Range.Start.Character,
+                Line = request.Range.Start.Line,
                 Selection = Helpers.FromRange(request.Range),
             };
 
@@ -60,27 +86,6 @@ public async override Task<CommandOrCodeActionContainer> Handle(CodeActionParams
 
             foreach (var ca in omnisharpResponse.CodeActions)
             {
-                var omnisharpCaRequest = new RunCodeActionRequest
-                {
-                    Identifier = ca.Identifier,
-                    FileName = Helpers.FromUri(request.TextDocument.Uri),
-                    Column = Convert.ToInt32(request.Range.Start.Character),
-                    Line = Convert.ToInt32(request.Range.Start.Line),
-                    Selection = Helpers.FromRange(request.Range),
-                    ApplyTextChanges = false,
-                    WantsTextChanges = true,
-                };
-
-                var omnisharpCaResponse = await _runActionHandler.Handle(omnisharpCaRequest);
-
-                var changes = omnisharpCaResponse.Changes.ToDictionary(
-                    x => Helpers.ToUri(x.FileName),
-                    x => ((ModifiedFileResponse)x).Changes.Select(edit => new TextEdit
-                    {
-                        NewText = edit.NewText,
-                        Range = Helpers.ToRange((edit.StartColumn, edit.StartLine), (edit.EndColumn, edit.EndLine))
-                    }));
-
                 CodeActionKind kind;
                 if (ca.Identifier.StartsWith("using ")) { kind = CodeActionKind.SourceOrganizeImports; }
                 else if (ca.Identifier.StartsWith("Inline ")) { kind = CodeActionKind.RefactorInline; }
@@ -94,12 +99,71 @@ public async override Task<CommandOrCodeActionContainer> Handle(CodeActionParams
                         Title = ca.Name,
                         Kind = kind,
                         Diagnostics = new Container<Diagnostic>(),
-                        Edit = new WorkspaceEdit { Changes = changes, }
+                        Edit = new WorkspaceEdit(),
+                        Command = Command.Create("omnisharp/executeCodeAction")
+                            .WithTitle(ca.Name)
+                            .WithArguments(new CommandData()
+                            {
+                                Uri = request.TextDocument.Uri,
+                                Identifier = ca.Identifier,
+                                Name = ca.Name,
+                                Range = request.Range,
+                            })
                     });
             }
 
             return new CommandOrCodeActionContainer(
                 codeActions.Select(ca => new CommandOrCodeAction(ca)));
         }
+
+        public async Task<Unit> Handle(ExecuteCommandParams request, CancellationToken cancellationToken)
+        {
+            Debug.Assert(request.Command == "omnisharp/executeCodeAction");
+            var data = request.Arguments[0].ToObject<CommandData>(_serializer.JsonSerializer);
+
+            var omnisharpCaRequest = new RunCodeActionRequest {
+                Identifier = data.Identifier,
+                FileName = data.Uri.GetFileSystemPath(),
+                Column = data.Range.Start.Character,
+                Line = data.Range.Start.Line,
+                Selection = Helpers.FromRange(data.Range),
+                ApplyTextChanges = false,
+                WantsTextChanges = true,
+                WantsAllCodeActionOperations = true
+            };
+
+            var omnisharpCaResponse = await _runActionHandler.Handle(omnisharpCaRequest);
+            if (omnisharpCaResponse.Changes != null)
+            {
+                var edit = Helpers.ToWorkspaceEdit(
+                    omnisharpCaResponse.Changes,
+                    _server.ClientSettings.Capabilities.Workspace.WorkspaceEdit.Value,
+                    _documentVersions
+                );
+                ;
+
+                await _server.Workspace.ApplyWorkspaceEdit(new ApplyWorkspaceEditParams()
+                {
+                    Label = data.Name,
+                    Edit = edit
+                }, cancellationToken);
+
+                // Do something with response?
+                //if (response.Applied)
+            }
+
+            return Unit.Value;
+        }
+
+        class CommandData
+        {
+            public DocumentUri Uri { get; set;}
+            public string Identifier { get; set;}
+            public string Name { get; set;}
+            public Range Range { get; set;}
+        }
+
+        ExecuteCommandRegistrationOptions IRegistration<ExecuteCommandRegistrationOptions> .GetRegistrationOptions() => _executeCommandRegistrationOptions;
+        void ICapability<ExecuteCommandCapability>.SetCapability(ExecuteCommandCapability capability) => _executeCommandCapability = capability;
     }
 }
diff --git a/src/OmniSharp.LanguageServerProtocol/Handlers/OmniSharpCodeLensHandler.cs b/src/OmniSharp.LanguageServerProtocol/Handlers/OmniSharpCodeLensHandler.cs
@@ -127,14 +127,5 @@ private static void ToCodeLens(TextDocumentIdentifier textDocument, FileMemberEl
                 }
             }
         }
-
-        public override bool CanResolve(CodeLens value)
-        {
-            var textDocumentUri = value.Data.ToObject<Uri>();
-
-            return textDocumentUri != null &&
-                   GetRegistrationOptions().DocumentSelector
-                       .IsMatch(new TextDocumentAttributes(textDocumentUri, string.Empty));
-        }
     }
 }
diff --git a/src/OmniSharp.LanguageServerProtocol/Handlers/OmniSharpCompletionHandler.cs b/src/OmniSharp.LanguageServerProtocol/Handlers/OmniSharpCompletionHandler.cs
@@ -137,12 +137,7 @@ public async override Task<CompletionList> Handle(CompletionParams request, Canc
 
         public override Task<CompletionItem> Handle(CompletionItem request, CancellationToken cancellationToken)
         {
-            throw new NotImplementedException();
-        }
-
-        public override bool CanResolve(CompletionItem value)
-        {
-            return false;
+            return Task.FromResult(request);
         }
     }
 }
diff --git a/src/OmniSharp.LanguageServerProtocol/Handlers/OmniSharpExecuteCommandHandler.cs b/src/OmniSharp.LanguageServerProtocol/Handlers/OmniSharpExecuteCommandHandler.cs