fixed did-change callback to ensure it's part of lsp request router

david-driscoll · david-driscoll · commit 69b540b475b0 · 2017-10-26T13:06:42.000-04:00
diff --git 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/Lsp/ClientCapabilityProvider.cs b/src/Lsp/ClientCapabilityProvider.cs
@@ -16,7 +16,7 @@ public ClientCapabilityProvider(IHandlerCollection collection)
             _collection = collection;
         }
 
-        public bool HasHandler<T>(Supports<T> capability)
+        public bool HasStaticHandler<T>(Supports<T> capability)
             where T : DynamicCapability, ConnectedCapability<IJsonRpcHandler>
         {
             if (!capability.IsSupported) return false;
@@ -26,13 +26,16 @@ public bool HasHandler<T>(Supports<T> 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<T>(Supports<T> capability)
+        public IOptionsGetter GetStaticOptions<T>(Supports<T> capability)
             where T : DynamicCapability, ConnectedCapability<IJsonRpcHandler>
         {
-            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
@@ -130,22 +130,22 @@ async Task<InitializeResult> IRequestHandler<InitializeParams, InitializeResult>
             var ccp = new ClientCapabilityProvider(_collection);
 
             var serverCapabilities = new ServerCapabilities() {
-                CodeActionProvider = ccp.HasHandler(textDocumentCapabilities.CodeAction),
-                CodeLensProvider = ccp.GetOptions(textDocumentCapabilities.CodeLens).Get<ICodeLensOptions, CodeLensOptions>(CodeLensOptions.Of),
-                CompletionProvider = ccp.GetOptions(textDocumentCapabilities.Completion).Get<ICompletionOptions, CompletionOptions>(CompletionOptions.Of),
-                DefinitionProvider = ccp.HasHandler(textDocumentCapabilities.Definition),
-                DocumentFormattingProvider = ccp.HasHandler(textDocumentCapabilities.Formatting),
-                DocumentHighlightProvider = ccp.HasHandler(textDocumentCapabilities.DocumentHighlight),
-                DocumentLinkProvider = ccp.GetOptions(textDocumentCapabilities.DocumentLink).Get<IDocumentLinkOptions, DocumentLinkOptions>(DocumentLinkOptions.Of),
-                DocumentOnTypeFormattingProvider = ccp.GetOptions(textDocumentCapabilities.OnTypeFormatting).Get<IDocumentOnTypeFormattingOptions, DocumentOnTypeFormattingOptions>(DocumentOnTypeFormattingOptions.Of),
-                DocumentRangeFormattingProvider = ccp.HasHandler(textDocumentCapabilities.RangeFormatting),
-                DocumentSymbolProvider = ccp.HasHandler(textDocumentCapabilities.DocumentSymbol),
-                ExecuteCommandProvider = ccp.GetOptions(workspaceCapabilities.ExecuteCommand).Get<IExecuteCommandOptions, ExecuteCommandOptions>(ExecuteCommandOptions.Of),
-                HoverProvider = ccp.HasHandler(textDocumentCapabilities.Hover),
-                ReferencesProvider = ccp.HasHandler(textDocumentCapabilities.References),
-                RenameProvider = ccp.HasHandler(textDocumentCapabilities.Rename),
-                SignatureHelpProvider = ccp.GetOptions(textDocumentCapabilities.SignatureHelp).Get<ISignatureHelpOptions, SignatureHelpOptions>(SignatureHelpOptions.Of),
-                WorkspaceSymbolProvider = ccp.HasHandler(workspaceCapabilities.Symbol)
+                CodeActionProvider = ccp.HasStaticHandler(textDocumentCapabilities.CodeAction),
+                CodeLensProvider = ccp.GetStaticOptions(textDocumentCapabilities.CodeLens).Get<ICodeLensOptions, CodeLensOptions>(CodeLensOptions.Of),
+                CompletionProvider = ccp.GetStaticOptions(textDocumentCapabilities.Completion).Get<ICompletionOptions, CompletionOptions>(CompletionOptions.Of),
+                DefinitionProvider = ccp.HasStaticHandler(textDocumentCapabilities.Definition),
+                DocumentFormattingProvider = ccp.HasStaticHandler(textDocumentCapabilities.Formatting),
+                DocumentHighlightProvider = ccp.HasStaticHandler(textDocumentCapabilities.DocumentHighlight),
+                DocumentLinkProvider = ccp.GetStaticOptions(textDocumentCapabilities.DocumentLink).Get<IDocumentLinkOptions, DocumentLinkOptions>(DocumentLinkOptions.Of),
+                DocumentOnTypeFormattingProvider = ccp.GetStaticOptions(textDocumentCapabilities.OnTypeFormatting).Get<IDocumentOnTypeFormattingOptions, DocumentOnTypeFormattingOptions>(DocumentOnTypeFormattingOptions.Of),
+                DocumentRangeFormattingProvider = ccp.HasStaticHandler(textDocumentCapabilities.RangeFormatting),
+                DocumentSymbolProvider = ccp.HasStaticHandler(textDocumentCapabilities.DocumentSymbol),
+                ExecuteCommandProvider = ccp.GetStaticOptions(workspaceCapabilities.ExecuteCommand).Get<IExecuteCommandOptions, ExecuteCommandOptions>(ExecuteCommandOptions.Of),
+                HoverProvider = ccp.HasStaticHandler(textDocumentCapabilities.Hover),
+                ReferencesProvider = ccp.HasStaticHandler(textDocumentCapabilities.References),
+                RenameProvider = ccp.HasStaticHandler(textDocumentCapabilities.Rename),
+                SignatureHelpProvider = ccp.GetStaticOptions(textDocumentCapabilities.SignatureHelp).Get<ISignatureHelpOptions, SignatureHelpOptions>(SignatureHelpOptions.Of),
+                WorkspaceSymbolProvider = ccp.HasStaticHandler(workspaceCapabilities.Symbol)
             };
 
             var textSyncHandlers = _collection
@@ -168,16 +168,21 @@ async Task<InitializeResult> IRequestHandler<InitializeParams, InitializeResult>
             }
             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
@@ -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<TextDocumentAttributes> 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<TextDocumentAttributes> attributes)
         {
             return attributes
@@ -105,6 +118,7 @@ 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;
                 }
             }
@@ -116,18 +130,25 @@ public async void 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<ErrorResponse> RouteRequest(Request request)
@@ -184,6 +205,11 @@ public async Task<ErrorResponse> 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
@@ -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
         /// </summary>
         public Container<TextDocumentContentChangeEvent> ContentChanges { get; set; }
     }
-}
+}
diff --git 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<T>(ClientCapabilityProvider provider, Supports<T> supports)
             where T : DynamicCapability, ConnectedCapability<IJsonRpcHandler>
         {
-            return provider.HasHandler(supports);
+            return provider.HasStaticHandler(supports);
         }
 
         private static IEnumerable<object[]> GetItems<T>(IEnumerable<T> types, Func<T, IEnumerable<object>> func)
diff --git 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')
         },
     }
 

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-using System;`
	`1`	`+using System;`
`2`	`2`	`using System.Collections.Concurrent;`
`3`	`3`	`using System.Collections.Generic;`
`4`	`4`	`using System.Threading;`
`@@ -128,7 +128,7 @@ public void Dispose()`
`128`	`128`	`}`
`129`	`129`	`}`
`130`	`130`
`131`		`- static class Events`
	`131`	`+ public static class Events`
`132`	`132`	`{`
`133`	`133`	`public static EventId UnhandledException = new EventId(1337_100);`
`134`	`134`	`public static EventId UnhandledRequest = new EventId(1337_101);`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ public ClientCapabilityProvider(IHandlerCollection collection)`
`16`	`16`	`_collection = collection;`
`17`	`17`	`}`
`18`	`18`
`19`		`- public bool HasHandler<T>(Supports<T> capability)`
	`19`	`+ public bool HasStaticHandler<T>(Supports<T> capability)`
`20`	`20`	`where T : DynamicCapability, ConnectedCapability<IJsonRpcHandler>`
`21`	`21`	`{`
`22`	`22`	`if (!capability.IsSupported) return false;`
`@@ -26,13 +26,16 @@ public bool HasHandler<T>(Supports<T> capability)`
`26`	`26`	`var handlerType = typeof(T).GetTypeInfo().ImplementedInterfaces`
`27`	`27`	`.Single(x => x.GetTypeInfo().IsGenericType && x.GetTypeInfo().GetGenericTypeDefinition() == typeof(ConnectedCapability<>))`
`28`	`28`	`.GetTypeInfo().GetGenericArguments()[0].GetTypeInfo();`
`29`		`- return !capability.Value.DynamicRegistration && _collection.Any(z => z.HandlerType.GetTypeInfo().IsAssignableFrom(handlerType));`
	`29`	`+ return !capability.Value.DynamicRegistration &&`
	`30`	`+ _collection.Any(z =>`
	`31`	`+ z.HandlerType.GetTypeInfo().IsAssignableFrom(handlerType) \|\|`
	`32`	`+ z.Handler.GetType().GetTypeInfo().IsAssignableFrom(handlerType));`
`30`	`33`	`}`
`31`	`34`
`32`		`- public IOptionsGetter GetOptions<T>(Supports<T> capability)`
	`35`	`+ public IOptionsGetter GetStaticOptions<T>(Supports<T> capability)`
`33`	`36`	`where T : DynamicCapability, ConnectedCapability<IJsonRpcHandler>`
`34`	`37`	`{`
`35`		`- return !HasHandler(capability) ? Null : new OptionsGetter(_collection);`
	`38`	`+ return !HasStaticHandler(capability) ? Null : new OptionsGetter(_collection);`
`36`	`39`	`}`
`37`	`40`
`38`	`41`	`private static readonly IOptionsGetter Null = new NullOptionsGetter();`
Original file line number	Diff line number	Diff line change
`@@ -111,7 +111,7 @@ private static bool HasHandler(ClientCapabilityProvider provider, Type type)`
`111`	`111`	`private static bool GenericHasHandler<T>(ClientCapabilityProvider provider, Supports<T> supports)`
`112`	`112`	`where T : DynamicCapability, ConnectedCapability<IJsonRpcHandler>`
`113`	`113`	`{`
`114`		`- return provider.HasHandler(supports);`
	`114`	`+ return provider.HasStaticHandler(supports);`
`115`	`115`	`}`
`116`	`116`
`117`	`117`	`private static IEnumerable<object[]> GetItems<T>(IEnumerable<T> types, Func<T, IEnumerable<object>> func)`