Skip to content

Commit

Permalink
tools/gopls: provide markdown for completion and signature help
Browse files Browse the repository at this point in the history
If the client prefers markdown, provide markdown.

There is a change to the LSP stubs, with Documentation fields becoming
Or-types (string|MarkupContent). If the client prefers, the returned
Documentation is markdown.

Fixes: #57300
Change-Id: I57300146333552da3849c1b6bfb97793042faee2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/463377
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
pjweinbgo committed Jan 31, 2023
1 parent 684a1c0 commit aa633e7
Show file tree
Hide file tree
Showing 12 changed files with 147 additions and 24 deletions.
11 changes: 9 additions & 2 deletions gopls/internal/lsp/cmd/signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,15 @@ func (r *signature) Run(ctx context.Context, args ...string) error {
// see toProtocolSignatureHelp in lsp/signature_help.go
signature := s.Signatures[0]
fmt.Printf("%s\n", signature.Label)
if signature.Documentation != "" {
fmt.Printf("\n%s\n", signature.Documentation)
switch x := signature.Documentation.Value.(type) {
case string:
if x != "" {
fmt.Printf("\n%s\n", x)
}
case protocol.MarkupContent:
if x.Value != "" {
fmt.Printf("\n%s\n", x.Value)
}
}

return nil
Expand Down
11 changes: 10 additions & 1 deletion gopls/internal/lsp/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ func toProtocolCompletionItems(candidates []completion.CompletionItem, rng proto
continue
}

doc := &protocol.Or_CompletionItem_documentation{
Value: protocol.MarkupContent{
Kind: protocol.Markdown,
Value: source.CommentToMarkdown(candidate.Documentation),
},
}
if options.PreferredContentFormat != protocol.Markdown {
doc.Value = candidate.Documentation
}
item := protocol.CompletionItem{
Label: candidate.Label,
Detail: candidate.Detail,
Expand All @@ -121,7 +130,7 @@ func toProtocolCompletionItems(candidates []completion.CompletionItem, rng proto
FilterText: strings.TrimLeft(candidate.InsertText, "&*"),

Preselect: i == 0,
Documentation: candidate.Documentation,
Documentation: doc,
Tags: candidate.Tags,
Deprecated: candidate.Deprecated,
}
Expand Down
12 changes: 7 additions & 5 deletions gopls/internal/lsp/completion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,13 @@ func expected(t *testing.T, test tests.Completion, items tests.CompletionItems)

toProtocolCompletionItem := func(item *completion.CompletionItem) protocol.CompletionItem {
pItem := protocol.CompletionItem{
Label: item.Label,
Kind: item.Kind,
Detail: item.Detail,
Documentation: item.Documentation,
InsertText: item.InsertText,
Label: item.Label,
Kind: item.Kind,
Detail: item.Detail,
Documentation: &protocol.Or_CompletionItem_documentation{
Value: item.Documentation,
},
InsertText: item.InsertText,
TextEdit: &protocol.TextEdit{
NewText: item.Snippet(),
},
Expand Down
17 changes: 17 additions & 0 deletions gopls/internal/lsp/fake/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1184,6 +1184,23 @@ func (e *Editor) Implementations(ctx context.Context, loc protocol.Location) ([]
return e.Server.Implementation(ctx, params)
}

func (e *Editor) SignatureHelp(ctx context.Context, loc protocol.Location) (*protocol.SignatureHelp, error) {
if e.Server == nil {
return nil, nil
}
path := e.sandbox.Workdir.URIToPath(loc.URI)
e.mu.Lock()
_, ok := e.buffers[path]
e.mu.Unlock()
if !ok {
return nil, fmt.Errorf("buffer %q is not open", path)
}
params := &protocol.SignatureHelpParams{
TextDocumentPositionParams: protocol.LocationTextDocumentPositionParams(loc),
}
return e.Server.SignatureHelp(ctx, params)
}

func (e *Editor) RenameFile(ctx context.Context, oldPath, newPath string) error {
closed, opened, err := e.renameBuffers(ctx, oldPath, newPath)
if err != nil {
Expand Down
10 changes: 4 additions & 6 deletions gopls/internal/lsp/protocol/generate/tables.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,10 @@ var usedGoplsStar = make(map[prop]bool)

// For gopls compatibility, use a different, typically more restrictive, type for some fields.
var renameProp = map[prop]string{
{"CancelParams", "id"}: "interface{}",
{"Command", "arguments"}: "[]json.RawMessage",
{"CompletionItem", "documentation"}: "string",
{"CompletionItem", "textEdit"}: "TextEdit",
{"Diagnostic", "code"}: "interface{}",
{"CancelParams", "id"}: "interface{}",
{"Command", "arguments"}: "[]json.RawMessage",
{"CompletionItem", "textEdit"}: "TextEdit",
{"Diagnostic", "code"}: "interface{}",

{"DocumentDiagnosticReportPartialResult", "relatedDocuments"}: "map[DocumentURI]interface{}",

Expand Down Expand Up @@ -181,7 +180,6 @@ var renameProp = map[prop]string{
{"ServerCapabilities", "typeDefinitionProvider"}: "interface{}",
{"ServerCapabilities", "typeHierarchyProvider"}: "interface{}",
{"ServerCapabilities", "workspaceSymbolProvider"}: "bool",
{"SignatureInformation", "documentation"}: "string",
{"TextDocumentEdit", "edits"}: "[]TextEdit",
{"TextDocumentSyncOptions", "save"}: "SaveOptions",
{"WorkspaceEdit", "documentChanges"}: "[]DocumentChanges",
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/protocol/tsclient.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion gopls/internal/lsp/protocol/tsjson.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions gopls/internal/lsp/protocol/tsprotocol.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion gopls/internal/lsp/protocol/tsserver.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions gopls/internal/lsp/regtest/wrappers.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,16 @@ func (e *Env) RenameFile(oldPath, newPath string) {
}
}

// SignatureHelp wraps Editor.SignatureHelp, calling t.Fatal on error
func (e *Env) SignatureHelp(loc protocol.Location) *protocol.SignatureHelp {
e.T.Helper()
sighelp, err := e.Editor.SignatureHelp(e.Ctx, loc)
if err != nil {
e.T.Fatal(err)
}
return sighelp
}

// Completion executes a completion request on the server.
func (e *Env) Completion(loc protocol.Location) *protocol.CompletionList {
e.T.Helper()
Expand Down
23 changes: 21 additions & 2 deletions gopls/internal/lsp/source/signature_help.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"go/ast"
"go/token"
"go/types"
"strings"

"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/gopls/internal/lsp/protocol"
Expand Down Expand Up @@ -117,7 +118,7 @@ FindCall:
}
return &protocol.SignatureInformation{
Label: name + s.Format(),
Documentation: s.doc,
Documentation: stringToSigInfoDocumentation(s.doc, snapshot.View().Options()),
Parameters: paramInfo,
}, activeParam, nil
}
Expand All @@ -134,7 +135,7 @@ func builtinSignature(ctx context.Context, snapshot Snapshot, callExpr *ast.Call
activeParam := activeParameter(callExpr, len(sig.params), sig.variadic, pos)
return &protocol.SignatureInformation{
Label: sig.name + sig.Format(),
Documentation: sig.doc,
Documentation: stringToSigInfoDocumentation(sig.doc, snapshot.View().Options()),
Parameters: paramInfo,
}, activeParam, nil

Expand Down Expand Up @@ -165,3 +166,21 @@ func activeParameter(callExpr *ast.CallExpr, numParams int, variadic bool, pos t
}
return activeParam
}

func stringToSigInfoDocumentation(s string, options *Options) *protocol.Or_SignatureInformation_documentation {
v := s
k := protocol.PlainText
if options.PreferredContentFormat == protocol.Markdown {
v = CommentToMarkdown(s)
// whether or not content is newline terminated may not matter for LSP clients,
// but our tests expect trailing newlines to be stripped.
v = strings.TrimSuffix(v, "\n") // TODO(pjw): change the golden files
k = protocol.Markdown
}
return &protocol.Or_SignatureInformation_documentation{
Value: protocol.MarkupContent{
Kind: k,
Value: v,
},
}
}
61 changes: 61 additions & 0 deletions gopls/internal/regtest/misc/hover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (
"testing"

"golang.org/x/tools/gopls/internal/lsp/fake"
"golang.org/x/tools/gopls/internal/lsp/protocol"
. "golang.org/x/tools/gopls/internal/lsp/regtest"
"golang.org/x/tools/internal/testenv"
)

func TestHoverUnexported(t *testing.T) {
Expand Down Expand Up @@ -270,3 +272,62 @@ go 1.16
env.Hover(env.RegexpSearch("go.mod", "go")) // no panic
})
}

func TestHoverCompletionMarkdown(t *testing.T) {
testenv.NeedsGo1Point(t, 19)
const source = `
-- go.mod --
module mod.com
go 1.19
-- main.go --
package main
// Just says [hello].
//
// [hello]: https://en.wikipedia.org/wiki/Hello
func Hello() string {
Hello() //Here
return "hello"
}
`
Run(t, source, func(t *testing.T, env *Env) {
// Hover, Completion, and SignatureHelp should all produce markdown
// check that the markdown for SignatureHelp and Completion are
// the same, and contained in that for Hover (up to trailing \n)
env.OpenFile("main.go")
loc := env.RegexpSearch("main.go", "func (Hello)")
hover, _ := env.Hover(loc)
hoverContent := hover.Value

loc = env.RegexpSearch("main.go", "//Here")
loc.Range.Start.Character -= 3 // Hello(_) //Here
completions := env.Completion(loc)
signatures := env.SignatureHelp(loc)

if len(completions.Items) != 1 {
t.Errorf("got %d completions, expected 1", len(completions.Items))
}
if len(signatures.Signatures) != 1 {
t.Errorf("got %d signatures, expected 1", len(signatures.Signatures))
}
item := completions.Items[0].Documentation.Value
var itemContent string
if x, ok := item.(protocol.MarkupContent); !ok || x.Kind != protocol.Markdown {
t.Fatalf("%#v is not markdown", item)
} else {
itemContent = strings.Trim(x.Value, "\n")
}
sig := signatures.Signatures[0].Documentation.Value
var sigContent string
if x, ok := sig.(protocol.MarkupContent); !ok || x.Kind != protocol.Markdown {
t.Fatalf("%#v is not markdown", item)
} else {
sigContent = x.Value
}
if itemContent != sigContent {
t.Errorf("item:%q not sig:%q", itemContent, sigContent)
}
if !strings.Contains(hoverContent, itemContent) {
t.Errorf("hover:%q does not containt sig;%q", hoverContent, sigContent)
}
})
}

0 comments on commit aa633e7

Please sign in to comment.