Skip to content

Commit d1ad86d

Browse files
david-driscolldavid-driscoll
authored
Merge pull request #41 from OmniSharp/fix/didchange-callback
Fix/didchange callback
2 parents b2bb198 + f128b27 commit d1ad86d

14 files changed

+109
-77
lines changed

‎src/JsonRpc/IRequestRouter.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
using System;
1+
using System;
22
using System.Threading.Tasks;
33
using OmniSharp.Extensions.JsonRpc.Server;
44

55
namespace OmniSharp.Extensions.JsonRpc
66
{
77
public interface IRequestRouter
88
{
9-
void RouteNotification(Notification notification);
9+
Task RouteNotification(Notification notification);
1010
Task<ErrorResponse> RouteRequest(Request request);
1111
}
1212
}

‎src/JsonRpc/InputHandler.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,10 +190,10 @@ private void HandleRequest(string request)
190190
_scheduler.Add(
191191
type,
192192
item.Notification.Method,
193-
() => {
193+
async () => {
194194
try
195195
{
196-
_requestRouter.RouteNotification(item.Notification);
196+
await _requestRouter.RouteNotification(item.Notification);
197197
}
198198
catch (Exception e)
199199
{
@@ -202,7 +202,6 @@ private void HandleRequest(string request)
202202
// If an exception happens... the whole system could be in a bad state, hence this throwing currently.
203203
throw;
204204
}
205-
return Task.CompletedTask;
206205
}
207206
);
208207
}

‎src/JsonRpc/ProcessScheduler.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System;
22
using System.Collections.Concurrent;
33
using System.Collections.Generic;
44
using System.Threading;
@@ -128,7 +128,7 @@ public void Dispose()
128128
}
129129
}
130130

131-
static class Events
131+
public static class Events
132132
{
133133
public static EventId UnhandledException = new EventId(1337_100);
134134
public static EventId UnhandledRequest = new EventId(1337_101);

‎src/JsonRpc/RequestRouter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public IDisposable Add(IJsonRpcHandler handler)
2222
return _collection.Add(handler);
2323
}
2424

25-
public async void RouteNotification(Notification notification)
25+
public async Task RouteNotification(Notification notification)
2626
{
2727
var handler = _collection.FirstOrDefault(x => x.Method == notification.Method);
2828

‎src/Lsp/ClientCapabilityProvider.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public ClientCapabilityProvider(IHandlerCollection collection)
1616
_collection = collection;
1717
}
1818

19-
public bool HasHandler<T>(Supports<T> capability)
19+
public bool HasStaticHandler<T>(Supports<T> capability)
2020
where T : DynamicCapability, ConnectedCapability<IJsonRpcHandler>
2121
{
2222
if (!capability.IsSupported) return false;
@@ -26,13 +26,16 @@ public bool HasHandler<T>(Supports<T> capability)
2626
var handlerType = typeof(T).GetTypeInfo().ImplementedInterfaces
2727
.Single(x => x.GetTypeInfo().IsGenericType && x.GetTypeInfo().GetGenericTypeDefinition() == typeof(ConnectedCapability<>))
2828
.GetTypeInfo().GetGenericArguments()[0].GetTypeInfo();
29-
return !capability.Value.DynamicRegistration && _collection.Any(z => z.HandlerType.GetTypeInfo().IsAssignableFrom(handlerType));
29+
return !capability.Value.DynamicRegistration &&
30+
_collection.Any(z =>
31+
z.HandlerType.GetTypeInfo().IsAssignableFrom(handlerType) ||
32+
z.Handler.GetType().GetTypeInfo().IsAssignableFrom(handlerType));
3033
}
3134

32-
public IOptionsGetter GetOptions<T>(Supports<T> capability)
35+
public IOptionsGetter GetStaticOptions<T>(Supports<T> capability)
3336
where T : DynamicCapability, ConnectedCapability<IJsonRpcHandler>
3437
{
35-
return !HasHandler(capability) ? Null : new OptionsGetter(_collection);
38+
return !HasStaticHandler(capability) ? Null : new OptionsGetter(_collection);
3639
}
3740

3841
private static readonly IOptionsGetter Null = new NullOptionsGetter();

‎src/Lsp/LanguageServer.cs

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -130,22 +130,22 @@ async Task<InitializeResult> IRequestHandler<InitializeParams, InitializeResult>
130130
var ccp = new ClientCapabilityProvider(_collection);
131131

132132
var serverCapabilities = new ServerCapabilities() {
133-
CodeActionProvider = ccp.HasHandler(textDocumentCapabilities.CodeAction),
134-
CodeLensProvider = ccp.GetOptions(textDocumentCapabilities.CodeLens).Get<ICodeLensOptions, CodeLensOptions>(CodeLensOptions.Of),
135-
CompletionProvider = ccp.GetOptions(textDocumentCapabilities.Completion).Get<ICompletionOptions, CompletionOptions>(CompletionOptions.Of),
136-
DefinitionProvider = ccp.HasHandler(textDocumentCapabilities.Definition),
137-
DocumentFormattingProvider = ccp.HasHandler(textDocumentCapabilities.Formatting),
138-
DocumentHighlightProvider = ccp.HasHandler(textDocumentCapabilities.DocumentHighlight),
139-
DocumentLinkProvider = ccp.GetOptions(textDocumentCapabilities.DocumentLink).Get<IDocumentLinkOptions, DocumentLinkOptions>(DocumentLinkOptions.Of),
140-
DocumentOnTypeFormattingProvider = ccp.GetOptions(textDocumentCapabilities.OnTypeFormatting).Get<IDocumentOnTypeFormattingOptions, DocumentOnTypeFormattingOptions>(DocumentOnTypeFormattingOptions.Of),
141-
DocumentRangeFormattingProvider = ccp.HasHandler(textDocumentCapabilities.RangeFormatting),
142-
DocumentSymbolProvider = ccp.HasHandler(textDocumentCapabilities.DocumentSymbol),
143-
ExecuteCommandProvider = ccp.GetOptions(workspaceCapabilities.ExecuteCommand).Get<IExecuteCommandOptions, ExecuteCommandOptions>(ExecuteCommandOptions.Of),
144-
HoverProvider = ccp.HasHandler(textDocumentCapabilities.Hover),
145-
ReferencesProvider = ccp.HasHandler(textDocumentCapabilities.References),
146-
RenameProvider = ccp.HasHandler(textDocumentCapabilities.Rename),
147-
SignatureHelpProvider = ccp.GetOptions(textDocumentCapabilities.SignatureHelp).Get<ISignatureHelpOptions, SignatureHelpOptions>(SignatureHelpOptions.Of),
148-
WorkspaceSymbolProvider = ccp.HasHandler(workspaceCapabilities.Symbol)
133+
CodeActionProvider = ccp.HasStaticHandler(textDocumentCapabilities.CodeAction),
134+
CodeLensProvider = ccp.GetStaticOptions(textDocumentCapabilities.CodeLens).Get<ICodeLensOptions, CodeLensOptions>(CodeLensOptions.Of),
135+
CompletionProvider = ccp.GetStaticOptions(textDocumentCapabilities.Completion).Get<ICompletionOptions, CompletionOptions>(CompletionOptions.Of),
136+
DefinitionProvider = ccp.HasStaticHandler(textDocumentCapabilities.Definition),
137+
DocumentFormattingProvider = ccp.HasStaticHandler(textDocumentCapabilities.Formatting),
138+
DocumentHighlightProvider = ccp.HasStaticHandler(textDocumentCapabilities.DocumentHighlight),
139+
DocumentLinkProvider = ccp.GetStaticOptions(textDocumentCapabilities.DocumentLink).Get<IDocumentLinkOptions, DocumentLinkOptions>(DocumentLinkOptions.Of),
140+
DocumentOnTypeFormattingProvider = ccp.GetStaticOptions(textDocumentCapabilities.OnTypeFormatting).Get<IDocumentOnTypeFormattingOptions, DocumentOnTypeFormattingOptions>(DocumentOnTypeFormattingOptions.Of),
141+
DocumentRangeFormattingProvider = ccp.HasStaticHandler(textDocumentCapabilities.RangeFormatting),
142+
DocumentSymbolProvider = ccp.HasStaticHandler(textDocumentCapabilities.DocumentSymbol),
143+
ExecuteCommandProvider = ccp.GetStaticOptions(workspaceCapabilities.ExecuteCommand).Get<IExecuteCommandOptions, ExecuteCommandOptions>(ExecuteCommandOptions.Of),
144+
HoverProvider = ccp.HasStaticHandler(textDocumentCapabilities.Hover),
145+
ReferencesProvider = ccp.HasStaticHandler(textDocumentCapabilities.References),
146+
RenameProvider = ccp.HasStaticHandler(textDocumentCapabilities.Rename),
147+
SignatureHelpProvider = ccp.GetStaticOptions(textDocumentCapabilities.SignatureHelp).Get<ISignatureHelpOptions, SignatureHelpOptions>(SignatureHelpOptions.Of),
148+
WorkspaceSymbolProvider = ccp.HasStaticHandler(workspaceCapabilities.Symbol)
149149
};
150150

151151
var textSyncHandlers = _collection
@@ -168,16 +168,21 @@ async Task<InitializeResult> IRequestHandler<InitializeParams, InitializeResult>
168168
}
169169
else
170170
{
171-
if (ccp.HasHandler(textDocumentCapabilities.Synchronization))
171+
if (ccp.HasStaticHandler(textDocumentCapabilities.Synchronization))
172172
{
173173
// TODO: Merge options
174-
serverCapabilities.TextDocumentSync = textSyncHandlers.FirstOrDefault()?.Options ?? new TextDocumentSyncOptions() {
175-
Change = TextDocumentSyncKind.None,
176-
OpenClose = false,
177-
Save = new SaveOptions() { IncludeText = false },
178-
WillSave = false,
179-
WillSaveWaitUntil = false
180-
};
174+
serverCapabilities.TextDocumentSync =
175+
textSyncHandlers.FirstOrDefault()?.Options ?? new TextDocumentSyncOptions() {
176+
Change = TextDocumentSyncKind.None,
177+
OpenClose = false,
178+
Save = new SaveOptions() { IncludeText = false },
179+
WillSave = false,
180+
WillSaveWaitUntil = false
181+
};
182+
}
183+
else
184+
{
185+
serverCapabilities.TextDocumentSync = TextDocumentSyncKind.None;
181186
}
182187
}
183188

‎src/Lsp/LspRequestRouter.cs

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -57,35 +57,48 @@ private ILspHandlerDescriptor FindDescriptor(string method, JToken @params)
5757
if (typeof(ITextDocumentIdentifierParams).GetTypeInfo().IsAssignableFrom(descriptor.Params))
5858
{
5959
var textDocumentIdentifierParams = @params.ToObject(descriptor.Params) as ITextDocumentIdentifierParams;
60-
var textDocumentSyncHandlers = _collection
61-
.Select(x => x.Handler is ITextDocumentSyncHandler r ? r : null)
62-
.Where(x => x != null)
63-
.Distinct();
64-
var attributes = textDocumentSyncHandlers
65-
.Select(x => x.GetTextDocumentAttributes(textDocumentIdentifierParams.TextDocument.Uri))
66-
.Where(x => x != null)
67-
.Distinct()
68-
.ToList();
60+
var attributes = GetTextDocumentAttributes(textDocumentIdentifierParams.TextDocument.Uri);
6961

7062
_logger.LogTrace("Found attributes {Count}, {Attributes}", attributes.Count, attributes.Select(x => $"{x.LanguageId}:{x.Scheme}:{x.Uri}"));
7163

7264
return GetHandler(method, attributes);
7365
}
74-
else if (typeof(DidOpenTextDocumentParams).GetTypeInfo().IsAssignableFrom(descriptor.Params))
66+
else if (@params?.ToObject(descriptor.Params) is DidOpenTextDocumentParams openTextDocumentParams)
7567
{
76-
var openTextDocumentParams = @params.ToObject(descriptor.Params) as DidOpenTextDocumentParams;
7768
var attributes = new TextDocumentAttributes(openTextDocumentParams.TextDocument.Uri, openTextDocumentParams.TextDocument.LanguageId);
7869

7970
_logger.LogTrace("Created attribute {Attribute}", $"{attributes.LanguageId}:{attributes.Scheme}:{attributes.Uri}");
8071

8172
return GetHandler(method, attributes);
8273
}
74+
else if (@params?.ToObject(descriptor.Params) is DidChangeTextDocumentParams didChangeDocumentParams)
75+
{
76+
// TODO: Do something with document version here?
77+
var attributes = GetTextDocumentAttributes(didChangeDocumentParams.TextDocument.Uri);
78+
79+
_logger.LogTrace("Found attributes {Count}, {Attributes}", attributes.Count, attributes.Select(x => $"{x.LanguageId}:{x.Scheme}:{x.Uri}"));
80+
81+
return GetHandler(method, attributes);
82+
}
8383

8484
// TODO: How to split these
8585
// Do they fork and join?
8686
return descriptor;
8787
}
8888

89+
private List<TextDocumentAttributes> GetTextDocumentAttributes(Uri uri)
90+
{
91+
var textDocumentSyncHandlers = _collection
92+
.Select(x => x.Handler is ITextDocumentSyncHandler r ? r : null)
93+
.Where(x => x != null)
94+
.Distinct();
95+
return textDocumentSyncHandlers
96+
.Select(x => x.GetTextDocumentAttributes(uri))
97+
.Where(x => x != null)
98+
.Distinct()
99+
.ToList();
100+
}
101+
89102
private ILspHandlerDescriptor GetHandler(string method, IEnumerable<TextDocumentAttributes> attributes)
90103
{
91104
return attributes
@@ -105,29 +118,37 @@ private ILspHandlerDescriptor GetHandler(string method, TextDocumentAttributes a
105118
_logger.LogTrace("Document Selector {DocumentSelector}", registrationOptions.DocumentSelector.ToString());
106119
if (registrationOptions.DocumentSelector == null || registrationOptions.DocumentSelector.IsMatch(attributes))
107120
{
121+
_logger.LogTrace("Handler Selected: {Handler} via {DocumentSelector} (targeting {HandlerInterface})", handler.Handler.GetType().FullName, registrationOptions.DocumentSelector.ToString(), handler.HandlerType.GetType().FullName);
108122
return handler;
109123
}
110124
}
111125
return null;
112126
}
113127

114-
public async void RouteNotification(Notification notification)
128+
public async Task RouteNotification(Notification notification)
115129
{
116130
var handler = FindDescriptor(notification.Method, notification.Params);
117131
if (handler is null) { return; }
118132

119-
Task result;
120-
if (handler.Params is null)
133+
try
121134
{
122-
result = ReflectionRequestHandlers.HandleNotification(handler);
135+
Task result;
136+
if (handler.Params is null)
137+
{
138+
result = ReflectionRequestHandlers.HandleNotification(handler);
139+
}
140+
else
141+
{
142+
var @params = notification.Params.ToObject(handler.Params);
143+
result = ReflectionRequestHandlers.HandleNotification(handler, @params);
144+
}
145+
146+
await result;
123147
}
124-
else
148+
catch (Exception e)
125149
{
126-
var @params = notification.Params.ToObject(handler.Params);
127-
result = ReflectionRequestHandlers.HandleNotification(handler, @params);
150+
_logger.LogCritical(Events.UnhandledRequest, e, "Failed to handle request {Method}", notification.Method);
128151
}
129-
130-
await result;
131152
}
132153

133154
public async Task<ErrorResponse> RouteRequest(Request request)
@@ -184,6 +205,11 @@ public async Task<ErrorResponse> RouteRequest(Request request)
184205
{
185206
return new RequestCancelled();
186207
}
208+
catch (Exception e)
209+
{
210+
_logger.LogCritical(Events.UnhandledRequest, e, "Failed to handle notification {Method}", request.Method);
211+
return new InternalError(id);
212+
}
187213
finally
188214
{
189215
_requests.TryRemove(id, out var _);

‎src/Lsp/Models/DidChangeTextDocumentParams.cs

Lines changed: 2 additions & 2 deletions
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

44
namespace OmniSharp.Extensions.LanguageServer.Models
@@ -18,4 +18,4 @@ public class DidChangeTextDocumentParams
1818
/// </summary>
1919
public Container<TextDocumentContentChangeEvent> ContentChanges { get; set; }
2020
}
21-
}
21+
}

‎test/JsonRpc.Tests/InputHandlerTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ public void ShouldHandleError()
206206
}
207207

208208
[Fact]
209-
public void ShouldHandleNotification()
209+
public async Task ShouldHandleNotification()
210210
{
211211
var inputStream = new MemoryStream(Encoding.ASCII.GetBytes("Content-Length: 2\r\n\r\n{}"));
212212
var outputHandler = Substitute.For<IOutputHandler>();
@@ -234,7 +234,7 @@ public void ShouldHandleNotification()
234234
});
235235
}))
236236
{
237-
incomingRequestRouter.Received().RouteNotification(notification);
237+
await incomingRequestRouter.Received().RouteNotification(notification);
238238
}
239239
}
240240

‎test/JsonRpc.Tests/MediatorTestsNotificationHandler.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System;
22
using System.Reflection;
33
using System.Threading.Tasks;
44
using NSubstitute;
@@ -23,10 +23,10 @@ public async Task ExecutesHandler()
2323

2424
var notification = new Notification("exit", null);
2525

26-
mediator.RouteNotification(notification);
26+
await mediator.RouteNotification(notification);
2727

2828
await exitHandler.Received(1).Handle();
2929
}
3030

3131
}
32-
}
32+
}

‎test/JsonRpc.Tests/MediatorTestsNotificationHandlerOfT.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System;
22
using System.Reflection;
33
using System.Threading.Tasks;
44
using Newtonsoft.Json;
@@ -33,10 +33,10 @@ public async Task ExecutesHandler()
3333
var @params = new CancelParams() { Id = Guid.NewGuid() };
3434
var notification = new Notification("$/cancelRequest", JObject.Parse(JsonConvert.SerializeObject(@params)));
3535

36-
mediator.RouteNotification(notification);
36+
await mediator.RouteNotification(notification);
3737

3838
await cancelRequestHandler.Received(1).Handle(Arg.Any<CancelParams>());
3939
}
4040

4141
}
42-
}
42+
}

‎test/Lsp.Tests/ClientCapabilityProviderTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ private static bool HasHandler(ClientCapabilityProvider provider, Type type)
111111
private static bool GenericHasHandler<T>(ClientCapabilityProvider provider, Supports<T> supports)
112112
where T : DynamicCapability, ConnectedCapability<IJsonRpcHandler>
113113
{
114-
return provider.HasHandler(supports);
114+
return provider.HasStaticHandler(supports);
115115
}
116116

117117
private static IEnumerable<object[]> GetItems<T>(IEnumerable<T> types, Func<T, IEnumerable<object>> func)

‎test/Lsp.Tests/LspRequestRouterTests.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public LspRequestRouterTests(ITestOutputHelper testOutputHelper)
3030
}
3131

3232
[Fact]
33-
public void ShouldRouteToCorrect_Notification()
33+
public async Task ShouldRouteToCorrect_Notification()
3434
{
3535
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));
3636
textDocumentSyncHandler.Handle(Arg.Any<DidSaveTextDocumentParams>()).Returns(Task.CompletedTask);
@@ -44,13 +44,13 @@ public void ShouldRouteToCorrect_Notification()
4444

4545
var request = new Notification("textDocument/didSave", JObject.Parse(JsonConvert.SerializeObject(@params)));
4646

47-
mediator.RouteNotification(request);
47+
await mediator.RouteNotification(request);
4848

49-
textDocumentSyncHandler.Received(1).Handle(Arg.Any<DidSaveTextDocumentParams>());
49+
await textDocumentSyncHandler.Received(1).Handle(Arg.Any<DidSaveTextDocumentParams>());
5050
}
5151

5252
[Fact]
53-
public void ShouldRouteToCorrect_Notification_WithManyHandlers()
53+
public async Task ShouldRouteToCorrect_Notification_WithManyHandlers()
5454
{
5555
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));
5656
var textDocumentSyncHandler2 = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cake"));
@@ -66,10 +66,10 @@ public void ShouldRouteToCorrect_Notification_WithManyHandlers()
6666

6767
var request = new Notification("textDocument/didSave", JObject.Parse(JsonConvert.SerializeObject(@params)));
6868

69-
mediator.RouteNotification(request);
69+
await mediator.RouteNotification(request);
7070

71-
textDocumentSyncHandler.Received(0).Handle(Arg.Any<DidSaveTextDocumentParams>());
72-
textDocumentSyncHandler2.Received(1).Handle(Arg.Any<DidSaveTextDocumentParams>());
71+
await textDocumentSyncHandler.Received(0).Handle(Arg.Any<DidSaveTextDocumentParams>());
72+
await textDocumentSyncHandler2.Received(1).Handle(Arg.Any<DidSaveTextDocumentParams>());
7373
}
7474

7575
[Fact]

0 commit comments

Comments
 (0)