dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.9k stars 4.01k forks source link

If option is sended to LSP client, its name and group name need to be frozen #67714

Open Cosifne opened 1 year ago

Cosifne commented 1 year ago

Background and Motivation

Currently, our LSP server starts to support to use workspace/configuration to get the client-side configuration. And we start to onboard a few IDE feature options to the LSP server. We use the option name (The name of the option) and option group name (The name of its feature) to construct the the message sent to the client.

In order not to break the client (client might choose whatever way to parse/persist the option based on the name), whenever a Roslyn option is onboarded to the LSP, it's option name and group name need to be frozen. (Note: editor config options are by default meet this rule, because its name and group are persisted in the .edtiorconfig file, so it can't be changed.)

Proposed API

Whenever an option is added to the list, option name and option group name would need to be frozen. It could be achieved by adding unit test to enforce requirement.

Usage Examples

When an option is added to list, its name & group name are considered as frozen because it means the option's name and group name are generalized to the client.

The option could be safely removed from the list (server wouldn't ask the client about the option, so compatibilty would not be broken) But it also means the client side stored option would not be readed by server.

Alternative Designs

No

Risks

Currently the IDE option name is only used within the Roslyn repo to map to the storage location. Example So by enforce this rule, it would limit the ability to rename options in the future. However, it's still achievable if we change the client & server side names at the same time, but we should only do it when the change is inevitable.

Cosifne commented 1 year ago

@tmat should be the feature owner. And @dibarbet and @jasonmalinowski for LSP owner.