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: show signature help when typing a struct as an argument to a function? #42149

Closed
bcmills opened this issue Oct 22, 2020 · 10 comments
Closed
Labels

Comments

@bcmills
Copy link
Member

@bcmills bcmills commented Oct 22, 2020

What version?

$ go version
go version devel +4c7a18d74 Thu Oct 22 01:20:16 2020 +0000 linux/amd64

$ go version -m $(which gopls)
/usr/local/google/home/bcmills/bin/gopls: devel +186f0220d0 Mon Oct 5 11:12:24 2020 -0400
        path    golang.org/x/tools/gopls
        mod     golang.org/x/tools/gopls        v0.5.1  h1:AF3Uh7HF08SZpKFfgJO6zfF3bbxyDXWqdkK4kMXiQ1o=
        dep     github.com/BurntSushi/toml      v0.3.1  h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
        dep     github.com/google/go-cmp        v0.5.1  h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k=
        dep     github.com/sergi/go-diff        v1.1.0  h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
        dep     golang.org/x/mod        v0.3.0  h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
        dep     golang.org/x/sync       v0.0.0-20200625203802-6e8e738ad208      h1:qwRHBd0NqMbJxfbotnDhm2ByMI1Shq4Y6oRJo21SGJA=
        dep     golang.org/x/tools      v0.0.0-20200930165741-f1523d29dbb9      h1:1R38tQp22dcHpTKJPjgVa16FhlDy/kHEaCM/ndi/FIc=
        dep     golang.org/x/xerrors    v0.0.0-20200804184101-5ec99f83aff1      h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
        dep     honnef.co/go/tools      v0.0.1-2020.1.5 h1:nI5egYTGJakVyOryqLs1cQO5dO0ksin5XXs2pspk75k=
        dep     mvdan.cc/gofumpt        v0.0.0-20200802201014-ab5a8192947d      h1:t8TAw9WgTLghti7RYkpPmqk4JtQ3+wcP5GgZqgWeWLQ=
        dep     mvdan.cc/xurls/v2       v2.2.0  h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A=

What did you do?

$ cat > example.go
package example

import "os"

func F() {
        var errs []*os.PathError
        errs = append(errs, &os.PathError{
                // Place cursor here.
        })
        _ = errs
}

$ emacs example.go
  • Move the cursor to the beginning of the // Place cursor here. comment.

What did you expect to see?

Hover information about the fields of the containing struct.

What did you see instead?

Hover information about the built-in append function.

[Trace - 11:29:14.338 AM] Sending request 'initialize - (1)'.
Params: {"processId":595917,"rootPath":"/tmp/tmp.6zQzInLjRX/example.com/","rootUri":"file:///tmp/tmp.6zQzInLjRX/example.com/","initializationOptions":null,"capabilities":{"workspace":{"applyEdit":true,"executeCommand":{"dynamicRegistration":false},"workspaceEdit":{"documentChanges":false},"didChangeWatchedFiles":{"dynamicRegistration":true},"symbol":{"dynamicRegistration":false},"configuration":true},"textDocument":{"synchronization":{"dynamicRegistration":false,"willSave":true,"willSaveWaitUntil":true,"didSave":true},"completion":{"dynamicRegistration":false,"completionItem":{"snippetSupport":true},"contextSupport":true},"hover":{"dynamicRegistration":false,"contentFormat":["markdown","plaintext"]},"signatureHelp":{"dynamicRegistration":false,"signatureInformation":{"parameterInformation":{"labelOffsetSupport":true}}},"references":{"dynamicRegistration":false},"definition":{"dynamicRegistration":false},"declaration":{"dynamicRegistration":false},"implementation":{"dynamicRegistration":false},"typeDefinition":{"dynamicRegistration":false},"documentSymbol":{"dynamicRegistration":false,"hierarchicalDocumentSymbolSupport":true,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}},"documentHighlight":{"dynamicRegistration":false},"codeAction":{"dynamicRegistration":false,"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"formatting":{"dynamicRegistration":false},"rangeFormatting":{"dynamicRegistration":false},"rename":{"dynamicRegistration":false},"publishDiagnostics":{"relatedInformation":false}},"experimental":null}}


[Trace - 11:29:14.339 AM] Received response 'initialize - (1)' in 1ms.
Result: {"capabilities":{"textDocumentSync":{"openClose":true,"change":2,"save":{}},"completionProvider":{"triggerCharacters":["."]},"hoverProvider":true,"signatureHelpProvider":{"triggerCharacters":["(",","]},"definitionProvider":true,"typeDefinitionProvider":true,"implementationProvider":true,"referencesProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"codeActionProvider":{"codeActionKinds":["quickfix","refactor.extract","refactor.rewrite","source.fixAll","source.organizeImports"]},"codeLensProvider":{},"documentLinkProvider":{},"workspaceSymbolProvider":true,"documentFormattingProvider":true,"documentOnTypeFormattingProvider":{"firstTriggerCharacter":""},"renameProvider":true,"foldingRangeProvider":true,"executeCommandProvider":{"commands":["generate","fill_struct","regenerate_cgo","test","tidy","undeclared_name","upgrade_dependency","vendor","extract_variable","extract_function","gc_details","generate_gopls_mod"]},"callHierarchyProvider":true,"workspace":{"workspaceFolders":{"supported":true,"changeNotifications":"workspace/didChangeWorkspaceFolders"}}},"serverInfo":{"name":"gopls","version":"Build info\n----------\ngolang.org/x/tools/gopls v0.5.1\n    golang.org/x/tools/gopls@v0.5.1 h1:AF3Uh7HF08SZpKFfgJO6zfF3bbxyDXWqdkK4kMXiQ1o=\n    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=\n    github.com/google/go-cmp@v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k=\n    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=\n    golang.org/x/mod@v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=\n    golang.org/x/sync@v0.0.0-20200625203802-6e8e738ad208 h1:qwRHBd0NqMbJxfbotnDhm2ByMI1Shq4Y6oRJo21SGJA=\n    golang.org/x/tools@v0.0.0-20200930165741-f1523d29dbb9 h1:1R38tQp22dcHpTKJPjgVa16FhlDy/kHEaCM/ndi/FIc=\n    golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=\n    honnef.co/go/tools@v0.0.1-2020.1.5 h1:nI5egYTGJakVyOryqLs1cQO5dO0ksin5XXs2pspk75k=\n    mvdan.cc/gofumpt@v0.0.0-20200802201014-ab5a8192947d h1:t8TAw9WgTLghti7RYkpPmqk4JtQ3+wcP5GgZqgWeWLQ=\n    mvdan.cc/xurls/v2@v2.2.0 h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A=\n"}}


[Trace - 11:29:14.341 AM] Sending notification 'initialized'.
Params: {}


[Trace - 11:29:14.341 AM] Received notification 'window/showMessage'.
Params: {"type":4,"message":"Loading packages..."}


[Trace - 11:29:14.341 AM] Received request 'workspace/configuration - (1)'.
Params: {"items":[{"scopeUri":"file:///tmp/tmp.6zQzInLjRX/example.com/","section":"gopls"},{"scopeUri":"file:///tmp/tmp.6zQzInLjRX/example.com/","section":"gopls-example.com"}]}


[Trace - 11:29:14.343 AM] Sending notification 'textDocument/didOpen'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.6zQzInLjRX/example.com/example.go","version":0,"languageId":"go","text":"package example\n\nimport \"os\"\n\nfunc F() {\n\tvar errs []*os.PathError\n\terrs = append(errs, &os.PathError{\n\t\t// Place cursor here.\n\t})\n\t_ = errs\n}\n"}}


[Trace - 11:29:14.344 AM] Sending notification 'workspace/didChangeConfiguration'.
Params: {"settings":{"gopls":{"staticcheck":true,"matcher":"CaseSensitive"}}}


[Trace - 11:29:14.346 AM] Sending request 'textDocument/documentSymbol - (2)'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.6zQzInLjRX/example.com/example.go"}}


[Trace - 11:29:14.351 AM] Sending response 'workspace/configuration - (1)' in 9ms.
Result: [{"staticcheck":true,"matcher":"CaseSensitive"},null]


[Trace - 11:29:14.418 AM] Received request 'client/registerCapability - (2)'.
Params: {"registrations":[{"id":"workspace/didChangeWatchedFiles-0","method":"workspace/didChangeWatchedFiles","registerOptions":{"watchers":[{"globPattern":"**/*.{go,mod,sum}","kind":7},{"globPattern":"/tmp/tmp.6zQzInLjRX/example.com/**/*.{go,mod,sum}","kind":7}]}}]}


[Trace - 11:29:14.418 AM] Received notification 'window/logMessage'.
Params: {"type":3,"message":"2020/10/22 11:29:14 go env for /tmp/tmp.6zQzInLjRX/example.com/\n(root /tmp/tmp.6zQzInLjRX/example.com)\n(go version go version devel +4c7a18d74 Thu Oct 22 01:20:16 2020 +0000 linux/amd64\n)\n(valid build configuration = true)\n(build flags: [])\nGOMODCACHE=/tmp/tmp.6zQzInLjRX/_gopath/pkg/mod\nGOPROXY=https://proxy.golang.org,direct\nGOMOD=/tmp/tmp.6zQzInLjRX/example.com/go.mod\nGOPATH=/tmp/tmp.6zQzInLjRX/_gopath\nGOROOT=/usr/local/google/home/bcmills/sdk/gotip\nGO111MODULE=auto\nGOINSECURE=\nGONOSUMDB=\nGOPRIVATE=\nGOSUMDB=sum.golang.org\nGOCACHE=/usr/local/google/home/bcmills/.cache/go-build\nGOFLAGS=\nGONOPROXY=\n\n"}


[Trace - 11:29:14.418 AM] Sending response 'client/registerCapability - (2)' in 0ms.
Result: 


[Trace - 11:29:14.674 AM] Received notification 'window/logMessage'.
Params: {"type":3,"message":"2020/10/22 11:29:14 go/packages.Load\n\tsnapshot=0\n\tdirectory=/tmp/tmp.6zQzInLjRX/example.com\n\tquery=[./... builtin]\n\tpackages=2\n"}


[Trace - 11:29:14.705 AM] Received notification 'window/showMessage'.
Params: {"type":3,"message":"Finished loading packages."}


[Trace - 11:29:14.705 AM] Received request 'workspace/configuration - (3)'.
Params: {"items":[{"scopeUri":"file:///tmp/tmp.6zQzInLjRX/example.com/","section":"gopls"},{"scopeUri":"file:///tmp/tmp.6zQzInLjRX/example.com/","section":"gopls-example.com"}]}


[Trace - 11:29:14.747 AM] Sending response 'workspace/configuration - (3)' in 41ms.
Result: [{"staticcheck":true,"matcher":"CaseSensitive"},null]


[Trace - 11:29:14.778 AM] Received response 'textDocument/documentSymbol - (2)' in 432ms.
Result: [{"name":"F","detail":"()","kind":12,"range":{"start":{"line":4,"character":0},"end":{"line":10,"character":1}},"selectionRange":{"start":{"line":4,"character":5},"end":{"line":4,"character":6}}}]


[Trace - 11:29:14.832 AM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///tmp/tmp.6zQzInLjRX/example.com/example.go","diagnostics":[{"range":{"start":{"line":0,"character":0},"end":{"line":0,"character":15}},"severity":2,"source":"ST1000","message":"at least one file in a package should have a package comment"}]}


[Trace - 11:29:18.761 AM] Sending request 'textDocument/signatureHelp - (3)'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.6zQzInLjRX/example.com/example.go"},"position":{"line":7,"character":2}}


[Trace - 11:29:18.762 AM] Sending request 'textDocument/hover - (4)'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.6zQzInLjRX/example.com/example.go"},"position":{"line":7,"character":2}}


[Trace - 11:29:18.767 AM] Received response 'textDocument/signatureHelp - (3)' in 5ms.
Result: {"signatures":[{"label":"append(slice []Type, elems ...Type) []Type","documentation":"The append built-in function appends elements to the end of a slice.","parameters":[{"label":"slice []Type"},{"label":"elems ...Type"}]}],"activeSignature":0,"activeParameter":1}


[Trace - 11:29:18.767 AM] Sending request 'textDocument/documentHighlight - (5)'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.6zQzInLjRX/example.com/example.go"},"position":{"line":7,"character":2}}


[Trace - 11:29:18.767 AM] Received response 'textDocument/hover - (4)' in 5ms.
Result: null


[Trace - 11:29:18.768 AM] Received response 'textDocument/documentHighlight - (5)' in 0ms.
Result: []


@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Oct 22, 2020

What you're seeing here is not actually the hover (which should return empty if your cursor isn't on a symbol), but rather signature help:

From the logs:

[Trace - 11:29:18.761 AM] Sending request 'textDocument/signatureHelp - (3)'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.6zQzInLjRX/example.com/example.go"},"position":{"line":7,"character":2}}

[Trace - 11:29:18.767 AM] Received response 'textDocument/signatureHelp - (3)' in 5ms.
Result: {"signatures":[{"label":"append(slice []Type, elems ...Type) []Type","documentation":"The append built-in function appends elements to the end of a slice.","parameters":[{"label":"slice []Type"},{"label":"elems ...Type"}]}],"activeSignature":0,"activeParameter":1}

I think this is working as intended here--signature help is meant to show up when you are typing the arguments to a function. Perhaps we could remove it when you are in a composite literal, but I'm not sure if that's more "correct".

@stamblerre stamblerre changed the title x/tools/gopls: unhelpful hover content inside a struct passed as an argument to 'append' x/tools/gopls: show signature help when typing a struct as an argument to a function? Oct 22, 2020
@bcmills
Copy link
Member Author

@bcmills bcmills commented Oct 22, 2020

Yeah, just removing the signature wouldn't be particularly helpful: I really want some guidance for the struct fields.

I guess a simpler reproducer is:

package example

import "os"

func F() error {
	return &os.PathError{
		// Place cursor here.
	}
}

In that case, I get no hovers or completions at all. I guess the idea is that I should use a Fill <struct> action here? (But I don't want to actually fill in the struct — I just want to see which fields I could set.)

My initial reaction was that I should get a hover with a doc summary for the struct type, but maybe that's not quite right either. Maybe I actually want completions for all of the struct fields that aren't already present in the declaration?

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Oct 22, 2020

Maybe I actually want completions for all of the struct fields that aren't already present in the declaration?

That is what you should be getting. In VS Code, you can trigger completion with Ctrl+Space, but I'm not sure how to manually trigger completions in Emacs. It would be nice if gopls could tell the client to automatically pop-up completions in such cases, but it's tricky because we can only provide "trigger characters" like . -- but you don't always want completions after typing {.

@bcmills
Copy link
Member Author

@bcmills bcmills commented Oct 22, 2020

Oh, neato! Yeah, if I run M-x company-complete I do get the expected completions.

So, I guess what's missing here is a mechanism in the LSP protocol, so that gopls could notify the client that completions are available / appropriate even though there is no trigger character?

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Oct 22, 2020

I suppose so, but even still, it probably wouldn't work well for gopls to be checking if the user typed the opening { for a struct on every file change. I think the solution here is to manually trigger completions, and at least, in VS Code, once you start typing the completions would pop up right away.

@bcmills
Copy link
Member Author

@bcmills bcmills commented Oct 22, 2020

it probably wouldn't work well for gopls to be checking if the user typed the opening { for a struct on every file change.

Could you elaborate? (It's already checking whether the user typed the opening ( for a function call on every file change, isn't it? Or is the signature help triggered some other way?)

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Oct 22, 2020

Signature help works similar to completions--there's a trigger character ((), and the client sends the request for it. There just isn't always a signature help result, whereas for completions, we almost always have some results.

@bcmills
Copy link
Member Author

@bcmills bcmills commented Oct 22, 2020

Ah, I see. That's unfortunate — I was really hoping for something a little more context-aware.

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Oct 22, 2020

Yeah I'm sorry that it's not more intuitive. OK to close this?

@bcmills
Copy link
Member Author

@bcmills bcmills commented Oct 22, 2020

Yep.

@bcmills bcmills closed this Oct 22, 2020
@stamblerre stamblerre removed this from the gopls/unplanned milestone Oct 23, 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
3 participants