vtsls icon indicating copy to clipboard operation
vtsls copied to clipboard

LSP violation: client's `CompletionItemCapabilityResolveSupport` are not considered

Open SomeoneToIgnore opened this issue 1 year ago • 2 comments

Server version: "@vtsls/language-server": "0.2.6",

Zed editor client specifies the following resolve capabilities: https://github.com/zed-industries/zed/blob/843b6611d3c0ffc6b36a721fd5c7c0d19f8ccd87/crates/lsp/src/lsp.rs#L674-L682

resolve_support: Some(CompletionItemCapabilityResolveSupport {
    properties: vec![
        "additionalTextEdits".to_string(),
        "command".to_string(),
        "documentation".to_string(),
        // NB: Do not have this resolved, otherwise Zed becomes slow to complete things
        // "textEdit".to_string(),
    ],
}),

Notice that there's no detail field in here.

The spec says: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_completion

By default the request can only delay the computation of the detail and documentation properties. Since 3.16.0 the client can signal that it can resolve more properties lazily. This is done using the completionItem#resolveSupport client capability which lists all properties that can be filled in during a ‘completionItem/resolve’ request. All other properties (usually sortText, filterText, insertText and textEdit) must be provided in the textDocument/completion response and must not be changed during resolve.

According to this excerpt, the absence of "detail" field in the resolve capabilities means that the server must provide it immediately.

Yet, for the snippet

const result = await fetch(someUrl);
result.ok

vtsls returns

{"jsonrpc":"2.0","id":28,"result":{"items":[{"label":"arrayBuffer","labelDetails":{},"kind":2,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"arrayBuffer"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":0,"cacheId":3}]},"data":{"providerId":2,"index":0,"cacheId":3}},{"label":"blob","labelDetails":{},"kind":2,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"blob"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":1,"cacheId":3}]},"data":{"providerId":2,"index":1,"cacheId":3}},{"label":"body","labelDetails":{},"kind":5,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"body"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":2,"cacheId":3}]},"data":{"providerId":2,"index":2,"cacheId":3}},{"label":"bodyUsed","labelDetails":{},"kind":5,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"bodyUsed"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":3,"cacheId":3}]},"data":{"providerId":2,"index":3,"cacheId":3}},{"label":"clone","labelDetails":{},"kind":2,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"clone"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":4,"cacheId":3}]},"data":{"providerId":2,"index":4,"cacheId":3}},{"label":"formData","labelDetails":{},"kind":2,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"formData"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":5,"cacheId":3}]},"data":{"providerId":2,"index":5,"cacheId":3}},{"label":"headers","labelDetails":{},"kind":5,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"headers"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":6,"cacheId":3}]},"data":{"providerId":2,"index":6,"cacheId":3}},{"label":"json","labelDetails":{},"kind":2,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"json"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":7,"cacheId":3}]},"data":{"providerId":2,"index":7,"cacheId":3}},{"label":"ok","labelDetails":{},"kind":5,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"ok"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":8,"cacheId":3}]},"data":{"providerId":2,"index":8,"cacheId":3,"match":[2,0,0]}},{"label":"redirected","labelDetails":{},"kind":5,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"redirected"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":9,"cacheId":3}]},"data":{"providerId":2,"index":9,"cacheId":3}},{"label":"status","labelDetails":{},"kind":5,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"status"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":10,"cacheId":3}]},"data":{"providerId":2,"index":10,"cacheId":3}},{"label":"statusText","labelDetails":{},"kind":5,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"statusText"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":11,"cacheId":3}]},"data":{"providerId":2,"index":11,"cacheId":3}},{"label":"text","labelDetails":{},"kind":2,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"text"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":12,"cacheId":3}]},"data":{"providerId":2,"index":12,"cacheId":3}},{"label":"type","labelDetails":{},"kind":5,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"type"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":13,"cacheId":3}]},"data":{"providerId":2,"index":13,"cacheId":3}},{"label":"url","labelDetails":{},"kind":5,"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"url"},"commitCharacters":[".",",",";"],"command":{"command":"_vtsls.completionCacheCommand","title":"","arguments":[{"providerId":2,"index":14,"cacheId":3}]},"data":{"providerId":2,"index":14,"cacheId":3}}],"isIncomplete":false}}

notice total absent of detail field in this response.

After resolving the same completion item, detail field is sent back:

{"jsonrpc":"2.0","id":29,"result":{"label":"ok","labelDetails":{},"kind":5,"detail":"(property) Response.ok: boolean","documentation":{"kind":"markdown","value":"[MDN Reference](https://developer.mozilla.org/docs/Web/API/Response/ok)"},"sortText":"11","insertTextFormat":1,"textEdit":{"insert":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"replace":{"start":{"line":23,"character":10},"end":{"line":23,"character":11}},"newText":"ok"},"commitCharacters":[".",",",";"],"command":{"title":"","command":"_vtsls.completionCacheCommand","arguments":[{"providerId":2,"index":8,"cacheId":3}]},"data":{"providerId":2,"index":8,"cacheId":3,"match":[2,0,0]}}}

SomeoneToIgnore avatar Dec 04 '24 12:12 SomeoneToIgnore

Thanks for your finding. This is fixable here but it could potentially incur performance degradation as we need to resolve every completion item before sending it to the client, due to that the underlying tsserver does not support the one-pass completion communication for completion, and the client communicating to tsserver (this lsp server) has to use a second CompletionEntryDetails request for the additional unresolved properties and it can only resolve one item per request.

On my local testing, one such request to tsserver takes 2-10ms. Suppose you have a list of 1000 completion items (common for medium size of typescript project) sent to the client, resolving all of them in advance could take several seconds, resulting in an unacceptably long delay.

However, for this specific case, there might be another solution that we could prefill the detail field with some existing information without asking tsserver to resolve additional details, but the tricky part here is that the content of detail field would change if the client do the actual resolve afterwards.

Anyway, I think this behavior is indeed a violation of the LSP but I deliberately chose that because of the performance consideration. Another thing interests me is the reason why zed would want the detail field accessible early as I think kind, label and labelDetails fields are sufficient for the initial UI rendering.

yioneko avatar Dec 04 '24 14:12 yioneko

Oof, does not sound very good indeed, thank you for the explanation.

Zed renders rich completion labels "inline" (which is not necessarily the final and "right" design but it used to work so far with other language servers). This way, we can show something more elaborate than just the label in the completion items, and if we start to resolve it, the UI becomes somewhat jumpy:

https://github.com/user-attachments/assets/ca3c8c60-780e-4626-b6ad-0314aa61c035

SomeoneToIgnore avatar Dec 04 '24 15:12 SomeoneToIgnore