omnisharp-roslyn icon indicating copy to clipboard operation
omnisharp-roslyn copied to clipboard

textDocument/definition URI is incompatible with (...)ExternalSourceService cache

Open Hoffs opened this issue 3 years ago • 12 comments

Hi,

I've been working on extending LSP client to pull metadata/file source when textDocument/definition returns result with $metadata$ URI (e.g. uri = "file:///%24metadata%24/Project/nvim-csharp-metadata/Assembly/System/Console/Symbol/System/Console.cs").

The problem I encountered that it seems to be impossible to get subsequent results using textDocument/definition when being run from $metadata$ file. For example, first definition request is done from normal file and gets:

{
  bufnr = 1,
  client_id = 1,
  method = "textDocument/definition",
  params = {
    position = {
      character = 20,
      line = 8
    },
    textDocument = {
      uri = "file:///home/hoffs/projects/nvim-csharp-metadata/Program.cs"
    }
  }
}
----- response ------
  {
    range = {
      end = {
        character = 36,
        line = 1774
      },
      start = {
        character = 27,
        line = 1774
      }
    },
    uri = "file:///%24metadata%24/Project/nvim-csharp-metadata/Assembly/System/Console/Symbol/System/Console.cs"
  }

Then that file can be fetched with custom call to o#/metadata, but then once in that temporary/pseudo file buffer, named $metadata$/Project/nvim-csharp-metadata/Assembly/System/Console/Symbol/System/Console.cs any textDocument/definition request is incompatible to properly resolve to cached document.

I believe the issue lies at FromUri call at:

https://github.com/OmniSharp/omnisharp-roslyn/blob/657b06470be88f4759e7db5969f84ce1bc0b14d2/src/OmniSharp.LanguageServerProtocol/Handlers/OmniSharpDefinitionHandler.cs#L35-L40

As this Uri->FileName gets translated to incompatible string for FindDocumentInCache.

https://github.com/OmniSharp/omnisharp-roslyn/blob/657b06470be88f4759e7db5969f84ce1bc0b14d2/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionServiceV2.cs#L33-L34

https://github.com/OmniSharp/omnisharp-roslyn/blob/657b06470be88f4759e7db5969f84ce1bc0b14d2/src/OmniSharp.Roslyn/BaseExternalSourceService.cs#L17-L25

LSP URI FileName/Cache Key attempted
file://$metadata$/Project/nvim-csharp-metadata/Assembly/System/Console/Symbol/System/Console.cs \\\\$metadata$\\Project\\nvim-csharp-metadata\\Assembly\\System\\Console\\Symbol\\System\\Console.cs
file:///$metadata$/Project/nvim-csharp-metadata/Assembly/System/Console/Symbol/System/Console.cs /$metadata$/Project/nvim-csharp-metadata/Assembly/System/Console/Symbol/System/Console.cs

The desired cache key is $metadata$/Project/nvim-csharp-metadata/Assembly/System/Console/Symbol/System/Console.cs which I believe is impossible to get when going through FromUri.

So while the metadata functionality is a bit outside the standard LSP spec, since it already returns and saves the $metadata$ documents in cache (without even calling o#/metadata), I believe it would be nice if it would properly work if textDocument/definition requests come from $metadata$ file.

If thats fine, this could either be done by not using FromUri or having special case for when URI is for $metadata$ file. IMO 2nd row in the table, where $metadata$ is supposedly under root (/$metadata$/...) should be the happy scenario when providing URI for $metadata$ documents, so then in the handler it could just have a check for StartsWith('/$metadata$/'... or something.

Hoffs avatar Sep 26 '21 14:09 Hoffs

Hi, not sure if this is helpful, but I'm also trying to get metadata working in a non-VSCode LSP client, and for your example, I think file:///Project/nvim-csharp-metadata/Assembly/System/Console/Symbol/System/[metadata] Console.cs would work. That seems to be the URL used in VSCode.

tadeokondrak avatar Oct 23 '21 18:10 tadeokondrak

Is there a way to at least disable this $metadata$ feature so that us LSP users don't run into this error constantly?

svermeulen avatar Jan 17 '22 17:01 svermeulen

is it reasonable to say that omnisharp-roslyn lsp should not return any URI that contains $metadata$, which does not comply with LSP standard, and most lsp clients don’t know how to handle it.

I’m using vim-lsp and also bump into this.

kflu avatar Jan 23 '22 20:01 kflu

And, for good measure here's my +1 to this issue. In the Eglot client, I get a nonsensical URL like /$metadata$/Project/my-project/Assembly/.... This $metadata$ variable does sound like something which could be set, somehow.

joaotavora avatar Jun 02 '22 06:06 joaotavora

+1

tilupe avatar Jan 15 '23 19:01 tilupe

Just would like to say, I started a new job in a dotnet shop, and would love to be able to have this support with my neovim editor

griggsca91 avatar May 17 '23 20:05 griggsca91

+1. This is also problematic on Helix editor. https://github.com/helix-editor/helix/issues/2430

Helix also dumped out a lot of received malformed notification from Language Server: Unhandled that has been coming from OmniSharp too. Not sure if this issue been mentioned before.

Multirious avatar Jan 18 '24 04:01 Multirious

Hey, guys, do we have any updates?

AugustoDeveloper avatar Feb 25 '24 04:02 AugustoDeveloper

I have just installed and configured https://github.com/Hoffs/omnisharp-extended-lsp.nvim and it seems to be working well for me. I have only used it for around 30 minutes so far, but so far, so good...

Edit: I did find that if I bind straight to this, my goto definition stops working for other file types. I've added an ftplugin specific binding just for cs files. As per https://github.com/KiLLeRRaT/.dotfiles/blob/65839bcdb41558d543395c0efa329f2fab6cc644/nvim-lua/.config/nvim/after/ftplugin/cs.lua#L9C1-L9C96

KiLLeRRaT avatar Feb 25 '24 23:02 KiLLeRRaT

I have just installed and configured https://github.com/Hoffs/omnisharp-extended-lsp.nvim and it seems to be working well for me. I have only used it for around 30 minutes so far, but so far, so good...

Yes, it works, but if you try to go to definition on metadata file, an error will be displayed.

AugustoDeveloper avatar Mar 02 '24 00:03 AugustoDeveloper

+1 on this using https://github.com/Hoffs/omnisharp-extended-lsp.nvim

jknopp avatar Mar 09 '24 20:03 jknopp

+1 on this using https://github.com/Hoffs/omnisharp-extended-lsp.nvim

See my edit on my first post about this and the ftplugin binding.

KiLLeRRaT avatar Mar 11 '24 17:03 KiLLeRRaT