Merge pull request #74 from OmniSharp/nullref-fail

fixed issue with non object JTokens in resolve command matcher.  Dete…

Author: david-driscoll

src/Server/Abstractions/IHandlerMatcherCollection.cs

@@ -7,6 +7,8 @@ public interface IHandlerMatcherCollection : IEnumerable<object>
     {
         IDisposable Add(object handler);
         IEnumerable<IHandlerMatcher> ForHandlerMatchers();
+        IEnumerable<IHandlerPreProcessorMatcher> ForHandlerPreProcessorMatcher();
+        IEnumerable<IHandlerPreProcessor> ForHandlerPreProcessor();
         IEnumerable<IHandlerPostProcessorMatcher> ForHandlerPostProcessorMatcher();
         IEnumerable<IHandlerPostProcessor> ForHandlerPostProcessor();
     }

src/Server/Abstractions/IHandlerPreProcessor.cs

@@ -0,0 +1,13 @@
+namespace OmniSharp.Extensions.LanguageServer.Server.Abstractions
+{
+    public interface IHandlerPreProcessor
+    {
+        /// <summary>
+        /// Does post processing for a request of descriptor type
+        /// </summary>
+        /// <param name="descriptor">The descriptor.</param>
+        /// <param name="parameters">The parameters.</param>
+        /// <returns></returns>
+        object Process(ILspHandlerDescriptor descriptor, object parameters);
+    }
+}

src/Server/Abstractions/IHandlerPreProcessorMatcher.cs

@@ -0,0 +1,16 @@
+using System.Collections.Generic;
+
+namespace OmniSharp.Extensions.LanguageServer.Server.Abstractions
+{
+    public interface IHandlerPreProcessorMatcher
+    {
+        /// <summary>
+        /// Finds any postproessor for a given descriptor and response
+        /// </summary>
+        /// <param name="descriptor">The descriptor.</param>
+        /// <param name="parameters">The parameters.</param>
+        /// <param name="response">The response.</param>
+        /// <returns></returns>
+        IEnumerable<IHandlerPreProcessor> FindPreProcessor(ILspHandlerDescriptor descriptor, object parameters);
+    }
+}

src/Server/HandlerDescriptor.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Reflection;
 using OmniSharp.Extensions.JsonRpc;
 using OmniSharp.Extensions.LanguageServer.Protocol;
@@ -23,10 +24,11 @@ public HandlerDescriptor(string method, string key, IJsonRpcHandler handler, Typ
             Params = @params;
             RegistrationType = registrationType;
             CapabilityType = capabilityType;
-            if (@params != null && typeof(ICanBeResolved).GetTypeInfo().IsAssignableFrom(@params))
-            {
-                CanBeResolvedHandlerType = typeof(ICanBeResolvedHandler<>).MakeGenericType(@params);
-            }
+
+            // If multiple are implemented this behavior is unknown
+            CanBeResolvedHandlerType = handler.GetType().GetTypeInfo()
+                .ImplementedInterfaces
+                .FirstOrDefault(x => x.GetTypeInfo().IsGenericType && x.GetTypeInfo().GetGenericTypeDefinition() == typeof(ICanBeResolvedHandler<>));
         }
 
         public IJsonRpcHandler Handler { get; }

src/Server/HandlerMatcherCollection.cs

@@ -13,6 +13,8 @@ public class HandlerMatcherCollection : IHandlerMatcherCollection
 
         private static readonly Type[] ValidHandlers = {
                 typeof(IHandlerMatcher),
+                typeof(IHandlerPreProcessor),
+                typeof(IHandlerPreProcessorMatcher),
                 typeof(IHandlerPostProcessor),
                 typeof(IHandlerPostProcessorMatcher)
             };
@@ -36,6 +38,16 @@ public IEnumerable<IHandlerMatcher> ForHandlerMatchers()
             return _handlers.OfType<IHandlerMatcher>();
         }
 
+        public IEnumerable<IHandlerPreProcessorMatcher> ForHandlerPreProcessorMatcher()
+        {
+            return _handlers.OfType<IHandlerPreProcessorMatcher>();
+        }
+
+        public IEnumerable<IHandlerPreProcessor> ForHandlerPreProcessor()
+        {
+            return _handlers.OfType<IHandlerPreProcessor>();
+        }
+
         public IEnumerable<IHandlerPostProcessorMatcher> ForHandlerPostProcessorMatcher()
         {
             return _handlers.OfType<IHandlerPostProcessorMatcher>();

src/Server/LspRequestRouter.cs

@@ -138,6 +138,13 @@ public async Task<ErrorResponse> RouteRequest(IHandlerDescriptor descriptor, Req
                             _logger.LogError(new EventId(-32602), cannotDeserializeRequestParams, "Failed to deserialise request parameters.");
                             return new InvalidParams(request.Id);
                         }
+                        var lspDescriptor = descriptor as ILspHandlerDescriptor;
+
+                        foreach (var preProcessor in _routeMatchers.ForHandlerPreProcessorMatcher()
+                            .SelectMany(strat => strat.FindPreProcessor(lspDescriptor, @params)))
+                        {
+                            @params = preProcessor.Process(lspDescriptor, @params);
+                        }
 
                         var result = ReflectionRequestHandlers.HandleRequest(descriptor, @params, cts.Token);
                         await result;
@@ -154,8 +161,6 @@ public async Task<ErrorResponse> RouteRequest(IHandlerDescriptor descriptor, Req
                             responseValue = property.GetValue(result);
                             _logger.LogDebug("Response value was {Type}", responseValue?.GetType().FullName);
 
-                            var lspDescriptor = descriptor as ILspHandlerDescriptor;
-
                             foreach (var postProcessor in _routeMatchers.ForHandlerPostProcessorMatcher()
                                 .SelectMany(strat => strat.FindPostProcessor(lspDescriptor, @params, responseValue)))
                             {

src/Server/Matchers/ResolveCommandMatcher.cs

@@ -9,7 +9,7 @@
 
 namespace OmniSharp.Extensions.LanguageServer.Server.Matchers
 {
-    public class ResolveCommandMatcher : IHandlerMatcher, IHandlerPostProcessorMatcher, IHandlerPostProcessor
+    public class ResolveCommandMatcher : IHandlerMatcher, IHandlerPostProcessorMatcher, IHandlerPreProcessor, IHandlerPostProcessor
     {
         private readonly ILogger _logger;
         internal static string PrivateHandlerTypeName = "$$___handlerType___$$";
@@ -30,8 +30,8 @@ public IEnumerable<ILspHandlerDescriptor> FindHandler(object parameters, IEnumer
             if (parameters is ICanBeResolved canBeResolved)
             {
                 string handlerType = null;
-                if (canBeResolved.Data != null)
-                    handlerType = canBeResolved.Data.Value<string>(PrivateHandlerTypeName);
+                if (canBeResolved.Data != null && canBeResolved.Data.Type == JTokenType.Object)
+                    handlerType = canBeResolved.Data?[PrivateHandlerTypeName]?.ToString();
 
                 if (string.IsNullOrWhiteSpace(handlerType))
                 {
@@ -67,7 +67,6 @@ public IEnumerable<ILspHandlerDescriptor> FindHandler(object parameters, IEnumer
                         descriptor.Handler.GetType().FullName);
                     if (descriptor.Handler.GetType().FullName == handlerType || descriptor.HandlerType.FullName == handlerType)
                     {
-                        canBeResolved.Data = canBeResolved.Data["data"];
                         yield return descriptor;
                     }
                 }
@@ -91,6 +90,23 @@ public IEnumerable<IHandlerPostProcessor> FindPostProcessor(ILspHandlerDescripto
             }
         }
 
+        public object Process(ILspHandlerDescriptor descriptor, object parameters)
+        {
+            if (parameters is ICanBeResolved canBeResolved)
+            {
+                string handlerType = null;
+                if (canBeResolved.Data != null && canBeResolved.Data.Type == JTokenType.Object)
+                    handlerType = canBeResolved.Data?[PrivateHandlerTypeName]?.ToString();
+
+                if (!string.IsNullOrWhiteSpace(handlerType))
+                {
+                    canBeResolved.Data = canBeResolved.Data["data"];
+                }
+            }
+
+            return parameters;
+        }
+
         public object Process(ILspHandlerDescriptor descriptor, object parameters, object response)
         {
             // Only pin the handler type, if we know the source handler (codelens) is also the resolver.

test/Lsp.Tests/Matchers/ResolveCommandMatcherTests.cs

@@ -139,6 +139,35 @@ public void Should_Handle_Null_Data()
             result.Should().Contain(x => x.Handler == resolveHandler);
         }
 
+        [Fact]
+        public void Should_Handle_Simple_Json_Data()
+        {
+            // Given
+            var handlerMatcher = new ResolveCommandMatcher(_logger);
+            var resolveHandler = Substitute.For<ICompletionResolveHandler>();
+            resolveHandler.CanResolve(Arg.Any<CompletionItem>()).Returns(true);
+
+            // When
+            var result = handlerMatcher.FindHandler(new CompletionItem() {
+                Data = new Uri("file:///c%3A/Users/mb/src/gh/Cake.Json/src/Cake.Json/Namespaces.cs")
+            },
+                    new List<HandlerDescriptor> {
+                        new HandlerDescriptor(DocumentNames.CompletionResolve,
+                            "Key",
+                            resolveHandler,
+                            resolveHandler.GetType(),
+                            typeof(CompletionItem),
+                            null,
+                            null,
+                            () => { }),
+                    })
+                .ToArray();
+
+            // Then
+            result.Should().NotBeNullOrEmpty();
+            result.Should().Contain(x => x.Handler == resolveHandler);
+        }
+
         [Fact]
         public void Should_Return_CompletionResolve_Descriptor()
         {
@@ -296,7 +325,7 @@ public void Should_FindPostProcessor_AsSelf_For_Completion()
                             "Key",
                             resolveHandler,
                             resolveHandler.GetType(),
-                            typeof(CodeLensParams),
+                            typeof(CompletionParams),
                             null,
                             null,
                             () => { });
@@ -325,7 +354,7 @@ public void Should_Update_CompletionItems_With_HandlerType()
                             "Key",
                             resolveHandler as IJsonRpcHandler,
                             resolveHandler.GetType(),
-                            typeof(CompletionItem),
+                            typeof(CompletionParams),
                             null,
                             null,
                             () => { });
@@ -363,7 +392,7 @@ public void Should_Update_CodeLensContainer_With_HandlerType()
                             "Key",
                             resolveHandler as IJsonRpcHandler,
                             resolveHandler.GetType(),
-                            typeof(CodeLens),
+                            typeof(CodeLensParams),
                             null,
                             null,
                             () => { });
@@ -385,5 +414,39 @@ public void Should_Update_CodeLensContainer_With_HandlerType()
             responseItem.Data[ResolveCommandMatcher.PrivateHandlerTypeName].Value<string>().Should().NotBeNullOrEmpty();
             responseItem.Data["data"]["hello"].Value<string>().Should().Be("world");
         }
+
+        [Fact]
+        public void Should_Update_CodeLens_Removing_HandlerType()
+        {
+            // Given
+            var handlerMatcher = new ResolveCommandMatcher(_logger);
+            var resolveHandler = Substitute.For(new Type[] {
+                typeof(ICodeLensHandler),
+                typeof(ICodeLensResolveHandler)
+            }, new object[0]);
+            (resolveHandler as ICodeLensResolveHandler).CanResolve(Arg.Any<CodeLens>()).Returns(true);
+            var descriptor = new HandlerDescriptor(
+                            DocumentNames.CodeLensResolve,
+                            "Key",
+                            resolveHandler as IJsonRpcHandler,
+                            resolveHandler.GetType(),
+                            typeof(CodeLens),
+                            null,
+                            null,
+                            () => { });
+
+            var item = new CodeLens() {
+                Data = JObject.FromObject(new { data = new { hello = "world" } })
+            };
+            item.Data[ResolveCommandMatcher.PrivateHandlerTypeName] = resolveHandler.GetType().FullName;
+
+            // When
+            var response = handlerMatcher.Process(descriptor, item);
+
+            // Then
+            response.Should().Be(item);
+            item.Data?[ResolveCommandMatcher.PrivateHandlerTypeName].Should().BeNull();
+            item.Data["hello"].Value<string>().Should().Be("world");
+        }
     }
 }