nvim-lspconfig icon indicating copy to clipboard operation
nvim-lspconfig copied to clipboard

vscode-csharp - new lsp server

Open titouancreach opened this issue 1 year ago • 34 comments

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:

  • https://github.com/dotnet/vscode-csharp
  • https://github.com/dotnet/vscode-csharp/issues/5708

Other clients which have this feature

No response

titouancreach avatar Jun 07 '23 07:06 titouancreach

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

glepnir avatar Jun 07 '23 08:06 glepnir

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

titouancreach avatar Jun 07 '23 08:06 titouancreach

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

  • https://github.com/dotnet/vscode-csharp/issues/5276#issuecomment-1581651560
  • https://github.com/dotnet/roslyn/pull/68461.

mdarocha avatar Jun 08 '23 16:06 mdarocha

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

bjorkstromm avatar Jun 13 '23 20:06 bjorkstromm

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.

lawrence-laz avatar Jun 13 '23 20:06 lawrence-laz

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

ChimpSpin avatar Jul 17 '23 19:07 ChimpSpin

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

jmederosalvarado avatar Aug 09 '23 11:08 jmederosalvarado

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.

lawrence-laz avatar Aug 09 '23 16:08 lawrence-laz

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:

  • Renaming symbols
  • Applying code actions
  • Code formatting
  • Go to definition
  • Loading "simple" nvim-cmp LSP suggestions
    • keywords
    • local symbols
    • class properties, etc.

What doesn't work for me:

  • Diagnostics or code errors
  • Loading "complex" nvim-cmp LSP suggestions
    • symbols from other namespaces
    • etc.
  • Applying nvim-cmp suggestions causes the LSP to crash with the following error:
"[LanguageServerHost]System.ArgumentException: The changes must not overlap. (Parameter 'changes')...

maxle5 avatar Aug 11 '23 01:08 maxle5

@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.

tilupe avatar Sep 11 '23 15:09 tilupe

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).

AnhQuanTrl avatar Oct 06 '23 08:10 AnhQuanTrl

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!

maxle5 avatar Oct 12 '23 10:10 maxle5

@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 avatar Oct 13 '23 01:10 boost1231

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.

boost1231 avatar Oct 16 '23 01:10 boost1231

@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?

tilupe avatar Oct 16 '23 15:10 tilupe

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.

jmederosalvarado avatar Oct 17 '23 11:10 jmederosalvarado

@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!

groovyghoul avatar Oct 18 '23 13:10 groovyghoul

@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.

boost1231 avatar Oct 19 '23 02:10 boost1231

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.

jmederosalvarado avatar Oct 19 '23 08:10 jmederosalvarado

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

boost1231 avatar Oct 20 '23 00:10 boost1231

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 avatar Oct 24 '23 04:10 AnhQuanTrl

@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

AnhQuanTrl avatar Oct 25 '23 08:10 AnhQuanTrl

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

jmederosalvarado avatar Oct 25 '23 08:10 jmederosalvarado

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

AnhQuanTrl avatar Oct 25 '23 08:10 AnhQuanTrl

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

boost1231 avatar Oct 28 '23 18:10 boost1231

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?

tilupe avatar Nov 17 '23 10:11 tilupe

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

ryuukk avatar Dec 20 '23 23:12 ryuukk

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)

616b2f avatar Dec 21 '23 08:12 616b2f

any news on this?

tudor07 avatar Feb 21 '24 11:02 tudor07

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")]],
    },
  },
}

zoriya avatar Apr 03 '24 14:04 zoriya