neovim / nvim-lspconfig

Quickstart configs for Nvim LSP
Apache License 2.0
9.96k stars 2.03k forks source link

vscode-csharp - new lsp server #2657

Open titouancreach opened 1 year ago

titouancreach commented 1 year ago

Language server

vscode-csharp

Requested feature

https://devblogs.microsoft.com/visualstudio/announcing-csharp-dev-kit-for-visual-studio-code/

Microsoft released a new version of vscode extension for C# on 06/06/2023.

by a new fully open-source Language Server Protocol (LSP) host, creating a performant, extensible, and flexible tooling environment that easily integrates new experiences into C# for VS Code

Here is the process that run instead of Omnisharp.dll:

79855 ?? Ss 0:41.46 dotnet /Users/redacted/.vscode-insiders/extensions/ms-dotnettools.csharp-2.0.206-darwin-arm64/.roslyn/Microsoft.CodeAnalysis.LanguageServer.dll --logLevel Information --starredCompletionComponentPath /Users/redacted/.vscode-insiders/extensions/ms-dotnettools.vscodeintellicode-csharp-0.1.9-darwin-arm64/components/starred-suggestions/node_modules/@vsintellicode/starred-suggestions-csharp --extension /Users/redacted/.vscode-insiders/extensions/ms-dotnettools.csdevkit-0.1.83-darwin-arm64/components/roslyn-visualstudio-languageservices-devkit/node_modules/@microsoft/visualstudio-languageservices-devkit/Microsoft.VisualStudio.LanguageServices.DevKit.dll --sessionId af60e874-a2e7-41d4-8854-b6e1646b9f601686121999271 --telemetryLevel all

other resources:

Other clients which have this feature

No response

glepnir commented 1 year ago

I just see this (powered by OmniSharp) .looks like also based on omnisharp?

titouancreach commented 1 year ago

I'm not really sure. The repo is confusing.

In this commit: references to omnisharp has been removed: https://github.com/dotnet/vscode-csharp/commit/08682dc08ce7283e934c6a9c8acf13d4b17db71f

In the process I put in the original post, the process is now called LanguageServer.dll and not Omnisharp.dll But I'm not sure

mdarocha commented 1 year ago

It seems the new dll is part of a new internal Roslyn LSP server:

bjorkstromm commented 1 year ago

Please be aware that it's not really clear whether or not it's allowed to use the new LSP server outside of VS family editors.

https://github.com/dotnet/roslyn/issues/68463

lawrence-laz commented 1 year ago

According to this comment, the intent is that the new LSP server will be part of roslyn and share the same license (MIT), though.

Hopefully this will get sorted out soon.

ChimpSpin commented 11 months ago

Looks to be sorted out. https://github.com/dotnet/roslyn/pull/68942

jmederosalvarado commented 11 months ago

has anyone tried to manually configure the new LSP in Neovim? would the license allow us to integrate it in this repo?

lawrence-laz commented 11 months ago

has anyone tried to manually configure the new LSP in Neovim? would the license allow us to integrate it in this repo?

The license is MIT, so there should be no issues there.

I have not tried to set it up manually though.

maxle5 commented 10 months ago

For anyone interested, I was able to set it up manually with this config (using the new LSP installed via the vscode extension):

local lsp_configurations = require('lspconfig.configs')
if not lsp_configurations.roslyn then
    lsp_configurations.roslyn = {
        default_config = {
            name = 'roslyn',
            cmd = {
                "dotnet",
                "c:\\users\\max\\.vscode\\extensions\\ms-dotnettools.csharp-2.0.346-win32-x64\\.roslyn\\Microsoft.CodeAnalysis.LanguageServer.dll",
                "--logLevel=Information",
                "--extensionLogDirectory=c:\\temp\\"
            },
            filetypes = { 'cs' },
            root_dir = require('lspconfig.util').root_pattern('*.sln'),
        }
    }
end

local on_attach = function(client, bufnr)
    -- keymaps
end

require("lspconfig").roslyn.setup({
    on_attach = on_attach,
})

What seems to work:

What doesn't work for me:

tilupe commented 9 months ago

@maxle5 thank you for this input. I could also get it to run, but it was barely usable. For me it seemed that only stuff within the same file worked, but even this wasn't reliable. I understand to little about language servers, but it seems weird, that it works so badly when this is already usable in vs-code... I hope there aren't any dependencies to the other tools of Microsoft which aren't open source.

AnhQuanTrl commented 9 months ago

Really love to have this. I think OmniSharp will be deprecated soon. A lot of issues in the github repo have not been resolved for months (for example auto importing not working).

maxle5 commented 8 months ago

Just an update, the manual config I posted above no longer works at all... It appears that a change was made to the LSP to use "named pipes" instead of stdout and I can no longer attach to the LSP from nvim.

This is not really something I am familiar with, so if anyone has any experience with this, I could really use some help!

boost1231 commented 8 months ago

@maxle5

I have gotten around the pipes issue by making the below modification to program.cs in the Roslyn repo: https://github.com/dotnet/roslyn/blob/main/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs#L119

    // var (clientPipeName, serverPipeName) = CreateNewPipeNames();
    // var pipeServer = new NamedPipeServerStream(serverPipeName,
    //     PipeDirection.InOut,
    //     maxNumberOfServerInstances: 1,
    //     PipeTransmissionMode.Byte,
    //     PipeOptions.CurrentUserOnly | PipeOptions.Asynchronous);

    // Send the named pipe connection info to the client 
    //Console.WriteLine(JsonConvert.SerializeObject(new NamedPipeInformation(clientPipeName)));

    // Wait for connection from client
    // await pipeServer.WaitForConnectionAsync(cancellationToken);

    using var stdIn = Console.OpenStandardInput();
    using var stdOut = Console.OpenStandardOutput();

    // UTF8Encoding utf8 = new UTF8Encoding();
    // var stuff = "Starting the LSP";
    // var bytes = utf8.GetBytes(stuff);
    // stdOut.Write(new ReadOnlySpan<byte>(bytes));

    // var server = new LanguageServerHost(pipeServer, pipeServer, exportProvider, languageServerLogger);
    var server = new LanguageServerHost(stdIn, stdOut, exportProvider, languageServerLogger);
    server.Start();

That being said, based on the PR you linked, it sounds like they found that a named pipe offers advantages to standard out. If that is the case, I suppose we have to open a feature request to Neovim to allow named pipe communication? Looking at the code this did not look like an option at the moment.

After getting the Roslyn Language Server to attach, I also had to make a "solution/open" notification request to the Language Server in order for things like "Go To Definition" to work. "solution/open" does not appear to be part of the LSP spec. Maybe we could make a custom handler for the "initialize" request response that would fire back a "solution/open" request?

I am currently working on the Diagnostics. In order to get them running at all, I had to use the development release of Neovim. It has support for the textDocument/diagnostic request. It seems to fire it off after it makes textDocument/didChange requests. The diagnostics request seems to work fine given it has the correct text of a ".cs" file. However, that was not always the case. The Language Server's copy of the ".cs" file gets corrupted. This appears to be happening in trying to keep the ".cs" file up to date with the textDocument/didChange requests. I'm trying to figure out if the requests are not being issued properly by Neovim or if they are not being handled properly by the server.

boost1231 commented 8 months ago

Looks like an issue was opened up in the Roslyn repo to address the issue with handling textDocument/didChange notifications: https://github.com/dotnet/roslyn/issues/70392

To continue testing, I applied this patch locally to the Roslyn LanguageSever code:

diff --git a/src/Features/LanguageServer/Protocol/Handler/DocumentChanges/DidChangeHandler.cs b/src/Features/LanguageServer/Protocol/Handler/DocumentChanges/DidChangeHandler.cs
index 6a8f66ffbfc..a09b9463de2 100644
--- a/src/Features/LanguageServer/Protocol/Handler/DocumentChanges/DidChangeHandler.cs
+++ b/src/Features/LanguageServer/Protocol/Handler/DocumentChanges/DidChangeHandler.cs
@@ -4,7 +4,6 @@

 using System;
 using System.Composition;
-using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Host.Mef;
@@ -33,11 +32,12 @@ public DidChangeHandler()
         {
             var text = context.GetTrackedDocumentSourceText(request.TextDocument.Uri);

-            // Per the LSP spec, each text change builds upon the previous, so we don't need to translate
-            // any text positions between changes, which makes this quite easy.
-            var changes = request.ContentChanges.Select(change => ProtocolConversions.ContentChangeEventToTextChange(change, text));
+            foreach (var change in request.ContentChanges)
+            {
+                var textChange = ProtocolConversions.ContentChangeEventToTextChange(change, text);
+                text = text.WithChanges(textChange);
+            }

-            text = text.WithChanges(changes);

             context.UpdateTrackedDocument(request.TextDocument.Uri, text);

Although I have not done extensive testing yet, Go To Definition, Find References, Diagnostics, Completions, etc. seem to be working for me.

tilupe commented 8 months ago

@boost1231 Thank you a lot for all this amazing work. I applied your fixes and attached the roslyn Lsp succesfully. It seemed to work quite well an fast, but one bug I ran into was, that the auto-completion didn't when I wanted to call a method of a service or a Property of a class. So when entering a "." after a service I didn't get the possible methods. Did you also have this problem or is it only with me?

jmederosalvarado commented 8 months ago

There are multiple issues in the current Roslyn LSP, it doesn't follow the LSP spec in some scenarios. I created a plugin that installs and sets everything up so that we can use the Roslyn LSP from neovim, the experience should be pretty good out of the box. I've been using it, and I have code-completion, go to definition, go to reference, etc. I intercept some calls to and from the lsp to workaround the scenarios where Roslyn doesn't follow the spec.

https://github.com/jmederosalvarado/roslyn.nvim

EDIT: documentation for the plugin is very poor, this is something I hacked during the weekend for myself. Feel free to open issues for anything you would like to see fixed.

groovyghoul commented 8 months ago

@jmederosalvarado Thank you so much! With the PR from @maxle5 this is working really well in Windows 11. Thank you @boost1231, @maxle5 and @jmederosalvarado for diving in for the rest of us!

boost1231 commented 8 months ago

@jmederosalvarado Thanks for putting this plugin together! I have not fully digested it yet, but just have a few questions.

  1. Were there any reasons you chose to add a command to install the Language Server as oppose to using a package manager like Mason.nvim?
  2. In regards to: "There are multiple issues in the current Roslyn LSP, it doesn't follow the LSP spec in some scenarios." Can you expand on what isn't following the spec? It looks like they fixed the issue with TextDocument/didChange notifications.
jmederosalvarado commented 8 months ago

Hi @boost1231

Were there any reasons you chose to add a command to install the Language Server as oppose to using a package manager like Mason.nvim?

Not at all, from the beginning my purpose was to integrate the installation process into Mason.nvim, but first I wanted to make sure the install process worked correctly, otherwise trying to reverse engineer the vscode extension for how the install should be done at the same time I implemented it in Mason would have been too complicated. This is the next thing I'm planning to focus on.

There are multiple issues in the current Roslyn LSP, it doesn't follow the LSP spec in some scenarios." Can you expand on what isn't following the spec? It looks like they fixed the issue with TextDocument/didChange notifications.

if you go to the hacks file in the plugin, you will see there are multiple scenario that need some kind of workaround for neovim to work well with roslyn. Not all of them are problems with roslyn, some are neovim lsp client issues, and for others I'm yet to figure out which one is not following the spec. I'll list them bellow and link to the issue in the corresponding repo:

  1. textDocument/didChange issue: https://github.com/dotnet/roslyn/issues/70392
  2. Neovim emitting warning on unknown diagnostic tag: this is a neovim issue, as unknown diagnostic tags should be ignored. PR to fix this: https://github.com/neovim/neovim/pull/25705
  3. On didChangeWatchedFiles registration roslyn is asking neovim to watch a baseDir that doesn't exist and neovim file watcher crashes. I'm yet to figure out if roslyn shouldn't be asking to watch missing directories or if neovim should ignore this watcher when the baseDir doesn't exist.
  4. Also on didChangeWatchedFiles roslyn is sending the baseDir to watch with a trailing /, but neovim is assuming no trailing slash exist and it's adding one to the pattern which results in pattern trying to match paths with // (two consecutive slashes). This one will probably be on the neovim side to just sanitize the uri received from the server.

So, (3) and (4) are fairly low priority for now, as you could just disable file watching. Neovim nightly introduces support for didChangeWatchedFiles. Neovim uses system file watchers to let the server know whenever a file changes. You can disable this capability by doing:

capabilities = vim.tbl_deep_extend("force", capabilities, {
    workspace = {
        didChangeWatchedFiles = {
            dynamicRegistration = false,
        },
    },
})

My intention is to slim down the plugin as much as possible, by porting install functionality to mason, and fixing in neovim the rest of the issues, but it will take some time to get there.

boost1231 commented 8 months ago

Thanks for the detailed response, and nice work! Much appreciated.

AnhQuanTrl commented 8 months ago

Thank you so much for the detailed work. This new library is so awesome. It support lots of things that Omnisharp failed to provide such as the ability to auto import after completion <3 and so much more.

AnhQuanTrl commented 8 months ago

@jmederosalvarado I have also added an issue in the Roslyn repo to add support for semanticTokens/full request: https://github.com/dotnet/roslyn/issues/70536

jmederosalvarado commented 8 months ago

Sounds good @AnhQuanTrl , have you thought of opening an issue in neovim asking for support for semanticTokens/range?

AnhQuanTrl commented 8 months ago

I found it is already requested in this issue: https://github.com/neovim/neovim/issues/23026.

boost1231 commented 8 months ago

I find that when I make code changes to FileA that breaks code in a FileB that is loaded in another hidden buffer, diagnostics are not reported when I switch to FileB.

The LSP spec has a "relatedDocuments" property that appears like it would support this very scenario. (https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#relatedFullDocumentDiagnosticReport). However, as far as I can tell, Roslyn does not populate that property even though the serverCapabilities it reports would indicate otherwise. In any case, I opened an issue: https://github.com/dotnet/roslyn/issues/70616 (edit: this was already closed, may have misinterpreted the use for the property)

In the mean time, I added this to the on_attach callback:

if client.supports_method(require('vim.lsp.protocol').Methods.textDocument_diagnostic) then

    vim.api.nvim_create_autocmd('BufEnter', {
      buffer = bufnr,
      callback = function()
            require('vim.lsp.util')._refresh(
                require('vim.lsp.protocol').Methods.textDocument_diagnostic,
                { only_visible = true, client_id = client.id })
      end,
    })

end

So that diagnostics are pulled every time I enter a buffer. It works in the simple case I tested it in.

Has anyone else experienced this issue? Or have any other suggestions on how to deal with it? I wonder if neovim should allow configuration around when diagnostics are pulled. It seems like how often and when could be a matter of preference. Here is a code link in the neovim repo that shows when diagnostics are pulled: https://github.com/neovim/neovim/blob/master/runtime/lua/vim/lsp/diagnostic.lua#L425

tilupe commented 7 months ago

Not sure if it belongs here, but wouldn't know where else to find all the people using the new vs-code-LS.

Does anybody know if it is possible that the language server also includes files which are created with a source generator?

ryuukk commented 6 months ago

Looks to be sorted out. dotnet/roslyn#68942

if you use vscode's extension to get the LSP, you need a commercial visual studio license

image

https://marketplace.visualstudio.com/items?itemName=ms-dotnettools.csdevkit (at the very bottom)

Whoever told you it's free to use is lying to you, this is microsoft, the evil lies in the detail

616b2f commented 6 months ago

the language capabilities (LSP) is provided by another extension (https://marketplace.visualstudio.com/items?itemName=ms-dotnettools.csharp) which is also installed alogside csdevkit, but if you read there:

image

To be on the save side, we should get it from the source and not rely on the packages that are distributed via vscode extensions. This is only how I understand it, I am not that experienced in the legal stuff.

PS: The new LSP server for c# is in the roslyn repository and is also licensed under MIT license ( https://github.com/dotnet/roslyn/tree/main/src/Features/LanguageServer)

tudor07 commented 4 months ago

any news on this?

zoriya commented 3 months ago

I took a look at this today, but the server wants to communicate via a unix socket/named pipe instead of stdin/out. Do you think this should be supported in nvim, lspconfig or should we ask for a --stdio option on roslyn-ls? I did not understand why they were moving away from stdin, it seemed something was printing garbage on stdout so it broke RPC.

PS: This is my current draft if anyone wants to take a look:

Draft ```lua local util = require 'lspconfig.util' local function open_sln(client, sln) client.notify("solution/open", { ["solution"] = vim.uri_from_fname(sln), }) end return { default_config = { cmd = { 'Microsoft.CodeAnalysis.LanguageServer', '--logLevel=Information', '--extensionLogDirectory=' .. vim.fs.dirname(vim.lsp.get_log_path()), }, filetypes = { 'cs', 'vb' }, root_dir = util.root_pattern('*.sln', '*.csproj'), init_options = {}, on_init = function(client) print("on_init") -- Not sure if client.root_dir exists, since the server/client can't communicate without unix sockets I did not get to this point. print(vim.inspect(client)) local sln_dir = client.root_dir if not sln_dir then return end local targets = vim.fn.glob(util.path.join(sln_dir, "*.sln")) if #targets == 0 then vim.notify("Starting in single file mode") elseif #targets == 1 then open_sln(client, targets[1]) else vim.ui.select(targets, { prompt = "Select solution file", }, function(sln) open_sln(client, sln) end) end end, }, docs = { description = [[ https://github.com/dotnet/vscode-csharp The language server behind C# Dev Kit for Visual Studio Code ]], default_config = { root_dir = [[root_pattern("*.sln", "*.csproj")]], }, }, } ```
616b2f commented 3 months ago

I took a look at this today, but the server wants to communicate via a unix socket/named pipe instead of stdin/out. Do you think this should be supported in nvim, lspconfig or should we ask for a --stdio option on roslyn-ls? I did not understand why they were moving away from stdin, it seemed something was printing garbage on stdout so it broke RPC.

PS: This is my current draft if anyone wants to take a look:

Draft

local util = require 'lspconfig.util'

local function open_sln(client, sln)
  client.notify("solution/open", {
    ["solution"] = vim.uri_from_fname(sln),
  })
end

return {
  default_config = {
    cmd = {
      'Microsoft.CodeAnalysis.LanguageServer',
      '--logLevel=Information',
      '--extensionLogDirectory=' .. vim.fs.dirname(vim.lsp.get_log_path()),
    },
    filetypes = { 'cs', 'vb' },
    root_dir = util.root_pattern('*.sln', '*.csproj'),
    init_options = {},
    on_init = function(client)
      print("on_init")
      -- Not sure if client.root_dir exists, since the server/client can't communicate without unix sockets I did not get to this point.
      print(vim.inspect(client))
      local sln_dir = client.root_dir
      if not sln_dir then
        return
      end

      local targets = vim.fn.glob(util.path.join(sln_dir, "*.sln"))
      if #targets == 0 then
        vim.notify("Starting in single file mode")
      elseif #targets == 1 then
        open_sln(client, targets[1])
      else
        vim.ui.select(targets, {
          prompt = "Select solution file",
        }, function(sln) open_sln(client, sln) end)
      end
    end,

  },
  docs = {
    description = [[
https://github.com/dotnet/vscode-csharp

The language server behind C# Dev Kit for Visual Studio Code
]],
    default_config = {
      root_dir = [[root_pattern("*.sln", "*.csproj")]],
    },
  },
}

neovim already supports named pipes, see https://github.com/neovim/neovim/pull/26032

Or do I miss something here?

zoriya commented 3 months ago

Neovim does support named pipes but no LSP in lspconfig is using it as far as I can tell. Maybe it's an easy one line change, maybe not. I must say, I never used the raw lsp API without lspconfig, so I'm not really knowledgeable in this area.

konradmalik commented 3 months ago

I think the issue here is that we don’t know the name of the pipe beforehand, and the pipe is created by roslyn-ls, not by neovim.

Roslyn-ls creates a new pipe with a random name, and sends the name as a json object after start to stdout. Neovim's implementation of connecting a LSP server via a named pipe assumes that neovim is the one creating the pipe if I'm not mistaken.

So the flow needed for roslyn-ls as of today would be: start a process; read its stdout; once a message pops up - parse it; get the pipe's name; connect to this pipe and use it to communicate using the LSP protocol as usual.

This requires a very custom module, and currently, https://github.com/jmederosalvarado/roslyn.nvim works by essentially copy-pasting the whole rpc.lua module from neovim and modifying here and there to get it to work. In my config, I do it similarly. Copying is needed because we need to use some functions/classes that are private.

tilupe commented 3 months ago

Not sure if this belongs here, but I saw that there is a razor ls in the dotnet project. Since razor is used for C# and goToReference etc. must work project wide in .cs files and .razor files, do they somehow need to communicate together? https://github.com/dotnet/razor/blob/main/src/Razor/src/rzls/Program.cs Could this be a reason that they used named pipes instead of stdin/out? Sorry I have very little understanding of that matter...

seblj commented 1 week ago

I have been looking into this as well:

This requires a very custom module, and currently, https://github.com/jmederosalvarado/roslyn.nvim works by essentially copy-pasting the whole rpc.lua module from neovim and modifying here and there to get it to work. In my config, I do it similarly. Copying is needed because we need to use some functions/classes that are private.

While trying to debug another issue today regarding performance, I found out that the code to getting roslyn to work in neovim can be simplified a lot, and that it isn't actually necessary to duplicate the whole rpc module: https://github.com/seblj/roslyn.nvim/commit/8f097106b61d51fe13d8c424a347f305c48ffefd

TLDR; You can just start the language server, and in the stdout handler, parse the name of the pipe and use it as a parameter for vim.lsp.rpc.connect. Then you can just schedule vim.lsp.start, and use vim.lsp.rpc.connect(pipe_name) as cmd, and everything should work

I am still having a couple of issues with the language server though, and on some repos I have, it will come to a serious halt. Neovim will be really slow to quit, and in some cases, I also get a lot of errors... I haven't really figured out these issues, and it also doesn't seem like vscode has the same issue. I don't know if anyone else have been looking into this, but I thought I should share my findings in removing the entire duplication of the rpc module😄

EDIT: I found out about why it just completely froze neovim sometimes. It has to do with watchfunc in neovim, and didChangeWatchedFiles. There is already an open issue about this in neovim from April 2023, so it may be worth it to turn it off if you notice any issues with this