Clarify expected use for inlayHint/refresh

[DanTup]

I'm implementing InlayHints and noticed an issue with them not refreshing in VS Code when I need them to.

file1

final foo = getThing();

file2

String getThing() {}

If I enable inlay hints, file1 gets a String hint on foo to show that the inferred type is Foo.

However, if I modify file2 and change String to int, VS Code does not know to refresh the labels in file1 (I have both files open side-by-side).

It seems like the answer to this is to call inlayHint/refresh, however:

• It doesn't take a filename, so will refresh them for every single file
• Doing this essentially on every file modification sounds like it could result in a lot of additional requests
• The spec makes it sound like we shouldn't be calling this often:

     * Note that this event is global and will force the client to refresh all
     * inlay hints currently shown. It should be used with absolute care and
     * is useful for situation where a server for example detects a project wide
     * change that requires such a calculation.
  

How is this intended to work? What are the responsibilities of the client, and what are the responsibilities of the server to ensure these labels are kept up-to-date when they reference functions from other files?

[dbaeumer]

@jrieken @alexdima sounds familiar :-) We had the same discussion with semantic tokens and captured the solution we landed on here: microsoft/vscode#130486 (comment)

But looks like we never implemented it in VS Code hence no adoption in LSP.

For inlay hints I propose to follow the same proposed pattern.

[DanTup]

@dbaeumer do I understand correctly that this proposal means the server will need to send the refresh call after document changes (and the commented above will need to be tweaked to make it not sound like something to generally avoid)? Does VS Code automatically refresh them for the current file after changes?

So from the servers perspective, whenever a file is modified, I think it should compute if any other files could be affected by the change (for example, whether any public API changed) and then send this ("global") event to invalidate all inlay hints the client has?

Is there any reason not to just also allow an array of Uris to be included, so if the server does not know exactly which ones were invalidated, the client could optimise a little?

[heejaechang]

pylance is having the same issue. almost any change in a file can cause semantic change in other files that depend on the file that has changed.

currently, since refresh doesn't take any params, we need to send this global refresh for both semantic token and inlay hint if any file that depends on the file is currently opened.

that cause all open files to get new semantic token and inlay hint requests even if most of them don't need to be refreshed. things get worse if some of them are notebooks since all of cells will be asked to be refreshed as well.

but for semantic token case, most of work could be avoided using the previousResultId, when files that no need to be refreshed gets the request, we can simply return previousResultId with no changes.

but for inlayHint case, there is no such capability, so there is no other way but fulfill the request. we could cache the previous results and re-send them, but unlike semantic token case, we can't avoid but keep sending a lot of the same data for every char user types for all open files.

I think either refresh should take list of files we want to refresh or inlayhint should have similar concept as previousResultId (this previousResultId could be some kind of opaque semantic version from the server, and client simply keep old data if that id is same as before)

[heejaechang]

@dbaeumer any update on this?

[dbaeumer]

@heejaechang I see the problem for inlay hints however this can't be fixed on the LSP client side only. As outlined here microsoft/vscode#130486 (comment) this needs also be addressed in the editor (e.g. VS Code) because you will have the same problem without LSP. I could simply add something to LSP but its capability would be off for VS Code which in the end will not help you. So we need to push for https://github.com/microsoft/vscode/issues/130486 to make progress on this.