Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inlay hints' text edits do not apply when being resolved #193124

Closed
SomeoneToIgnore opened this issue Sep 14, 2023 · 9 comments
Closed

Inlay hints' text edits do not apply when being resolved #193124

SomeoneToIgnore opened this issue Sep 14, 2023 · 9 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug inlay-hints insiders-released Patch has been released in VS Code Insiders verified Verification succeeded

Comments

@SomeoneToIgnore
Copy link

Does this issue occur when all extensions are disabled?: Yes/No

  • VS Code Version:
  • OS Version:
Version: 1.82.1
Commit: 6509174151d557a81c9d0b5f8a5a1e9274db5585
Date: 2023-09-08T08:49:22.656Z
Electron: 25.8.0
ElectronBuildId: 23503258
Chromium: 114.0.5735.289
Node.js: 18.15.0
V8: 11.4.183.29-electron.0
OS: Darwin arm64 22.6.0

Steps to Reproduce:

  1. Have a language server that is able to resolve textEdits field of inlay hints, e.g. rust-analyzer
  2. Get a hint, hover it and double click it, to insert hint's edit
    Expected: the document gets edited with hint contents
    Actual: nothing happens, but hint resolve definitely happens as LSP log shows, and we can cmd+click certain hints (which means r-a resolved label.location)

Additional information: if textEdits is not resolved (see patch for rust-analyzer in rust-lang/rust-analyzer#15604 or remove the corresponding client hint resolve capability), double clicking the hint works.

@jrieken jrieken added info-needed Issue requires more information from poster inlay-hints labels Sep 15, 2023
@jrieken
Copy link
Member

jrieken commented Sep 15, 2023

Please attach a minimal but complete code sample that allows me to reproduce this

@SomeoneToIgnore
Copy link
Author

SomeoneToIgnore commented Sep 15, 2023

Alas, I'm not aware of any other langserver capable of sending hints' edits, so I'm using rust-analyzer as an example, any latest version has this.

  1. Create a new Rust project somewhere: cargo init test_resolve
  2. adjust ./test_resolve/src/main.rs to contain anything with variables, e.g.
fn main() {
    let test = "foo".to_string();
}
  1. test variable will have a : String hint after you interact with the editor (VSCode does not issue another hint request after hint refresh, see Support for workspace/inlayHint/refresh to ensure inlay hints are correctly shown in editors rust-lang/rust-analyzer#13369 (comment), so you have to scroll or somehow edit the document for the first hints to appear)
  2. double click the hint — it would not insert its edit
  3. cmd + click the hint — it will navigate elsewhere

rust-analyzer resolves both properties (textEdit that does not work and label.location that works and navigates).

LSP trace I'm getting for this sequence
[[Trace - 09:55:50] Sending request 'initialize - (0)'.
[Trace - 09:55:50] Received response 'initialize - (0)' in 5ms.
[Trace - 09:55:50] Sending notification 'initialized'.
[Trace - 09:55:50] Sending notification 'textDocument/didOpen'.
[Trace - 09:55:50] Received notification 'experimental/serverStatus'.
[Trace - 09:55:50] Received request 'client/registerCapability - (0)'.
[Trace - 09:55:50] Sending response 'client/registerCapability - (0)'. Processing request took 0ms
[Trace - 09:55:50] Received request 'window/workDoneProgress/create - (1)'.
[Trace - 09:55:50] Sending response 'window/workDoneProgress/create - (1)'. Processing request took 0ms
[Trace - 09:55:50] Received notification '$/progress'.
[Trace - 09:55:50] Sending request 'textDocument/codeAction - (1)'.
[Trace - 09:55:50] Received response 'textDocument/codeAction - (1)' in 2ms.
[Trace - 09:55:50] Sending request 'textDocument/inlayHint - (2)'.
[Trace - 09:55:50] Received response 'textDocument/inlayHint - (2)' in 22ms.
[Trace - 09:55:50] Received notification '$/progress'.
[Trace - 09:55:50] Sending request 'textDocument/inlayHint - (3)'.
[Trace - 09:55:50] Sending notification '$/cancelRequest'.
[Trace - 09:55:50] Sending request 'textDocument/inlayHint - (4)'.
[Trace - 09:55:50] Received response 'textDocument/inlayHint - (3)' in 28ms.
[Trace - 09:55:50] Received response 'textDocument/inlayHint - (4)' in 3ms.
[Trace - 09:55:51] Received request 'client/registerCapability - (2)'.
[Trace - 09:55:51] Sending response 'client/registerCapability - (2)'. Processing request took 1ms
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received request 'window/workDoneProgress/create - (3)'.
[Trace - 09:55:51] Sending response 'window/workDoneProgress/create - (3)'. Processing request took 0ms
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received request 'window/workDoneProgress/create - (4)'.
[Trace - 09:55:51] Sending response 'window/workDoneProgress/create - (4)'. Processing request took 0ms
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Sending request 'textDocument/foldingRange - (5)'.
[Trace - 09:55:51] Sending request 'textDocument/semanticTokens/range - (6)'.
[Trace - 09:55:51] Received response 'textDocument/foldingRange - (5)' in 0ms.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received response 'textDocument/semanticTokens/range - (6)' in 28ms.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Sending request 'textDocument/semanticTokens/full - (7)'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received request 'workspace/semanticTokens/refresh - (5)'.
[Trace - 09:55:51] Sending response 'workspace/semanticTokens/refresh - (5)'. Processing request took 0ms
[Trace - 09:55:51] Received request 'workspace/codeLens/refresh - (6)'.
[Trace - 09:55:51] Sending response 'workspace/codeLens/refresh - (6)'. Processing request took 0ms
[Trace - 09:55:51] Received request 'window/workDoneProgress/create - (7)'.
[Trace - 09:55:51] Sending response 'window/workDoneProgress/create - (7)'. Processing request took 0ms
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received request 'workspace/semanticTokens/refresh - (8)'.
[Trace - 09:55:51] Sending response 'workspace/semanticTokens/refresh - (8)'. Processing request took 0ms
[Trace - 09:55:51] Received request 'workspace/codeLens/refresh - (9)'.
[Trace - 09:55:51] Sending response 'workspace/codeLens/refresh - (9)'. Processing request took 0ms
[Trace - 09:55:51] Received request 'window/workDoneProgress/create - (10)'.
[Trace - 09:55:51] Sending response 'window/workDoneProgress/create - (10)'. Processing request took 0ms
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received request 'window/workDoneProgress/create - (11)'.
[Trace - 09:55:51] Sending response 'window/workDoneProgress/create - (11)'. Processing request took 0ms
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received request 'client/registerCapability - (12)'.
[Trace - 09:55:51] Sending response 'client/registerCapability - (12)'. Processing request took 0ms
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received request 'window/workDoneProgress/create - (13)'.
[Trace - 09:55:51] Sending response 'window/workDoneProgress/create - (13)'. Processing request took 0ms
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received request 'window/workDoneProgress/create - (14)'.
[Trace - 09:55:51] Sending response 'window/workDoneProgress/create - (14)'. Processing request took 0ms
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received request 'workspace/semanticTokens/refresh - (15)'.
[Trace - 09:55:51] Sending response 'workspace/semanticTokens/refresh - (15)'. Processing request took 0ms
[Trace - 09:55:51] Received request 'workspace/codeLens/refresh - (16)'.
[Trace - 09:55:51] Sending response 'workspace/codeLens/refresh - (16)'. Processing request took 0ms
[Trace - 09:55:51] Received request 'workspace/inlayHint/refresh - (17)'.
[Trace - 09:55:51] Sending response 'workspace/inlayHint/refresh - (17)'. Processing request took 0ms
[Trace - 09:55:51] Received notification 'experimental/serverStatus'.
[Trace - 09:55:51] Received request 'window/workDoneProgress/create - (18)'.
[Trace - 09:55:51] Sending response 'window/workDoneProgress/create - (18)'. Processing request took 0ms
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Received notification '$/progress'.
[Trace - 09:55:51] Sending request 'textDocument/codeAction - (8)'.
[Trace - 09:55:51] Sending request 'textDocument/documentSymbol - (9)'.
[Trace - 09:55:51] Received response 'textDocument/documentSymbol - (9)' in 0ms.
[Trace - 09:55:51] Sending request 'textDocument/codeLens - (10)'.
[Trace - 09:55:53] Received notification '$/progress'.
[Trace - 09:55:53] Received notification '$/progress'.
[Trace - 09:55:53] Received notification '$/progress'.
[Trace - 09:55:53] Received response 'textDocument/codeLens - (10)' in 1474ms.
[Trace - 09:55:53] Received notification '$/progress'.
[Trace - 09:55:53] Received notification '$/progress'.
[Trace - 09:55:53] Received notification '$/progress'.
[Trace - 09:55:53] Received notification '$/progress'.
[Trace - 09:55:54] Received notification 'textDocument/publishDiagnostics'.
[Trace - 09:55:54] Received response 'textDocument/semanticTokens/full - (7)' in 3362ms.
[Trace - 09:55:54] Received response 'textDocument/codeAction - (8)' in 2643ms.
[Trace - 09:55:54] Sending request 'textDocument/semanticTokens/full/delta - (11)'.
[Trace - 09:55:54] Received response 'textDocument/semanticTokens/full/delta - (11)' in 2ms.
/////////////////////// above logs are for me opening an editor and waiting, doing nothing

[Trace - 09:56:19] Sending request 'textDocument/inlayHint - (12)'.
[Trace - 09:56:19] Received response 'textDocument/inlayHint - (12)' in 2ms.
[Trace - 09:56:19] Sending request 'textDocument/inlayHint - (13)'.
[Trace - 09:56:19] Received response 'textDocument/inlayHint - (13)' in 1ms.
/////////////////////// above logs appeared after scrolling below and back, to make the hint appear

[Trace - 09:57:09] Sending request 'inlayHint/resolve - (14)'.
[Trace - 09:57:09] Received response 'inlayHint/resolve - (14)' in 3ms.
[Trace - 09:57:09] Sending notification 'textDocument/didOpen'.
[Trace - 09:57:09] Sending request 'textDocument/hover - (15)'.
[Trace - 09:57:09] Sending notification 'textDocument/didClose'.
[Trace - 09:57:09] Received notification 'textDocument/publishDiagnostics'.
[Trace - 09:57:09] Received response 'textDocument/hover - (15)' in 20ms.
/////////////////////// above logs appeared after hovering the hint, I get the pop-up, ability to cmd+click it, but no double click to apply the edit

I would assume you can adjust VSCode to not to send textEdit inlay hint resolve capability, that would make rust-analyzer to send these edits back before resolving and double click edit will work.
Alternatively, patch rust-analyzer to forbid resolving textEdit (see patch in rust-lang/rust-analyzer#15604 (comment)) and do cargo xtask install --server on rust-analyzer + make VSCode settings.json aware of the installed server: "rust-analyzer.server.path": "rust-analyzer", (or use any other installation path if needed).
This will bring the same results: double click starts to work.

@SomeoneToIgnore
Copy link
Author

SomeoneToIgnore commented Sep 15, 2023

If it's helpful, here are the json responses rust-analyzer language server emits for the example above

  • during first response
{
    "jsonrpc": "2.0",
    "id": 13,
    "result": [
        {
            "position": {
                "line": 1,
                "character": 12
            },
            "label": [
                {
                    "value": ": "
                },
                {
                    "value": "String"
                },
                {
                    "value": ""
                }
            ],
            "kind": 1,
            "paddingLeft": false,
            "paddingRight": false,
            "data": {
                "file_id": 0
            }
        }
    ]
}
  • after resolving on hint hover
{
    "jsonrpc": "2.0",
    "id": 15,
    "result": {
        "position": {
            "line": 1,
            "character": 12
        },
        "label": [
            {
                "value": ": "
            },
            {
                "value": "String",
                "location": {
                    "uri": "file:///Users/someonetoignore/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/string.rs",
                    "range": {
                        "start": {
                            "line": 364,
                            "character": 11
                        },
                        "end": {
                            "line": 364,
                            "character": 17
                        }
                    }
                }
            },
            {
                "value": ""
            }
        ],
        "kind": 1,
        "textEdits": [
            {
                "range": {
                    "start": {
                        "line": 1,
                        "character": 12
                    },
                    "end": {
                        "line": 1,
                        "character": 12
                    }
                },
                "newText": ": String"
            }
        ],
        "paddingLeft": false,
        "paddingRight": false
    }
}

there seems nothing in label.location and textEdits that could get obsolete during inlay hint hover, so I'm not sure why the former works and the latter does not after resolving.
If textEdit contents gets moved into the initial response, double click starts to work.

SomeoneToIgnore added a commit to SomeoneToIgnore/rust-analyzer that referenced this issue Sep 19, 2023
VSCode behaves strangely, allowing to navigate into label location, but
not allowing to apply hint's text edit, after hint is resolved.
See microsoft/vscode#193124 for details.

For now, stub hint resolution for VSCode specifically.
SomeoneToIgnore added a commit to SomeoneToIgnore/rust-analyzer that referenced this issue Sep 19, 2023
VSCode behaves strangely, allowing to navigate into label location, but
not allowing to apply hint's text edit, after hint is resolved.
See microsoft/vscode#193124 for details.

For now, stub hint resolution for VSCode specifically.
bors added a commit to rust-lang/rust-analyzer that referenced this issue Sep 19, 2023
Do not resolve inlayHint.textEdit for VSCode client

Closes #15604

VSCode behaves strangely, allowing to navigate into label location, but not allowing to apply hint's text edit, after hint is resolved. See microsoft/vscode#193124 for details.

For now, stub hint resolution for VSCode specifically.
@SomeoneToIgnore
Copy link
Author

A small update: I've added a workaround into rust-analyzer, so now, in order to reproduce it with VSCode, somebody has to either change the clientInfo.name value to something different or use rust-analyzer without the patch.

@vscodenpa
Copy link

Hey @jrieken, this issue might need further attention.

@SomeoneToIgnore, you can help us out by closing this issue if the problem no longer exists, or adding more information.

@SomeoneToIgnore
Copy link
Author

Can confirm this is still a thing with the edits only.

@heejaechang
Copy link

heejaechang commented Jan 9, 2024

Hi, I am a dev from pylance and we are having the same problem.

it looks like it is a bug either in vscode or lsp depends on what is the expected behavior.

when LSP is initialized, it clearly says it supports resolving textEdits as it is shown in the source code.

https://github.com/microsoft/vscode-languageserver-node/blob/d1c83dbbb0a37621a2028ac871f6ce3409e7baba/client/src/common/inlayHint.ts#L39-L41

but vscode never actually resolve textEdits property according to the source code.

this.hint.tooltip = newHint?.tooltip ?? this.hint.tooltip;
this.hint.label = newHint?.label ?? this.hint.label;

so either LSP should not say it support textEdits or vscode should set textEdits it got from resolveInlayHint

...

but that said, for us, we want textEdits to be supported since we want to make some improvements where text edit we generate is actually valid and context aware (that also means it is too expensive to generate for all inlay hints upfront)

and also, it looks like all it needs to fix the issue is just adding one line to the vscode above.

this.hint.textEdits = newHint?.textEdits ?? this.hint.textEdits;

@heejaechang
Copy link

there are actually multiple places in vscode codebase that calls resolveInlayHint and this one seems handle textEdits

const result: extHostProtocol.IInlayHintDto = {
label: '', // fill-in below
cacheId: id,
tooltip: typeConvert.MarkdownString.fromStrict(hint.tooltip),
position: typeConvert.Position.from(hint.position),
textEdits: hint.textEdits && hint.textEdits.map(typeConvert.TextEdit.from),
kind: hint.kind && typeConvert.InlayHintKind.from(hint.kind),
paddingLeft: hint.paddingLeft,
paddingRight: hint.paddingRight,

but none of others do.

@jrieken jrieken added this to the December / January 2024 milestone Jan 9, 2024
@jrieken jrieken removed the info-needed Issue requires more information from poster label Jan 9, 2024
@jrieken
Copy link
Member

jrieken commented Jan 9, 2024

Thank you @heejaechang !

@jrieken jrieken added the bug Issue identified by VS Code Team member as probable bug label Jan 9, 2024
jrieken added a commit that referenced this issue Jan 9, 2024
@jrieken jrieken closed this as completed in 8adf59f Jan 9, 2024
@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jan 9, 2024
SomeoneToIgnore added a commit to SomeoneToIgnore/rust-analyzer that referenced this issue Jan 10, 2024
After microsoft/vscode#193124 was fixed,
this change is not needed anymore.
@aeschli aeschli added the verified Verification succeeded label Jan 25, 2024
SomeoneToIgnore added a commit to SomeoneToIgnore/rust-analyzer that referenced this issue Feb 1, 2024
After microsoft/vscode#193124 was fixed,
this change is not needed anymore.
@aiday-mar aiday-mar added this to the December / January 2024 milestone Feb 6, 2024
Veykril pushed a commit to SomeoneToIgnore/rust-analyzer that referenced this issue Mar 11, 2024
After microsoft/vscode#193124 was fixed,
this change is not needed anymore.
bors added a commit to rust-lang/rust-analyzer that referenced this issue Mar 11, 2024
Stop eagerly resolving inlay hint text edits for VSCode

Send less json over the wire.
After microsoft/vscode#193124 was fixed, this change is not needed anymore.

VSCode 1.86.0 now supports double click for unresolved hint data too.
@microsoft microsoft locked and limited conversation to collaborators Jun 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug inlay-hints insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants