Skip to content
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

Wrong IRegistration definition #136

Closed
gep13 opened this issue Apr 4, 2019 · 8 comments
Closed

Wrong IRegistration definition #136

gep13 opened this issue Apr 4, 2019 · 8 comments
Labels
bug Something isn't working

Comments

@gep13
Copy link
Contributor

gep13 commented Apr 4, 2019

Following on from the fix that was provided here:

#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 added a commit to gep13/csharp-language-server-protocol that referenced this issue Apr 4, 2019
@bjorkstromm bjorkstromm added the bug Something isn't working label Apr 4, 2019
david-driscoll added a commit that referenced this issue Apr 5, 2019
(GH-136) Corrected IRegistration definition
@gep13
Copy link
Contributor Author

gep13 commented Apr 5, 2019

@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
Copy link
Member

bjorkstromm commented Apr 13, 2019

This is also a bug in VS LSP protocol:
image

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
Copy link
Contributor Author

gep13 commented Apr 16, 2019

@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
Copy link
Member

@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.

@bjorkstromm bjorkstromm reopened this Apr 16, 2019
@tgjones
Copy link

tgjones commented May 21, 2019

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
Copy link

tgjones commented May 21, 2019

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
Copy link

tgjones commented May 21, 2019

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

@bjorkstromm
Copy link
Member

Fixed in #140

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants