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

LSP: completion returns snippets when client does not support them #4139

Closed
matthijskooijman opened this issue Sep 6, 2020 · 6 comments · Fixed by #8960
Closed

LSP: completion returns snippets when client does not support them #4139

matthijskooijman opened this issue Sep 6, 2020 · 6 comments · Fixed by #8960

Comments

@matthijskooijman
Copy link
Contributor

I'm using YouCompleteMe as an LSP client, and noticed an assertion error in that client. I asked they YCM developers, and they said:

That assertion means that the server is sending snippets instead of plaintext completion, despite what YCM declares as its capabilities. In other words the server violates the protocol.

I'm not familiar with the protocol, so I'm not sure what this means exactly, but I hope it is clear to you and helpful.

To reproduce it, I've used this code snippet:

<?php

class bar {
        /** @var int */
        var $fooz = 1;

        function foo() {
                $var = new bar();
                $var->;
        }
}

Triggering a completion after the unfinished $var->.

The YCM log, which includes all JSONRPC messages sent, is shown below. Psalm itself does not produce any output.

2020-09-06 12:17:58,990 - DEBUG - No global extra conf, not calling method YcmCorePreload
2020-09-06 12:17:59,059 - INFO - Received ready request
2020-09-06 12:17:59,067 - INFO - Received signature help available request
2020-09-06 12:17:59,149 - INFO - Received event notification
2020-09-06 12:17:59,149 - DEBUG - Event name: BufferVisit
2020-09-06 12:17:59,154 - INFO - Received event notification
2020-09-06 12:17:59,154 - DEBUG - Event name: FileReadyToParse
2020-09-06 12:17:59,154 - INFO - Adding buffer identifiers for file: /var/www/html/hypha/system/datatypes/festival.php
2020-09-06 12:17:59,155 - INFO - Starting phpCompleter: ['/home/matthijs/bin/psalm', '--language-server']
2020-09-06 12:17:59,163 - INFO - phpCompleter started with PID 113403
2020-09-06 12:17:59,164 - DEBUG - TX: Sending message: b'Content-Length: 1026\r\n\r\n{"id":1,"jsonrpc":"2.0","method":"initialize","params":{"capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"completion":{"completionItem":{"documentationFormat":["plaintext","markdown"]},"completionItemKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25]}},"hover":{"contentFormat":["plaintext","markdown"]},"signatureHelp":{"signatureInformation":{"documentationFormat":["plaintext","markdown"],"parameterInformation":{"labelOffsetSupport":false}}},"synchronization":{"didSave":true}},"workspace":{"applyEdit":true,"didChangeWatchedFiles":{"dynamicRegistration":true},"documentChanges":true,"symbol":{"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}}}},"initializationOptions":{},"processId":113362,"rootPath":"/var/www/html/hypha","rootUri":"file:///var/www/html/hypha"}}'
2020-09-06 12:17:59,254 - INFO - Received filetype completion available request
2020-09-06 12:18:01,126 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"initializing"},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,126 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"initializing: getting code base"},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,127 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"initializing: scanning files"},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,532 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"initializing: registering stub files"},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,656 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"initialized"},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,656 - DEBUG - RX: Received message: b'{"result":{"capabilities":{"textDocumentSync":{"openClose":null,"change":1,"willSave":null,"willSaveWaitUntil":null,"save":null},"hoverProvider":true,"completionProvider":{"resolveProvider":false,"triggerCharacters":["$",">",":"]},"signatureHelpProvider":{"triggerCharacters":["(",","]},"definitionProvider":true,"referencesProvider":false,"documentHighlightProvider":null,"documentSymbolProvider":false,"workspaceSymbolProvider":false,"codeActionProvider":null,"codeLensProvider":null,"documentFormattingProvider":null,"documentRangeFormattingProvider":null,"documentOnTypeFormattingProvider":null,"renameProvider":null,"xworkspaceReferencesProvider":false,"xdefinitionProvider":false,"dependenciesProvider":false}},"id":1,"jsonrpc":"2.0"}'
2020-09-06 12:18:01,656 - INFO - Language server requires sync type of Full
2020-09-06 12:18:01,657 - DEBUG - php: Server declares trigger characters: ['$', '>', ':']
2020-09-06 12:18:01,657 - INFO - php: Using trigger characters for semantic triggers: $,>,:
2020-09-06 12:18:01,658 - DEBUG - php: Server declares signature trigger characters: ['(', ',']
2020-09-06 12:18:01,658 - INFO - php: Using characters for signature triggers: (,,
2020-09-06 12:18:01,658 - DEBUG - TX: Sending notification: b'Content-Length: 52\r\n\r\n{"jsonrpc":"2.0","method":"initialized","params":{}}'
2020-09-06 12:18:01,659 - DEBUG - TX: Sending notification: b'Content-Length: 86\r\n\r\n{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{}}}'
2020-09-06 12:18:01,659 - DEBUG - Refreshing file /var/www/html/hypha/system/datatypes/festival.php: State is Open/action Open
2020-09-06 12:18:01,659 - DEBUG - TX: Sending notification: b'Content-Length: 308\r\n\r\n{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"php","text":"<?php\\n\\nclass bar {\\n\\t/** @var int */\\n\\tvar $fooz = 1;\\n\\n\\tfunction foo() {\\n\\t\\t$var = new bar();\\n\\t\\t$var->;\\n\\t}\\n}\\n","uri":"file:///var/www/html/hypha/system/datatypes/festival.php","version":1}}}'
2020-09-06 12:18:01,660 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"analyzing"},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,660 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"running"},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,662 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"running"},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,664 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"analyzing"},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,664 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"running"},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,666 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"analyzing"},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,719 - DEBUG - RX: Received message: b'{"method":"textDocument\\/publishDiagnostics","params":{"uri":"file:\\/\\/\\/var\\/www\\/html\\/hypha\\/system\\/datatypes\\/festival.php","diagnostics":[{"range":{"start":{"line":8,"character":8},"end":{"line":8,"character":9}},"severity":1,"code":"https:\\/\\/psalm.dev\\/173","source":"Psalm","message":"Syntax error, unexpected \';\', expecting T_STRING or T_VARIABLE or \'{\' or \'$\' on line 9"},{"range":{"start":{"line":6,"character":10},"end":{"line":6,"character":13}},"severity":2,"code":"https:\\/\\/psalm.dev\\/050","source":"Psalm","message":"Method bar::foo does not have a return type, expecting void"}]},"jsonrpc":"2.0"}'
2020-09-06 12:18:01,720 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"running"},"jsonrpc":"2.0"}'
2020-09-06 12:18:05,144 - INFO - Received completion request
2020-09-06 12:18:05,144 - DEBUG - Using filetype completion: True
2020-09-06 12:18:05,145 - DEBUG - Refreshing file /var/www/html/hypha/system/datatypes/festival.php: State is Open/action None
2020-09-06 12:18:05,145 - DEBUG - TX: Sending message: b'Content-Length: 188\r\n\r\n{"id":2,"jsonrpc":"2.0","method":"textDocument/completion","params":{"position":{"character":8,"line":8},"textDocument":{"uri":"file:///var/www/html/hypha/system/datatypes/festival.php"}}}'
2020-09-06 12:18:05,150 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"analyzing"},"jsonrpc":"2.0"}'
2020-09-06 12:18:05,150 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"running"},"jsonrpc":"2.0"}'
2020-09-06 12:18:05,152 - DEBUG - RX: Received message: b'{"result":{"isIncomplete":false,"items":[{"label":"foo","kind":2,"detail":"public function foo() : mixed","documentation":null,"sortText":"1","filterText":"foo","insertText":"foo()","textEdit":null,"additionalTextEdits":null,"command":{"title":"Trigger parameter hints","command":"editor.action.triggerParameterHints","arguments":null},"data":null,"insertTextFormat":2},{"label":"$fooz","kind":10,"detail":"public int","documentation":null,"sortText":"1","filterText":"fooz","insertText":"fooz","textEdit":null,"additionalTextEdits":null,"command":null,"data":null,"insertTextFormat":null}]},"id":2,"jsonrpc":"2.0"}'
2020-09-06 12:18:05,152 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"analyzing"},"jsonrpc":"2.0"}'
2020-09-06 12:18:05,153 - DEBUG - RX: Received message: b'{"method":"telemetry\\/event","params":{"type":3,"message":"running"},"jsonrpc":"2.0"}'
Traceback (most recent call last):
  File "/home/matthijs/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/third_party/bottle/bottle.py", line 868, in _handle
    return route.call(**args)
  File "/home/matthijs/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/third_party/bottle/bottle.py", line 1748, in wrapper
    rv = callback(*a, **ka)
  File "/home/matthijs/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/watchdog_plugin.py", line 97, in wrapper
    return callback( *args, **kwargs )
  File "/home/matthijs/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/hmac_plugin.py", line 62, in wrapper
    body = callback( *args, **kwargs )
  File "/home/matthijs/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/handlers.py", line 122, in GetCompletions
    completions = filetype_completer.ComputeCandidates( request_data )
  File "/home/matthijs/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/completers/completer.py", line 286, in ComputeCandidates
    candidates = self._GetCandidatesFromSubclass( request_data )
  File "/home/matthijs/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/completers/language_server/language_server_completer.py", line 1126, in _GetCandidatesFromSubclass
    raw_completions, is_incomplete = self.ComputeCandidatesInner( request_data,
  File "/home/matthijs/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/completers/language_server/language_server_completer.py", line 1112, in ComputeCandidatesInner
    return ( self._CandidatesFromCompletionItems( items,
  File "/home/matthijs/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/completers/language_server/language_server_completer.py", line 1227, in _CandidatesFromCompletionItems
    _InsertionTextForItem( request_data, item ) )
  File "/home/matthijs/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/completers/language_server/language_server_completer.py", line 2560, in _InsertionTextForItem
    assert lsp.INSERT_TEXT_FORMAT[
AssertionError
@matthijskooijman
Copy link
Contributor Author

Digging a little deeper, it seems the client does not advertise for snippets, by omitting the snippetSupport entry from its completion capabilities. And Psalm unconditionally generates snippets for method calls here:

psalm/src/Psalm/Codebase.php

Lines 1418 to 1430 in 422271b

$completion_item = new \LanguageServerProtocol\CompletionItem(
$method_storage->cased_name,
\LanguageServerProtocol\CompletionItemKind::METHOD,
(string)$method_storage,
null,
(string)$method_storage->visibility,
$method_storage->cased_name,
$method_storage->cased_name . (count($method_storage->params) !== 0 ? '($0)' : '()'),
null,
null,
new Command('Trigger parameter hints', 'editor.action.triggerParameterHints')
);
$completion_item->insertTextFormat = \LanguageServerProtocol\InsertTextFormat::SNIPPET;

@matthijskooijman matthijskooijman changed the title LSP completion returns snippets when client does not support them LSP: completion returns snippets when client does not support them Sep 6, 2020
@bstaletic
Copy link

Just calling attention to the relevant LSP messages:

2020-09-06 12:17:59,164 - DEBUG - TX: Sending message: b'Content-Length: 1026\r\n\r\n{"id":1,"jsonrpc":"2.0","method":"initialize","params":{"capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"completion":{"completionItem":{"documentationFormat":["plaintext","markdown"]},"completionItemKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25]}},"hover":{"contentFormat":["plaintext","markdown"]},"signatureHelp":{"signatureInformation":{"documentationFormat":["plaintext","markdown"],"parameterInformation":{"labelOffsetSupport":false}}},"synchronization":{"didSave":true}},"workspace":{"applyEdit":true,"didChangeWatchedFiles":{"dynamicRegistration":true},"documentChanges":true,"symbol":{"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}}}},"initializationOptions":{},"processId":113362,"rootPath":"/var/www/html/hypha","rootUri":"file:///var/www/html/hypha"}}'

The client capabilities do not mention textDocument.completion.completionItem.snippetSupport. According to the protocol, this should be interpreted as "client does not support snippet completion".

2020-09-06 12:18:05,145 - DEBUG - TX: Sending message: b'Content-Length: 188\r\n\r\n{"id":2,"jsonrpc":"2.0","method":"textDocument/completion","params":{"position":{"character":8,"line":8},"textDocument":{"uri":"file:///var/www/html/hypha/system/datatypes/festival.php"}}}'
2020-09-06 12:18:05,152 - DEBUG - RX: Received message: b'{"result":{"isIncomplete":false,"items":[{"label":"foo","kind":2,"detail":"public function foo() : mixed","documentation":null,"sortText":"1","filterText":"foo","insertText":"foo()","textEdit":null,"additionalTextEdits":null,"command":{"title":"Trigger parameter hints","command":"editor.action.triggerParameterHints","arguments":null},"data":null,"insertTextFormat":2},{"label":"$fooz","kind":10,"detail":"public int","documentation":null,"sortText":"1","filterText":"fooz","insertText":"fooz","textEdit":null,"additionalTextEdits":null,"command":null,"data":null,"insertTextFormat":null}]},"id":2,"jsonrpc":"2.0"}'

The above is the textDocument/completion request and response. Out of two completion items, only the first is problematic.

{"label":"foo","kind":2,"detail":"public function foo() : mixed","documentation":null,"sortText":"1","filterText":"foo","insertText":"foo()","textEdit":null,"additionalTextEdits":null,"command":{"title":"Trigger parameter hints","command":"editor.action.triggerParameterHints","arguments":null},"data":null,"insertTextFormat":2}

It contains "insertTextFormat":2, which corresponds to snippets.

Ideally, for a "snippetless" client, like YCM (actually ycmd), anything from ( to the end should not be in insertText.

Basically, just don't do this if client doesn't support snippets.

@weirdan weirdan added the bug label Sep 6, 2020
@muglug
Copy link
Collaborator

muglug commented Sep 6, 2020

Mind creating a PR? You are very well-positioned to verify whether any fix is actually good

matthijskooijman added a commit to matthijskooijman/psalm that referenced this issue Sep 6, 2020
Returning completion of kind SNIPPET is only allowed when the client has
advertised support. This changes the method completion code to only
return a snippet when the client supports them, and when the method has
arguments. Otherwise, a plaintext completion is returned.

TODO: This does not actually check the client capabilities yet, but only
implements the output handling.

This fixes vimeo#4139.
matthijskooijman added a commit to matthijskooijman/psalm that referenced this issue Sep 6, 2020
Returning completion of kind SNIPPET is only allowed when the client has
advertised support. This changes the method completion code to only
return a snippet when the client supports them, and when the method has
arguments. Otherwise, a plaintext completion is returned.

TODO: This does not actually check the client capabilities yet, but only
implements the output handling.

This fixes vimeo#4139.
@matthijskooijman
Copy link
Contributor Author

Mind creating a PR? You are very well-positioned to verify whether any fix is actually good

I wasn't going to, but you make an excellent point, so I did anyway. I created #4144 which implements the "output side" of things and works well for my client now. It does not actually check the client capabilities yet, since it seems that the upstream protocol project does not implement this part of the client capabilities yet (I think or at least I'm not sure where to load the value from). Maybe someone else knows and can add that part?

I also found some other small problems (fixed in #4143), that suddenly make Psalm LSP suddenly (seemingly) reliable in my setup, while it was rather erratic before :-)

matthijskooijman added a commit to matthijskooijman/psalm that referenced this issue Sep 8, 2020
Returning completion of kind SNIPPET is only allowed when the client has
advertised support. This changes the method completion code to only
return a snippet when the client supports them, and when the method has
arguments. Otherwise, a plaintext completion is returned.

TODO: This does not actually check the client capabilities yet, but only
implements the output handling.

This fixes vimeo#4139.
matthijskooijman added a commit to matthijskooijman/psalm that referenced this issue Oct 20, 2020
Returning completion of kind SNIPPET is only allowed when the client has
advertised support. This changes the method completion code to only
return a snippet when the client supports them, and when the method has
arguments. Otherwise, a plaintext completion is returned.

When the function has no arguments, this now also returns a completion
without parenthesis, for consistency with other language server
implementation.

TODO: This does not actually check the client capabilities yet, but only
implements the output handling.

This fixes vimeo#4139.
@tm1000
Copy link
Contributor

tm1000 commented Mar 31, 2022

@orklah can you assign this to me. I will work on it in my work on Language Server (I've already fixed it actually)

@orklah
Copy link
Collaborator

orklah commented Mar 31, 2022

Done! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment