From 53d33d9aaf7964aac097c4c127d4021bbcbc09d0 Mon Sep 17 00:00:00 2001 From: David Driscoll Date: Fri, 12 Jul 2019 21:27:15 -0400 Subject: [PATCH] Added a fix for #141 with test validating that it works --- .gitignore | 1 + LSP.sln | 4 +-- src/JsonRpc/IRequestRouter.cs | 14 ++++---- src/JsonRpc/RequestRouterBase.cs | 4 +-- src/Server/LanguageServer.cs | 51 ++++++++++++++++++++++++--- src/Server/LspRequestRouter.cs | 16 +++++++-- test/Lsp.Tests/LanguageServerTests.cs | 25 ++++++++++++- 7 files changed, 97 insertions(+), 18 deletions(-) diff --git a/.gitignore b/.gitignore index b66fb4f5e..937228463 100644 --- a/.gitignore +++ b/.gitignore @@ -41,3 +41,4 @@ tools/*/ coverage.*.xml coverage.json coverage.info +/codealike.json diff --git a/LSP.sln b/LSP.sln index 66a1c82d8..0101f00db 100644 --- a/LSP.sln +++ b/LSP.sln @@ -1,6 +1,6 @@ Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio 15 -VisualStudioVersion = 15.0.27130.2003 +# Visual Studio Version 16 +VisualStudioVersion = 16.0.29025.244 MinimumVisualStudioVersion = 10.0.40219.1 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{D764E024-3D3F-4112-B932-2DB722A1BACC}" ProjectSection(SolutionItems) = preProject diff --git a/src/JsonRpc/IRequestRouter.cs b/src/JsonRpc/IRequestRouter.cs index 363a92379..249032d03 100644 --- a/src/JsonRpc/IRequestRouter.cs +++ b/src/JsonRpc/IRequestRouter.cs @@ -5,16 +5,18 @@ namespace OmniSharp.Extensions.JsonRpc { - public interface IRequestRouter + public interface IRequestRouter + { + Task RouteNotification(Notification notification, CancellationToken token); + Task RouteRequest(Request request, CancellationToken token); + void CancelRequest(object id); + } + + public interface IRequestRouter : IRequestRouter { TDescriptor GetDescriptor(Notification notification); TDescriptor GetDescriptor(Request request); - - Task RouteNotification(Notification notification, CancellationToken token); Task RouteNotification(TDescriptor descriptor, Notification notification, CancellationToken token); - Task RouteRequest(Request request, CancellationToken token); Task RouteRequest(TDescriptor descriptor, Request request, CancellationToken token); - - void CancelRequest(object id); } } diff --git a/src/JsonRpc/RequestRouterBase.cs b/src/JsonRpc/RequestRouterBase.cs index 037e85667..c599654b0 100644 --- a/src/JsonRpc/RequestRouterBase.cs +++ b/src/JsonRpc/RequestRouterBase.cs @@ -198,12 +198,12 @@ private string GetId(object id) return id?.ToString(); } - Task IRequestRouter.RouteNotification(Notification notification, CancellationToken token) + Task IRequestRouter.RouteNotification(Notification notification, CancellationToken token) { return RouteNotification(GetDescriptor(notification), notification, token); } - Task IRequestRouter.RouteRequest(Request request, CancellationToken token) + Task IRequestRouter.RouteRequest(Request request, CancellationToken token) { return RouteRequest(GetDescriptor(request), request, token); } diff --git a/src/Server/LanguageServer.cs b/src/Server/LanguageServer.cs index b737bae6f..242b20fbd 100644 --- a/src/Server/LanguageServer.cs +++ b/src/Server/LanguageServer.cs @@ -61,7 +61,14 @@ public static Task From(Action optionsAc { var options = new LanguageServerOptions(); optionsAction(options); - return From(options); + return From(options, token); + } + + public static ILanguageServer PreInit(Action optionsAction) + { + var options = new LanguageServerOptions(); + optionsAction(options); + return PreInit(options); } public static async Task From(LanguageServerOptions options, CancellationToken token) @@ -91,6 +98,38 @@ public static async Task From(LanguageServerOptions options, Ca return server; } + /// + /// Create the server without connecting to the client + /// + /// Mainly used for unit testing + /// + /// + /// + public static ILanguageServer PreInit(LanguageServerOptions options) + { + var server = new LanguageServer( + options.Input, + options.Output, + options.Reciever, + options.RequestProcessIdentifier, + options.LoggerFactory, + options.Serializer, + options.Services, + options.HandlerTypes.Select(x => x.Assembly) + .Distinct().Concat(options.HandlerAssemblies), + options.Handlers, + options.NamedHandlers, + options.NamedServiceHandlers, + options.InitializeDelegates, + options.InitializedDelegates + ); + + if (options.AddDefaultLoggingProvider) + options.LoggerFactory.AddProvider(new LanguageServerLoggerProvider(server)); + + return server; + } + internal LanguageServer( Stream input, Stream output, @@ -132,7 +171,9 @@ internal LanguageServer( services.AddSingleton(this); services.AddTransient(); services.AddTransient(); - services.AddSingleton, LspRequestRouter>(); + services.AddSingleton(); + services.AddSingleton>(_ => _.GetRequiredService()); + services.AddSingleton>(_ => _.GetRequiredService()); services.AddSingleton(); services.AddTransient(typeof(IPipelineBehavior<,>), typeof(ResolveCommandPipeline<,>)); @@ -285,7 +326,7 @@ async Task IRequestHandler MinimumLogLevel = LogLevel.Trace; } - _clientVersion = request.Capabilities.GetClientVersion(); + _clientVersion = request.Capabilities?.GetClientVersion() ?? ClientVersion.Lsp2; _serializer.SetClientCapabilities(_clientVersion.Value, request.Capabilities); var supportedCapabilities = new List(); @@ -312,8 +353,8 @@ async Task IRequestHandler await Task.WhenAll(_initializeDelegates.Select(c => c(this, request))); - var textDocumentCapabilities = ClientSettings.Capabilities.TextDocument; - var workspaceCapabilities = ClientSettings.Capabilities.Workspace; + var textDocumentCapabilities = ClientSettings.Capabilities?.TextDocument ?? new TextDocumentClientCapabilities(); + var workspaceCapabilities = ClientSettings.Capabilities?.Workspace ?? new WorkspaceClientCapabilities(); var ccp = new ClientCapabilityProvider(_collection); diff --git a/src/Server/LspRequestRouter.cs b/src/Server/LspRequestRouter.cs index 9d2505b83..5958c76a7 100644 --- a/src/Server/LspRequestRouter.cs +++ b/src/Server/LspRequestRouter.cs @@ -19,7 +19,7 @@ namespace OmniSharp.Extensions.LanguageServer.Server { - internal class LspRequestRouter : RequestRouterBase + internal class LspRequestRouter : RequestRouterBase, IRequestRouter { private readonly IEnumerable _collection; private readonly IEnumerable _handlerMatchers; @@ -68,6 +68,18 @@ private ILspHandlerDescriptor FindDescriptor(string method, JToken @params) return _handlerMatchers.SelectMany(strat => strat.FindHandler(paramsValue, lspHandlerDescriptors)).FirstOrDefault() ?? descriptor; } - + + IHandlerDescriptor IRequestRouter.GetDescriptor(Notification notification) => GetDescriptor(notification); + IHandlerDescriptor IRequestRouter.GetDescriptor(Request request) => GetDescriptor(request); + Task IRequestRouter.RouteNotification(IHandlerDescriptor descriptor, Notification notification, CancellationToken token) => + RouteNotification( + descriptor is ILspHandlerDescriptor d ? d : throw new Exception("This should really never happen, seriously, only hand this correct descriptors"), + notification, + token); + Task IRequestRouter.RouteRequest(IHandlerDescriptor descriptor, Request request, CancellationToken token) => + RouteRequest( + descriptor is ILspHandlerDescriptor d ? d : throw new Exception("This should really never happen, seriously, only hand this correct descriptors"), + request, + token); } } diff --git a/test/Lsp.Tests/LanguageServerTests.cs b/test/Lsp.Tests/LanguageServerTests.cs index b6dcdf70f..0ed57bb2f 100644 --- a/test/Lsp.Tests/LanguageServerTests.cs +++ b/test/Lsp.Tests/LanguageServerTests.cs @@ -2,10 +2,14 @@ using System.IO; using System.Threading; using System.Threading.Tasks; +using FluentAssertions; using Microsoft.Extensions.Logging; using NSubstitute; +using OmniSharp.Extensions.Embedded.MediatR; using OmniSharp.Extensions.LanguageServer.Client; using OmniSharp.Extensions.LanguageServer.Client.Processes; +using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; using OmniSharp.Extensions.LanguageServer.Protocol.Server; using OmniSharp.Extensions.LanguageServer.Server; using Xunit; @@ -19,7 +23,7 @@ public LanguageServerTests(ITestOutputHelper testOutputHelper) : base(testOutput { } - [Fact(Skip="Disabled to see if build passes on ci")] + [Fact(Skip = "Disabled to see if build passes on ci")] public async Task Works_With_IWorkspaceSymbolsHandler() { var process = new NamedPipeServerProcess(Guid.NewGuid().ToString("N"), LoggerFactory); @@ -50,5 +54,24 @@ await Task.WhenAll( var server = await serverStart; server.AddHandlers(handler); } + + [Fact] + public async Task GH141_CrashesWithEmptyInitializeParams() + { + var process = new NamedPipeServerProcess(Guid.NewGuid().ToString("N"), LoggerFactory); + await process.Start(); + var server = LanguageServer.PreInit(x => x + .WithInput(process.ClientOutputStream) + .WithOutput(process.ClientInputStream) + .WithLoggerFactory(LoggerFactory) + .AddDefaultLoggingProvider() + .WithMinimumLogLevel(LogLevel.Trace) + ) as IRequestHandler; + + var handler = server as IRequestHandler; + + Func a = async () => await handler.Handle(new InitializeParams() { }, CancellationToken.None); + a.Should().NotThrow(); + } } }