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.

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

x/tools/gopls: some properties/defaults from `gopls api-json` are invalid #41964

Closed
hyangah opened this issue Oct 14, 2020 · 3 comments
Closed

x/tools/gopls: some properties/defaults from `gopls api-json` are invalid #41964

hyangah opened this issue Oct 14, 2020 · 3 comments
Assignees
Labels
Milestone

Comments

@hyangah
Copy link
Contributor

@hyangah hyangah commented Oct 14, 2020

$ gopls -v version
Build info
----------
golang.org/x/tools/gopls master
    golang.org/x/tools/gopls@v0.0.0-20201013201025-64a9e34f3752 h1:WOieoYgx8c4Yz9/oW7VgjvzIsjTN0++21yoFC15q+RY=
    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/google/go-cmp@v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/mod@v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
    golang.org/x/sync@v0.0.0-20200625203802-6e8e738ad208 h1:qwRHBd0NqMbJxfbotnDhm2ByMI1Shq4Y6oRJo21SGJA=
    golang.org/x/tools@v0.0.0-20201013201025-64a9e34f3752 h1:2ntEwh02rqo2jSsrYmp4yKHHjh0CbXP3ZtSUetSB+q8=
    golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
    honnef.co/go/tools@v0.0.1-2020.1.5 h1:nI5egYTGJakVyOryqLs1cQO5dO0ksin5XXs2pspk75k=
    mvdan.cc/gofumpt@v0.0.0-20200802201014-ab5a8192947d h1:t8TAw9WgTLghti7RYkpPmqk4JtQ3+wcP5GgZqgWeWLQ=
    mvdan.cc/xurls/v2@v2.2.0 h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A=

But reproducible with the latest stable version of gopls (gopls settings-json).

Configure the vscode gopls section with the default values listed in settings.md@64a9e34

  "gopls": {
    "env": [],
    "semanticTokens": false,
    "experimentalPackageCacheKey": false,
    "completionBudget": 100000000,
    "literalCompletions": true,
  }

See gopls complaining about the default values and triggering multiple notification popups.


[Trace - 23:17:01.044 PM] Sending response 'workspace/configuration - (2)' in 4ms.
Result: [{"env":[],"semanticTokens":false,"experimentalPackageCacheKey":false,"completionBudget":100000000,"literalCompletions":true},null]

[Trace - 23:17:01.044 PM] Received notification 'window/showMessage'.
Params: {"type":1,"message":"invalid config gopls.env type []interface {}"}

[Trace - 23:17:01.044 PM] Received notification 'window/showMessage'.
Params: {"type":1,"message":"unexpected gopls setting \"semanticTokens\""}

[Trace - 23:17:01.044 PM] Received notification 'window/showMessage'.
Params: {"type":1,"message":"unexpected gopls setting \"experimentalPackageCacheKey\""}

[Trace - 23:17:01.044 PM] Received notification 'window/showMessage'.
Params: {"type":1,"message":"Invalid type float64 for string option \"completionBudget\""}

[Trace - 23:17:01.045 PM] Received notification 'window/showMessage'.
Params: {"type":1,"message":"unexpected gopls setting \"literalCompletions\""}

Issues:

  • semanticTokens, experimentalPackageCacheKey, literalCompletions were unexpected
  • completionBudget is time.Duration type. The default value 100000000 is interpreted as float64. I tried to supply "100000000" (string type) which still fails due to parsing error ( Params: {"type":1,"message":"failed to parse duration \"100000000\": time: missing unit in duration \"100000000\""})
    Note: the error message does not include the property name which needs to be addressed too.
  • env section is defined as []string following source.UserOptions.Env
    In VS Code, environment variable collection is often map[string]string type, so string array type is unusual.
    And, gopls doesn't accept default [] but accepts {}.
@gopherbot gopherbot added this to the Unreleased milestone Oct 14, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v1.0.0 Oct 14, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 14, 2020

Change https://golang.org/cl/262345 mentions this issue: internal/lsp: fix mismatches between doc and settings

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 14, 2020

Change https://golang.org/cl/262351 mentions this issue: internal/lsp: fix env setting type

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 14, 2020

Change https://golang.org/cl/262353 mentions this issue: internal/lsp/source: include setting name in all settings errors

gopherbot pushed a commit to golang/tools that referenced this issue Oct 15, 2020
Fix a few settings whose documentation was bad in various ways:
 - semanticTokens' was not camel case; changed to semanticTokens.
 - experimentalPackageCacheKey was not hooked up to a setting; do so.
 - literalCompletions is for tests only; hide it.
 - completionBudget should be specified as a duration string, not number
 of nanoseconds.
 - experimentalDiagnosticsDelay should have been affected similarly to
 completionBudget but is not mentioned in the bug.

env is next.

Updates golang/go#41964.

Change-Id: I3470958cbdfc46daa76da297fc1a56da4a3f10ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/262345
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopherbot pushed a commit to golang/tools that referenced this issue Oct 15, 2020
Every setting parse error should include the name of the error for user
sanity. Add it in the errorf method, clean up redundancy, and make
existing errors more consistent/correct.

Updates golang/go#41964.

Change-Id: I8f980ceb5d82d4238edcd4095ea0e9572ac9c271
Reviewed-on: https://go-review.googlesource.com/c/tools/+/262353
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.5.2 Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants