Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/tools/gopls: regression in semantic tokens (Data field is JSON "null") #67885

Closed
adonovan opened this issue Jun 7, 2024 · 4 comments
Closed

x/tools/gopls: regression in semantic tokens (Data field is JSON "null") #67885

adonovan opened this issue Jun 7, 2024 · 4 comments
Assignees
@findleyr
Labels
gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
gopls/v0.16.0

Comments

@adonovan
Copy link
Member

adonovan commented Jun 7, 2024

@neild reports that neovim with gopls at master produces an error similar to neovim/neovim#21387. I suspect https://go.dev/cl/577655 introduced a shortcut path with a nil Data slice. This patch appears to be an effective workaround:

diff --git a/gopls/internal/server/semantic.go b/gopls/internal/server/semantic.go
index ca3df78e10..7f7a3474d3 100644
--- a/gopls/internal/server/semantic.go
+++ b/gopls/internal/server/semantic.go
@@ -38,7 +38,7 @@ func (s *server) semanticTokens(ctx context.Context, td protocol.TextDocumentIde
                // Previously, an error was returned here to achieve the same effect, but
                // that had the side effect of very noisy "semantictokens are disabled"
                // logs on every keystroke.
-               return new(protocol.SemanticTokens), nil
+               return &protocol.SemanticTokens{Data: []uint32{}}, nil
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jun 7, 2024
@gopherbot gopherbot added this to the Unreleased milestone Jun 7, 2024
@adonovan adonovan added release-blocker NeedsFix The path to resolution is known, but the work has not been done. and removed gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Jun 7, 2024
@adonovan adonovan modified the milestones: Unreleased, gopls/v0.16.0 Jun 7, 2024
@findleyr
Copy link
Member

findleyr commented Jun 7, 2024

Thanks all for catching this (right before we were about to cut the prerelease, too!).

I suspect adding "omitempty" would also fix this, and may be more principled.

@findleyr findleyr self-assigned this Jun 7, 2024
@adonovan
Copy link
Member Author

adonovan commented Jun 7, 2024

Yes, the patch was just evidence. I think the cleanest solution would be to make sure that every slice in our JSON schema is marked omitempty.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/591415 mentions this issue: gopls/internal/server: return a non-nil slice for empty token result

@findleyr
Copy link
Member

findleyr commented Jun 7, 2024

It looks like omitempty may not be appropriate here, because the fields are not marked as optional in the schema.

For now, went with the fix suggested in Alan's patch: just return a non-nil empty slice.

@seankhliao seankhliao added the gopls Issues related to the Go language server, gopls. label Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@findleyr findleyr

Labels
gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
gopls/v0.16.0
Development

No branches or pull requests
4 participants
@adonovan @gopherbot @seankhliao @findleyr