Issafalcon / neotest-dotnet

Neotest adapter for dotnet
MIT License
69 stars 23 forks source link

[Feature] Remove dependency on Omnisharp LSP to allow F# support #32

Closed WillEhrendreich closed 1 year ago

WillEhrendreich commented 1 year ago

hey, thank you so much for this project, I'm so excited!!!

i'm wondering if there is an easy way for it not to depend upon omnisharp being attached to the buffer? with .fs files it wont trigger to attach, but right now it seems the functionality isn't abastracted away from using omnisharp directly.

thanks!

looks like there's already been some contribution from @adelarsq! Good to see you here! haha.

Issafalcon commented 1 year ago

Hi @WillEhrendreich - Glad you're looking forward to using it! 😃

I'll be honest, I have 0 experience with F#, or the F# ecosystem, and only now have I learnt that Omnisharp basically doesn't support F#! So yes, that makes things a bit trickier (I made the incorrect assumption Omnisharp lsp would cover both F# and C#).

At the moment, I'm relying on Omnisharp to do 2 things:

  1. Determine if there are tests within the file
  2. Figure out which testing framework is being used by the tests

As I've worked on this project, I've moved further and further away from using the Omnisharp LSP endpoints, and it sounds like I need to get rid of it as a dependency altogether! Moving to a tree-sitter based approach to cover off the 2 tasks above would be the way to go I think.

There is another obvious barrier before this adapter can be used with F# tests, and that is the fact I am yet to actually create and test the required tree-sitter queries needed to identify the various tests in all the possible test framework use cases for F#.

For now, I think step one is removing Omnisharp dependency, which should make the adapter LSP agnostic. I'll be adding tickets to get the remaining F# treesitter stuff in place.

WillEhrendreich commented 1 year ago

Yeah, I'd love using treesitter for it too, but as of right now, there is only a very incomplete grammar set up, no queries, nothing like that yet.

https://github.com/nvim-treesitter/nvim-treesitter/issues/2282#issuecomment-1104155140

it's all still way over my head, I'm barely up an running with getting neovim usable on a day to day basis.

I'm not sure how to meaningfully contribute to the project.

https://github.com/baronfel/tree-sitter-fsharp

Baronfel, or Chet Husk, is an Ionide/FsAutocomplete maintainer and a PM at microsoft, so he's definitely a great guy to have started the project, but, as you might guess, he's incredibly busy, and hasn't been able to contribute to the project in a while.

yatli, another maintainer of the grammar, is another microsoft guy, so, awesome, but busy.

Fun fact he's also a Nvim guy, he built an fsharp vim gui called fvim.

https://github.com/yatli/fvim

All of that is to say, that if there's any way to NOT use tree-sitter, like just having generic nvim lsp handlers do the job, then that's going to be a great big win for the project.

Issafalcon commented 1 year ago

Well, this certainly puts a damper on the F# support!

I didn't realise that the ecosystem for F# was that limited. The FsAutoComplete LSP looks promising. If it had some of the additional endpoints that the Omnisharp server does (https://github.com/OmniSharp/omnisharp-roslyn/blob/master/src/OmniSharp.Abstractions/OmniSharpEndpoints.cs) like the codeStructure or discoverTests, it might have been feasible to try and use that LSP to discover test positions and parse them. However, without those, and without a developed parser / grammar for the F# tree-sitter to hang queries off, I can't say that it would be possible to add support to F# in this adapter at the moment.

I'll keep an eye out on the tree-sitter parser situation, and as soon as it's good to go, getting F# support up and running for this adapter should be quite quick (at least for the most common use cases). I would imagine that might be the path of least resistance, unless the LSP handlers for getting code structure etc are added first. Then I'll re-assess the route of using LSP as the source of truth for F# files.

adelarsq commented 1 year ago

At moment there is no work on a treesitter for F#. There is baronfel/tree-sitter-fsharp, but he said that there are some corners on the project to solve before anything.

For LSP we have FsAutoComplete that already have a setup on nvim-lspconfig, but I don't know how LSP can help on this.

WillEhrendreich commented 1 year ago

Thanks for the link to that, I wish i would have waited a bit, lol. I found that already but it did take a bit. the omnisharp project structure is a little confusing at first dive in with no one to ask what's what.

I'm trying to assess what each endpoint actually DOES.

fsautocomplete has the normal lsp endpoints, for sure, and a couple of specific ones.

I'm working on a PR now to generalize it, so that basically if it's Omnisharp then carry on as before, if it's an fsautocomplete based one then do .. whatever equivalent endpoint calls are there for that.. I'm still exploring.. lol.

but if there's some usable equivalent call on fsautocomplete's side, this should be a piece of cake.. (famous last words)

Issafalcon commented 1 year ago

Ha, yes the omnisharp codebase isn't that easy to navigate. I found the easiest thing was to just make direct calls to the endpoints and see what came back.

It would be great if you could figure out if there is something on fsautocomplete we could hook onto. Check out what's in the Omnisharp folder of this adapter, especially https://github.com/Issafalcon/neotest-dotnet/blob/88c4215c98487d8bb3df324b1f7865f9ca630177/lua/neotest-dotnet/omnisharp-lsp/requests.lua. I left a comment on there with the code structure response structure to remind me. If something like that exists, then it will work. So, given a file path, we get back at least the test framework info, the row and column positions of namespaces, classes and methods etc, and whether the methods are tests or not. Or if we can get that info from a couple of different requests, either way.

However, there is one thing to bear in mind, and that was the reason I had to abandon using Omnisharp-LSP altogether, and that's because it wasn't possible to get test positions of all the parameterized tests. I don't think this will be possible with an LSP, without jumping through some hoops. I may be wrong though, as my F# knowledge is lacking.