Skip to content

Commit 023ac40

Browse files
david-driscolldavid-driscoll
authored
Merge pull request #68 from OmniSharp/fix/routing
Updated to fix an issue where routing wouldn't work correctly
2 parents 6960413 + f0b65f1 commit 023ac40

File tree

6 files changed

+160
-41
lines changed

6 files changed

+160
-41
lines changed

‎src/Directory.Build.props

+4
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,8 @@
77
<InformationalVersion Condition="'$(GitVersion_InformationalVersion)' != ''">$(GitVersion_InformationalVersion)</InformationalVersion>
88
<LangVersion>Latest</LangVersion>
99
</PropertyGroup>
10+
<ItemGroup>
11+
<PackageReference Include="SourceLink.Create.GitHub" Version="2.7.6" PrivateAssets="All" />
12+
<DotNetCliToolReference Include="dotnet-sourcelink-git" Version="2.7.6" />
13+
</ItemGroup>
1014
</Project>

‎src/Server/HandlerCollection.cs

+16-3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ namespace OmniSharp.Extensions.LanguageServer.Server
1313
class HandlerCollection : IHandlerCollection
1414
{
1515
internal readonly HashSet<HandlerDescriptor> _handlers = new HashSet<HandlerDescriptor>();
16+
internal readonly HashSet<ITextDocumentSyncHandler> _documentSyncHandlers = new HashSet<ITextDocumentSyncHandler>();
17+
18+
public IEnumerable<ITextDocumentSyncHandler> TextDocumentSyncHandlers()
19+
{
20+
return _documentSyncHandlers;
21+
}
1622

1723
public IEnumerator<ILspHandlerDescriptor> GetEnumerator()
1824
{
@@ -50,9 +56,13 @@ public IDisposable Add(params IJsonRpcHandler[] handlers)
5056
}
5157
}
5258

53-
foreach (var handler in descriptors)
59+
foreach (var descriptor in descriptors)
5460
{
55-
_handlers.Add(handler);
61+
_handlers.Add(descriptor);
62+
if (descriptor.Handler is ITextDocumentSyncHandler documentSyncHandler)
63+
{
64+
_documentSyncHandlers.Add(documentSyncHandler);
65+
}
5666
}
5767

5868
return new ImmutableDisposable(descriptors);
@@ -87,7 +97,10 @@ private HandlerDescriptor GetDescriptor(string method, Type implementedType, IJs
8797
@params,
8898
registration,
8999
capability,
90-
() => _handlers.RemoveWhere(instance => instance.Handler == handler));
100+
() => {
101+
_handlers.RemoveWhere(instance => instance.Handler == handler);
102+
_documentSyncHandlers.RemoveWhere(instance => instance == handler);
103+
});
91104
}
92105

93106
private Type UnwrapGenericType(Type genericType, Type type)

‎src/Server/LanguageServer.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ internal LanguageServer(
7272
_serializer = serializer;
7373
_handlerMactherCollection = new HandlerMatcherCollection
7474
{
75-
new TextDocumentMatcher(_loggerFactory.CreateLogger<TextDocumentMatcher>()),
75+
new TextDocumentMatcher(_loggerFactory.CreateLogger<TextDocumentMatcher>(), _collection.TextDocumentSyncHandlers),
7676
new ExecuteCommandMatcher(_loggerFactory.CreateLogger<ExecuteCommandMatcher>())
7777
};
7878

‎src/Server/Matchers/TextDocumentMatcher.cs

+8-10
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@ namespace OmniSharp.Extensions.LanguageServer.Server.Matchers
1111
public class TextDocumentMatcher : IHandlerMatcher
1212
{
1313
private readonly ILogger _logger;
14+
private readonly Func<IEnumerable<ITextDocumentSyncHandler>> _getSyncHandlers;
1415

15-
public TextDocumentMatcher(ILogger logger)
16+
public TextDocumentMatcher(ILogger logger, Func<IEnumerable<ITextDocumentSyncHandler>> getSyncHandlers)
1617
{
1718
_logger = logger;
19+
_getSyncHandlers = getSyncHandlers;
1820
}
1921

2022
public IEnumerable<ILspHandlerDescriptor> FindHandler(object parameters, IEnumerable<ILspHandlerDescriptor> descriptors)
@@ -53,11 +55,7 @@ public IEnumerable<ILspHandlerDescriptor> FindHandler(object parameters, IEnumer
5355

5456
private List<TextDocumentAttributes> GetTextDocumentAttributes(IEnumerable<ILspHandlerDescriptor> method, Uri uri)
5557
{
56-
var textDocumentSyncHandlers = method
57-
.Select(x => x.Handler is ITextDocumentSyncHandler r ? r : null)
58-
.Where(x => x != null)
59-
.Distinct();
60-
return textDocumentSyncHandlers
58+
return _getSyncHandlers()
6159
.Select(x => x.GetTextDocumentAttributes(uri))
6260
.Where(x => x != null)
6361
.Distinct()
@@ -67,7 +65,7 @@ private List<TextDocumentAttributes> GetTextDocumentAttributes(IEnumerable<ILspH
6765
private IEnumerable<ILspHandlerDescriptor> GetHandler(IEnumerable<ILspHandlerDescriptor> method, IEnumerable<TextDocumentAttributes> attributes)
6866
{
6967
return attributes
70-
.SelectMany(x => GetHandler(method, x));
68+
.SelectMany(x => GetHandler(method, x));
7169
}
7270

7371
private IEnumerable<ILspHandlerDescriptor> GetHandler(IEnumerable<ILspHandlerDescriptor> method, TextDocumentAttributes attributes)
@@ -78,9 +76,9 @@ private IEnumerable<ILspHandlerDescriptor> GetHandler(IEnumerable<ILspHandlerDes
7876
_logger.LogTrace("Checking handler {Method}:{Handler}", method, handler.Handler.GetType().FullName);
7977
var registrationOptions = handler.Registration.RegisterOptions as TextDocumentRegistrationOptions;
8078

81-
_logger.LogTrace("Registration options {OptionsName}", registrationOptions.GetType().FullName);
82-
_logger.LogTrace("Document Selector {DocumentSelector}", registrationOptions.DocumentSelector.ToString());
83-
if (registrationOptions.DocumentSelector == null || registrationOptions.DocumentSelector.IsMatch(attributes))
79+
_logger.LogTrace("Registration options {OptionsName}", registrationOptions?.GetType().FullName);
80+
_logger.LogTrace("Document Selector {DocumentSelector}", registrationOptions?.DocumentSelector.ToString());
81+
if (registrationOptions?.DocumentSelector == null || registrationOptions.DocumentSelector.IsMatch(attributes))
8482
{
8583
_logger.LogTrace("Handler Selected: {Handler} via {DocumentSelector} (targeting {HandlerInterface})", handler.Handler.GetType().FullName, registrationOptions.DocumentSelector.ToString(), handler.HandlerType.FullName);
8684
yield return handler;

‎test/Lsp.Tests/LspRequestRouterTests.cs

+74-19
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
using OmniSharp.Extensions.LanguageServer.Server;
1818
using OmniSharp.Extensions.LanguageServer.Server.Abstractions;
1919
using OmniSharp.Extensions.LanguageServer.Server.Handlers;
20+
using OmniSharp.Extensions.LanguageServer.Server.Matchers;
2021
using Xunit;
2122
using Xunit.Abstractions;
2223
using Xunit.Sdk;
@@ -27,17 +28,11 @@ namespace Lsp.Tests
2728
public class LspRequestRouterTests
2829
{
2930
private readonly TestLoggerFactory _testLoggerFactory;
30-
private readonly IHandlerMatcherCollection _handlerMatcherCollection = new HandlerMatcherCollection();
31+
//private readonly IHandlerMatcherCollection handlerMatcherCollection = new HandlerMatcherCollection();
3132

3233
public LspRequestRouterTests(ITestOutputHelper testOutputHelper)
3334
{
3435
_testLoggerFactory = new TestLoggerFactory(testOutputHelper);
35-
var logger = Substitute.For<ILogger>();
36-
var matcher = Substitute.For<IHandlerMatcher>();
37-
matcher.FindHandler(Arg.Any<object>(), Arg.Any<IEnumerable<ILspHandlerDescriptor>>())
38-
.Returns(new List<HandlerDescriptor>());
39-
40-
_handlerMatcherCollection.Add(matcher);
4136
}
4237

4338
[Fact]
@@ -47,7 +42,10 @@ public async Task ShouldRouteToCorrect_Notification()
4742
textDocumentSyncHandler.Handle(Arg.Any<DidSaveTextDocumentParams>()).Returns(Task.CompletedTask);
4843

4944
var collection = new HandlerCollection { textDocumentSyncHandler };
50-
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
45+
var handlerMatcherCollection = new HandlerMatcherCollection {
46+
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
47+
};
48+
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());
5149

5250
var @params = new DidSaveTextDocumentParams() {
5351
TextDocument = new TextDocumentIdentifier(new Uri("file:///c:/test/123.cs"))
@@ -69,7 +67,10 @@ public async Task ShouldRouteToCorrect_Notification_WithManyHandlers()
6967
textDocumentSyncHandler2.Handle(Arg.Any<DidSaveTextDocumentParams>()).Returns(Task.CompletedTask);
7068

7169
var collection = new HandlerCollection { textDocumentSyncHandler, textDocumentSyncHandler2 };
72-
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
70+
var handlerMatcherCollection = new HandlerMatcherCollection {
71+
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
72+
};
73+
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());
7374

7475
var @params = new DidSaveTextDocumentParams() {
7576
TextDocument = new TextDocumentIdentifier(new Uri("file:///c:/test/123.cake"))
@@ -79,8 +80,8 @@ public async Task ShouldRouteToCorrect_Notification_WithManyHandlers()
7980

8081
await mediator.RouteNotification(mediator.GetDescriptor(request), request);
8182

82-
await textDocumentSyncHandler.Received(1).Handle(Arg.Any<DidSaveTextDocumentParams>());
83-
await textDocumentSyncHandler2.Received(0).Handle(Arg.Any<DidSaveTextDocumentParams>());
83+
await textDocumentSyncHandler.Received(0).Handle(Arg.Any<DidSaveTextDocumentParams>());
84+
await textDocumentSyncHandler2.Received(1).Handle(Arg.Any<DidSaveTextDocumentParams>());
8485
}
8586

8687
[Fact]
@@ -96,7 +97,10 @@ public async Task ShouldRouteToCorrect_Request()
9697
.Returns(new CommandContainer());
9798

9899
var collection = new HandlerCollection { textDocumentSyncHandler, codeActionHandler };
99-
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
100+
var handlerMatcherCollection = new HandlerMatcherCollection {
101+
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
102+
};
103+
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());
100104

101105
var id = Guid.NewGuid().ToString();
102106
var @params = new DidSaveTextDocumentParams() {
@@ -131,19 +135,61 @@ public async Task ShouldRouteToCorrect_Request_WithManyHandlers()
131135
.Returns(new CommandContainer());
132136

133137
var collection = new HandlerCollection { textDocumentSyncHandler, textDocumentSyncHandler2, codeActionHandler, codeActionHandler2 };
134-
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
138+
var handlerMatcherCollection = new HandlerMatcherCollection {
139+
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
140+
};
141+
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());
135142

136143
var id = Guid.NewGuid().ToString();
137-
var @params = new DidSaveTextDocumentParams() {
144+
var @params = new CodeActionParams() {
138145
TextDocument = new TextDocumentIdentifier(new Uri("file:///c:/test/123.cake"))
139146
};
140147

141148
var request = new Request(id, DocumentNames.CodeAction, JObject.Parse(JsonConvert.SerializeObject(@params, new Serializer(ClientVersion.Lsp3).Settings)));
142149

143150
await mediator.RouteRequest(mediator.GetDescriptor(request), request);
144151

145-
await codeActionHandler.Received(1).Handle(Arg.Any<CodeActionParams>(), Arg.Any<CancellationToken>());
146-
await codeActionHandler2.Received(0).Handle(Arg.Any<CodeActionParams>(), Arg.Any<CancellationToken>());
152+
await codeActionHandler.Received(0).Handle(Arg.Any<CodeActionParams>(), Arg.Any<CancellationToken>());
153+
await codeActionHandler2.Received(1).Handle(Arg.Any<CodeActionParams>(), Arg.Any<CancellationToken>());
154+
}
155+
156+
[Fact]
157+
public async Task ShouldRouteToCorrect_Request_WithManyHandlers_CodeLensHandler()
158+
{
159+
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));
160+
var textDocumentSyncHandler2 = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cake"));
161+
textDocumentSyncHandler.Handle(Arg.Any<DidSaveTextDocumentParams>()).Returns(Task.CompletedTask);
162+
textDocumentSyncHandler2.Handle(Arg.Any<DidSaveTextDocumentParams>()).Returns(Task.CompletedTask);
163+
164+
var codeActionHandler = Substitute.For<ICodeLensHandler>();
165+
codeActionHandler.GetRegistrationOptions().Returns(new CodeLensRegistrationOptions() { DocumentSelector = DocumentSelector.ForPattern("**/*.cs") });
166+
codeActionHandler
167+
.Handle(Arg.Any<CodeLensParams>(), Arg.Any<CancellationToken>())
168+
.Returns(new CodeLensContainer());
169+
170+
var codeActionHandler2 = Substitute.For<ICodeLensHandler>();
171+
codeActionHandler2.GetRegistrationOptions().Returns(new CodeLensRegistrationOptions() { DocumentSelector = DocumentSelector.ForPattern("**/*.cake") });
172+
codeActionHandler2
173+
.Handle(Arg.Any<CodeLensParams>(), Arg.Any<CancellationToken>())
174+
.Returns(new CodeLensContainer());
175+
176+
var collection = new HandlerCollection { textDocumentSyncHandler, textDocumentSyncHandler2, codeActionHandler, codeActionHandler2 };
177+
var handlerMatcherCollection = new HandlerMatcherCollection {
178+
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
179+
};
180+
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());
181+
182+
var id = Guid.NewGuid().ToString();
183+
var @params = new CodeLensParams() {
184+
TextDocument = new TextDocumentIdentifier(new Uri("file:///c:/test/123.cs"))
185+
};
186+
187+
var request = new Request(id, DocumentNames.CodeLens, JObject.Parse(JsonConvert.SerializeObject(@params, new Serializer(ClientVersion.Lsp3).Settings)));
188+
189+
await mediator.RouteRequest(mediator.GetDescriptor(request), request);
190+
191+
await codeActionHandler2.Received(0).Handle(Arg.Any<CodeLensParams>(), Arg.Any<CancellationToken>());
192+
await codeActionHandler.Received(1).Handle(Arg.Any<CodeLensParams>(), Arg.Any<CancellationToken>());
147193
}
148194

149195
[Fact]
@@ -155,7 +201,10 @@ public async Task ShouldRouteTo_CorrectRequestWhenGivenNullParams()
155201
.Returns(Task.CompletedTask);
156202

157203
var collection = new HandlerCollection { handler };
158-
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
204+
var handlerMatcherCollection = new HandlerMatcherCollection {
205+
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
206+
};
207+
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());
159208

160209
var id = Guid.NewGuid().ToString();
161210
var request = new Request(id, GeneralNames.Shutdown, new JObject());
@@ -177,7 +226,10 @@ public async Task ShouldHandle_Request_WithNullParameters()
177226
};
178227

179228
var collection = new HandlerCollection { shutdownHandler };
180-
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
229+
var handlerMatcherCollection = new HandlerMatcherCollection {
230+
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
231+
};
232+
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());
181233

182234
JToken @params = JValue.CreateNull(); // If the "params" property present but null, this will be JTokenType.Null.
183235

@@ -201,7 +253,10 @@ public async Task ShouldHandle_Request_WithMissingParameters()
201253
};
202254

203255
var collection = new HandlerCollection { shutdownHandler };
204-
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
256+
var handlerMatcherCollection = new HandlerMatcherCollection {
257+
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
258+
};
259+
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());
205260

206261
JToken @params = null; // If the "params" property was missing entirely, this will be null.
207262

0 commit comments

Comments
 (0)