Merge pull request #65 from OmniSharp/fix/registration

Fixed an issue where handler descriptors would fail to be created of …

Author: david-driscoll

src/Server/HandlerCollection.cs

@@ -71,12 +71,12 @@ private HandlerDescriptor GetDescriptor(string method, Type implementedType, IJs
             }
 
             var key = "default";
-            if (handler is IRegistration<TextDocumentRegistrationOptions>)
+            // This protects against the case where class implements many, possibly conflicting, interfaces.
+            if (registration != null &&
+                typeof(TextDocumentRegistrationOptions).GetTypeInfo().IsAssignableFrom(registration) &&
+                handler is IRegistration<TextDocumentRegistrationOptions> handlerRegistration)
             {
-                if (GetTextDocumentRegistrationOptionsMethod
-                    .MakeGenericMethod(registration)
-                    .Invoke(handler, new object[] { handler }) is TextDocumentRegistrationOptions options)
-                    key = options.DocumentSelector;
+                key = handlerRegistration.GetRegistrationOptions()?.DocumentSelector ?? key;
             }
 
             return new HandlerDescriptor(
@@ -90,15 +90,6 @@ private HandlerDescriptor GetDescriptor(string method, Type implementedType, IJs
                 () => _handlers.RemoveWhere(instance => instance.Handler == handler));
         }
 
-        private static readonly MethodInfo GetTextDocumentRegistrationOptionsMethod = typeof(HandlerCollection).GetTypeInfo()
-            .GetMethod(nameof(GetTextDocumentRegistrationOptions), BindingFlags.Static | BindingFlags.NonPublic);
-
-        private static TextDocumentRegistrationOptions GetTextDocumentRegistrationOptions<T>(IRegistration<T> instance)
-            where T : TextDocumentRegistrationOptions
-        {
-            return instance.GetRegistrationOptions();
-        }
-
         private Type UnwrapGenericType(Type genericType, Type type)
         {
             return type?.GetTypeInfo()

test/Lsp.Tests/HandlerResolverTests.cs

@@ -11,6 +11,7 @@
 using OmniSharp.Extensions.LanguageServer.Protocol.Models;
 using Xunit;
 using HandlerCollection = OmniSharp.Extensions.LanguageServer.Server.HandlerCollection;
+using System.Collections.Generic;
 
 namespace Lsp.Tests
 {
@@ -133,7 +134,8 @@ public void Should_Contain_AllDefinedMethods_OnLanguageServer(Type requestHandle
             var sub = (IJsonRpcHandler)Substitute.For(new Type[] { requestHandler, type2 }, new object[0]);
             if (sub is IRegistration<TextDocumentRegistrationOptions> reg)
                 reg.GetRegistrationOptions()
-                    .Returns(new TextDocumentRegistrationOptions() {
+                    .Returns(new TextDocumentRegistrationOptions()
+                    {
                         DocumentSelector = new DocumentSelector()
                     });
             handler.Add(sub);
@@ -150,13 +152,15 @@ public void Should_Contain_AllDefinedMethods_OnLanguageServer_WithDifferentKeys(
             var sub = (IJsonRpcHandler)Substitute.For(new Type[] { requestHandler, type2 }, new object[0]);
             if (sub is IRegistration<TextDocumentRegistrationOptions> reg)
                 reg.GetRegistrationOptions()
-                    .Returns(new TextDocumentRegistrationOptions() {
+                    .Returns(new TextDocumentRegistrationOptions()
+                    {
                         DocumentSelector = new DocumentSelector()
                     });
             var sub2 = (IJsonRpcHandler)Substitute.For(new Type[] { requestHandler, type2 }, new object[0]);
             if (sub2 is IRegistration<TextDocumentRegistrationOptions> reg2)
                 reg2.GetRegistrationOptions()
-                    .Returns(new TextDocumentRegistrationOptions() {
+                    .Returns(new TextDocumentRegistrationOptions()
+                    {
                         DocumentSelector = new DocumentSelector()
                     });
             handler.Add(sub);
@@ -172,12 +176,31 @@ public void Should_AllowSpecificHandlers_ToBeAdded(string method, Type handlerTy
         {
             var handler = new HandlerCollection();
             var sub = (IJsonRpcHandler)Substitute.For(new Type[] { handlerType }, new object[0]);
-            var sub2= (IJsonRpcHandler)Substitute.For(new Type[] { handlerType }, new object[0]);
+            var sub2 = (IJsonRpcHandler)Substitute.For(new Type[] { handlerType }, new object[0]);
             handler.Add(method, sub);
             handler.Add(method, sub2);
             handler._handlers.Should().Contain(x => x.Method == method);
             handler._handlers.Should().Contain(x => x.Method == method);
             handler._handlers.Count.Should().Be(1);
         }
+
+        [Theory]
+        [MemberData(nameof(Should_DealWithClassesThatImplementMultipleHandlers_WithoutConflictingRegistrations_Data))]
+        public void Should_DealWithClassesThatImplementMultipleHandlers_WithoutConflictingRegistrations(string method, IJsonRpcHandler sub)
+        {
+            var handler = new HandlerCollection();
+            handler.Add(sub);
+
+            var descriptor = handler._handlers.First(x => x.Method == method);
+            descriptor.Key.Should().Be("default");
+        }
+
+        public static IEnumerable<object[]> Should_DealWithClassesThatImplementMultipleHandlers_WithoutConflictingRegistrations_Data()
+        {
+            yield return new object[] {
+                DocumentNames.CodeLensResolve,
+                Substitute.For(new Type[] { typeof(ICodeLensHandler), typeof(ICodeLensResolveHandler) }, new object[0])
+            };
+        }
     }
 }

test/Lsp.Tests/LspRequestRouterTests.cs

@@ -170,7 +170,7 @@ public async Task ShouldHandle_Request_WithNullParameters()
         {
             bool wasShutDown = false;
 
-            ShutdownHandler shutdownHandler = new ShutdownHandler();
+            var shutdownHandler = new ShutdownHandler();
             shutdownHandler.Shutdown += shutdownRequested =>
             {
                 wasShutDown = true;
@@ -194,7 +194,7 @@ public async Task ShouldHandle_Request_WithMissingParameters()
         {
             bool wasShutDown = false;
 
-            ShutdownHandler shutdownHandler = new ShutdownHandler();
+            var shutdownHandler = new ShutdownHandler();
             shutdownHandler.Shutdown += shutdownRequested =>
             {
                 wasShutDown = true;