diff --git a/src/JsonRpc/IRequestRouter.cs b/src/JsonRpc/IRequestRouter.cs index 6805c5573..99bc56896 100644 --- a/src/JsonRpc/IRequestRouter.cs +++ b/src/JsonRpc/IRequestRouter.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Threading.Tasks; using OmniSharp.Extensions.JsonRpc.Server; @@ -6,7 +6,7 @@ namespace OmniSharp.Extensions.JsonRpc { public interface IRequestRouter { - void RouteNotification(Notification notification); + Task RouteNotification(Notification notification); Task RouteRequest(Request request); } } diff --git a/src/JsonRpc/InputHandler.cs b/src/JsonRpc/InputHandler.cs index 1849b74c7..a2ab890e1 100644 --- a/src/JsonRpc/InputHandler.cs +++ b/src/JsonRpc/InputHandler.cs @@ -190,10 +190,10 @@ private void HandleRequest(string request) _scheduler.Add( type, item.Notification.Method, - () => { + async () => { try { - _requestRouter.RouteNotification(item.Notification); + await _requestRouter.RouteNotification(item.Notification); } catch (Exception e) { @@ -202,7 +202,6 @@ private void HandleRequest(string request) // If an exception happens... the whole system could be in a bad state, hence this throwing currently. throw; } - return Task.CompletedTask; } ); } diff --git a/src/JsonRpc/ProcessScheduler.cs b/src/JsonRpc/ProcessScheduler.cs index 1fb1fc6b4..2ddddb2e4 100644 --- a/src/JsonRpc/ProcessScheduler.cs +++ b/src/JsonRpc/ProcessScheduler.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Threading; @@ -128,7 +128,7 @@ public void Dispose() } } - static class Events + public static class Events { public static EventId UnhandledException = new EventId(1337_100); public static EventId UnhandledRequest = new EventId(1337_101); diff --git a/src/JsonRpc/RequestRouter.cs b/src/JsonRpc/RequestRouter.cs index 44e8c3aa8..7d9441ad5 100644 --- a/src/JsonRpc/RequestRouter.cs +++ b/src/JsonRpc/RequestRouter.cs @@ -22,7 +22,7 @@ public IDisposable Add(IJsonRpcHandler handler) return _collection.Add(handler); } - public async void RouteNotification(Notification notification) + public async Task RouteNotification(Notification notification) { var handler = _collection.FirstOrDefault(x => x.Method == notification.Method); diff --git a/src/Lsp/ClientCapabilityProvider.cs b/src/Lsp/ClientCapabilityProvider.cs index e131135d2..f4de50a82 100644 --- a/src/Lsp/ClientCapabilityProvider.cs +++ b/src/Lsp/ClientCapabilityProvider.cs @@ -16,7 +16,7 @@ public ClientCapabilityProvider(IHandlerCollection collection) _collection = collection; } - public bool HasHandler(Supports capability) + public bool HasStaticHandler(Supports capability) where T : DynamicCapability, ConnectedCapability { if (!capability.IsSupported) return false; @@ -26,13 +26,16 @@ public bool HasHandler(Supports capability) var handlerType = typeof(T).GetTypeInfo().ImplementedInterfaces .Single(x => x.GetTypeInfo().IsGenericType && x.GetTypeInfo().GetGenericTypeDefinition() == typeof(ConnectedCapability<>)) .GetTypeInfo().GetGenericArguments()[0].GetTypeInfo(); - return !capability.Value.DynamicRegistration && _collection.Any(z => z.HandlerType.GetTypeInfo().IsAssignableFrom(handlerType)); + return !capability.Value.DynamicRegistration && + _collection.Any(z => + z.HandlerType.GetTypeInfo().IsAssignableFrom(handlerType) || + z.Handler.GetType().GetTypeInfo().IsAssignableFrom(handlerType)); } - public IOptionsGetter GetOptions(Supports capability) + public IOptionsGetter GetStaticOptions(Supports capability) where T : DynamicCapability, ConnectedCapability { - return !HasHandler(capability) ? Null : new OptionsGetter(_collection); + return !HasStaticHandler(capability) ? Null : new OptionsGetter(_collection); } private static readonly IOptionsGetter Null = new NullOptionsGetter(); diff --git a/src/Lsp/LanguageServer.cs b/src/Lsp/LanguageServer.cs index ba803ec7d..923fa0535 100644 --- a/src/Lsp/LanguageServer.cs +++ b/src/Lsp/LanguageServer.cs @@ -130,22 +130,22 @@ async Task IRequestHandler var ccp = new ClientCapabilityProvider(_collection); var serverCapabilities = new ServerCapabilities() { - CodeActionProvider = ccp.HasHandler(textDocumentCapabilities.CodeAction), - CodeLensProvider = ccp.GetOptions(textDocumentCapabilities.CodeLens).Get(CodeLensOptions.Of), - CompletionProvider = ccp.GetOptions(textDocumentCapabilities.Completion).Get(CompletionOptions.Of), - DefinitionProvider = ccp.HasHandler(textDocumentCapabilities.Definition), - DocumentFormattingProvider = ccp.HasHandler(textDocumentCapabilities.Formatting), - DocumentHighlightProvider = ccp.HasHandler(textDocumentCapabilities.DocumentHighlight), - DocumentLinkProvider = ccp.GetOptions(textDocumentCapabilities.DocumentLink).Get(DocumentLinkOptions.Of), - DocumentOnTypeFormattingProvider = ccp.GetOptions(textDocumentCapabilities.OnTypeFormatting).Get(DocumentOnTypeFormattingOptions.Of), - DocumentRangeFormattingProvider = ccp.HasHandler(textDocumentCapabilities.RangeFormatting), - DocumentSymbolProvider = ccp.HasHandler(textDocumentCapabilities.DocumentSymbol), - ExecuteCommandProvider = ccp.GetOptions(workspaceCapabilities.ExecuteCommand).Get(ExecuteCommandOptions.Of), - HoverProvider = ccp.HasHandler(textDocumentCapabilities.Hover), - ReferencesProvider = ccp.HasHandler(textDocumentCapabilities.References), - RenameProvider = ccp.HasHandler(textDocumentCapabilities.Rename), - SignatureHelpProvider = ccp.GetOptions(textDocumentCapabilities.SignatureHelp).Get(SignatureHelpOptions.Of), - WorkspaceSymbolProvider = ccp.HasHandler(workspaceCapabilities.Symbol) + CodeActionProvider = ccp.HasStaticHandler(textDocumentCapabilities.CodeAction), + CodeLensProvider = ccp.GetStaticOptions(textDocumentCapabilities.CodeLens).Get(CodeLensOptions.Of), + CompletionProvider = ccp.GetStaticOptions(textDocumentCapabilities.Completion).Get(CompletionOptions.Of), + DefinitionProvider = ccp.HasStaticHandler(textDocumentCapabilities.Definition), + DocumentFormattingProvider = ccp.HasStaticHandler(textDocumentCapabilities.Formatting), + DocumentHighlightProvider = ccp.HasStaticHandler(textDocumentCapabilities.DocumentHighlight), + DocumentLinkProvider = ccp.GetStaticOptions(textDocumentCapabilities.DocumentLink).Get(DocumentLinkOptions.Of), + DocumentOnTypeFormattingProvider = ccp.GetStaticOptions(textDocumentCapabilities.OnTypeFormatting).Get(DocumentOnTypeFormattingOptions.Of), + DocumentRangeFormattingProvider = ccp.HasStaticHandler(textDocumentCapabilities.RangeFormatting), + DocumentSymbolProvider = ccp.HasStaticHandler(textDocumentCapabilities.DocumentSymbol), + ExecuteCommandProvider = ccp.GetStaticOptions(workspaceCapabilities.ExecuteCommand).Get(ExecuteCommandOptions.Of), + HoverProvider = ccp.HasStaticHandler(textDocumentCapabilities.Hover), + ReferencesProvider = ccp.HasStaticHandler(textDocumentCapabilities.References), + RenameProvider = ccp.HasStaticHandler(textDocumentCapabilities.Rename), + SignatureHelpProvider = ccp.GetStaticOptions(textDocumentCapabilities.SignatureHelp).Get(SignatureHelpOptions.Of), + WorkspaceSymbolProvider = ccp.HasStaticHandler(workspaceCapabilities.Symbol) }; var textSyncHandlers = _collection @@ -168,16 +168,21 @@ async Task IRequestHandler } else { - if (ccp.HasHandler(textDocumentCapabilities.Synchronization)) + if (ccp.HasStaticHandler(textDocumentCapabilities.Synchronization)) { // TODO: Merge options - serverCapabilities.TextDocumentSync = textSyncHandlers.FirstOrDefault()?.Options ?? new TextDocumentSyncOptions() { - Change = TextDocumentSyncKind.None, - OpenClose = false, - Save = new SaveOptions() { IncludeText = false }, - WillSave = false, - WillSaveWaitUntil = false - }; + serverCapabilities.TextDocumentSync = + textSyncHandlers.FirstOrDefault()?.Options ?? new TextDocumentSyncOptions() { + Change = TextDocumentSyncKind.None, + OpenClose = false, + Save = new SaveOptions() { IncludeText = false }, + WillSave = false, + WillSaveWaitUntil = false + }; + } + else + { + serverCapabilities.TextDocumentSync = TextDocumentSyncKind.None; } } diff --git a/src/Lsp/LspRequestRouter.cs b/src/Lsp/LspRequestRouter.cs index 656141805..08d429208 100644 --- a/src/Lsp/LspRequestRouter.cs +++ b/src/Lsp/LspRequestRouter.cs @@ -57,35 +57,48 @@ private ILspHandlerDescriptor FindDescriptor(string method, JToken @params) if (typeof(ITextDocumentIdentifierParams).GetTypeInfo().IsAssignableFrom(descriptor.Params)) { var textDocumentIdentifierParams = @params.ToObject(descriptor.Params) as ITextDocumentIdentifierParams; - var textDocumentSyncHandlers = _collection - .Select(x => x.Handler is ITextDocumentSyncHandler r ? r : null) - .Where(x => x != null) - .Distinct(); - var attributes = textDocumentSyncHandlers - .Select(x => x.GetTextDocumentAttributes(textDocumentIdentifierParams.TextDocument.Uri)) - .Where(x => x != null) - .Distinct() - .ToList(); + var attributes = GetTextDocumentAttributes(textDocumentIdentifierParams.TextDocument.Uri); _logger.LogTrace("Found attributes {Count}, {Attributes}", attributes.Count, attributes.Select(x => $"{x.LanguageId}:{x.Scheme}:{x.Uri}")); return GetHandler(method, attributes); } - else if (typeof(DidOpenTextDocumentParams).GetTypeInfo().IsAssignableFrom(descriptor.Params)) + else if (@params?.ToObject(descriptor.Params) is DidOpenTextDocumentParams openTextDocumentParams) { - var openTextDocumentParams = @params.ToObject(descriptor.Params) as DidOpenTextDocumentParams; var attributes = new TextDocumentAttributes(openTextDocumentParams.TextDocument.Uri, openTextDocumentParams.TextDocument.LanguageId); _logger.LogTrace("Created attribute {Attribute}", $"{attributes.LanguageId}:{attributes.Scheme}:{attributes.Uri}"); return GetHandler(method, attributes); } + else if (@params?.ToObject(descriptor.Params) is DidChangeTextDocumentParams didChangeDocumentParams) + { + // TODO: Do something with document version here? + var attributes = GetTextDocumentAttributes(didChangeDocumentParams.TextDocument.Uri); + + _logger.LogTrace("Found attributes {Count}, {Attributes}", attributes.Count, attributes.Select(x => $"{x.LanguageId}:{x.Scheme}:{x.Uri}")); + + return GetHandler(method, attributes); + } // TODO: How to split these // Do they fork and join? return descriptor; } + private List GetTextDocumentAttributes(Uri uri) + { + var textDocumentSyncHandlers = _collection + .Select(x => x.Handler is ITextDocumentSyncHandler r ? r : null) + .Where(x => x != null) + .Distinct(); + return textDocumentSyncHandlers + .Select(x => x.GetTextDocumentAttributes(uri)) + .Where(x => x != null) + .Distinct() + .ToList(); + } + private ILspHandlerDescriptor GetHandler(string method, IEnumerable attributes) { return attributes @@ -105,29 +118,37 @@ private ILspHandlerDescriptor GetHandler(string method, TextDocumentAttributes a _logger.LogTrace("Document Selector {DocumentSelector}", registrationOptions.DocumentSelector.ToString()); if (registrationOptions.DocumentSelector == null || registrationOptions.DocumentSelector.IsMatch(attributes)) { + _logger.LogTrace("Handler Selected: {Handler} via {DocumentSelector} (targeting {HandlerInterface})", handler.Handler.GetType().FullName, registrationOptions.DocumentSelector.ToString(), handler.HandlerType.GetType().FullName); return handler; } } return null; } - public async void RouteNotification(Notification notification) + public async Task RouteNotification(Notification notification) { var handler = FindDescriptor(notification.Method, notification.Params); if (handler is null) { return; } - Task result; - if (handler.Params is null) + try { - result = ReflectionRequestHandlers.HandleNotification(handler); + Task result; + if (handler.Params is null) + { + result = ReflectionRequestHandlers.HandleNotification(handler); + } + else + { + var @params = notification.Params.ToObject(handler.Params); + result = ReflectionRequestHandlers.HandleNotification(handler, @params); + } + + await result; } - else + catch (Exception e) { - var @params = notification.Params.ToObject(handler.Params); - result = ReflectionRequestHandlers.HandleNotification(handler, @params); + _logger.LogCritical(Events.UnhandledRequest, e, "Failed to handle request {Method}", notification.Method); } - - await result; } public async Task RouteRequest(Request request) @@ -184,6 +205,11 @@ public async Task RouteRequest(Request request) { return new RequestCancelled(); } + catch (Exception e) + { + _logger.LogCritical(Events.UnhandledRequest, e, "Failed to handle notification {Method}", request.Method); + return new InternalError(id); + } finally { _requests.TryRemove(id, out var _); diff --git a/src/Lsp/Models/DidChangeTextDocumentParams.cs b/src/Lsp/Models/DidChangeTextDocumentParams.cs index bc5827ca4..d9ec891d0 100644 --- a/src/Lsp/Models/DidChangeTextDocumentParams.cs +++ b/src/Lsp/Models/DidChangeTextDocumentParams.cs @@ -1,4 +1,4 @@ -using Newtonsoft.Json; +using Newtonsoft.Json; using Newtonsoft.Json.Serialization; namespace OmniSharp.Extensions.LanguageServer.Models @@ -18,4 +18,4 @@ public class DidChangeTextDocumentParams /// public Container ContentChanges { get; set; } } -} \ No newline at end of file +} diff --git a/test/JsonRpc.Tests/InputHandlerTests.cs b/test/JsonRpc.Tests/InputHandlerTests.cs index 3af28e8e6..61c7f617c 100644 --- a/test/JsonRpc.Tests/InputHandlerTests.cs +++ b/test/JsonRpc.Tests/InputHandlerTests.cs @@ -206,7 +206,7 @@ public void ShouldHandleError() } [Fact] - public void ShouldHandleNotification() + public async Task ShouldHandleNotification() { var inputStream = new MemoryStream(Encoding.ASCII.GetBytes("Content-Length: 2\r\n\r\n{}")); var outputHandler = Substitute.For(); @@ -234,7 +234,7 @@ public void ShouldHandleNotification() }); })) { - incomingRequestRouter.Received().RouteNotification(notification); + await incomingRequestRouter.Received().RouteNotification(notification); } } diff --git a/test/JsonRpc.Tests/MediatorTestsNotificationHandler.cs b/test/JsonRpc.Tests/MediatorTestsNotificationHandler.cs index 4ac9ba493..fddc2c0b6 100644 --- a/test/JsonRpc.Tests/MediatorTestsNotificationHandler.cs +++ b/test/JsonRpc.Tests/MediatorTestsNotificationHandler.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Reflection; using System.Threading.Tasks; using NSubstitute; @@ -23,10 +23,10 @@ public async Task ExecutesHandler() var notification = new Notification("exit", null); - mediator.RouteNotification(notification); + await mediator.RouteNotification(notification); await exitHandler.Received(1).Handle(); } } -} \ No newline at end of file +} diff --git a/test/JsonRpc.Tests/MediatorTestsNotificationHandlerOfT.cs b/test/JsonRpc.Tests/MediatorTestsNotificationHandlerOfT.cs index e47e4af06..6c0a54a77 100644 --- a/test/JsonRpc.Tests/MediatorTestsNotificationHandlerOfT.cs +++ b/test/JsonRpc.Tests/MediatorTestsNotificationHandlerOfT.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Reflection; using System.Threading.Tasks; using Newtonsoft.Json; @@ -33,10 +33,10 @@ public async Task ExecutesHandler() var @params = new CancelParams() { Id = Guid.NewGuid() }; var notification = new Notification("$/cancelRequest", JObject.Parse(JsonConvert.SerializeObject(@params))); - mediator.RouteNotification(notification); + await mediator.RouteNotification(notification); await cancelRequestHandler.Received(1).Handle(Arg.Any()); } } -} \ No newline at end of file +} diff --git a/test/Lsp.Tests/ClientCapabilityProviderTests.cs b/test/Lsp.Tests/ClientCapabilityProviderTests.cs index 8391de310..25cc38557 100644 --- a/test/Lsp.Tests/ClientCapabilityProviderTests.cs +++ b/test/Lsp.Tests/ClientCapabilityProviderTests.cs @@ -111,7 +111,7 @@ private static bool HasHandler(ClientCapabilityProvider provider, Type type) private static bool GenericHasHandler(ClientCapabilityProvider provider, Supports supports) where T : DynamicCapability, ConnectedCapability { - return provider.HasHandler(supports); + return provider.HasStaticHandler(supports); } private static IEnumerable GetItems(IEnumerable types, Func> func) diff --git a/test/Lsp.Tests/LspRequestRouterTests.cs b/test/Lsp.Tests/LspRequestRouterTests.cs index 4c506c3da..40918b144 100644 --- a/test/Lsp.Tests/LspRequestRouterTests.cs +++ b/test/Lsp.Tests/LspRequestRouterTests.cs @@ -30,7 +30,7 @@ public LspRequestRouterTests(ITestOutputHelper testOutputHelper) } [Fact] - public void ShouldRouteToCorrect_Notification() + public async Task ShouldRouteToCorrect_Notification() { var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs")); textDocumentSyncHandler.Handle(Arg.Any()).Returns(Task.CompletedTask); @@ -44,13 +44,13 @@ public void ShouldRouteToCorrect_Notification() var request = new Notification("textDocument/didSave", JObject.Parse(JsonConvert.SerializeObject(@params))); - mediator.RouteNotification(request); + await mediator.RouteNotification(request); - textDocumentSyncHandler.Received(1).Handle(Arg.Any()); + await textDocumentSyncHandler.Received(1).Handle(Arg.Any()); } [Fact] - public void ShouldRouteToCorrect_Notification_WithManyHandlers() + public async Task ShouldRouteToCorrect_Notification_WithManyHandlers() { var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs")); var textDocumentSyncHandler2 = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cake")); @@ -66,10 +66,10 @@ public void ShouldRouteToCorrect_Notification_WithManyHandlers() var request = new Notification("textDocument/didSave", JObject.Parse(JsonConvert.SerializeObject(@params))); - mediator.RouteNotification(request); + await mediator.RouteNotification(request); - textDocumentSyncHandler.Received(0).Handle(Arg.Any()); - textDocumentSyncHandler2.Received(1).Handle(Arg.Any()); + await textDocumentSyncHandler.Received(0).Handle(Arg.Any()); + await textDocumentSyncHandler2.Received(1).Handle(Arg.Any()); } [Fact] diff --git a/vscode-testextension/src/extension.ts b/vscode-testextension/src/extension.ts index 0f8e92aaa..575d340a7 100644 --- a/vscode-testextension/src/extension.ts +++ b/vscode-testextension/src/extension.ts @@ -13,7 +13,7 @@ import { Trace } from 'vscode-jsonrpc'; export function activate(context: ExtensionContext) { // The server is implemented in node - let serverExe = 'D:/Development/Omnisharp/omnisharp-roslyn/bin/Debug/OmniSharp.Stdio/net46/OmniSharp.exe'; + let serverExe = 'C:/Other/omnisharp-roslyn/bin/Debug/OmniSharp.Stdio/net46/OmniSharp.exe'; // let serverExe = 'D:/Development/Omnisharp/omnisharp-roslyn/artifacts/publish/OmniSharp.Stdio/win7-x64/OmniSharp.exe'; // let serverExe = context.asAbsolutePath('D:/Development/Omnisharp/omnisharp-roslyn/artifacts/publish/OmniSharp.Stdio/win7-x64/OmniSharp.exe'); // The debug options for the server @@ -39,8 +39,7 @@ export function activate(context: ExtensionContext) { synchronize: { // Synchronize the setting section 'languageServerExample' to the server configurationSection: 'languageServerExample', - // Notify the server about file changes to '.clientrc files contain in the workspace - fileEvents: workspace.createFileSystemWatcher('**/.clientrc') + fileEvents: workspace.createFileSystemWatcher('**/*.cs') }, }