Fix no inlay hints / unresolved tokens until manual edit to refresh

[Veetaha]

Fixes #5349

Now we return ContentModified during the workspace loading. This signifies the language
client to retry the operation (i.e. the client will
continue polling the server while it returns ContentModified).
I believe that there might be cases of overly big projects where the backoff
logic we have setup in sendRequestWithRetry (which we use for inlay hints)
might bail too early (currently the largest retry standby time is 10 seconds).
However, I've tried on one of my project with 500+ dependencies and it is still enough.

Here are the examples before/after the change (the gifs are quite lengthy because they show testing rather large cargo workspace).

Before

Here you can see that the client receives empty array of inlay hints and does nothing more.
Same applies to semantic tokens. The client receives unresolved tokens and does nothing more.
The user needs to do a manual edit to refresh the editor.

After

Here the server returns ContentModified, so the client periodically retries the requests and eventually receives the wellformed response.

[kjeremy]

I think this approach makes sense! Do we need to let exit/shutdown go through? I haven't looked too closely at it.

[jonas-schievink]

I believe that there might be cases of overly big projects where the backoff
logic we have setup in sendRequestWithRetry (which we use for inlay hints)
might bail too early (currently the largest retry standby time is 10 seconds).

My understanding is that the Loading status only persists until the VFS has loaded all files into memory, which hopefully won't take that long.

I wouldn't be surprised if some clients will have problems with initialize returning ContentModified though, so I think I agree with @kjeremy that we shouldn't return it for initialize and shutdown.

[kjeremy]

During initialize the server should respond to a request with -32002 which is serverNotInitialized. At the point in this PR though we've already made it past the initialize phase (we enter main_loop after initialization has completed) so I don't think that applies here.

The shutdown request from the client should probably not respond with ContentModified though. See: https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#shutdown

[jonas-schievink]

Ah, I see

[Veetaha]

Yup, I tried returning ServerNotInitialized at the same place I've put ContentModified and the language client just considered this an unrecoverable error.
It's just that we have such a code flow where part of initialization is in the main loop. I think the reason for this is allowing for some interactions during the initial loading, but this leads to inconsistencies. Maybe some requests do may work while loading is in process?

[kjeremy]

Yup, I tried returning ServerNotInitialized at the same place I've put ContentModified and the language client just considered this an unrecoverable error.

That makes sense since we've already processed the initialize request and given the response back to the client.

It's just that we have such a code flow where part of initialization is in the main loop. I think the reason for this is allowing for some interactions during the initial loading, but this leads to inconsistencies. Maybe some requests do may work while loading is in process?

At this point in the code the LSP client considers us initialized. The spec makes no mention of the expected behavior (as in "what does initialization really mean?"). I think your solution to return ContentModified here for every request (except shutdown!) is entirely reasonable especially in light of our reloading capability.

[Veetaha]

I've added a condition to let shutdown request pass through, though we do nothing in response to it right now.
It will be reasonable to add a shutdown state to gate further requests as kjeremy mentioned here #5696, but let's leave it for other RP?

[Veetaha]

Hmm, is this a bug with cargo fmt, but for some reason it adds two closing curly braces at the end of on_request function and shifts the comment....
UPD, oh yeah, not quals in rust is not !== but !=

[kjeremy]

Does cargo xtask format make a difference?

[Veetaha]

Nvm, that's tripple equals what makes cargo fmt wild...

[kjeremy]

UPD, oh yeah, not quals in rust is not !== but !=

Relevant :) rust-lang/rust#75321

[Veetaha]

Lol, what a coincidence :D

[kjeremy]

It will be reasonable to add a shutdown state to gate further requests as kjeremy mentioned here #5696, but let's leave it for other RP?

Yeah I think that should be a separate PR.

[jonas-schievink]

bors r+

[bors]

Build succeeded:

• Rust (macos-latest)
• Rust (ubuntu-latest)
• Rust (windows-latest)
• TypeScript (macos-latest)
• TypeScript (ubuntu-latest)
• TypeScript (windows-latest)