Wrong IRegistration definition

[gep13]

Following on from the fix that was provided here:

https://github.com/OmniSharp/csharp-language-server-protocol/issues/132

When trying to run the Chocolatey Language Server again in Visual Studio, I was met with another error:

Error converting value {null} to type 'System.Boolean'. Path 'capabilities.codeActionProvider'.

Full stack trace here:

at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.EnsureType(JsonReader reader, Object value, CultureInfo culture, JsonContract contract, Type targetType) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent) at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType) at Newtonsoft.Json.Linq.JToken.ToObject(Type objectType, JsonSerializer jsonSerializer) at Newtonsoft.Json.Linq.JToken.ToObject[T](JsonSerializer jsonSerializer) at StreamJsonRpc.JsonRpcMessage.GetResult[T](JsonSerializer serializer) at StreamJsonRpc.JsonRpc.<>c__DisplayClass93_1`1.b__0(JsonRpcMessage response) --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) at StreamJsonRpc.JsonRpc.d__93`1.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) at Microsoft.VisualStudio.LanguageServer.Client.RemoteLanguageClientInstance.d__52.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.VisualStudio.Threading.ThreadingTools.d__12.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.VisualStudio.LanguageServer.Client.RemoteLanguageClientInstance.d__50.MoveNext()

After discussing this with @mholo65 he thinks he has figured out what the problem is.

He believes that here:

https://github.com/OmniSharp/csharp-language-server-protocol/blob/master/src/Protocol/Document/Server/ICodeActionHandler.cs

Is using the wrong IRegistration<TextDocumentRegistrationOptions> and instead, it should be using this:

https://github.com/OmniSharp/csharp-language-server-protocol/blob/4050dad05963ac5d7df3579345bde9edd25971fb/src/Protocol/Models/CodeActionRegistrationOptions.cs

I tend to agree with him. I am going to create a PR to correct this in a short while.

[gep13]

@david-driscoll so while this issue has now been resolved, I have just tried the latest MyGet bits on my Extension, and it is still showing the same error message, so I suspect that there is still a problem.

The extension in question is here:

https://github.com/gep13/chocolatey-vs

If you open the sln in VS 2019, and debug it, when the experimental instance opens, if you open the ConsoleApp1 sln, and then open the nuspec file (it doesn't actually have anything in it) it should activate the extension, and start the LSP Client, which will then throw an exception in the OnServerInitializeFailedAsync method.

Would it be possible to re-open this issue, or should I open a new one?

[bjorkstromm]

This is also a bug in VS LSP protocol:

According to spec codeActionProvider is boolean or CodeActionOptions, however the documentation also states

The server provides code actions. The CodeActionOptions return type is only valid if the client signals code action literal support via the property textDocument.codeAction.codeActionLiteralSupport.

I think OmniSharp LSP should honor that and check the client capabilities before setting that property in the server capabilities.

[gep13]

@mholo65 Am I right in thinking that even with this change, I won't fix the issue that we are currently seeing, i.e. Chocolatey.Language.Server works in VSCode but not VS. Or have I misunderstood. Should we also raise an issue "somewhere" about the VS implementation?

[bjorkstromm]

@gep13 once we get static registration working as expected on the server side, it should work. I’ll see if I can find some spare time today or tomorrow.

[tgjones]

I'd like to take a look at this, because it's currently blocking me using OmniSharp.Extensions.LanguageServer in Visual Studio. Does one of you mind pointing me in the right direction for where the fix mentioned above should be done?

[tgjones]

Is it as straightforward as changing the ServerCapabilities initialisation in LanguageServer.Handle to this, or am I missing something?

CodeActionProvider = ccp.HasStaticHandler(textDocumentCapabilities.CodeAction) ? (BooleanOr<CodeActionOptions>) ccp.GetStaticOptions(textDocumentCapabilities.CodeAction).Get<ICodeActionOptions, CodeActionOptions>(CodeActionOptions.Of) : false,

[tgjones]

I've created a pull request #139 with a possible fix.

[bjorkstromm]

Fixed in #140