Adding a dependency to Cargo.toml require RLS + IDE restart

[norru]

Adding a dependency to Cargo.toml doesn't trigger a rebuild. As a result, any reference to the added dependency results in a compilation error.

For

[dependencies]
...
notify = "*"
...

main.rs:

extern crate notify;

RLS log:

org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"code":"E0463","message":"can't find crate for `notify`\n\ncan't find crate","range":{"end":{"character":20,"line":38},"start":{"character":0,"line":38}},"severity":1,"source":"rustc"}],"uri":"file:///home/norru/Projects/ivt/parallax_testbed/src/main.rs"}}

Workaround: restarting the RLS (and the IDE) triggers a rebuild and the missing crate is linked correctly.

[alexheretic]

Rls should do a full cargo rebuild on manifest changes, is it possible your client isn't telling Rls?

[norru]

Why should my client tell RLS? RLS alreay knows I'm changing Cargo.toml (at least so it says in the logs).

EDIT: I've just checked and in fact RLS doesn't seem to be aware of my Cargo.toml changes.

The role of the RLS is so that that the client doesn't have to worry about file changes.

@mickaelistria

[mickaelistria]

We can try to associate RLS with the Cargo.toml files so it gets notified of updates on that file (and could even provide some edition assistance for it).

[norru]

It's well worth a shot. No idea how to do it though.

[norru]

If the RLS already does this then this issue should be moved to Corrosion. @mickaelistria WDYT?

[alexheretic]

The role of the RLS is so that that the client doesn't have to worry about file changes

That isn't true, LSP relies on clients watching for file changes and notifying the language server. Sending workspace/didChangeWatchedFiles for all Cargo.toml & Cargo.lock files (plus ./target directory deletion, but not ./target contents) will allow Rls to trigger a full cargo build on changes to these.

[mickaelistria]

Does RLS somehow declare automatically which files it's supposed to watch for best results, or do we have to hardcode it in our integration? IMHO best behavior should be default and shouldn't require extra-effort.
But would sending fileSaved and documentChanges notification work? It would seem more simple than dealing with extra didChangeWatchedFiles notification.

[norru]

Am I misunderstanding how this works? Is this another feature that has to be implemented in each and every client?

Servers are allowed to run their own file watching mechanism and not rely on clients to provide file events.

Out of curiosity, what's the rationale behind RLS not actually doing this (in fact it does look like it's not recommended although the reasons for the recommendation on the link above are very unconvincing)

@mickaelistria putting things together, it does look like the file monitoring should be done by LSP4E. I have opened an issue in the Eclipse bug reporter. https://bugs.eclipse.org/bugs/show_bug.cgi?id=543627

On a side note, is there any chance that we can have LSP4E issue reporting on GitHub? The Eclipse one is very clunky and cross-referencing all the other GitHub project is very inconvenient.

[norru]

fileSaved and documentChanges notification work? It would seem more simple than dealing with extra didChangeWatchedFiles notification.

It may work if we assume all file changes are done within the Corrosion "registered" editors which is not true but it's what most developers would do anyway. It's possibly a useful stepping stone towards "correct" implementation.

Again, I wonder why this is left to each and every client while this clearly is an option that should be implemented in the server as a default - with the option of disabling on in the server if the client can provide a more efficient implementation.

[norru]

I have a local version of Eclipse which now detects changes in Cargo.toml via didSave but still adding a dependency requires a restart.

LSP4E to org.eclipse.corrosion.rls:{"jsonrpc":"2.0","method":"textDocument/didSave","params":{"textDocument":{"uri":"file:///home/nico/Workspaces/runtime-EclipseApplication(1)/new_rust_project/Cargo.toml"},"text":"[package]\nname \u003d \"new_rust_project\"\nversion \u003d \"0.1.0\"\nauthors \u003d [\"Nicola Orru \u003cnigu.orru@gmail.com\u003e\"]\nedition \u003d \"2018\"\n\n[dependencies]\ncgmath \u003d \"*\"\n"}}
LSP4E to org.eclipse.corrosion.rls:Content-Length: 228


LSP4E to org.eclipse.corrosion.rls:{"jsonrpc":"2.0","id":"25","method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///home/nico/Workspaces/runtime-EclipseApplication(1)/new_rust_project/Cargo.toml"},"position":{"line":7,"character":0}}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","id":"25","result":[]}
LSP4E to org.eclipse.corrosion.rls:Content-Length: 309


LSP4E to org.eclipse.corrosion.rls:{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"version":17,"uri":"file:///home/nico/Workspaces/runtime-EclipseApplication(1)/new_rust_project/Cargo.toml"},"contentChanges":[{"range":{"start":{"line":7,"character":0},"end":{"line":7,"character":0}},"rangeLength":0,"text":"#"}]}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"id":"progress_41","title":"Building"}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"id":"progress_41","message":"new_rust_project cfg(test)","title":"Building"}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"id":"progress_41","message":"new_rust_project","title":"Building"}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"done":true,"id":"progress_41","title":"Building"}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"id":"progress_40","title":"Indexing"}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"code":"dead_code","message":"constant item is never used: `UNUSED`\n\nnote: #[warn(dead_code)] on by default","range":{"end":{"character":22,"line":2},"start":{"character":0,"line":2}},"severity":2,"source":"rustc"},{"code":"clippy::bool_comparison","message":"equality checks against true are unnecessary\n\nnote: #[warn(clippy::bool_comparison)] on by default\nhelp: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison\nhelp: try simplifying it as shown: `test`","range":{"end":{"character":16,"line":6},"start":{"character":4,"line":6}},"severity":2,"source":"clippy"}],"uri":"file:///home/nico/Workspaces/runtime-EclipseApplication(1)/new_rust_project/src/main.rs"}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"done":true,"id":"progress_40","title":"Indexing"}}
LSP4E to org.eclipse.corrosion.rls:Content-Length: 384


LSP4E to org.eclipse.corrosion.rls:{"jsonrpc":"2.0","method":"textDocument/didSave","params":{"textDocument":{"uri":"file:///home/nico/Workspaces/runtime-EclipseApplication(1)/new_rust_project/Cargo.toml"},"text":"[package]\nname \u003d \"new_rust_project\"\nversion \u003d \"0.1.0\"\nauthors \u003d [\"Nicola Orru \u003cnigu.orru@gmail.com\u003e\"]\nedition \u003d \"2018\"\n\n[dependencies]\n#cgmath \u003d \"*\"\n"}}
LSP4E to org.eclipse.corrosion.rls:Content-Length: 228


LSP4E to org.eclipse.corrosion.rls:{"jsonrpc":"2.0","id":"26","method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///home/nico/Workspaces/runtime-EclipseApplication(1)/new_rust_project/Cargo.toml"},"position":{"line":7,"character":1}}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","id":"26","result":[]}
LSP4E to org.eclipse.corrosion.rls:Content-Length: 183


LSP4E to org.eclipse.corrosion.rls:{"jsonrpc":"2.0","id":"27","method":"textDocument/codeLens","params":{"textDocument":{"uri":"file:///home/nico/Workspaces/runtime-EclipseApplication(1)/new_rust_project/Cargo.toml"}}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","id":"27","result":[]}
LSP4E to org.eclipse.corrosion.rls:Content-Length: 189


LSP4E to org.eclipse.corrosion.rls:{"jsonrpc":"2.0","id":"28","method":"textDocument/documentSymbol","params":{"textDocument":{"uri":"file:///home/nico/Workspaces/runtime-EclipseApplication(1)/new_rust_project/Cargo.toml"}}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","id":"28","result":[]}

[norru]

Accidentally closed

[norru]

@alexheretic Here's what I see after adding the cgmath dependency to Cargo.toml.

RLS appears to be ignoring the change.

LSP4E to org.eclipse.corrosion.rls:{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"version":2,"uri":"file:///home/nico/Workspaces/runtime-EclipseApplication(1)/new_rust_project/Cargo.toml"},"contentChanges":[{"range":{"start":{"line":7,"character":0},"end":{"line":7,"character":1}},"rangeLength":1,"text":""}]}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"id":"progress_59","title":"Building"}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"id":"progress_59","message":"new_rust_project","title":"Building"}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"id":"progress_59","message":"new_rust_project cfg(test)","title":"Building"}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"done":true,"id":"progress_59","title":"Building"}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"id":"progress_58","title":"Indexing"}}
org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"code":"E0463","message":"can't find crate for `cgmath`\n\ncan't find crate","range":{"end":{"character":20,"line":0},"start":{"character":0,"line":0}},"severity":1,"source":"rustc"}],"uri":"file:///home/nico/Workspaces/runtime-EclipseApplication(1)/new_rust_project/src/main.rs"}}

LSP4E to org.eclipse.corrosion.rls:{"jsonrpc":"2.0","method":"textDocument/didSave","params":{"textDocument":{"uri":"file:///home/nico/Workspaces/runtime-EclipseApplication(1)/new_rust_project/Cargo.toml"},"text":"[package]\nname \u003d \"new_rust_project\"\nversion \u003d \"0.1.0\"\nauthors \u003d [\"Nicola Orru \u003cnigu.orru@gmail.com\u003e\"]\nedition \u003d \"2018\"\n\n[dependencies]\ncgmath \u003d \"*\"\n "}}

org.eclipse.corrosion.rls to LSP4E:{"jsonrpc":"2.0","method":"window/progress","params":{"done":true,"id":"progress_58","title":"Indexing"}}
LSP4E to org.eclipse.corrosion.rls:Content-Length: 183

[mickaelistria]

Thanks for the experiment @norru .
So it seems like a good solution that would be simple enough both from client and server side would be that RLS reacts to "didSave" on Cargo.toml just like it reacts to didChangeWatchedFile. What do RLS contributors think about it? Is it safe? Is it simple enough?