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: gopls does not reset workspace/configuration if missing from the client #71227

Open
h9jiang opened this issue Jan 10, 2025 · 2 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@h9jiang
Copy link
Member

h9jiang commented Jan 10, 2025

gopls version

master head, following experiment is based on commit 6efe0f4b404b25e02999c3e34db08771f855fc28

go env

NA

What did you do?

Locate semantic token information from gopls of a string or a number. Toggle feature noSemanticString or noSemanticNumber.

What did you see happen?

By default, number's token info is returned from the gopls, you can see semantic token type = number.

image

Then if I set "ui.noSemanticNumber": true, the semantic token info goes away. WAI

image

If I set the "ui.noSemanticNumber": false,, the semantic token info comes back. WAI

image

Set it to "ui.noSemanticNumber": true,, the token info goes away. WAI. Then if I comment this out. What I expect is, the semantic token information will come back. Because, no setting means default value "ui.noSemanticNumber": false,.

What did you expect to see?

image

As a result it does not come back.

Editor and settings

No response

Logs

I will focus on the last two workspace/configuration logs

Where I set it to true

[Trace - 5:26:49 PM] Sending response 'workspace/configuration - (14)'. Processing request took 0ms
Result: [
    {
        "ui.semanticTokens": true,
        "ui.noSemanticNumber": true,
        "ui.inlayhint.hints": {
            "assignVariableTypes": false,
            "compositeLiteralFields": false,
            "compositeLiteralTypes": false,
            "constantValues": false,
            "functionTypeParameters": false,
            "parameterNames": false,
            "rangeVariableTypes": false
        },
        "ui.vulncheck": "Off",
        "linkifyShowMessage": true
    }
]

Where I comment it out

[Trace - 5:27:44 PM] Sending response 'workspace/configuration - (16)'. Processing request took 0ms
Result: [
    {
        "ui.semanticTokens": true,
        "ui.inlayhint.hints": {
            "assignVariableTypes": false,
            "compositeLiteralFields": false,
            "compositeLiteralTypes": false,
            "constantValues": false,
            "functionTypeParameters": false,
            "parameterNames": false,
            "rangeVariableTypes": false
        },
        "ui.vulncheck": "Off",
        "linkifyShowMessage": true
    }
]

I think what happened is, when gopls saw "ui.noSemanticNumber": X, in the json, gopls will apply this X in memory. WAI.
But when gopls saw "ui.noSemanticNumber": X, is missing in the json, gopls do nothing. In this case, gopls should set the ui.noSemanticNumber = false because false is the default value.

@findleyr

@h9jiang h9jiang added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Jan 10, 2025
@gopherbot gopherbot added this to the Unreleased milestone Jan 10, 2025
@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Jan 10, 2025
@findleyr
Copy link
Member

I agree this is a bug, but perhaps not one that is easy to fix.

The problem is that on didChangeConfiguration notifications, gopls makes a workspace/configuration request to the client with a specific scope URI set to the workspace folder. IIUC, the VS Code client will respond with the workspace settings.json.

Has that response already merged user settings.json? Or was user settings.json sent as the initializationOptions?

If the former, we should always clobber the existing options with defaults, and then apply the result of workspace/configuration. If the latter, we need to keep a copy of the original initializationOptions that can be overlaid with workspace options.

Also: in our workspace/configuration request we ask for the "gopls" section of configuration, but are tolerant of users sending us configuration without the "gopls." prefix. Therefore, some clients will send an empty response to "workspace/configuration", and so only work if we overlay the "initializationOptions" of the original "initialize" request.

In summary: something we're doing is certainly wrong, but any change to our current behavior is liable to be complicated, and break some existing users. Therefore, we are unlikely to be able to solve this problem soon.

@findleyr findleyr modified the milestones: Unreleased, gopls/backlog Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants