-
Notifications
You must be signed in to change notification settings - Fork 150
Description
When a JSON-RPC request without params
(i.e. them being null
in Java) is emitted, that gets written as "params": null
instead of just omitting the params
field from the output. The latter should be done instead.
Specifications
JSON-RPC specification says:
params
A Structured value that holds the parameter values to be used during the invocation of the method. This member MAY be omitted.
where "two structured types (Objects and Arrays)".
LSP specification says (in TypeScript syntax):
/** * The method's params. */ params?: array | object;
which exactly matches the JSON-RPC specification with the ?
meaning omission, but not null
(which would have to be explicitly given as a | null
variant).
Implementation
The writing of params
here:
https://github.com/eclipse/lsp4j/blob/22f8edce55379bd9d4ae1b96bc614e74549bc0f9/org.eclipse.lsp4j.jsonrpc/src/main/java/org/eclipse/lsp4j/jsonrpc/json/adapters/MessageTypeAdapter.java#L404-L409
uses the writeNullValue
function here:
https://github.com/eclipse/lsp4j/blob/22f8edce55379bd9d4ae1b96bc614e74549bc0f9/org.eclipse.lsp4j.jsonrpc/src/main/java/org/eclipse/lsp4j/jsonrpc/json/adapters/MessageTypeAdapter.java#L449-L457
which goes out of its way to output a null
instead of omitting missing params
.
This clearly violates the specifications and trips up other JSON-RPC/LSP implementations that require incoming requests to follow the specification to the letter.
Background
According to git blame, this behavior was introduced in #137, the goal of which was to fix the behavior for response result
that indeed may not be omitted but may be null
. Looks like the same logic was erroneously applied to request/notification params
as well.