Skip to content

Updated support for capabilities to be more generous to clients that … #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/Server/ClientCapabilityProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ public ClientCapabilityProvider(IHandlerCollection collection)
public bool HasStaticHandler<T>(Supports<T> capability)
where T : DynamicCapability, ConnectedCapability<IJsonRpcHandler>
{
if (!capability.IsSupported) return false;
if (capability.Value == null) return false;
if (capability.Value.DynamicRegistration == true) return false;
// Dynamic registration will cause us to double register things if we report our capabilities staticly.
// However if the client does not tell us it's capabilities we should just assume that they do not support
// dynamic registraiton but we should report any capabilities statically
if (capability.IsSupported && capability.Value != null && capability.Value.DynamicRegistration == true) return false;

var handlerTypes = typeof(T).GetTypeInfo().ImplementedInterfaces
.Where(x => x.GetTypeInfo().IsGenericType && x.GetTypeInfo().GetGenericTypeDefinition() == typeof(ConnectedCapability<>))
Expand Down
74 changes: 59 additions & 15 deletions test/Lsp.Tests/ClientCapabilityProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using OmniSharp.Extensions.LanguageServer.Protocol;
using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
using OmniSharp.Extensions.LanguageServer.Protocol.Server.Capabilities;
using OmniSharp.Extensions.LanguageServer.Server;
using Xunit;
Expand Down Expand Up @@ -42,15 +43,24 @@ public static IEnumerable<object[]> AllowSupportedCapabilities()
});
}

[Theory, MemberData(nameof(DisallowUnsupportedCapabilities))]
public void Should_DisallowUnsupportedCapabilities(IJsonRpcHandler handler, object instance)
[Theory, MemberData(nameof(AllowUnsupportedCapabilities))]
public void Should_AllowUnsupportedCapabilities(IJsonRpcHandler handler, object instance)
{
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));

var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue) { textDocumentSyncHandler, handler };
var provider = new ClientCapabilityProvider(collection);

HasHandler(provider, instance).Should().BeFalse();
HasHandler(provider, instance).Should().BeTrue();
}

public static IEnumerable<object[]> AllowUnsupportedCapabilities()
{
return GetItems(Capabilities, type => {
var handlerTypes = GetHandlerTypes(type);
var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), false) };
});
}

[Fact]
Expand Down Expand Up @@ -81,17 +91,29 @@ public void Should_Invoke_Reduce_Delegate()
stub.Received().Invoke(Arg.Any<IEnumerable<IExecuteCommandOptions>>());
}

public static IEnumerable<object[]> DisallowUnsupportedCapabilities()
[Theory, MemberData(nameof(AllowNullSupportsCapabilities))]
public void Should_AllowNullSupportedCapabilities(IJsonRpcHandler handler, object instance)
{
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));

var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue) { textDocumentSyncHandler, handler };
var provider = new ClientCapabilityProvider(collection);

HasHandler(provider, instance).Should().BeTrue();
}

public static IEnumerable<object[]> AllowNullSupportsCapabilities()
{
return GetItems(Capabilities, type => {
var handlerTypes = GetHandlerTypes(type);
var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), false) };
return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), true) };
});
}

[Theory, MemberData(nameof(DisallowNullSupportsCapabilities))]
public void Should_DisallowNullSupportedCapabilities(IJsonRpcHandler handler, object instance)

[Theory, MemberData(nameof(DisallowDynamicSupportsCapabilities))]
public void Should_DisallowDynamicSupportedCapabilities(IJsonRpcHandler handler, object instance)
{
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));

Expand All @@ -101,27 +123,49 @@ public void Should_DisallowNullSupportedCapabilities(IJsonRpcHandler handler, ob
HasHandler(provider, instance).Should().BeFalse();
}

public static IEnumerable<object[]> DisallowNullSupportsCapabilities()
public static IEnumerable<object[]> DisallowDynamicSupportsCapabilities()
{
return GetItems(Capabilities, type => {
var handlerTypes = GetHandlerTypes(type);
var handler = Substitute.For(handlerTypes.ToArray(), new object[0]);
return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), true) };
var capability = Activator.CreateInstance(type);
if (capability is DynamicCapability dyn) dyn.DynamicRegistration = true;
return new[] { handler, Activator.CreateInstance(typeof(Supports<>).MakeGenericType(type), true, capability) };
});
}

private static bool HasHandler(ClientCapabilityProvider provider, object instance)
[Fact]
public void Should_Handle_Mixed_Capabilities()
{
return (bool)typeof(ClientCapabilityProviderTests).GetTypeInfo()
.GetMethod(nameof(GenericHasHandler), BindingFlags.Static | BindingFlags.NonPublic)
.MakeGenericMethod(instance.GetType().GetTypeInfo().GetGenericArguments()[0]).Invoke(null, new[] { provider, instance });
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));

var codeActionHandler = Substitute.For<ICodeActionHandler>();
var definitionHandler = Substitute.For<IDefinitionHandler>();
var typeDefinitionHandler = Substitute.For<ITypeDefinitionHandler>();

var collection = new HandlerCollection(SupportedCapabilitiesFixture.AlwaysTrue) { textDocumentSyncHandler, codeActionHandler, definitionHandler, typeDefinitionHandler };
var provider = new ClientCapabilityProvider(collection);
var capabilities = new ClientCapabilities() {
TextDocument = new TextDocumentClientCapabilities() {
CodeAction = new Supports<CodeActionCapability>(true, new CodeActionCapability() {
DynamicRegistration = false,
}),
TypeDefinition = new Supports<TypeDefinitionCapability>(true, new TypeDefinitionCapability() {
DynamicRegistration = true,
})
}
};

provider.GetStaticOptions(capabilities.TextDocument.CodeAction).Get<ICodeActionOptions, CodeActionOptions>(CodeActionOptions.Of).Should().NotBeNull();
provider.HasStaticHandler(capabilities.TextDocument.Definition).Should().BeTrue();
provider.HasStaticHandler(capabilities.TextDocument.TypeDefinition).Should().BeFalse();
}

private static bool HasHandler(ClientCapabilityProvider provider, Type type)
private static bool HasHandler(ClientCapabilityProvider provider, object instance)
{
return (bool)typeof(ClientCapabilityProviderTests).GetTypeInfo()
.GetMethod(nameof(GenericHasHandler), BindingFlags.Static | BindingFlags.NonPublic)
.MakeGenericMethod(type).Invoke(null, new object[] { provider, null });
.MakeGenericMethod(instance.GetType().GetTypeInfo().GetGenericArguments()[0]).Invoke(null, new[] { provider, instance });
}

private static bool GenericHasHandler<T>(ClientCapabilityProvider provider, Supports<T> supports)
Expand Down
1 change: 1 addition & 0 deletions test/Lsp.Tests/LanguageServerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public async Task GH141_CrashesWithEmptyInitializeParams()
.WithLoggerFactory(LoggerFactory)
.AddDefaultLoggingProvider()
.WithMinimumLogLevel(LogLevel.Trace)
.AddHandlers(TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs")))
) as IRequestHandler<InitializeParams, InitializeResult>;

var handler = server as IRequestHandler<InitializeParams, InitializeResult>;
Expand Down