docs: Update options for language servers

[cameron-martin]

Previously the docs suggested using facebook's proof-of-concept bazel language server, but there is a more up-to-date version available based on this. There is also another implementation, starpls, that is based on rust-analyzer. This also updates the default values of the lsp settings to not favour a particular language server.

[vogelsgesang]

I wonder, if in the longterm we should aim for also replacing buildifier-based functionality (formatting, linting) with the language servers. As a part-time vim user, I personally would like that, because I can easily use a language server from vim, but I can't directly use buildifier. Also, it would probably simplify this extension a bit, if we have a single dependency (the language server) instead of two dependencies (the language server and buildifier).

(not really something to change in this pull request. Just a general idea)

[withered-magic]

rust-analyzer does something similar where cargo check (or clippy? forgot which one) is actually run by the language server itself and not the extension iirc

[vogelsgesang]

yes, clangd (for C++) also does something similar: They also embed the clang-tidy checks into clangd such that the check results can be displayed directly in the IDE. clangd does not actually call clang-tidy as a separate binary, though, but instead directly links against the clang-tidy library. Not sure how buildifier is built and if it could also be consumed as a library.

[cameron-martin]

rust-analyzer does forward the output of cargo check, but there is some doubt on whether this is the correct thing to. See rust-lang/rust-analyzer#13437 (comment).

[withered-magic]

But yeah I would probably advocate for some sort of middle ground where the language servers are the ones responsible for installing buildifier and running it on changes, instead of implementing that functionality themselves. It'd guarantee that we won't have any conflicts with any existing workflows that use buildifier, e.g. people running it in CI. (Otherwise if the language servers implemented buildifier's functionality themselves, then to avoid said conflicts we'd have to make sure their behaviors match exactly which would be difficult to do).

[withered-magic]

Ooo interesting, I didn't see that thread!

[vogelsgesang]

responsible for installing buildifier

What do we think about auto-installing other tools as part of an language server / this extension?

Leaving my personal usage of vim aside, the major hurdle for VS Code users with so many auxiliary tools (bazel, buildifier, language servers) is that this leads to a higher entry barrier since there are more tools that need to be set-up. We could counteract this by teaching the this extension to automatically download bazelisk, buildifier and a language server, in case those aren't found on the system, yet.

But I am not sure if we are comfortable with installing additional tools on the users' machines from a security point of view (potential supply chain attacks?)

[cameron-martin]

I can't comment on the security side, but I like the idea of this extension downloading tools. rust-analyzer used to do that before it came bundled with rust and the user experience was great.

[vogelsgesang]

I created #371 to track the idea of automatically bootstrapping the tools

[withered-magic]

A bit off topic, but should we maybe add a disclaimer about certain functionalities not working correctly for either language server unless you're on a certain Bazel version? The main scenario I had in mind is for workspaces using bzlmod, since things like goto def won't work at all without the bazel mod dump_repo_mappings command, which wasn't added till 7.1.0 (pretty recently!).

[cameron-martin]

@withered-magic sounds like a good idea, feel free to open a PR.