CompletionItemKind of 0 (None) causes language server clients to fail

[gotgenes]

Environment data

• Language Server version: master branch (last compiled on 51b2532ddc0161aecadda182b33f1b15ed6093b0
• OS and version: MacOS 10.14.6
• Python version (& distribution if applicable, e.g. Anaconda): 3.7.4

Expected behaviour

Python Language Server emits valid completion items.

Actual behaviour

Python Language Server emits completion items with kind value of 0 as described in autozimu/LanguageClient-neovim#633 (comment)

Zero is not a valid CompletionItemKind according to the Language Server specification
https://microsoft.github.io/language-server-protocol/specification

/**
* The kind of a completion entry.
*/
namespace CompletionItemKind {
  export const Text = 1;
  export const Method = 2;
  export const Function = 3;
  export const Constructor = 4;
  export const Field = 5;
  export const Variable = 6;
  export const Class = 7;
  export const Interface = 8;
  export const Module = 9;
  export const Property = 10;
  export const Unit = 11;
  export const Value = 12;
  export const Enum = 13;
  export const Keyword = 14;
  export const Snippet = 15;
  export const Color = 16;
  export const File = 17;
  export const Reference = 18;
  export const Folder = 19;
  export const EnumMember = 20;
  export const Constant = 21;
  export const Struct = 22;
  export const Event = 23;
  export const Operator = 24;
  export const TypeParameter = 25;
}

The result of Python Language Server emitting completion items with "kind":0 breaks completion for language clients, like LanguageClient-neovim, that are set up to handle the Language Client spec.

Logs

N/A

Code Snippet / Additional lnformation

The example I gave in autozimu/LanguageClient-neovim#633 (comment) was

import logging

logging.getLogge

[MikhailArkhipov]

Unfortunately, there is no enum item for "I don't know what it is" i.e. Unknown in Python. In VS Code 0 yields ABC icon (i.e. default completion). We are open to suggestions what to return on Unknown type.

[jakebailey]

CompletionItem allows kind to be null, so I think the best thing would be to make it nullable in our message and ensure that it never goes to zero.

[jakebailey]

That, or do what the VS Code client does and convert any invalid choices to Text:

https://github.com/microsoft/vscode-languageserver-node/blob/5532367a530c4e8bea957fb10a09493468571ac4/client/src/protocolConverter.ts#L358-L364

[MikhailArkhipov]

Text is probably safer

[gotgenes]

Thank you for looking into this!

I would say either

1. omit the entries with kind 0 from the completion items, or
2. convert entries with kind 0 to CompletionItemKind.Text if it really is worth keeping those cases. (I'm not sure under what conditions PythonMemberType.Unknown gets encountered.)

[jakebailey]

#1482 includes that second item.

Note that the protocol does say that it can be null, so hopefully the vim language server client could accept that.

[MikhailArkhipov]

Unknown is something that analysis did not manage to figure type for. Example

x = a

a is undefined so variable x will have value of Unknown. It rarely leaks to completions but it might.

[jakebailey]

Unknown happens all the time when we can't infer something, so it's going to show up as members of classes, in the global scope, etc; making sure we include those items is pretty critical.

Using Text helps, though from my testing, VSC has a special icon it uses when the kind in CompletionItem was null.

[gotgenes]

Looking through LanguageClient-neovim, I found that it uses the lsp-types crate for parsing the messages sent back from a Language Server. The version it currently uses, 0.60.0, has Option<CompletionItemKind> for the kind property. If I understand this correctly, it should properly handle cases where kind is null. So mapping PythonMemberType.Unknown to null should allow this and other language clients to handle these completion items.

[jakebailey]

That's reasonable, we just can't change it without some extra work for users of the LS's public API (which includes completion items). Changing it to Text for now stops the failures until we can coordinate a change.

[gotgenes]

Thanks!

Here's the relevant commit for anyone arriving at this issue later: 8e3289c#diff-d1348423ac842b3fb8e33aaafe1b10de

Is there an issue tracking changing this to null in a future release?

[jakebailey]

Not at the moment, no. Feel free to make one.