A proposal to import custom JsonRpc implementation based on System.Text.Json

[razzmatazz]

Currently Ionide.LanguageServerProtocol is using StreamJsonRpc which is somewhat cumbersome to use and has several deficiencies in the context of Ionide.LSP:

• Uses Newtonsoft.Json (instead of STJ)
• Request serialization is achieved by holding onto Task and not returning from handler until proper measures are taken (i.e. req # generated, pushed to request queue, etc)
• Is a C# library and does not meld into F# Async contexts (e.g. ambient cancellation) nicely

I propose importing JsonRpc.fs from csharp-ls which has been actually tested in the field and provides Ordinal # in request context where the user of the library can implement custom scheduler to serialize read/write ops, for example.

Please note, that replacing StreamJsonRpc with JsonRpc.fs may not be trivial as it does not do Task-based serialization, like StreamJsonRpc does and may open servers using Ionide.LSP to more races unless we change some of the logic in Ionide.LSP to expose JsonRpcRequestContext.Ordinal and/or wrap JsonRpc.fs to enforce request handler serialization.

The alternatives are:

• I keep it in csharp-ls
• Ship it is as a separate nuget/library, outside of Ionide.LanguageServerProtocol

There are also tests, that could be included with the import (mostly Claude-written, may need restructuring):

• https://github.com/razzmatazz/csharp-language-server/blob/main/tests/CSharpLanguageServer.Tests/JsonRpcTests.fs

[TheAngryByrd]

Yeah, I'm kind of frustrated with the StreamJsonRpc library but I am worried that maintaining our implementation would be tedious too.

I'd be interested to see a PR that allows us to swap in either solution if there was a minimal set of API changes we could make so we could have a gradual transition and allow people to find bugs with the newer implementation.

[Numpsy]

Uses Newtonsoft.Json (instead of STJ)

It looks like the version of StreamJsonRpc used here is quite old, and I believe the current version is able to use System.Text.Json instead of Newtonsoft. (they were doing some work to try to get it working with NativeAOT and STJ is better for that)

[TheAngryByrd]

Yeah probably makes sense to update our StreamJson if we’re missing STJ support

[razzmatazz]

I have taken an interative approach to isolate Newtonsoft.Json in Ionide.LSP API, with the first PR being #74

This updates LSPAny to be an actual wrapper class for JToken (not a type alias anymore) and then updates serialize/deserialize fns to actually produce/consume LSPAny with the intent that it would make later conversion to STJ a mechanical affair. There is a bit of extra allocation introduced per each LSPAny for a wrapper but with transition to STJ the overhead would go away as if JToken is replaced with JsonElement from STJ, then it would no longer spray heap with thousands of JValues/JObjects/JArrays, but a pair of byte[]+metadata ob jects that is in JsonElement.

[TheAngryByrd]

This a great start!

FYI ill be on vacation for 2 weeks so unlikely ill be able to look over/merge changes

[razzmatazz]

This a great start!

FYI ill be on vacation for 2 weeks so unlikely ill be able to look over/merge changes

Friendly reminder,–I'd love to to continue to work on top of that PR, if merged.