lsp icon indicating copy to clipboard operation
lsp copied to clipboard

Spec compliance: null vs missing values

Open thomasjm opened this issue 5 years ago • 3 comments

I just noticed that some types don't perfectly obey the LSP spec regarding when a value is allowed to be missing vs. when it must be present and null.

For example, if you look at InitializeParams, the processId field has type processId: integer | null, while the locale field is written locale?: string. Thus the latter is allowed to be missing but the former is not.

All optional fields in this package seem to be represented as Maybe, and encoded to JSON with omitNothingFields = True:

https://github.com/alanz/lsp/blob/36b655d9cc538f79b14c847ca383d97ffcc18162/lsp-types/src/Language/LSP/Types/Utils.hs#L109 https://github.com/alanz/lsp/blob/36b655d9cc538f79b14c847ca383d97ffcc18162/lsp-types/src/Language/LSP/Types/Initialize.hs#L44

This caused a problem for me when trying to write a Haskell client for a Python language server powered by pygls. Upon sending an initialize message with an empty processId, I get an NPE because it assumes the value will be present:

    self._server.process_id = params.processId
AttributeError: 'Object' object has no attribute 'processId'

To be compliant with the spec, perhaps we need to replace these "required optional values" with a different Maybe-like wrapper that always encodes to null when empty?

thomasjm avatar Dec 17 '20 16:12 thomasjm

Note: wow, I am slightly sleep deprived and didn't notice that I already filed an issue for this back in May (#246). I think this one is the better write-up so I'll close the other one :)

thomasjm avatar Dec 17 '20 16:12 thomasjm

I wonder if there's any existing package that offers "another Maybe". But I figure it's not too much trouble to define one anyway, like how we treat union types.

banacorn avatar Dec 23 '20 05:12 banacorn

We already do something similar for the List type. See https://github.com/alanz/lsp/blob/793810cb1681e293f7b5fbdac6821e51e87e3c17/lsp-types/src/Language/LSP/Types/Common.hs#L37-L50

alanz avatar Jan 07 '21 22:01 alanz

Closed by lsp-types-2.x

thomasjm avatar Jul 05 '23 07:07 thomasjm