go-language-server / protocol

Package protocol implements Language Server Protocol specification in Go
https://pkg.go.dev/go.lsp.dev/protocol
BSD 3-Clause "New" or "Revised" License
98 stars 15 forks source link

Fix client capability for semantic tokens #27

Closed markdumay closed 3 years ago

markdumay commented 3 years ago

First of all, great to see you guys are making the effort to port the LSP specification to Go. I started experimenting with your package quite recently. I encountered a small problem when trying to unmarshal the InitializeParams as generated by vscode-languageclient with version 7.0.0. VSCode generates the client capabilities automatically, but handling the payload of InitializeParams on the server side (written in Go using your protocol package) gives the following error:

Message: json: cannot unmarshal object into Go struct field SemanticTokensWorkspaceClientCapabilitiesRequests.capabilities.textDocument.semanticTokens.requests.full of type bool

It seems the following section of the InitializeParams JSON is the cause of the issue.

[...]
    "textDocument": {
      "semanticTokens": {
        "requests": {
          "range": true,
          "full": {
            "delta": true
          }
      }
    }
[...]

According to the official specification, the value for full can be either boolean or another embedded struct.

This PR fixes the issue by adjusting the struct SemanticTokensWorkspaceClientCapabilitiesRequests in capabilities_client.go. It simply changes the type of Full from bool to interface{}. Let me know if you need more evidence or supporting files to reproduce the error.

zchee commented 3 years ago

@markdumay Thanks for contributing! I understand you said problem and approve this change. But I forgotten enabled run forked pull request on CircleCI. I'll dig CircleCI docs, but could you amend and force push this branch? Thanks.

markdumay commented 3 years ago

Great, good to hear you approve the proposed change. I made a small documentation change and submitted a force push. It seems this has triggered the CircleCI workflows. There is now an error though:

./capabilities_client_gojay.go:1824: cannot use v.Full (type interface {}) as type bool in argument to enc.BoolKeyOmitEmpty: need type assertion
./capabilities_client_gojay.go:1836: cannot use &v.Full (type *interface {}) as type *bool in argument to dec.Bool
WARN invalid TestEvent: FAIL    go.lsp.dev/protocol [build failed]

It looks like the capabilities_client_gojay.go file is out of sync. Is there a specific command that you run to regenerate the gojay files?

zchee commented 3 years ago

@markdumay sorry for late, I'll comment later, maybe ~6h.

zchee commented 3 years ago

@markdumay Unfortunately, the gojay bindings are written by my hand.

Could you patch the below diff? It should work.

diff --git a/capabilities_client_gojay.go b/capabilities_client_gojay.go
index 3c2be32..759d87a 100644
--- a/capabilities_client_gojay.go
+++ b/capabilities_client_gojay.go
@@ -1821,7 +1821,7 @@ var (
 // MarshalJSONObject implements gojay.MarshalerJSONObject.
 func (v *SemanticTokensWorkspaceClientCapabilitiesRequests) MarshalJSONObject(enc *gojay.Encoder) {
    enc.BoolKeyOmitEmpty(keyRange, v.Range)
-   enc.BoolKeyOmitEmpty(keyFull, v.Full)
+   enc.AddInterfaceKey(keyFull, v.Full)
 }

 // IsNil implements gojay.MarshalerJSONObject.
@@ -1833,7 +1833,7 @@ func (v *SemanticTokensWorkspaceClientCapabilitiesRequests) UnmarshalJSONObject(
    case keyRange:
        return dec.Bool(&v.Range)
    case keyFull:
-       return dec.Bool(&v.Full)
+       return dec.Interface(&v.Full)
    }
    return nil
 }
markdumay commented 3 years ago

Thanks @zchee, I force pushed the changes. CircleCI now indicates I'm not authorized to run the workflows though.

codecov[bot] commented 3 years ago

Codecov Report

Merging #27 (9f6c20c) into main (c071fd8) will not change coverage. The diff coverage is 100.0%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main     #27   +/-   ##
=====================================
  Coverage   60.5%   60.5%           
=====================================
  Files         40      40           
  Lines       5538    5538           
=====================================
  Hits        3356    3356           
  Misses      2143    2143           
  Partials      39      39           
Flag Coverage Δ
gojay 67.0% <100.0%> (ø)
json 20.4% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
capabilities_client.go 100.0% <ø> (ø)
capabilities_client_gojay.go 91.4% <100.0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c071fd8...9f6c20c. Read the comment docs.

zchee commented 3 years ago

@markdumay merged. Thanks contribute!

zchee commented 3 years ago

@markdumay also released a new tag.

https://github.com/go-language-server/protocol/releases/tag/v0.11.2

markdumay commented 3 years ago

Great, glad to help. Keep up the good work!