Merge pull request #156 from OmniSharp/fix-registration

david-driscoll · web-flow · commit 2c88d7cebd43 · 2019-07-19T19:37:36.000-04:00
Move registration options to the descriptor
diff --git a/src/JsonRpc/JsonRpc.csproj b/src/JsonRpc/JsonRpc.csproj
@@ -1,4 +1,4 @@
-﻿<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
     <PropertyGroup>
         <TargetFrameworks>netstandard2.0</TargetFrameworks>
         <PlatformTarget>AnyCPU</PlatformTarget>
@@ -13,7 +13,7 @@
         <PackageReference Include="Microsoft.Extensions.DependencyInjection" />
         <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" />
         <PackageReference Include="Newtonsoft.Json" />
-        <Compile Include="../../submodules/MediatR/src/MediatR/**/*.cs" Exclude="**/AssemblyInfo.cs" Visible="false" />
-        <Compile Include="../../submodules/MediatR.Extensions.Microsoft.DependencyInjection/src/MediatR.Extensions.Microsoft.DependencyInjection/**/*.cs" Exclude="**/AssemblyInfo.cs" Visible="false" />
+        <Compile Include="../../submodules/MediatR/src/MediatR/**/*.cs" Exclude="**/*AssemblyInfo.cs" Visible="false" />
+        <Compile Include="../../submodules/MediatR.Extensions.Microsoft.DependencyInjection/src/MediatR.Extensions.Microsoft.DependencyInjection/**/*.cs" Exclude="**/*AssemblyInfo.cs" Visible="false" />
     </ItemGroup>
 </Project>
diff --git a/src/JsonRpc/RequestRouterBase.cs b/src/JsonRpc/RequestRouterBase.cs
@@ -65,7 +65,7 @@ public async Task RouteNotification(TDescriptor descriptor, Notification notific
                                 @params = notification.Params?.ToObject(descriptor.Params, _serializer.JsonSerializer);
                             }
 
-                            await HandleNotification(mediator, descriptor, @params ?? EmptyRequest.Instance, token);
+                            await HandleNotification(mediator, descriptor, @params ?? Activator.CreateInstance(descriptor.Params), token);
                         }
                     }
                     catch (Exception e)
diff --git a/src/Protocol/Models/Registration.cs b/src/Protocol/Models/Registration.cs
@@ -1,4 +1,4 @@
-﻿using Newtonsoft.Json;
+using Newtonsoft.Json;
 using Newtonsoft.Json.Serialization;
 using OmniSharp.Extensions.LanguageServer.Protocol.Serialization;
 
diff --git a/src/Server/Abstractions/ILspHandlerDescriptor.cs b/src/Server/Abstractions/ILspHandlerDescriptor.cs
@@ -6,9 +6,11 @@ namespace OmniSharp.Extensions.LanguageServer.Server.Abstractions
 {
     public interface ILspHandlerDescriptor : IHandlerDescriptor
     {
+        Guid Id { get; }
         bool HasRegistration { get; }
         Type RegistrationType { get; }
-        Registration Registration { get; }
+        object RegistrationOptions { get; }
+        bool AllowsDynamicRegistration { get; }
 
         bool HasCapability { get; }
         Type CapabilityType { get; }
diff --git a/src/Server/ClientCapabilityProvider.cs b/src/Server/ClientCapabilityProvider.cs
@@ -92,29 +92,29 @@ public TOptions Get<TInterface, TOptions>(Func<TInterface, TOptions> action)
                 where TOptions : class
             {
                 return _collection
-                    .Select(x => x.Registration?.RegisterOptions is TInterface cl ? action(cl) : null)
+                    .Select(x => x.RegistrationOptions is TInterface cl ? action(cl) : null)
                     .FirstOrDefault(x => x != null);
             }
 
             public Supports<TOptions> Can<TInterface, TOptions>(Func<TInterface, TOptions> action)
                 where TOptions : class
             {
                 var options = _collection
-                    .Select(x => x.Registration?.RegisterOptions is TInterface cl ? action(cl) : null)
+                    .Select(x => x.RegistrationOptions is TInterface cl ? action(cl) : null)
                     .FirstOrDefault(x => x != null);
                 if (options == null)
                     return Supports.OfBoolean<TOptions>(false);
 
                 return _collection
-                    .Select(x => x.Registration?.RegisterOptions is TInterface cl ? action(cl) : null)
+                    .Select(x => x.RegistrationOptions is TInterface cl ? action(cl) : null)
                     .FirstOrDefault(x => x != null);
             }
 
             public TOptions Reduce<TInterface, TOptions>(Func<IEnumerable<TInterface>, TOptions> action)
                 where TOptions : class
             {
                 return action(_collection
-                    .Select(x => x.Registration?.RegisterOptions is TInterface cl ? cl : default)
+                    .Select(x => x.RegistrationOptions is TInterface cl ? cl : default)
                     .Where(x => x != null));
             }
         }
diff --git a/src/Server/HandlerCollection.cs b/src/Server/HandlerCollection.cs
@@ -145,7 +145,6 @@ private HandlerDescriptor GetDescriptor(string method, Type handlerType, IJsonRp
 
             Type @params = null;
             object registrationOptions = null;
-            Registration registration = null;
             if (@interface.GetTypeInfo().IsGenericType)
             {
                 @params = @interface.GetTypeInfo().GetGenericArguments()[0];
@@ -156,15 +155,6 @@ private HandlerDescriptor GetDescriptor(string method, Type handlerType, IJsonRp
                 registrationOptions = GetRegistrationMethod
                     .MakeGenericMethod(registrationType)
                     .Invoke(null, new object[] { handler });
-
-                if (_supportedCapabilities.AllowsDynamicRegistration(capabilityType))
-                {
-                    registration = new Registration() {
-                        Id = Guid.NewGuid().ToString(),
-                        Method = method,
-                        RegisterOptions = registrationOptions
-                    };
-                }
             }
 
             var key = "default";
@@ -193,7 +183,8 @@ private HandlerDescriptor GetDescriptor(string method, Type handlerType, IJsonRp
                 @interface,
                 @params,
                 registrationType,
-                registration,
+                registrationOptions,
+                registrationType != null && _supportedCapabilities.AllowsDynamicRegistration(capabilityType),
                 capabilityType,
                 () => {
                     _handlers.RemoveWhere(d => d.Handler == handler);
diff --git a/src/Server/HandlerDescriptor.cs b/src/Server/HandlerDescriptor.cs
@@ -22,11 +22,13 @@ public HandlerDescriptor(
             Type handlerType,
             Type @params,
             Type registrationType,
-            Registration registration,
+            object registrationOptions,
+            bool allowsDynamicRegistration,
             Type capabilityType,
             Action disposeAction)
         {
             _disposeAction = disposeAction;
+            Id = Guid.NewGuid();
             Method = method;
             Key = key;
             ImplementationType = handler.GetType();
@@ -35,7 +37,8 @@ public HandlerDescriptor(
             Params = @params;
             Response = Response;
             RegistrationType = registrationType;
-            Registration = registration;
+            RegistrationOptions = registrationOptions;
+            AllowsDynamicRegistration = allowsDynamicRegistration;
             CapabilityType = capabilityType;
 
             var requestInterface = @params?.GetInterfaces()
@@ -63,9 +66,11 @@ public HandlerDescriptor(
         public Type ImplementationType { get; }
         public Type HandlerType { get; }
 
+        public Guid Id { get; }
         public bool HasRegistration => RegistrationType != null;
         public Type RegistrationType { get; }
-        public Registration Registration { get; }
+        public object RegistrationOptions { get; }
+        public bool AllowsDynamicRegistration { get; }
 
         public bool HasCapability => CapabilityType != null;
         public Type CapabilityType { get; }
diff --git a/src/Server/ISupportedCapabilities.cs b/src/Server/ISupportedCapabilities.cs
@@ -6,7 +6,6 @@ namespace OmniSharp.Extensions.LanguageServer.Server
 {
     public interface ISupportedCapabilities
     {
-        bool AllowsDynamicRegistration(ILspHandlerDescriptor descriptor);
         bool AllowsDynamicRegistration(Type capabilityType);
         void SetCapability(ILspHandlerDescriptor descriptor, IJsonRpcHandler handler);
     }
diff --git a/src/Server/LanguageServer.cs b/src/Server/LanguageServer.cs
@@ -328,8 +328,12 @@ private IDisposable RegisterHandlers(LspHandlerDescriptorDisposable handlerDispo
             using (var scope = _serviceProvider.CreateScope())
             {
                 var registrations = handlerDisposable.Descriptors
-                    .Select(x => x.Registration)
-                    .Where(x => x != null)
+                    .Where(d => d.AllowsDynamicRegistration)
+                    .Select(d => new Registration() {
+                        Id = d.Id.ToString(),
+                        Method = d.Method,
+                        RegisterOptions = d.RegistrationOptions
+                    })
                     .ToArray();
 
                 // Fire and forget
diff --git a/src/Server/Matchers/ExecuteCommandMatcher.cs b/src/Server/Matchers/ExecuteCommandMatcher.cs
@@ -28,7 +28,7 @@ public IEnumerable<ILspHandlerDescriptor> FindHandler(object parameters, IEnumer
                 _logger.LogTrace("Registration options {OptionsName}", executeCommandParams.GetType().FullName);
                 foreach (var descriptor in descriptors)
                 {
-                    if (descriptor.Registration?.RegisterOptions is ExecuteCommandRegistrationOptions registrationOptions && registrationOptions.Commands.Any(x => x == executeCommandParams.Command))
+                    if (descriptor.RegistrationOptions is ExecuteCommandRegistrationOptions registrationOptions && registrationOptions.Commands.Any(x => x == executeCommandParams.Command))
                     {
                         _logger.LogTrace("Checking handler {Method}:{Handler}",
                             executeCommandParams.Command,
diff --git a/src/Server/Matchers/TextDocumentMatcher.cs b/src/Server/Matchers/TextDocumentMatcher.cs
@@ -75,7 +75,7 @@ private IEnumerable<ILspHandlerDescriptor> GetHandler(IEnumerable<ILspHandlerDes
             foreach (var descriptor in descriptors)
             {
                 _logger.LogTrace("Checking handler {Method}:{Handler}", method, descriptor.ImplementationType.FullName);
-                var registrationOptions = descriptor.Registration?.RegisterOptions as ITextDocumentRegistrationOptions;
+                var registrationOptions = descriptor.RegistrationOptions as ITextDocumentRegistrationOptions;
 
                 _logger.LogTrace("Registration options {OptionsName}", registrationOptions?.GetType().FullName);
                 _logger.LogTrace("Document Selector {DocumentSelector}", registrationOptions?.DocumentSelector.ToString());
diff --git a/src/Server/SupportedCapabilities.cs b/src/Server/SupportedCapabilities.cs
@@ -28,16 +28,6 @@ public void Add(IEnumerable<ISupports> supports)
             }
         }
 
-        public bool AllowsDynamicRegistration(ILspHandlerDescriptor descriptor)
-        {
-            if (descriptor.HasCapability && _supports.TryGetValue(descriptor.CapabilityType, out var capability))
-            {
-                if (capability is DynamicCapability dc)
-                    return dc.DynamicRegistration;
-            }
-            return false;
-        }
-
         public bool AllowsDynamicRegistration(Type capabilityType)
         {
             if (_supports.TryGetValue(capabilityType, out var capability))
diff --git a/test/Lsp.Tests/Matchers/ExecuteCommandHandlerMatcherTests.cs b/test/Lsp.Tests/Matchers/ExecuteCommandHandlerMatcherTests.cs
@@ -58,6 +58,9 @@ public void Should_Return_Handler_Descriptor()
             // Given
             var handlerMatcher = AutoSubstitute.Resolve<ExecuteCommandMatcher>();
             var executeCommandHandler = Substitute.For<IExecuteCommandHandler>().With(new Container<string>("Command"));
+            var registrationsOptions = new ExecuteCommandRegistrationOptions() {
+                Commands = new Container<string>("Command")
+            };
 
             // When
             var result = handlerMatcher.FindHandler(new ExecuteCommandParams { Command = "Command" },
@@ -68,11 +71,8 @@ public void Should_Return_Handler_Descriptor()
                         executeCommandHandler.GetType(),
                         typeof(ExecuteCommandParams),
                         typeof(ExecuteCommandRegistrationOptions),
-                        new Registration() {
-                            RegisterOptions = new ExecuteCommandRegistrationOptions() {
-                                Commands = new Container<string>("Command")
-                            }
-                        },
+                        registrationsOptions,
+                        true,
                         typeof(ExecuteCommandCapability),
                         () => { })
                 });
diff --git a/test/Lsp.Tests/Matchers/ResolveCommandMatcherTests.cs b/test/Lsp.Tests/Matchers/ResolveCommandMatcherTests.cs
@@ -67,6 +67,7 @@ public void Should_Not_Throw_Given_Another_Descriptor()
                         typeof(CodeLensParams),
                         null,
                         null,
+                        false,
                         null,
                         () => { });
             var handlerMatcher = new ResolveCommandPipeline<CodeLensParams, CodeLensContainer>(
@@ -100,6 +101,7 @@ public void Should_Return_CodeLensResolve_Descriptor()
                             typeof(CodeLens),
                             null,
                             null,
+                            false,
                             null,
                             () => { }),
                         new HandlerDescriptor(DocumentNames.CodeLensResolve,
@@ -109,6 +111,7 @@ public void Should_Return_CodeLensResolve_Descriptor()
                             typeof(CodeLens),
                             null,
                             null,
+                            false,
                             null,
                             () => { }),
                     })
@@ -137,6 +140,7 @@ public void Should_Handle_Null_Data()
                             typeof(CompletionItem),
                             null,
                             null,
+                            false,
                             null,
                             () => { }),
                     })
@@ -167,6 +171,7 @@ public void Should_Handle_Simple_Json_Data()
                             typeof(CompletionItem),
                             null,
                             null,
+                            false,
                             null,
                             () => { }),
                     })
@@ -199,6 +204,7 @@ public void Should_Return_CompletionResolve_Descriptor()
                             typeof(CompletionItem),
                             null,
                             null,
+                            false,
                             null,
                             () => { }),
                         new HandlerDescriptor(DocumentNames.CompletionResolve,
@@ -208,6 +214,7 @@ public void Should_Return_CompletionResolve_Descriptor()
                             typeof(CompletionItem),
                             null,
                             null,
+                            false,
                             null,
                             () => { }),
                     })
@@ -247,6 +254,7 @@ public void Should_Deal_WithHandlers_That_Not_Also_Resolvers()
                             typeof(CompletionItem),
                             null,
                             null,
+                            false,
                             null,
                             () => { }),
                         new HandlerDescriptor(DocumentNames.CompletionResolve,
@@ -256,6 +264,7 @@ public void Should_Deal_WithHandlers_That_Not_Also_Resolvers()
                             typeof(CompletionItem),
                             null,
                             null,
+                            false,
                             null,
                             () => { }),
                     })
@@ -291,6 +300,7 @@ public void Should_Deal_WithHandlers_That_Not_Also_Resolvers2()
                             typeof(CompletionItem),
                             null,
                             null,
+                            false,
                             null,
                             () => { }),
                         new HandlerDescriptor(DocumentNames.CompletionResolve,
@@ -300,6 +310,7 @@ public void Should_Deal_WithHandlers_That_Not_Also_Resolvers2()
                             typeof(CompletionItem),
                             null,
                             null,
+                            false,
                             null,
                             () => { }),
                     })
@@ -327,6 +338,7 @@ public async Task Should_Update_CompletionItems_With_HandlerType()
                             typeof(CompletionParams),
                             null,
                             null,
+                            false,
                             null,
                             () => { });
             var handlerMatcher = new ResolveCommandPipeline<CompletionParams, CompletionList>(
@@ -369,6 +381,7 @@ public async Task Should_Update_CodeLensContainer_With_HandlerType()
                             typeof(CodeLensParams),
                             null,
                             null,
+                            false,
                             null,
                             () => { });
             var handlerMatcher = new ResolveCommandPipeline<CodeLensParams, CodeLensContainer>(
@@ -411,6 +424,7 @@ public async Task Should_Update_CodeLens_Removing_HandlerType()
                             typeof(CodeLens),
                             null,
                             null,
+                            false,
                             null,
                             () => { });
             var handlerMatcher = new ResolveCommandPipeline<CodeLens, CodeLens>(
diff --git a/vscode-testextension/src/extension.ts b/vscode-testextension/src/extension.ts

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ public async Task RouteNotification(TDescriptor descriptor, Notification notific`
`65`	`65`	`@params = notification.Params?.ToObject(descriptor.Params, _serializer.JsonSerializer);`
`66`	`66`	`}`
`67`	`67`
`68`		`- await HandleNotification(mediator, descriptor, @params ?? EmptyRequest.Instance, token);`
	`68`	`+ await HandleNotification(mediator, descriptor, @params ?? Activator.CreateInstance(descriptor.Params), token);`
`69`	`69`	`}`
`70`	`70`	`}`
`71`	`71`	`catch (Exception e)`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-using Newtonsoft.Json;`
	`1`	`+using Newtonsoft.Json;`
`2`	`2`	`using Newtonsoft.Json.Serialization;`
`3`	`3`	`using OmniSharp.Extensions.LanguageServer.Protocol.Serialization;`
`4`	`4`
Original file line number	Diff line number	Diff line change
`@@ -6,9 +6,11 @@ namespace OmniSharp.Extensions.LanguageServer.Server.Abstractions`
`6`	`6`	`{`
`7`	`7`	`public interface ILspHandlerDescriptor : IHandlerDescriptor`
`8`	`8`	`{`
	`9`	`+ Guid Id { get; }`
`9`	`10`	`bool HasRegistration { get; }`
`10`	`11`	`Type RegistrationType { get; }`
`11`		`- Registration Registration { get; }`
	`12`	`+ object RegistrationOptions { get; }`
	`13`	`+ bool AllowsDynamicRegistration { get; }`
`12`	`14`
`13`	`15`	`bool HasCapability { get; }`
`14`	`16`	`Type CapabilityType { get; }`
Original file line number	Diff line number	Diff line change
`@@ -92,29 +92,29 @@ public TOptions Get<TInterface, TOptions>(Func<TInterface, TOptions> action)`
`92`	`92`	`where TOptions : class`
`93`	`93`	`{`
`94`	`94`	`return _collection`
`95`		`- .Select(x => x.Registration?.RegisterOptions is TInterface cl ? action(cl) : null)`
	`95`	`+ .Select(x => x.RegistrationOptions is TInterface cl ? action(cl) : null)`
`96`	`96`	`.FirstOrDefault(x => x != null);`
`97`	`97`	`}`
`98`	`98`
`99`	`99`	`public Supports<TOptions> Can<TInterface, TOptions>(Func<TInterface, TOptions> action)`
`100`	`100`	`where TOptions : class`
`101`	`101`	`{`
`102`	`102`	`var options = _collection`
`103`		`- .Select(x => x.Registration?.RegisterOptions is TInterface cl ? action(cl) : null)`
	`103`	`+ .Select(x => x.RegistrationOptions is TInterface cl ? action(cl) : null)`
`104`	`104`	`.FirstOrDefault(x => x != null);`
`105`	`105`	`if (options == null)`
`106`	`106`	`return Supports.OfBoolean<TOptions>(false);`
`107`	`107`
`108`	`108`	`return _collection`
`109`		`- .Select(x => x.Registration?.RegisterOptions is TInterface cl ? action(cl) : null)`
	`109`	`+ .Select(x => x.RegistrationOptions is TInterface cl ? action(cl) : null)`
`110`	`110`	`.FirstOrDefault(x => x != null);`
`111`	`111`	`}`
`112`	`112`
`113`	`113`	`public TOptions Reduce<TInterface, TOptions>(Func<IEnumerable<TInterface>, TOptions> action)`
`114`	`114`	`where TOptions : class`
`115`	`115`	`{`
`116`	`116`	`return action(_collection`
`117`		`- .Select(x => x.Registration?.RegisterOptions is TInterface cl ? cl : default)`
	`117`	`+ .Select(x => x.RegistrationOptions is TInterface cl ? cl : default)`
`118`	`118`	`.Where(x => x != null));`
`119`	`119`	`}`
`120`	`120`	`}`
Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,6 @@ namespace OmniSharp.Extensions.LanguageServer.Server`
`6`	`6`	`{`
`7`	`7`	`public interface ISupportedCapabilities`
`8`	`8`	`{`
`9`		`- bool AllowsDynamicRegistration(ILspHandlerDescriptor descriptor);`
`10`	`9`	`bool AllowsDynamicRegistration(Type capabilityType);`
`11`	`10`	`void SetCapability(ILspHandlerDescriptor descriptor, IJsonRpcHandler handler);`
`12`	`11`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ public IEnumerable<ILspHandlerDescriptor> FindHandler(object parameters, IEnumer`
`28`	`28`	`_logger.LogTrace("Registration options {OptionsName}", executeCommandParams.GetType().FullName);`
`29`	`29`	`foreach (var descriptor in descriptors)`
`30`	`30`	`{`
`31`		`- if (descriptor.Registration?.RegisterOptions is ExecuteCommandRegistrationOptions registrationOptions && registrationOptions.Commands.Any(x => x == executeCommandParams.Command))`
	`31`	`+ if (descriptor.RegistrationOptions is ExecuteCommandRegistrationOptions registrationOptions && registrationOptions.Commands.Any(x => x == executeCommandParams.Command))`
`32`	`32`	`{`
`33`	`33`	`_logger.LogTrace("Checking handler {Method}:{Handler}",`
`34`	`34`	`executeCommandParams.Command,`