Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vscode-csharp - new lsp server #2657

Open
titouancreach opened this issue Jun 7, 2023 · 34 comments
Open

vscode-csharp - new lsp server #2657

titouancreach opened this issue Jun 7, 2023 · 34 comments
Labels
enhancement New feature or request

Comments

@titouancreach
Copy link
Contributor

titouancreach commented Jun 7, 2023

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

@titouancreach titouancreach added the enhancement New feature or request label Jun 7, 2023
@glepnir
Copy link
Member

glepnir commented Jun 7, 2023

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

@titouancreach
Copy link
Contributor Author

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

In this commit: references to omnisharp has been removed:
dotnet/vscode-csharp@08682dc

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
Copy link

mdarocha commented Jun 8, 2023

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

@bjorkstromm
Copy link

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.

dotnet/roslyn#68463

@lawrence-laz
Copy link
Contributor

lawrence-laz commented Jun 13, 2023

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
Copy link

Looks to be sorted out. dotnet/roslyn#68942

@jmederosalvarado
Copy link

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

@lawrence-laz
Copy link
Contributor

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
Copy link

maxle5 commented Aug 11, 2023

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

@tilupe
Copy link

tilupe commented Sep 11, 2023

@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
Copy link

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
Copy link

maxle5 commented Oct 12, 2023

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
Copy link

boost1231 commented Oct 13, 2023

@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
Copy link

Looks like an issue was opened up in the Roslyn repo to address the issue with handling textDocument/didChange notifications: dotnet/roslyn#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
Copy link

tilupe commented Oct 16, 2023

@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
Copy link

jmederosalvarado commented Oct 17, 2023

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
Copy link

@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
Copy link

@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
Copy link

jmederosalvarado commented Oct 19, 2023

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: [LSP] textDocument/didChange handler behavior doesn't follow lsp spec dotnet/roslyn#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: Change log level to DEBUG instead of WARN when unknown tag is found in diagnostic neovim#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
Copy link

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

@AnhQuanTrl
Copy link

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
Copy link

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

@jmederosalvarado
Copy link

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

@AnhQuanTrl
Copy link

I found it is already requested in this issue: neovim/neovim#23026.

@boost1231
Copy link

boost1231 commented Oct 28, 2023

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: dotnet/roslyn#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
Copy link

tilupe commented Nov 17, 2023

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
Copy link

ryuukk commented Dec 20, 2023

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
Copy link
Sponsor

616b2f commented Dec 21, 2023

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

This comment was marked as off-topic.

@zoriya
Copy link

zoriya commented Apr 3, 2024

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

@616b2f
Copy link
Sponsor

616b2f commented Apr 4, 2024

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 neovim/neovim#26032

Or do I miss something here?

@zoriya
Copy link

zoriya commented Apr 4, 2024

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
Copy link
Contributor

konradmalik commented Apr 4, 2024

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
Copy link

tilupe commented Apr 5, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests