Generate missing custom unmarshalers

[radeksimko]

The LSP specification is evolving and until there is a machine-readable spec from which we could generate Go structs periodically I think it's safe to say the structs will become outdated sooner or later.

As changes to the spec are additive, this is generally fine, until a strict JSON decoder is used, e.g.

buf := bytes.NewBufferString(str)
dec := json.NewDecoder(buf)
dec.DisallowUnknownFields()
var v lsp.InitializeParams
if err := dec.Decode(&v); err != nil {
	log.Fatal(err)
}

This can fail to decode, as shown here with workspaceFolders being an extra field which InitializeParams doesn't support yet.

We could add workspaceFolders to the struct, but as outlined above, this is not really a sustainable solution.

---

Adding unmarshaler for each struct by hand seems like a lot of (unnecessary) work, so I'm proposing addition of a generator which can generate any missing unmarshaler while retaining the ability to implement custom ones.

I'm willing to add generator for some tests for these unmarshalers if there is any interest.

---

I'm open to alternative solutions and any feedback you may have to this PR.

Related: creachadair/jrpc2#5

[keegancsmith]

Given it is additive, why don't we just update the structs to support the latest version of the spec? Wouldn't that satisfy your need? The spec does change, but not that often.

The rpc layer being strict is (kinda) useful when you control both the client and the server, but seems not great for other environments. Even then, this makes upgrade paths far more difficult. I think in particular for LSP this is not a great idea. Do you have a link to how you are using the rpc framework? That may help me understand why it is a good idea :)

Generating the custom unmarshallers does seem like an ugly hack to be honest, so I need a bit more convincing. The linked issue isn't concrete enough for me.

[keegancsmith]

We haven't done work on this package in a while, maybe it would make sense to create a new one which exports the lsp package used by gopls? https://pkg.go.dev/golang.org/x/tools/internal/lsp?tab=doc

[radeksimko]

Given it is additive, why don't we just update the structs to support the latest version of the spec? Wouldn't that satisfy your need? The spec does change, but not that often.

Even if go-lsp stays on top of spec changes, there's the long tail of:

a) language servers actually updating in time, and
b) end-users of these LS updating in time

Generally the problem is that clients will move at a different pace to servers and neither side has any way of controlling that pace, because the protocol doesn't do any version negotiation.

The rpc layer being strict is (kinda) useful when you control both the client and the server, but seems not great for other environments.

That is a good point. Maybe jrpc2 should still allow non-strict mode for these scenarios. I should bring that up again there.

Do you have a link to how you are using the rpc framework? That may help me understand why it is a good idea :)

I don't think it's unique in any way, but this is the main touchpoint between go-lsp and jrpc2:
https://github.com/hashicorp/terraform-ls/blob/26be653d23d4f7d90bc817ecc2ea999cebe66c1f/langserver/handlers/service.go

Generating the custom unmarshallers does seem like an ugly hack to be honest, so I need a bit more convincing.

To be quite honest I'm not that excited about this solution either in retrospect, but given the context above I couldn't come up with a better solution other than re-exporting gopls' internal package(s) or building our own library in a similar way, which is not something I fancied at such an early stage of building our LS when your library proved useful in solving majority of our use cases. 😄

Relatedly @njuCZ kindly pointed out to me a few days ago, that this PR actually prevents certain fields from being unmarshaled, which is an effect of custom unmarshalers being used in combination with embedded structs. So I am glad you didn't merge it in retrospect actually. 😅

We haven't done work on this package in a while, maybe it would make sense to create a new one which exports the lsp package used by gopls? https://pkg.go.dev/golang.org/x/tools/internal/lsp?tab=doc

💯 👍 I'm hoping the gopls team will eventually un-internalize these packages, but I haven't checked the roadmap (not sure if there is any public one) and whether they plan to do that at all.

Either way if you are up for this I'm happy to help - in fact I will probably do it in coming months if you don't, because we need a decent solution for terraform-ls.

I think that generated structs would be best for everyone and it would be much easier if the LSP spec had an official machine-readable version.

With all that in mind I'm happy to close this PR now and explore alternative solutions.

[keegancsmith]

Given it is additive, why don't we just update the structs to support the latest version of the spec? Wouldn't that satisfy your need? The spec does change, but not that often.

Even if go-lsp stays on top of spec changes, there's the long tail of:

a) language servers actually updating in time, and
b) end-users of these LS updating in time

Generally the problem is that clients will move at a different pace to servers and neither side has any way of controlling that pace, because the protocol doesn't do any version negotiation.

Agreed, so we need it to always be relaxed when parsing.

Do you have a link to how you are using the rpc framework? That may help me understand why it is a good idea :)

I don't think it's unique in any way, but this is the main touchpoint between go-lsp and jrpc2:
https://github.com/hashicorp/terraform-ls/blob/26be653d23d4f7d90bc817ecc2ea999cebe66c1f/langserver/handlers/service.go

Thanks for the link. I like how that code looks :) Also I see terraform has a bunch of useful packages related to exec and filesystem that I will probably go and explore at some point. So thanks!

Relatedly @njuCZ kindly pointed out to me a few days ago, that this PR actually prevents certain fields from being unmarshaled, which is an effect of custom unmarshalers being used in combination with embedded structs. So I am glad you didn't merge it in retrospect actually.

Phew :)

We haven't done work on this package in a while, maybe it would make sense to create a new one which exports the lsp package used by gopls? https://pkg.go.dev/golang.org/x/tools/internal/lsp?tab=doc

💯 👍 I'm hoping the gopls team will eventually un-internalize these packages, but I haven't checked the roadmap (not sure if there is any public one) and whether they plan to do that at all.

There is precedent for exporting these useful packages outside of the golang.org domain. For example this package contains many useful internal pkgs from the stdlib: https://github.com/rogpeppe/go-internal

They might even be open to mirroring the lsp pkg from gopls.

With all that in mind I'm happy to close this PR now and explore alternative solutions.

Thanks for the detailed response! Sounds good.