Clippy RLS support

[norru]

Hello,

In VSC the RLS allows transparent linting via native RLS Clippy integration.

rust-lang/rls#149

It would be a great addition to have this in Corrosion.

[norru]

It turns out there is a way to do it in code (but it would be nice to have the setting as well).

Adding the following lines of code to the root of a crate enables all lints for that crate.

#![allow(unknown_lints)]
#![warn(clippy)] 

[LucasBullen]

Are you aware of any command line / preferences option that could enable clippy for RLS? If not then a command would have to edit the root of a crate and I'm not too fond of that.

[norru]

According to RLS README at https://github.com/rust-lang-nursery/rls the only current way to send config is to implement DidChangeConfiguration. There was a command line option but AFAIK it's been removed.

Configuration

The RLS can be configured on a per-project basis; using the Visual Studio Code extension this will be done via the workspace settings file settings.json.

Other editors will have their own way of sending the workspace/DidChangeConfiguration method.

The setting to send for clippy is clippy_preference=off/on/opt-in (String, defaults to "opt-in")

[mickaelistria]

LSP4E doesn't have generic way of sending extra DidChangeConfiguration so far. But it has support for extra InitializeOptions. Is this something RLS can allow (enabling clippy with InitializationOptions).

[norru]

Quick googling:

palantir/python-language-server#57

The Rust language service uses workspace/DidChangeConfiguration as described in https://github.com/rust-lang-nursery/rls#configuration

[norru]

@mickaelistria I think there should be some sort of conversation between LSP4E and the RLS teams. In general, the RLS team appears to use the VSCode client as a reference implementation. As a rule of thumb, they usually reply in ways such as "in VS code it works like X so you have to implement X".

[mickaelistria]

I've opened rust-lang/rls#1026

"in VS code it works like X so you have to implement X".

That kind of answers shows they are focusing on VSCode more than on the protocol or the language server itself. It's a pity.

[norru]

I do agree it's a pity - any way to improve the situation? It might be just a matter of establishing better communication channels between RLS and LSP4E, to show that LSP4E is a viable and potentially popular a client as VSC is.

[mickaelistria]

Just being clear about what would work better for us and making sure we send decent proposals of enhancement should be enough to convince them to implement it, or to merge some good pull requests. I don't think this requires extra-process beyond technical discussions and proposals on tickets.

[norru]

Fair enough

[norru]

Is the plan to get the option of restarting the server after changing the Clippy settings? Otherwise it does make sense to implement the didChangeConfiguration message.

[mickaelistria]

We'll ultimately need ways to send a didChangeConfiguration message (or maybe just to have a configuration file and have RLS checking it for changes, which would be much simpler). But at the moment, I think we should focus on enabling decent rich default feature set before focusing on giving options to users to tweak them.
In reality, many users don't even try to tweak things and stick to the default without looking for more options.

[norru]

Well, I am not "many users" and I value configurability highly. In fact, that is the main reason why I stuck to Eclipse instead of switching to IntelliJ or VSC (which is customizable for many aspects but the GUI is stiff).

In fact I am also miffed by the fact that I cannot easily change the color scheme of the syntax highligher in Corrosion (I'd be happy to edit a CSS manually if nothing else is available).

This is also one of the reasons why I don't use Apple products anymore.

[mickaelistria]

I don't think the features of the Language Server that are really specific to RLS (such as Clippy linter settings) should require a didChangeConfiguration event and a specific IDE integration. I think it's be best to have them declared in a configuration file that the LS would read.
Would that seem configurable/simple enough to you?

[norru]

Editable configuration file in a custom location (a la Maven) sounds good to me. Will probably need integration with Corrosion as to be able to specify such location - and get the RLS to reload the configuration if it changes.

[norru]

I personally think a .rls.json file in the root of each project would be great and way more portable - however this doesn't look like it's the strategy for the RLS team (whereas they prefer to have a tight control of the configuration from within their host IDE, absolutely no idea why).

[mickaelistria]

@norru do you have a link to a github ticket on RLS discussing it? I could try to add some support to the idea.

[norru]

@mickaelistria
At some point they had it and then

rust-lang/vscode-rust#107 "rls.toml is deprecated" (in favour of "whatever VS Code does").

Yeah, passing configuration is a standard part of the LSP, so any client can do it. Exactly how the client exposes config options to the user is up to the client.

In fact this does look like LSP4E should implement this.

[mickaelistria]

Thanks @norru . I think the approach taken by RLS is at our disservice and at RLS's disservice too. I've reported rust-lang/rls#1047 so its developer can understand better how they're moving away from standards and a putting at risk the value of their language server which may turn VSCode specific or become too expensive to catch up with.

FYI, so you won't be surprised, our current policy at Red Hat on such topic is to go for 0 (strictly 0) development specific to a language server or another, except for the discovery/provisioning part (that's supposed to be stable and extremely cheap to maintain). This is because our interest in Language Servers and IDEs spans over several tools and we want to avoid having to implement LS-specific configuration/preference page on each one of the tools we want to be able to play with.
I can welcome PRs adding specific integration with some specific configuration in Corrosion, but I think it's really going at the opposite direction and is an architectural regression when we should all design workflows and integrations that are cheap and powerful (like editing a file ;)

[norru]

Yep, I understand. I am quite baffled.

[norru]

Just thinking, for a stopgap, perhaps you could add a primitive implementation for didChangeConfiguration which just loads the content of the message from, say, .rls.conf and sends it verbatim (or as unchanged as possible) to the RLS via LSP4e?

File change events can also be tracked so that whenever the .rls.conf file is changed, LSP client (Corrosion?) reloads the file and resends the config. This .rls.conf path could be made configurable as a Workspace or Project setting in Eclipse as well (see how m2e does it).

I do understand that it's very wasteful for each RLS client to implement a similar mechanism, but this could be the only alternative solution to playing catch-up with Eclipse dialog settings (assuming the RLS team establishes they don't want to change their strategy).

[mickaelistria]

Seems like we can now use the initializationOptions for that, cool!
@norru Do you know how the initializationOptions should be modified in https://github.com/eclipse/corrosion/blob/master/org.eclipse.corrosion/src/org/eclipse/corrosion/edit/RLSStreamConnectionProvider.java to enable it? You can have a look at https://github.com/eclipse/wildwebdeveloper/blob/master/org.eclipse.wildwebdeveloper/src/org/eclipse/wildwebdeveloper/css/CSSLanguageServer.java#L58 for an example.

[norru]

Actually, no idea :) I'll read the code at some point. I currently have no bandwidth to contribute code to Corrosion itself though...

[norru]

Fixed in #183