Refactor the architecture

[tcx4c70]

This PR is a huge rework that almost refactors the whole architecture of the software.

background

1. When I use csharp-ls in my daily work, I find that sometimes csharp-ls throws tons of "KeyNotFoundException" then stops working. I can't find the root cause of the exception but only find the reason why csharp-ls will re-throw the same exception (it re-throw the exception after logging it). Even if I remove the line, csharp-ls will not throw the same exception multiple times but it will still stop working (and will not recover from it) once the exception occurs. I'm sorry to say that it seems that the architecture is hard to debug (as lest for me).
2. When I'm going to develop new feature for csharp-ls, I find that both Server.fs and RoslynHelpers.fs are very large (both of them have more than 1000 lines). I don't think it's a good smell so I try to split them.
3. If the solution is versy large, usually csharp-ls is very slow to respond a request. After some investigation, I think it might be due to the strong consistency we use now. Every time we start to handle a request, we will require a read-only or read-write access to the scope (I think it's a bit like read-write lock?) and read-write access is exclusive. Since textDocument/didChange needs read-write access, every time I type two or three chars, csharp-ls will block other requests until the current textDocument/didChange has been done. But we really need such strong consistency? IMO, eventual consistency is enough. Even if some unconsistency occur because of the order of handling textDocument/didChange, textDocument/didSave will correct them. But eventual consistency will improve perfromance significantly.
4. It's a little hard to test. I don't find a easy way to do unit test/function test.

big changes

1. The biggest change is the architecture. After 6427f98, I find that the architecture I'm thinking is a bit like MVC:

   1. Workspace project: The mode module. It's responsible to maintain the state of MSWorkspaces.
   2. Lsp project: The view module. It routes the requests to Handlers module and implements the listeners such that Workspace can notify LSP client easily.
   3. Handlers project: The controller module. It handles all requests. Usually it gets some status from Workspace module and convert it to LSP types. And it will notify the status changes to Workspace module. For now, most code of this module are just copy & paste from the original code then apply some formatting & refactoring.
   4. Common project: Some common type definitions, type conversions, type extensions, etc.

2. Remove the scope concept. All requests can be handled concurrently after Workspace initializes.

3. Remove csharp.solution setting and workspace/didChangeConfiguration handler. If we wants to support multiple workspace folders, it seems that csharp.solution is not suitable. So I remove the setting and the workspace/didChangeConfiguration. And csharp-ls will find and load .sln/.csproj from:

   1. workspaceFolders
   2. rootUri
   3. rootPath
   4. cwd

   I think it should fix Cant seem to understand .sln with multiple .csproj #62, No parent project could be resolved #57

4. Log to a real logger. Change to use FsLibLog instead of sending all logs/information to client.

5. Register the handlers after initialization if client support dynamic registration. During the initialization, client might send lots of requests to us but we can't handle them before finishing initialization. If client doesn't receive the response in some time, it might cancel the requests and send new ones. Defering the registration of capabilities to avoid the unnecessary requests during initialization.

small changes

Besides the big changes, there are some small big fixes/enhancements in this PR. For example:

1. Fix only the first chars of parameters are highlight in signature help (https://github.com/tcx4c70/csharp-language-server/blob/621b8a576b946a4787a972fd29cf108799375442/src/Handlers/SignatureHelp.fs#L19).
2. Fix no symbols are showed for the first query of workspace/symbols (https://github.com/tcx4c70/csharp-language-server/blob/621b8a576b946a4787a972fd29cf108799375442/src/Workspace/WorkspaceManager.fs#L281C1-L286C129).
3. Escape ` in the markdown documentation (621b8a5).
4. Fix crash due to empty item.Tags (9e92165).

I think these fixes can be cherry-picked to master branch, but I didn't do that. (These changes have already been merged to master branch)

some TODOs need to be done in this PR

• Support workspace/didChangeWatchedFiles.
• Merge the exes. For now, the new exe is in src/Lsp/bin/Release/net7.0/CSharpLanguageServer.Lsp. Add a new argument --legacy/-L to switch between them.

some TODOs I'm going to do in further PRs

1. Add unit tests/function tests.
2. More refactors in Handlers module. I think a very long function is hard to read in a functional language, while there are some functions in Handlers module that are very long.
3. More bugfix. For example:
   1. There are lots of duplicate items of code lens and references.
4. More enhancement. For example:
   1. Add parameters to completion items so that user can distinguish override versions.
   2. Monitor the LSP client process and shutdown csharp-ls itself if the client dies and doen't kill csharp-ls.
   3. Send refresh requests (e.g., workspace/semanticTokens/refresh and workspace/inlayHint/refresh) to client at apposite time
5. Add some configuration so that user can control some behavior/enable some feature. For example:
   1. Add configuration to disable/enable some kind of inlay hint.
   2. Add configuration to disable/enable some kind of code lens. Usually I don't care the reference number of a private member in a class.
6. Send the process of initialization to client.
7. Support pull diagnositc.
8. Remove the original implement in src/CSharpLanguageServer if there is no big regression.

[razzmatazz]

Hey @tcx4c70!
Sorry for responding this late, I had a lot going in my life lately, in addition to change of $JOB and birth of a child :)
Ideally I would like to have this big of a change integrated piecemal, will try to help in you in reviewing and merging in the changes.

[ckangnz]

hey @tcx4c70 thanks so much for your hard dedicated work!
Looking forward to the new architecture! I'm still waiting for the language server to be smarter to understand the root folder better. Do we know when this is expected to release?

[tcx4c70]

Hi @ckangnz , when you say "smarter to understand the root folder better", do you mean the big changes 3? If so and you're eager to get the fix, maybe I can cherry-pick it without the mutiple workspace folder support to master branch.

BTW, I think the responsibility of understanding the root folder should be in language client rather than language server. What language server shoud do is to reuse the knowledge of root folder client tells (workspaceFolders or rootUri or rootPath). However, for now, csharp-ls just ignore the knowledge and find .sln/.csproj from cwd if client doesn't tell csharp-ls csharp.solution.

[ckangnz]

Hey thanks for the response!
I actually write that issue you linked

#62

I'm just waiting for this to be updated..! No need to rush though. Take your time. Really appreciate your contribution and I'm looking forward to be able to code c# with vim again

[ckangnz]

I just pulled down the latest version and it works! Thank you so much 🙂

[rudiejd]

hey @tcx4c70 @razzmatazz 🙋 happy holidays!! Any ideas when this will be merged? I'd like to give implementing progress a go while I have some time off, but I wanted to figure out whether I should base off of this branch or master.

Also more than willing to help chunk this up despite my piss poor F# skills, because this tool is invaluable to me for work

[dakamakat]

Thats really good changes ! Hope it would be merged soon , @tcx4c70 do you test these changes on large projects in terms of response speed or it's still in development ?

[razzmatazz]

hey @tcx4c70 @razzmatazz 🙋 happy holidays!! Any ideas when this will be merged? I'd like to give implementing progress a go while I have some time off, but I wanted to figure out whether I should base off of this branch or master.

Also more than willing to help chunk this up despite my piss poor F# skills, because this tool is invaluable to me for work

Hey @tcx4c70
Sorry for taking so long to respond.

What I want to do is to pull changes gradually (w/ cherrypicks), while testing on my own.

Q1. Would you agree with this approach? (Instead of merging the whole PR at once)

Q2. I am seeing problems running this on my machine (dotnet8 on an arm64 mac):

Unhandled exception. HarmonyLib.HarmonyException: Patching exception in method System.Void Microsoft.CodeAnalysis.MSBuild.MSBuildWorkspace::SaveDocumentText(Microsoft.CodeAnalysis.DocumentId id, System.String fullPath, Microsoft.CodeAnalysis.Text.SourceText newText, System.Text.Encoding encoding)
 ---> System.NotImplementedException: The method or operation is not implemented.
   at HarmonyLib.PatchFunctions.UpdateWrapper(MethodBase original, PatchInfo patchInfo)
   at HarmonyLib.PatchClassProcessor.ProcessPatchJob(Job job)
   --- End of inner exception stack trace ---
   at HarmonyLib.PatchClassProcessor.ReportException(Exception exception, MethodBase original)
   at HarmonyLib.PatchClassProcessor.Patch()
   at HarmonyLib.Harmony.<PatchAll>b__10_0(Type type)
   at HarmonyLib.CollectionExtensions.Do[T](IEnumerable`1 sequence, Action`1 action)
   at HarmonyLib.Harmony.PatchAll(Assembly assembly)
   at HarmonyLib.Harmony.PatchAll()
   at CSharpLanguageServer.MSBuildWorkspacePatcher.MSBuildWorkspacePatcher.Patch() in /Users/bob/src/csharp-language-server/src/MSBuildWorkspacePatcher/MSBuildWorkspacePatcher.cs:line 43
   at CSharpLanguageServer.Program.entry(String[] args) in /Users/bob/src/csharp-language-server/src/CSharpLanguageServer/Program.fs:line 16

[razzmatazz]

I began work to ingest this PR by cherry-picking commits to master

[tcx4c70]

hey @tcx4c70 @razzmatazz 🙋 happy holidays!! Any ideas when this will be merged? I'd like to give implementing progress a go while I have some time off, but I wanted to figure out whether I should base off of this branch or master.
Also more than willing to help chunk this up despite my piss poor F# skills, because this tool is invaluable to me for work

Hey @tcx4c70 Sorry for taking so long to respond.

What I want to do is to pull changes gradually (w/ cherrypicks), while testing on my own.

Q1. Would you agree with this approach? (Instead of merging the whole PR at once)

Sure, please go ahead with any way you like. I will rebase to rewrite git history to make cherrypick easier.

Q2. I am seeing problems running this on my machine (dotnet8 on an arm64 mac):

Unhandled exception. HarmonyLib.HarmonyException: Patching exception in method System.Void Microsoft.CodeAnalysis.MSBuild.MSBuildWorkspace::SaveDocumentText(Microsoft.CodeAnalysis.DocumentId id, System.String fullPath, Microsoft.CodeAnalysis.Text.SourceText newText, System.Text.Encoding encoding)
 ---> System.NotImplementedException: The method or operation is not implemented.
   at HarmonyLib.PatchFunctions.UpdateWrapper(MethodBase original, PatchInfo patchInfo)
   at HarmonyLib.PatchClassProcessor.ProcessPatchJob(Job job)
   --- End of inner exception stack trace ---
   at HarmonyLib.PatchClassProcessor.ReportException(Exception exception, MethodBase original)
   at HarmonyLib.PatchClassProcessor.Patch()
   at HarmonyLib.Harmony.<PatchAll>b__10_0(Type type)
   at HarmonyLib.CollectionExtensions.Do[T](IEnumerable`1 sequence, Action`1 action)
   at HarmonyLib.Harmony.PatchAll(Assembly assembly)
   at HarmonyLib.Harmony.PatchAll()
   at CSharpLanguageServer.MSBuildWorkspacePatcher.MSBuildWorkspacePatcher.Patch() in /Users/bob/src/csharp-language-server/src/MSBuildWorkspacePatcher/MSBuildWorkspacePatcher.cs:line 43
   at CSharpLanguageServer.Program.entry(String[] args) in /Users/bob/src/csharp-language-server/src/CSharpLanguageServer/Program.fs:line 16

I don't have a macbook, but I'll test it on my orange 5 plus, which has an arm64 CPU.