Re-use document symbols functionality for workspace symbols

[mhanberg]

Previously, the workspace symbols feature used the :code.all_loaded functionality of the OTP standard library to "discover" all modules, and then was able to parse out the different type of symbols from that.

In doing so, this loaded tons of symbols that are completely unrelated to your workspace. As a prior user of the document symbols feature, this really threw me off. As I understand it, the workspace symbols support is for quick navigation of functions, modules, types in your own codebase, so if the source code is not available in your own repository, you really wouldn't be navigating to it.

The workspace symbols feature was also implemented differently than the document symbols feature. This commit works to align the two features to make them work similarly.

In addition, this also allows you to pass an empty query to LSP request in order to retrieve a full list of all of the symbols. This aligns the Elixir implementation with the spec, and makes it useful for those who use a fuzzy searching plugin in their editor to do the querying.

Here are two videos demonstrating the differences between the current and (potentially) new behavior. I uploaded the to YouTube as the vides were over 10MB.

Current Behavior: https://youtu.be/PUkykG7IK8I New Behavior: https://youtu.be/7CmKaGyTfqk

Notes

• I believe this is technically a breaking change, as the results returned in the request are slightly different than they were before.
• With the previous implementation, I found that in some codebases (like https://github.com/mhanberg/temple), no symbols were being returned for the project. I suspect it had something to do with the library not having an Application, so no code is loaded at first. The videos uploaded demonstrate it with the Wallaby codebase, which does use an Application however, so I'm not entirely sure why.
• I have tested this on the Wallaby codebase (https://github.com/elixir-wallaby/wallaby) and it seems to return instantly, so I don't think there should be any kind of performance regression, but it would be worth checking against larger codebases.
• I have also tested this on the aws-elixir codebase, which has about 235kloc, and the performance is very good.

PS

I understand this is a significant and unsolicited change, so thank you in advance for reviewing this patch and considering it for merging.

[mhanberg]

The tests pass locally for me on the failed job (elixir 1.13 and otp 24.1), but i can't test the otp 22 failure because i am on mac (I think only 23+ works on mac now a days).

I think it might have been a dialyzer timeout or something, but I can't restart the failed jobs myself it seems.

[mhanberg]

I will be testing this in the VSCode extension soon.

I think that the neovim lsp client might be forgiving in whether it sees a DocumentSymbol or a SymbolInformation, whereas there is a chance that VSCode might be more strict.

[mhanberg]

seems fine

[mhanberg]

All builds green now 😄

[lukaszsamson]

Nice work @mhanberg but I'm not completely convinced we should go this road. Let me state my reasons.

1. Relying on DocumentSymbols provider is in fact relying on AST parsing and hence it's severely limited in understanding macro generated code. The introspection based on loaded modules approach on the other hand is able to extract complete and accurate information about modules/functions/types. Moreover, with elixir >= 1.10 now required and growing support from the upstream it's highly likely elixir-LS features will start migrating to compiler tracers.
2. Yes, the current approach loads a lot of modules but the fact it does so means that your project (or elixir-LS itself) is using those dependencies (or at least loads OTP apps with those modules). Wether those modules should be returned is another thing and other language servers take various approaches. I thing it would be best if it was configurable. See https://github.com/elixir-lsp/elixir-ls/pull/261
3. I know it's not an extremely important feature but we lose ability to jump to erlang source files in elixir projects.
4. Empty query is not supported currently only due to performance reasons (https://github.com/elixir-lsp/elixir-ls/pull/603). With scope limited to mix project it would probably be fine.

Anyway, I plan to play around with it and see how it works in practice

[mhanberg]

@lukaszsamson thank you for taking the time to look over this patch, I really appreciate it.

Relying on DocumentSymbols provider is in fact relying on AST parsing and hence it's severely limited in understanding macro generated code.

This is something I was thinking about when writing this patch. I am trying to think of the utility of navigating to functions that don't exist in the source code. I figure you'd rather go to the actual source code (the macro that generates the code) than the module where it gets injected.

Small example, but I'm not certain the value i'd get from the action/2 function that get injected into every Phoenix controller showing up in the list of symbols.

Is there a useful example that you use regularly?

Moreover, with elixir >= 1.10 now required and growing support from the upstream it's highly likely elixir-LS features will start migrating to compiler tracers.

That seems cool to me. 👍

Yes, the current approach loads a lot of modules but the fact it does so means that your project (or elixir-LS itself) is using those dependencies (or at least loads OTP apps with those modules). Wether those modules should be returned is another thing and other language servers take various approaches. I thing it would be best if it was configurable. See https://github.com/elixir-lsp/elixir-ls/pull/261

I think configuration is a valid point (while, not applicable with this patch, as it can't pull in functions/modules/types that are used in the project but not present in the source code).

One of the things I noticed which instigated me writing this patch was that the symbols didn't seem to show up at all, except for some (to me, random) OTP functions. You can observe this in the screen recording I shared.

Do you know why that is?

I know it's not an extremely important feature but we lose ability to jump to Erlang source files in elixir projects.

I think this is on oversight in the patch. I think this could possibly be remedied. Thank you for pointing it out 👍.

---

Thanks again for reviewing this patch. I appreciate all the comments and for the consideration.

I have been using this branch for the last week at work and have been enjoying it, let me know what you think after using it.

[mhanberg]

I'm going to try and port the workspace symbols to compiler tracers in the next week or so.

[lukaszsamson]

I already have a PoC branch with DocumentSymbols using a database built by compile tracers. It looks promising but requires more work and integration with current AST based approach. Currently compile tracers can only get high quality info only about modules and defs and some limited info about attributes. Anything more than that requires loading the module and doing the usual introspection that WorkspaceSymbols currently do.

[mhanberg]

Gotcha, thanks for the update 🚀

[lukaszsamson]

https://github.com/elixir-lsp/elixir-ls/pull/1050 should improve workspace symbols experience for now. In the future I plan to build a common database/index layer. If that happens parts of this PR may be yet reused