Cannot read property 'range' of null: CompletionItem.textEdit cannot be null

[felixfbecker]

The protocol defines that all properties on CompletionItem except label are optional. However, sending a completion item with only a label results in this exception:

Cannot read property 'range' of null: TypeError: Cannot read property 'range' of null
    at asTextEdit (C:\Users\felix\git\OpenSource\vscode-php-intellisense\node_modules\vscode-languageclient\lib\protocolConverter.js:108:46)
    at C:\Users\felix\git\OpenSource\vscode-php-intellisense\node_modules\vscode-languageclient\lib\protocolConverter.js:101:67
    at set (C:\Users\felix\git\OpenSource\vscode-php-intellisense\node_modules\vscode-languageclient\lib\protocolConverter.js:89:13)
    at asCompletionItem (C:\Users\felix\git\OpenSource\vscode-php-intellisense\node_modules\vscode-languageclient\lib\protocolConverter.js:101:9)
    at Array.map (native)
    at asCompletionResult (C:\Users\felix\git\OpenSource\vscode-php-intellisense\node_modules\vscode-languageclient\lib\protocolConverter.js:82:26)
    at process._tickCallback (internal/process/next_tick.js:103:7)

That is because here textEdit is passed to asTextEdit, which passes textEdit.range to asRange. textEdit can be null though, which leads to the exception.

The same happens with asCommand.

I highly recommend you to take a look at strictNullChecks - it prevents these bugs at compile-time!

[felixfbecker]

Looking more at the code, it seems like the set() helper only checks if the value is undefined, but allows null just fine. Many static languages unfortunately only know null and no undefined.
(Btw all these helper functions are really confusing - why a function that does nothing but return value === null, or call a function if a condition is true (like an if)?)

[felixfbecker]

@dbaeumer I'm not insisting that #128 gets accepted, if you would like to see a different solution I would happily provide a different PR, but can we please resolve this? It blocks completion support for PHP

[dbaeumer]

Although all these properties are defined as optional they aren't really all optional :-). I (and Joh) were too lazy to define different type defining the legal combinations. What I will do is to change the code that if all properties are missing I will create a completion item which does nothing.

[felixfbecker]

Thanks for the explanation! Is there any reference which combination of properties are actually legal?

[dbaeumer]

Actually no. Will add to the list of things to do. What happens as a fallback is that the item does nothing when selected but such an item is not very useful :-)

[felixfbecker]

Could you give a rough description for the meantime?

[dbaeumer]

It should either have a { insertString } a { insertString, range } (new in 3.0) or a { textEdit }

[dbaeumer]

I fixed the code to create a noop completion item in case all properties except label are missing.

[felixfbecker]

Isn't insertString + range exactly the same as textEdit?
Also, the LSP docs state that if insertText is not set, label is used in its place

[dbaeumer]

@felixfbecker you are correct that the label is used. My code fix does result in this. Sorry for the incorrect comment. My change creates an empty CompletionItem.