Since we're not directly going through the request queue, we need to provide the client capabilities to the server upfront. This better matches how VS actually works.

matches previous behavior, have #57243 filed for me to followup on this

no longer needed since we don't have LSP push diagnostics and we don't need to specify jsonrpcmethods explicitly anymore 🎉

we now directly create the jsonrpc stream and LanguageServerTarget (previously we just manually sent requests to the RequestDispatcher). Tests will now go through serialization / deserialization as well as the full functionality of the LanguageServerTarget (yay more test coverage)

This seems really good. 😄

We don't need the supported languages here to filter out handlers for other servers - instead we only import the handler providers that match the desired contract name

the firstHandler and additionalHandlerTypes is so we get a compile error if you don't pass in at least one handler type

Duplicate information - we now use the method attribute on the handler to get the method name (doesn't require instantiating the handler)

this is where we register the method + request and response types with streamjsonrpc.

todo: do we really need this, or can we jsut pass around a tuple of these values...

this type is mainly to hold onto the method name associated with the streamjsonrpc entrypoint. Since streamjsonrpc doesn't pass the method name to us when they call our entry point.

I don't think a tuple would work, as the target needs to have both the actual entry point method (for streamjsonrpc to call) and save the method so we can pass it to the queue when streamjsonrpc calls us

thank you for breasking up all the red with a little green. it's festive.

for my edification, why ddo we still have this method?

workspace/ExecuteCommand contains a command name string and an object typed payload. The server is expected to know what to do with the payload based on the command name (since commands are originally created by the server).

In roslyn, we model this by creating IRequestHandler for each command name that we support. So in order to know which IRequestHandler to delegate the command request to, we need to deserialize the ExecuteCommandParams to retrieve the command name. Then we can pass it to whatever handler says it handles that command.

And since the types for workspace/ExecuteCommand are all always object anyway, it doesn't matter if we deserialize first

i don't love that for testing it seems like we expose all of thsee things. how come?

90% of the tests don't look at this, but there are specific tests that test implementation details of the queue, request dispatcher, or handlers.

For example, a few code action tests look at the handler instance to verify the cached contents match expectations. And the queue is looked at to verify that the tracked text at each point matches the DidChange requests, or that when we shutdown all the tasks in queue are cancelled.

Since these are TestAccessors the API surface they have access to is still limited, so the tests aren't actually able to arbitrarily access these internal parts.

super nice. :)

i'm skipping the tests

There's behavior changes happening here?

This is caused by actually doing serialization - the client extension converters add the whitespace classified text runs

That way any type mismatch is a useful throw instead of this which might be more confusing.

this gives me nullable warnings because GetCustomAttribute returns a nullable Attribute, so this way seemed clearer than bang

This seems really good. 😄