Compatibility with statically typed languages

[spoenemann]

Some parts of the protocol cannot be implemented properly in statically typed languages such as Java. Examples:

• Completion Request response result: CompletionItem[] | CompletionList
• Hover: type MarkedString = string | { language: string; value: string };

Of course this can be handled during the conversion between JSON and the specific Java data structures. But wouldn't it be better to drop some flexibility in favor of more explicit typing, so this kind of conversion would not be necessary? For example, with a JSON library such as Gson most of the protocol parsing and serialization can be done with the generic capabilities of the library, but ambiguities such as those mentioned above require to implement special cases for the respective types.

[david-driscoll]

I would support this breaking change, to make things more consistent.

[dbaeumer]

Point well taken. Would a return type of the form

{
    items: CompletionItem[];
    list: CompletionList
}

Work for you ?

[Seairth]

In this case, just return a CompletionList. It already contains CompletionItem[]. You could also make CompletionList.isIncomplete optional to ease migration.

[akosyakov]

@dbaeumer What about MarkedString, how will it be modeled?

[spoenemann]

I would vote for dropping the simple string option:

type MarkedString = { language: string; value: string };

[akosyakov]

@spoenemann string and { language, value } were handled differently, i.e. string is rendered as markdown and a pair as source with a syntax highlighting. if it is still true just dropping string won't work, whether MarkedString is markdown or source should be explicitly communicated.

[akosyakov]

Actually { language: string; value: string }; can be replaced by string: https://github.com/Microsoft/vscode/blob/c67ef57cda90b5f28499646f7cc94e8dcc5b0586/src/vs/base/browser/htmlContentRenderer.ts#L83, so we can use plain strings in the general case.

[mickaelistria]

I support @spoenemann here on the idea of dropping the direct string in a newer version of the protocol. It makes parsers more complex without adding much value.
Instead the current behaviour of an untyped MarkedString could happen if language is not set, in which case it's up to the client to decide how to handle it. Ideally, the absence of language on a MarkedString should resolve as plain text, and markdown or html should always be added as a language. Or, if it's worth saving a few bytes, the language server ServerCapabilities element could define a default language once, that will be used by clients whenever the language of MarkedString is not set.

[felixfbecker]

@mickaelistria You can't use a MarkedString with language set to markdown to get the rendered markdown, that would result in actual syntax highlighting for the markdown code. The cleanest would be to just allow a markdown string, but that would require every client to have a markdown parser. Or he could just display the plain markdown code of course, since MD is designed to be readable even in text form. It can also contain HTML.

[spoenemann]

In LSP4J we have solved this issue with an Either type, so from the Java perspective we can close this.

[dbaeumer]

@spoenemann noticed that my Java skills got a little rusty. Would this be a solution for C# as well ?

[spoenemann]

Probably yes, provided you can customize the JSON input / output. For Java we use the Gson library and we implemented a type adapter for Either.

[dbaeumer]

Given were we are with the protocol I think we can't break this right now. I try to make things not worse for statically types languages, but the priority right now is to now break existing clients.

I will close this issue since I don't see us acting (by breaking) on this in the next 6 - 12 months. Please ping if you think otherwise.