Skip to content

Commit b85e6da

Browse files
david-driscolldavid-driscoll
authored
Merge pull request #74 from OmniSharp/nullref-fail
fixed issue with non object JTokens in resolve command matcher. Dete…
2 parents def59b4 + ae45bf7 commit b85e6da

File tree

8 files changed

+142
-13
lines changed

8 files changed

+142
-13
lines changed

‎src/Server/Abstractions/IHandlerMatcherCollection.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ public interface IHandlerMatcherCollection : IEnumerable<object>
77
{
88
IDisposable Add(object handler);
99
IEnumerable<IHandlerMatcher> ForHandlerMatchers();
10+
IEnumerable<IHandlerPreProcessorMatcher> ForHandlerPreProcessorMatcher();
11+
IEnumerable<IHandlerPreProcessor> ForHandlerPreProcessor();
1012
IEnumerable<IHandlerPostProcessorMatcher> ForHandlerPostProcessorMatcher();
1113
IEnumerable<IHandlerPostProcessor> ForHandlerPostProcessor();
1214
}

‎src/Server/Abstractions/IHandlerPreProcessor.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
namespace OmniSharp.Extensions.LanguageServer.Server.Abstractions
2+
{
3+
public interface IHandlerPreProcessor
4+
{
5+
/// <summary>
6+
/// Does post processing for a request of descriptor type
7+
/// </summary>
8+
/// <param name="descriptor">The descriptor.</param>
9+
/// <param name="parameters">The parameters.</param>
10+
/// <returns></returns>
11+
object Process(ILspHandlerDescriptor descriptor, object parameters);
12+
}
13+
}

‎src/Server/Abstractions/IHandlerPreProcessorMatcher.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
using System.Collections.Generic;
2+
3+
namespace OmniSharp.Extensions.LanguageServer.Server.Abstractions
4+
{
5+
public interface IHandlerPreProcessorMatcher
6+
{
7+
/// <summary>
8+
/// Finds any postproessor for a given descriptor and response
9+
/// </summary>
10+
/// <param name="descriptor">The descriptor.</param>
11+
/// <param name="parameters">The parameters.</param>
12+
/// <param name="response">The response.</param>
13+
/// <returns></returns>
14+
IEnumerable<IHandlerPreProcessor> FindPreProcessor(ILspHandlerDescriptor descriptor, object parameters);
15+
}
16+
}

‎src/Server/HandlerDescriptor.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Linq;
34
using System.Reflection;
45
using OmniSharp.Extensions.JsonRpc;
56
using OmniSharp.Extensions.LanguageServer.Protocol;
@@ -23,10 +24,11 @@ public HandlerDescriptor(string method, string key, IJsonRpcHandler handler, Typ
2324
Params = @params;
2425
RegistrationType = registrationType;
2526
CapabilityType = capabilityType;
26-
if (@params != null && typeof(ICanBeResolved).GetTypeInfo().IsAssignableFrom(@params))
27-
{
28-
CanBeResolvedHandlerType = typeof(ICanBeResolvedHandler<>).MakeGenericType(@params);
29-
}
27+
28+
// If multiple are implemented this behavior is unknown
29+
CanBeResolvedHandlerType = handler.GetType().GetTypeInfo()
30+
.ImplementedInterfaces
31+
.FirstOrDefault(x => x.GetTypeInfo().IsGenericType && x.GetTypeInfo().GetGenericTypeDefinition() == typeof(ICanBeResolvedHandler<>));
3032
}
3133

3234
public IJsonRpcHandler Handler { get; }

‎src/Server/HandlerMatcherCollection.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ public class HandlerMatcherCollection : IHandlerMatcherCollection
1313

1414
private static readonly Type[] ValidHandlers = {
1515
typeof(IHandlerMatcher),
16+
typeof(IHandlerPreProcessor),
17+
typeof(IHandlerPreProcessorMatcher),
1618
typeof(IHandlerPostProcessor),
1719
typeof(IHandlerPostProcessorMatcher)
1820
};
@@ -36,6 +38,16 @@ public IEnumerable<IHandlerMatcher> ForHandlerMatchers()
3638
return _handlers.OfType<IHandlerMatcher>();
3739
}
3840

41+
public IEnumerable<IHandlerPreProcessorMatcher> ForHandlerPreProcessorMatcher()
42+
{
43+
return _handlers.OfType<IHandlerPreProcessorMatcher>();
44+
}
45+
46+
public IEnumerable<IHandlerPreProcessor> ForHandlerPreProcessor()
47+
{
48+
return _handlers.OfType<IHandlerPreProcessor>();
49+
}
50+
3951
public IEnumerable<IHandlerPostProcessorMatcher> ForHandlerPostProcessorMatcher()
4052
{
4153
return _handlers.OfType<IHandlerPostProcessorMatcher>();

‎src/Server/LspRequestRouter.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,13 @@ public async Task<ErrorResponse> RouteRequest(IHandlerDescriptor descriptor, Req
138138
_logger.LogError(new EventId(-32602), cannotDeserializeRequestParams, "Failed to deserialise request parameters.");
139139
return new InvalidParams(request.Id);
140140
}
141+
var lspDescriptor = descriptor as ILspHandlerDescriptor;
142+
143+
foreach (var preProcessor in _routeMatchers.ForHandlerPreProcessorMatcher()
144+
.SelectMany(strat => strat.FindPreProcessor(lspDescriptor, @params)))
145+
{
146+
@params = preProcessor.Process(lspDescriptor, @params);
147+
}
141148

142149
var result = ReflectionRequestHandlers.HandleRequest(descriptor, @params, cts.Token);
143150
await result;
@@ -154,8 +161,6 @@ public async Task<ErrorResponse> RouteRequest(IHandlerDescriptor descriptor, Req
154161
responseValue = property.GetValue(result);
155162
_logger.LogDebug("Response value was {Type}", responseValue?.GetType().FullName);
156163

157-
var lspDescriptor = descriptor as ILspHandlerDescriptor;
158-
159164
foreach (var postProcessor in _routeMatchers.ForHandlerPostProcessorMatcher()
160165
.SelectMany(strat => strat.FindPostProcessor(lspDescriptor, @params, responseValue)))
161166
{

‎src/Server/Matchers/ResolveCommandMatcher.cs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
namespace OmniSharp.Extensions.LanguageServer.Server.Matchers
1111
{
12-
public class ResolveCommandMatcher : IHandlerMatcher, IHandlerPostProcessorMatcher, IHandlerPostProcessor
12+
public class ResolveCommandMatcher : IHandlerMatcher, IHandlerPostProcessorMatcher, IHandlerPreProcessor, IHandlerPostProcessor
1313
{
1414
private readonly ILogger _logger;
1515
internal static string PrivateHandlerTypeName = "$$___handlerType___$$";
@@ -30,8 +30,8 @@ public IEnumerable<ILspHandlerDescriptor> FindHandler(object parameters, IEnumer
3030
if (parameters is ICanBeResolved canBeResolved)
3131
{
3232
string handlerType = null;
33-
if (canBeResolved.Data != null)
34-
handlerType = canBeResolved.Data.Value<string>(PrivateHandlerTypeName);
33+
if (canBeResolved.Data != null && canBeResolved.Data.Type == JTokenType.Object)
34+
handlerType = canBeResolved.Data?[PrivateHandlerTypeName]?.ToString();
3535

3636
if (string.IsNullOrWhiteSpace(handlerType))
3737
{
@@ -67,7 +67,6 @@ public IEnumerable<ILspHandlerDescriptor> FindHandler(object parameters, IEnumer
6767
descriptor.Handler.GetType().FullName);
6868
if (descriptor.Handler.GetType().FullName == handlerType || descriptor.HandlerType.FullName == handlerType)
6969
{
70-
canBeResolved.Data = canBeResolved.Data["data"];
7170
yield return descriptor;
7271
}
7372
}
@@ -91,6 +90,23 @@ public IEnumerable<IHandlerPostProcessor> FindPostProcessor(ILspHandlerDescripto
9190
}
9291
}
9392

93+
public object Process(ILspHandlerDescriptor descriptor, object parameters)
94+
{
95+
if (parameters is ICanBeResolved canBeResolved)
96+
{
97+
string handlerType = null;
98+
if (canBeResolved.Data != null && canBeResolved.Data.Type == JTokenType.Object)
99+
handlerType = canBeResolved.Data?[PrivateHandlerTypeName]?.ToString();
100+
101+
if (!string.IsNullOrWhiteSpace(handlerType))
102+
{
103+
canBeResolved.Data = canBeResolved.Data["data"];
104+
}
105+
}
106+
107+
return parameters;
108+
}
109+
94110
public object Process(ILspHandlerDescriptor descriptor, object parameters, object response)
95111
{
96112
// Only pin the handler type, if we know the source handler (codelens) is also the resolver.

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

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,35 @@ public void Should_Handle_Null_Data()
139139
result.Should().Contain(x => x.Handler == resolveHandler);
140140
}
141141

142+
[Fact]
143+
public void Should_Handle_Simple_Json_Data()
144+
{
145+
// Given
146+
var handlerMatcher = new ResolveCommandMatcher(_logger);
147+
var resolveHandler = Substitute.For<ICompletionResolveHandler>();
148+
resolveHandler.CanResolve(Arg.Any<CompletionItem>()).Returns(true);
149+
150+
// When
151+
var result = handlerMatcher.FindHandler(new CompletionItem() {
152+
Data = new Uri("file:///c%3A/Users/mb/src/gh/Cake.Json/src/Cake.Json/Namespaces.cs")
153+
},
154+
new List<HandlerDescriptor> {
155+
new HandlerDescriptor(DocumentNames.CompletionResolve,
156+
"Key",
157+
resolveHandler,
158+
resolveHandler.GetType(),
159+
typeof(CompletionItem),
160+
null,
161+
null,
162+
() => { }),
163+
})
164+
.ToArray();
165+
166+
// Then
167+
result.Should().NotBeNullOrEmpty();
168+
result.Should().Contain(x => x.Handler == resolveHandler);
169+
}
170+
142171
[Fact]
143172
public void Should_Return_CompletionResolve_Descriptor()
144173
{
@@ -296,7 +325,7 @@ public void Should_FindPostProcessor_AsSelf_For_Completion()
296325
"Key",
297326
resolveHandler,
298327
resolveHandler.GetType(),
299-
typeof(CodeLensParams),
328+
typeof(CompletionParams),
300329
null,
301330
null,
302331
() => { });
@@ -325,7 +354,7 @@ public void Should_Update_CompletionItems_With_HandlerType()
325354
"Key",
326355
resolveHandler as IJsonRpcHandler,
327356
resolveHandler.GetType(),
328-
typeof(CompletionItem),
357+
typeof(CompletionParams),
329358
null,
330359
null,
331360
() => { });
@@ -363,7 +392,7 @@ public void Should_Update_CodeLensContainer_With_HandlerType()
363392
"Key",
364393
resolveHandler as IJsonRpcHandler,
365394
resolveHandler.GetType(),
366-
typeof(CodeLens),
395+
typeof(CodeLensParams),
367396
null,
368397
null,
369398
() => { });
@@ -385,5 +414,39 @@ public void Should_Update_CodeLensContainer_With_HandlerType()
385414
responseItem.Data[ResolveCommandMatcher.PrivateHandlerTypeName].Value<string>().Should().NotBeNullOrEmpty();
386415
responseItem.Data["data"]["hello"].Value<string>().Should().Be("world");
387416
}
417+
418+
[Fact]
419+
public void Should_Update_CodeLens_Removing_HandlerType()
420+
{
421+
// Given
422+
var handlerMatcher = new ResolveCommandMatcher(_logger);
423+
var resolveHandler = Substitute.For(new Type[] {
424+
typeof(ICodeLensHandler),
425+
typeof(ICodeLensResolveHandler)
426+
}, new object[0]);
427+
(resolveHandler as ICodeLensResolveHandler).CanResolve(Arg.Any<CodeLens>()).Returns(true);
428+
var descriptor = new HandlerDescriptor(
429+
DocumentNames.CodeLensResolve,
430+
"Key",
431+
resolveHandler as IJsonRpcHandler,
432+
resolveHandler.GetType(),
433+
typeof(CodeLens),
434+
null,
435+
null,
436+
() => { });
437+
438+
var item = new CodeLens() {
439+
Data = JObject.FromObject(new { data = new { hello = "world" } })
440+
};
441+
item.Data[ResolveCommandMatcher.PrivateHandlerTypeName] = resolveHandler.GetType().FullName;
442+
443+
// When
444+
var response = handlerMatcher.Process(descriptor, item);
445+
446+
// Then
447+
response.Should().Be(item);
448+
item.Data?[ResolveCommandMatcher.PrivateHandlerTypeName].Should().BeNull();
449+
item.Data["hello"].Value<string>().Should().Be("world");
450+
}
388451
}
389452
}

0 commit comments

Comments
 (0)