Merge pull request #148 from OmniSharp/fix/capabilities

Updated support for capabilities to be more generous to clients that …

Author: david-driscoll

src/Server/ClientCapabilityProvider.cs

@@ -21,9 +21,10 @@ public ClientCapabilityProvider(IHandlerCollection collection)
         public bool HasStaticHandler<T>(Supports<T> capability)
             where T : DynamicCapability, ConnectedCapability<IJsonRpcHandler>
         {
-            if (!capability.IsSupported) return false;
-            if (capability.Value == null) return false;
-            if (capability.Value.DynamicRegistration == true) return false;
+            // Dynamic registration will cause us to double register things if we report our capabilities staticly.
+            // However if the client does not tell us it's capabilities we should just assume that they do not support
+            // dynamic registraiton but we should report any capabilities statically
+            if (capability.IsSupported && capability.Value != null && capability.Value.DynamicRegistration == true) return false;
 
             var handlerTypes = typeof(T).GetTypeInfo().ImplementedInterfaces
                 .Where(x => x.GetTypeInfo().IsGenericType && x.GetTypeInfo().GetGenericTypeDefinition() == typeof(ConnectedCapability<>))

test/Lsp.Tests/ClientCapabilityProviderTests.cs

@@ -9,6 +9,7 @@
 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.Extensions.LanguageServer.Protocol.Server.Capabilities;
 using OmniSharp.Extensions.LanguageServer.Server;
 using Xunit;
@@ -42,15 +43,24 @@ public static IEnumerable<object[]> AllowSupportedCapabilities()
             });
         }
 
-        [Theory, MemberData(nameof(DisallowUnsupportedCapabilities))]
-        public void Should_DisallowUnsupportedCapabilities(IJsonRpcHandler handler, object instance)
+        [Theory, MemberData(nameof(AllowUnsupportedCapabilities))]
+        public void Should_AllowUnsupportedCapabilities(IJsonRpcHandler handler, object instance)
         {
             var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));
 
             var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue) { textDocumentSyncHandler, handler };
             var provider = new ClientCapabilityProvider(collection);
 
-            HasHandler(provider, instance).Should().BeFalse();
+            HasHandler(provider, instance).Should().BeTrue();
+        }
+
+        public static IEnumerable<object[]> AllowUnsupportedCapabilities()
+        {
+            return GetItems(Capabilities, type => {
+                var handlerTypes = GetHandlerTypes(type);
+                var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
+                return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), false) };
+            });
         }
 
         [Fact]
@@ -81,17 +91,29 @@ public void Should_Invoke_Reduce_Delegate()
             stub.Received().Invoke(Arg.Any<IEnumerable<IExecuteCommandOptions>>());
         }
 
-        public static IEnumerable<object[]> DisallowUnsupportedCapabilities()
+        [Theory, MemberData(nameof(AllowNullSupportsCapabilities))]
+        public void Should_AllowNullSupportedCapabilities(IJsonRpcHandler handler, object instance)
+        {
+            var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));
+
+            var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue) { textDocumentSyncHandler, handler };
+            var provider = new ClientCapabilityProvider(collection);
+
+            HasHandler(provider, instance).Should().BeTrue();
+        }
+
+        public static IEnumerable<object[]> AllowNullSupportsCapabilities()
         {
             return GetItems(Capabilities, type => {
                 var handlerTypes = GetHandlerTypes(type);
                 var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
-                return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), false) };
+                return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), true) };
             });
         }
 
-        [Theory, MemberData(nameof(DisallowNullSupportsCapabilities))]
-        public void Should_DisallowNullSupportedCapabilities(IJsonRpcHandler handler, object instance)
+
+        [Theory, MemberData(nameof(DisallowDynamicSupportsCapabilities))]
+        public void Should_DisallowDynamicSupportedCapabilities(IJsonRpcHandler handler, object instance)
         {
             var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));
 
@@ -101,27 +123,49 @@ public void Should_DisallowNullSupportedCapabilities(IJsonRpcHandler handler, ob
             HasHandler(provider, instance).Should().BeFalse();
         }
 
-        public static IEnumerable<object[]> DisallowNullSupportsCapabilities()
+        public static IEnumerable<object[]> DisallowDynamicSupportsCapabilities()
         {
             return GetItems(Capabilities, type => {
                 var handlerTypes = GetHandlerTypes(type);
                 var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
-                return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), true) };
+                var capability = Activator.CreateInstance(type);
+                if (capability is DynamicCapability dyn) dyn.DynamicRegistration = true;
+                return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), true, capability) };
             });
         }
 
-        private static bool HasHandler(ClientCapabilityProvider provider, object instance)
+        [Fact]
+        public void Should_Handle_Mixed_Capabilities()
         {
-            return (bool)typeof(ClientCapabilityProviderTests).GetTypeInfo()
-                .GetMethod(nameof(GenericHasHandler), BindingFlags.Static | BindingFlags.NonPublic)
-                .MakeGenericMethod(instance.GetType().GetTypeInfo().GetGenericArguments()[0]).Invoke(null, new[] { provider, instance });
+            var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));
+
+            var codeActionHandler = Substitute.For<ICodeActionHandler>();
+            var definitionHandler = Substitute.For<IDefinitionHandler>();
+            var typeDefinitionHandler = Substitute.For<ITypeDefinitionHandler>();
+
+            var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue) { textDocumentSyncHandler, codeActionHandler, definitionHandler, typeDefinitionHandler };
+            var provider = new ClientCapabilityProvider(collection);
+            var capabilities = new ClientCapabilities() {
+                TextDocument = new TextDocumentClientCapabilities() {
+                    CodeAction = new Supports<CodeActionCapability>(true, new CodeActionCapability() {
+                        DynamicRegistration = false,
+                    }),
+                    TypeDefinition = new Supports<TypeDefinitionCapability>(true, new TypeDefinitionCapability() {
+                        DynamicRegistration = true,
+                    })
+                }
+            };
+
+            provider.GetStaticOptions(capabilities.TextDocument.CodeAction).Get<ICodeActionOptions, CodeActionOptions>(CodeActionOptions.Of).Should().NotBeNull();
+            provider.HasStaticHandler(capabilities.TextDocument.Definition).Should().BeTrue();
+            provider.HasStaticHandler(capabilities.TextDocument.TypeDefinition).Should().BeFalse();
         }
 
-        private static bool HasHandler(ClientCapabilityProvider provider, Type type)
+        private static bool HasHandler(ClientCapabilityProvider provider, object instance)
         {
             return (bool)typeof(ClientCapabilityProviderTests).GetTypeInfo()
                 .GetMethod(nameof(GenericHasHandler), BindingFlags.Static | BindingFlags.NonPublic)
-                .MakeGenericMethod(type).Invoke(null, new object[] { provider, null });
+                .MakeGenericMethod(instance.GetType().GetTypeInfo().GetGenericArguments()[0]).Invoke(null, new[] { provider, instance });
         }
 
         private static bool GenericHasHandler<T>(ClientCapabilityProvider provider, Supports<T> supports)

test/Lsp.Tests/LanguageServerTests.cs

@@ -66,6 +66,7 @@ public async Task GH141_CrashesWithEmptyInitializeParams()
                 .WithLoggerFactory(LoggerFactory)
                 .AddDefaultLoggingProvider()
                 .WithMinimumLogLevel(LogLevel.Trace)
+                .AddHandlers(TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs")))
             ) as IRequestHandler<InitializeParams, InitializeResult>;
 
             var handler = server as IRequestHandler<InitializeParams, InitializeResult>;