LSP features not working in second dotnet solution with OmniSharp

[starteleport]

Description

I'm not sure if it's lspconfig-related or omnisharp-related, so posting it here to start with.

Neovim version

NVIM v0.9.0-dev-774+g151b9fc52-dirty
Build type: Release
LuaJIT 2.1.0-beta3

Nvim-lspconfig version

85cd2ec

Operating system and version

macOs 13.1

Affected language servers

omnisharp

Steps to reproduce

Step 1: open file from solution 1

LspInfo:

 Language client log: /Users/starteleport/.local/state/nvim/lsp.log
 Detected filetype:   cs
 
 1 client(s) attached to this buffer: 
 
 Client: omnisharp (id: 1, bufnr: [1])
 	filetypes:       cs, vb
 	autostart:       true
 	root directory:  /Users/starteleport/my_code/solution1
 	cmd:             omnisharp -z -s /Users/starteleport/my_code/solution1 --hostPID 59024 DotNet:enablePackageRestore=false --encoding utf-8 --languageserver FormattingOptions:EnableEditorConfigSupport=true Sdk:IncludePrereleases=true
 
 Configured servers list: omnisharp

Step 2: check whether things like goto definition work within solution 1

Step 3: open file from solution 2

LspInfo:

 Language client log: /Users/starteleport/.local/state/nvim/lsp.log
 Detected filetype:   cs
 
 1 client(s) attached to this buffer: 
 
 Client: omnisharp (id: 1, bufnr: [1, 7])
 	filetypes:       cs, vb
 	autostart:       true
 	root directory:  /Users/starteleport/my_code/solution2
 	cmd:             omnisharp -z -s /Users/starteleport/my_code/solution1 --hostPID 59024 DotNet:enablePackageRestore=false --encoding utf-8 --languageserver FormattingOptions:EnableEditorConfigSupport=true Sdk:IncludePrereleases=true -z -s /Users/starteleport/my_code/solution2 --hostPID 59024 DotNet:enablePackageRestore=false --encoding utf-8 --languageserver FormattingOptions:EnableEditorConfigSupport=true Sdk:IncludePrereleases=true
 
 Configured servers list: omnisharp

Step 4: check whether things like goto definition work within solution 2

Step 5: issue ps -A to verify how many instances of omnisharp are started and with which parameters

Actual behavior

Goto definition from step 2 work within solution 1.

Goto definition from step 4 do not work within solution 2.

LspInfo in step 4 show suspicious cmd value that expands even further as I open subsequent buffers.

cmd value in step 4 does not match the output from ps -A in step 5

Expected behavior

Goto definition from step 2 work within solution 1.

Goto definition from step 4 work within solution 2.

LspInfo in step 4 shows adequate cmd value that matches the output of ps -A.

Minimal config

local on_windows = vim.loop.os_uname().version:match 'Windows'

local function join_paths(...)
  local path_sep = on_windows and '\\' or '/'
  local result = table.concat({ ... }, path_sep)
  return result
end

vim.cmd [[set runtimepath=$VIMRUNTIME]]

local temp_dir = vim.loop.os_getenv 'TEMP' or '/tmp'

vim.cmd('set packpath=' .. join_paths(temp_dir, 'nvim', 'site'))

local package_root = join_paths(temp_dir, 'nvim', 'site', 'pack')
local lspconfig_path = join_paths(package_root, 'test', 'start', 'nvim-lspconfig')

if vim.fn.isdirectory(lspconfig_path) ~= 1 then
  vim.fn.system { 'git', 'clone', 'https://github.com/neovim/nvim-lspconfig', lspconfig_path }
end

vim.lsp.set_log_level 'trace'
require('vim.lsp.log').set_format_func(vim.inspect)
local nvim_lsp = require 'lspconfig'
local on_attach = function(_, bufnr)
  local function buf_set_option(...)
    vim.api.nvim_buf_set_option(bufnr, ...)
  end

  buf_set_option('omnifunc', 'v:lua.vim.lsp.omnifunc')

  -- Mappings.
  local opts = { buffer = bufnr, noremap = true, silent = true }
  vim.keymap.set('n', 'gD', vim.lsp.buf.declaration, opts)
  vim.keymap.set('n', 'gd', vim.lsp.buf.definition, opts)
  vim.keymap.set('n', 'K', vim.lsp.buf.hover, opts)
  vim.keymap.set('n', 'gi', vim.lsp.buf.implementation, opts)
  vim.keymap.set('n', '<C-k>', vim.lsp.buf.signature_help, opts)
  vim.keymap.set('n', '<space>wa', vim.lsp.buf.add_workspace_folder, opts)
  vim.keymap.set('n', '<space>wr', vim.lsp.buf.remove_workspace_folder, opts)
  vim.keymap.set('n', '<space>wl', function()
    print(vim.inspect(vim.lsp.buf.list_workspace_folders()))
  end, opts)
  vim.keymap.set('n', '<space>D', vim.lsp.buf.type_definition, opts)
  vim.keymap.set('n', '<space>rn', vim.lsp.buf.rename, opts)
  vim.keymap.set('n', 'gr', vim.lsp.buf.references, opts)
  vim.keymap.set('n', '<space>e', vim.diagnostic.open_float, opts)
  vim.keymap.set('n', '[d', vim.diagnostic.goto_prev, opts)
  vim.keymap.set('n', ']d', vim.diagnostic.goto_next, opts)
  vim.keymap.set('n', '<space>q', vim.diagnostic.setloclist, opts)
end

-- Add the server that troubles you here
local name = 'omnisharp'
local cmd = { 'omnisharp' } -- needed for elixirls, omnisharp, sumneko_lua
if not name then
  print 'You have not defined a server name, please edit minimal_init.lua'
end
if not nvim_lsp[name].document_config.default_config.cmd and not cmd then
  print [[You have not defined a server default cmd for a server
    that requires it please edit minimal_init.lua]]
end

nvim_lsp[name].setup {
  cmd = cmd,
  on_attach = on_attach,
}

print [[You can find your log at $HOME/.cache/nvim/lsp.log. Please paste in a github issue under a details tag as described in the issue template.]]

LSP log

https://gist.github.com/starteleport/055fedceaea9e2ef22dd2ff9e6be43d9

[glepnir]

to be honest I don't find the textDocument/definition request in your log.

[starteleport]

Sorry, wrong log file. I've updated the gist as well as NeoVim/lspconfig revisions in bug description (just updated everything to the latest masters).

[waynebowie99]

I believe I figured out part of the problem. The on_new_config function for omnisharp inserts the extra configurations to the cmd table. The issue with this is that it doesn't seem like the cmd table is ever cleared or recreated. This is what causing the repeating config options at the end of the command.

This section gets repeated many times. I've noticed on Windows this happens as soon as I open one solution
-z -s /Users/starteleport/my_code/solution1 --hostPID 59024 DotNet:enablePackageRestore=false --encoding utf-8 --languageserver FormattingOptions:EnableEditorConfigSupport=true Sdk:IncludePrereleases=true

I created a temporary solution that works for my use case here. I'm just setting the new_config.cmd to a completely new table and setting the omnisharp dll elsewhere. Someone would have to come up with a proper solution.

I'm open to helping out when I can

[starteleport]

@glepnir, were you able to reproduce this? Can I help here somehow?

[glepnir]

sry a little busy recently . I don't write dotnet. so need some time to install env and try to reproduce it.

[JustALawnGnome7]

@waynebowie99 You've certainly identified the problem! I've spent countless hours and have come across multiple Issues (the GitHub kind) in other repos that have been opened—and subsequently closed—that were ultimately the fault of this bug.

Shoot, I've spend dozens of hours just trying to learn Lua in hopes of troubleshooting this. I think this is what's preventing me from using Hoffs/omnisharp-extended-lsp.nvim in my LazyVim configuration.

Does anybody more experienced than me have some time to work on this?

[JustALawnGnome7]

@glepnir As I'm looking over the code @waynebowie99 shared back on Feb 2, it seems he's on the right track by resetting new_config.cmd (i.e. setting the arguments to omnisharp from scratch). If this is indeed the right approach, how do we do this so that lspconfig's default path to omnisharp is used in subsequent command line resets—so that @waynebowie99's dll_path field is unnecessary?

Second, is it even correct for us to be trying to set new_config.cmd in most cases? In your professional opinion, is it a problem that on_new_config is being called every time we open a *.cs file under the same directory as the other files we have open?

[waynebowie99]

This issue #2018 was the one that caused this breaking change. Maybe @williamboman has some insight on this and how to fix the problem

Thanks @JustALawnGnome7 for continuing this work on this!

[JustALawnGnome7]

I've submitted a PR that updates the logic behind on_new_config. The change saves the value of cmd—or rather its first parameter's cmd field—to be used in subsequent calls to on_new_config instead of appending duplicate arguments to cmd whenever another *.cs source file is opened.

It should be noted, however, that this still doesn't cause OmniSharp to work with multiple projects at the same time.

[JustALawnGnome7]

Whoops, looks like I botched the commit messages and created quite a mess. I've re-submitted the change under a new PR. (#2692)

[williamboman]

So this required some extensive debugging. There seems to be two problems here:

1. I had so far been under the impression that each client configuration table gets "deep copied" before use. This is not the case, it only creates a shallow copy of the table, so if you mutate a table field directly (such as table.insert(new_config.cmd, "something")) it'll mutate the shared config table. This is easily fixable (I've suggested an easier approach in fix(omnisharp): update on_new_config #2692). Some other server configurations seem to suffer the same issue, I'll probably open a separate PR to fix these.

2. The Omnisharp server does not seem to handle multiple workspaces per server instance (see Add support for multiple root folders OmniSharp/omnisharp-roslyn#909 and other linked issues from VSCode users). For some reason the server seems to announce that it supports this capability, which causes lspconfig to attach a new workspace to an existing client. Any new opened workspace will effectively be unusable due to this. The following autocmd should patch the server capabilities to avoid this happening (note that I've also included a reset of semanticTokensProvider because of Semantic tokens do not conform to the LSP specification OmniSharp/omnisharp-roslyn#2483):

vim.api.nvim_create_autocmd("LspAttach", {
    callback = function(args)
        local client = vim.lsp.get_client_by_id(args.data.client_id)

        if client.name == "omnisharp" then
            client.server_capabilities.semanticTokensProvider = nil
            client.server_capabilities.workspace.workspaceFolders.supported = false
        end
    end,
})

-- (the better way would probably be to try to configure this via client capabilities, but I haven't given that a try yet)

The above could potentially also be somehow included directly in nvim-lspconfig for a better out-of-the-box experience.