`textDocument/references` also returns `@spec` as a reference

[ogomezba]

Hello! Thank you for the great work. I've been using Expert for a week and it has been a fantastic experience.

Feel free to close the issue if it is intended, but I've noticed that, when asking the LSP server for textDocument/references, it also includes the @spec of the function itself as a reference. Is this the intended behaviour? Is any change planned for this for the future?

Thank you!

[mhanberg]

Can you share screenshots or a sample project we could use to reproduce?

[ogomezba]

Sure! So I reproduce it by following these steps:

1. Create a new Phoenix project with mix phx.new --no-ecto expert_testing
2. Create a new module inside the project lib/expert_testing/my_module.ex
3. Insert the contents:

   defmodule ExpertTesting.MyModule do
     def hello do
       world()
     end
   
     @spec world() :: :ok
     defp world() do
       IO.puts("Hello World")
     end
   end
   

4. Use the "Go to References" functionality of the client (in the video VSCode, but the same occurs in Neovim)

https://github.com/user-attachments/assets/72e0783b-d0d0-4afc-b022-5702c190088e

Please let me know if you need any additional information (versions, etc.)

[ogomezba]

Hi @mhanberg ! I have some time that I can dedicate to this. But I would like to confirm if:

1. This is also experienced by others and is not just something wrong on my side.
2. It would be desirable.

Assuming the first clause is true, my argument for the second is that, at least in Neovim and VSCode, the default behaviour when only a reference is found is that it takes you directly to the reference itself, which is a default I personally like. On the other hand, if more than one reference is found, typically a UI is presented with the different references. Considering what I mentioned in the issue, this happens even when there is only one reference to the function, but the function has a spec, which I find a bit cumbersome.

Let me know your thoughts, and if you think it's worth working on this, I can dedicate time to it.

[mhanberg]

I see, I think i misunderstand the issue before.

To summarize, it seems like its returning the definition, its spec, and the usage all as references.

In Next LS, it doesn't provide itself as a reference, which I find intuitive, and also doesn't include specs.

I feel like the spec should not be included, but I'm not entirely sure about returning itself as a reference. It might be helpful to see how other languages LSPs handle this situation.

Also, we don't have any concept of configuration yet, but when we do, this sort of thing might be configurable.

[ogomezba]

Completely agree. I just wanted to start a bit of the discussion, but I was indeed thinking that this might be a good "configurable" behaviour. I can take a look at other LSPs and check how others behave, but I completely agree with the definition being returned being unintuitive.

One usage that I've found is that, in case you have just the function definition and a usage, using textDocument/references on the usage, both in Neovim and VSCode, takes you to the definition immediately (as it is the only other "reference"), behaving basically like a textDocument/definition. Maybe others rely on this behaviour, but I personally find it unnecessary, but just my opinion.

[mhanberg]

One usage that I've found is that, in case you have just the function definition and a usage, using textDocument/references on the usage, both in Neovim and VSCode, takes you to the definition immediately (as it is the only other "reference"), behaving basically like a textDocument/definition. Maybe others rely on this behaviour, but I personally find it unnecessary, but just my opinion.

This is client behavior, I'm not sure it's part of the server. I think the same thing happens if definition returns multiple locations (I've seen that in typescript)

I'll double check the spec tho.

[ogomezba]

Yeah, just pointing it out as it is the default client behaviour in those two clients.

I've checked the spec and it looks like there is already a context param that can be passed to references to specify if the declaration of the symbol itself should be returned.

[john-eevee]

I'm not entirely sure about returning itself as a reference. It might be helpful to see how other languages LSPs handle this situation.

Checked the Java LSP and Dart in VSCode; they return the definition as a reference. In the future Expert could offer the configuration to disable it, but I don't see a problem with it right now, as the definition is a reference on itself.

[ogomezba]

Hey @john-eevee ! Thank you for checking. I think that, indeed, returning the definition is good behaviour, and even more if the LSP (and Expert itself itself) support an includeDeclaration flag in the textDocument/references method. In my case, passing includeDeclaration as false avoids having the declaration as a reference (which I found especially annoying when having different definition clauses for the same function).

Still, being able to configure if the @specs should be returned also as reference would be nice in my opinion. But it is something that can be considered after https://github.com/elixir-lang/expert/issues/155.

I think we can close this issue if you all are okay with that.