Skip to content

Commit a52197f

Browse files
bjorkstrommbjorkstromm
committed
Move registration options to the descriptor
- Fixes issue where clients that doesn't support dynamic registrations will not end up with the correct handler matching the document (due to that registration is null). See OmniSharp-Roslyn issue #1505 and #1525
1 parent df30f5e commit a52197f

11 files changed

+38
-17
lines changed

‎src/JsonRpc/JsonRpc.csproj

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
33
<TargetFrameworks>netstandard2.0</TargetFrameworks>
44
<PlatformTarget>AnyCPU</PlatformTarget>
@@ -13,7 +13,7 @@
1313
<PackageReference Include="Microsoft.Extensions.DependencyInjection" />
1414
<PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" />
1515
<PackageReference Include="Newtonsoft.Json" />
16-
<Compile Include="../../submodules/MediatR/src/MediatR/**/*.cs" Exclude="**/AssemblyInfo.cs" Visible="false" />
17-
<Compile Include="../../submodules/MediatR.Extensions.Microsoft.DependencyInjection/src/MediatR.Extensions.Microsoft.DependencyInjection/**/*.cs" Exclude="**/AssemblyInfo.cs" Visible="false" />
16+
<Compile Include="../../submodules/MediatR/src/MediatR/**/*.cs" Exclude="**/*AssemblyInfo.cs" Visible="false" />
17+
<Compile Include="../../submodules/MediatR.Extensions.Microsoft.DependencyInjection/src/MediatR.Extensions.Microsoft.DependencyInjection/**/*.cs" Exclude="**/*AssemblyInfo.cs" Visible="false" />
1818
</ItemGroup>
1919
</Project>

‎src/Protocol/Models/Registration.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using Newtonsoft.Json;
1+
using Newtonsoft.Json;
22
using Newtonsoft.Json.Serialization;
33
using OmniSharp.Extensions.LanguageServer.Protocol.Serialization;
44

‎src/Server/Abstractions/ILspHandlerDescriptor.cs

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ public interface ILspHandlerDescriptor : IHandlerDescriptor
88
{
99
bool HasRegistration { get; }
1010
Type RegistrationType { get; }
11+
object RegisterOptions { get; }
1112
Registration Registration { get; }
1213

1314
bool HasCapability { get; }

‎src/Server/ClientCapabilityProvider.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -92,29 +92,29 @@ public TOptions Get<TInterface, TOptions>(Func<TInterface, TOptions> action)
9292
where TOptions : class
9393
{
9494
return _collection
95-
.Select(x => x.Registration?.RegisterOptions is TInterface cl ? action(cl) : null)
95+
.Select(x => x.RegisterOptions is TInterface cl ? action(cl) : null)
9696
.FirstOrDefault(x => x != null);
9797
}
9898

9999
public Supports<TOptions> Can<TInterface, TOptions>(Func<TInterface, TOptions> action)
100100
where TOptions : class
101101
{
102102
var options = _collection
103-
.Select(x => x.Registration?.RegisterOptions is TInterface cl ? action(cl) : null)
103+
.Select(x => x.RegisterOptions is TInterface cl ? action(cl) : null)
104104
.FirstOrDefault(x => x != null);
105105
if (options == null)
106106
return Supports.OfBoolean<TOptions>(false);
107107

108108
return _collection
109-
.Select(x => x.Registration?.RegisterOptions is TInterface cl ? action(cl) : null)
109+
.Select(x => x.RegisterOptions is TInterface cl ? action(cl) : null)
110110
.FirstOrDefault(x => x != null);
111111
}
112112

113113
public TOptions Reduce<TInterface, TOptions>(Func<IEnumerable<TInterface>, TOptions> action)
114114
where TOptions : class
115115
{
116116
return action(_collection
117-
.Select(x => x.Registration?.RegisterOptions is TInterface cl ? cl : default)
117+
.Select(x => x.RegisterOptions is TInterface cl ? cl : default)
118118
.Where(x => x != null));
119119
}
120120
}

‎src/Server/HandlerCollection.cs

+1
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ private HandlerDescriptor GetDescriptor(string method, Type handlerType, IJsonRp
193193
@interface,
194194
@params,
195195
registrationType,
196+
registrationOptions,
196197
registration,
197198
capabilityType,
198199
() => {

‎src/Server/HandlerDescriptor.cs

+3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public HandlerDescriptor(
2222
Type handlerType,
2323
Type @params,
2424
Type registrationType,
25+
object registerOptions,
2526
Registration registration,
2627
Type capabilityType,
2728
Action disposeAction)
@@ -35,6 +36,7 @@ public HandlerDescriptor(
3536
Params = @params;
3637
Response = Response;
3738
RegistrationType = registrationType;
39+
RegisterOptions = registerOptions;
3840
Registration = registration;
3941
CapabilityType = capabilityType;
4042

@@ -65,6 +67,7 @@ public HandlerDescriptor(
6567

6668
public bool HasRegistration => RegistrationType != null;
6769
public Type RegistrationType { get; }
70+
public object RegisterOptions { get; }
6871
public Registration Registration { get; }
6972

7073
public bool HasCapability => CapabilityType != null;

‎src/Server/Matchers/ExecuteCommandMatcher.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public IEnumerable<ILspHandlerDescriptor> FindHandler(object parameters, IEnumer
2828
_logger.LogTrace("Registration options {OptionsName}", executeCommandParams.GetType().FullName);
2929
foreach (var descriptor in descriptors)
3030
{
31-
if (descriptor.Registration?.RegisterOptions is ExecuteCommandRegistrationOptions registrationOptions && registrationOptions.Commands.Any(x => x == executeCommandParams.Command))
31+
if (descriptor.RegisterOptions is ExecuteCommandRegistrationOptions registrationOptions && registrationOptions.Commands.Any(x => x == executeCommandParams.Command))
3232
{
3333
_logger.LogTrace("Checking handler {Method}:{Handler}",
3434
executeCommandParams.Command,

‎src/Server/Matchers/TextDocumentMatcher.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ private IEnumerable<ILspHandlerDescriptor> GetHandler(IEnumerable<ILspHandlerDes
7575
foreach (var descriptor in descriptors)
7676
{
7777
_logger.LogTrace("Checking handler {Method}:{Handler}", method, descriptor.ImplementationType.FullName);
78-
var registrationOptions = descriptor.Registration?.RegisterOptions as ITextDocumentRegistrationOptions;
78+
var registrationOptions = descriptor.RegisterOptions as ITextDocumentRegistrationOptions;
7979

8080
_logger.LogTrace("Registration options {OptionsName}", registrationOptions?.GetType().FullName);
8181
_logger.LogTrace("Document Selector {DocumentSelector}", registrationOptions?.DocumentSelector.ToString());

‎test/Lsp.Tests/Matchers/ExecuteCommandHandlerMatcherTests.cs

+5-3
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ public void Should_Return_Handler_Descriptor()
5858
// Given
5959
var handlerMatcher = AutoSubstitute.Resolve<ExecuteCommandMatcher>();
6060
var executeCommandHandler = Substitute.For<IExecuteCommandHandler>().With(new Container<string>("Command"));
61+
var registrationsOptions = new ExecuteCommandRegistrationOptions() {
62+
Commands = new Container<string>("Command")
63+
};
6164

6265
// When
6366
var result = handlerMatcher.FindHandler(new ExecuteCommandParams { Command = "Command" },
@@ -68,10 +71,9 @@ public void Should_Return_Handler_Descriptor()
6871
executeCommandHandler.GetType(),
6972
typeof(ExecuteCommandParams),
7073
typeof(ExecuteCommandRegistrationOptions),
74+
registrationsOptions,
7175
new Registration() {
72-
RegisterOptions = new ExecuteCommandRegistrationOptions() {
73-
Commands = new Container<string>("Command")
74-
}
76+
RegisterOptions = registrationsOptions
7577
},
7678
typeof(ExecuteCommandCapability),
7779
() => { })

‎test/Lsp.Tests/Matchers/ResolveCommandMatcherTests.cs

+14
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ public void Should_Not_Throw_Given_Another_Descriptor()
6868
null,
6969
null,
7070
null,
71+
null,
7172
() => { });
7273
var handlerMatcher = new ResolveCommandPipeline<CodeLensParams, CodeLensContainer>(
7374
new RequestContext() { Descriptor = handlerDescriptor },
@@ -101,6 +102,7 @@ public void Should_Return_CodeLensResolve_Descriptor()
101102
null,
102103
null,
103104
null,
105+
null,
104106
() => { }),
105107
new HandlerDescriptor(DocumentNames.CodeLensResolve,
106108
"Key2",
@@ -110,6 +112,7 @@ public void Should_Return_CodeLensResolve_Descriptor()
110112
null,
111113
null,
112114
null,
115+
null,
113116
() => { }),
114117
})
115118
.ToArray();
@@ -138,6 +141,7 @@ public void Should_Handle_Null_Data()
138141
null,
139142
null,
140143
null,
144+
null,
141145
() => { }),
142146
})
143147
.ToArray();
@@ -168,6 +172,7 @@ public void Should_Handle_Simple_Json_Data()
168172
null,
169173
null,
170174
null,
175+
null,
171176
() => { }),
172177
})
173178
.ToArray();
@@ -200,6 +205,7 @@ public void Should_Return_CompletionResolve_Descriptor()
200205
null,
201206
null,
202207
null,
208+
null,
203209
() => { }),
204210
new HandlerDescriptor(DocumentNames.CompletionResolve,
205211
"Key2",
@@ -209,6 +215,7 @@ public void Should_Return_CompletionResolve_Descriptor()
209215
null,
210216
null,
211217
null,
218+
null,
212219
() => { }),
213220
})
214221
.ToArray();
@@ -248,6 +255,7 @@ public void Should_Deal_WithHandlers_That_Not_Also_Resolvers()
248255
null,
249256
null,
250257
null,
258+
null,
251259
() => { }),
252260
new HandlerDescriptor(DocumentNames.CompletionResolve,
253261
"Key2",
@@ -257,6 +265,7 @@ public void Should_Deal_WithHandlers_That_Not_Also_Resolvers()
257265
null,
258266
null,
259267
null,
268+
null,
260269
() => { }),
261270
})
262271
.ToArray();
@@ -292,6 +301,7 @@ public void Should_Deal_WithHandlers_That_Not_Also_Resolvers2()
292301
null,
293302
null,
294303
null,
304+
null,
295305
() => { }),
296306
new HandlerDescriptor(DocumentNames.CompletionResolve,
297307
"Key2",
@@ -301,6 +311,7 @@ public void Should_Deal_WithHandlers_That_Not_Also_Resolvers2()
301311
null,
302312
null,
303313
null,
314+
null,
304315
() => { }),
305316
})
306317
.ToArray();
@@ -328,6 +339,7 @@ public async Task Should_Update_CompletionItems_With_HandlerType()
328339
null,
329340
null,
330341
null,
342+
null,
331343
() => { });
332344
var handlerMatcher = new ResolveCommandPipeline<CompletionParams, CompletionList>(
333345
new RequestContext() { Descriptor = descriptor },
@@ -370,6 +382,7 @@ public async Task Should_Update_CodeLensContainer_With_HandlerType()
370382
null,
371383
null,
372384
null,
385+
null,
373386
() => { });
374387
var handlerMatcher = new ResolveCommandPipeline<CodeLensParams, CodeLensContainer>(
375388
new RequestContext() { Descriptor = descriptor },
@@ -412,6 +425,7 @@ public async Task Should_Update_CodeLens_Removing_HandlerType()
412425
null,
413426
null,
414427
null,
428+
null,
415429
() => { });
416430
var handlerMatcher = new ResolveCommandPipeline<CodeLens, CodeLens>(
417431
new RequestContext() { Descriptor = descriptor },

‎vscode-testextension/src/extension.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,16 @@ export function activate(context: ExtensionContext) {
1515

1616
// The server is implemented in node
1717
// let serverExe = 'dotnet';
18-
let serverExe = 'D:\\development\\omnisharp\\omnisharp-roslyn\\bin\\Debug\\OmniSharp.Stdio.Driver\\net461\\OmniSharp.exe';
18+
let serverExe = 'C:/Users/mb/src/gh/omnisharp-roslyn/artifacts/publish/OmniSharp.Stdio.Driver/win7-x64/OmniSharp.exe';
1919
// let serverExe = context.asAbsolutePath('D:/Development/Omnisharp/omnisharp-roslyn/artifacts/publish/OmniSharp.Stdio/win7-x64/OmniSharp.exe');
2020
// The debug options for the server
21-
// let debugOptions = { execArgv: ['-lsp', '-d' };5
21+
// let debugOptions = { execArgv: ['-lsp', '-d' };
2222

2323
// If the extension is launched in debug mode then the debug server options are used
2424
// Otherwise the run options are used
2525
let serverOptions: ServerOptions = {
26-
run: { command: serverExe, args: ['-lsp', '-d'] },
27-
debug: { command: serverExe, args: ['-lsp', '-d'] }
26+
run: { command: serverExe, args: ['-lsp'] },
27+
debug: { command: serverExe, args: ['-lsp'] }
2828
}
2929

3030
// Options to control the language client

0 commit comments

Comments
 (0)