diff --git a/gopls/internal/goxls/testdata/overload/godef.gop b/gopls/internal/goxls/testdata/overload/godef.gop new file mode 100644 index 00000000000..48cfe63dbf1 --- /dev/null +++ b/gopls/internal/goxls/testdata/overload/godef.gop @@ -0,0 +1,57 @@ +import ( + "golang.org/lsptests/overload/a" + "golang.org/lsptests/overload/b" +) + +func _() { + add "hello" //@godef("add", addString) + add 100, 200 //@godef("add", addInt) + add 100, 200, 300 //@godef("add", addFloat) +} + +func _() { + var m M + m.add "hello" //@godef("add", m_addString) + m.add 100, 200 //@godef("add", m_addInt) + m.add 100, 200, 300 //@godef("add", m_addFloat) +} + +func _() { + Demo //@godef("Demo",Demo__0) + Demo 100 //@godef("Demo",Demo__1) + Demo 100, 200 //@godef("Demo",Demo__2) + Demo "hello" //@godef("Demo",Demo__3) +} + +func _() { + var n N + n.add 100 //@godef("add",Add__0) + n.add "hello" //@godef("add",Add__1) +} + +func _() { + a.demo //@godef("demo",a_demo__0) + a.demo 100 //@godef("demo",a_demo__1) + a.demo 100, 200 //@godef("demo",a_demo__2) + a.demo "hello" //@godef("demo",a_demo__3) +} + +func _() { + var n a.N + n.add 100 //@godef("add",a_add__0) + n.add "hello" //@godef("add",a_add__1) +} + +func _() { + b.gopDemo //@godef("gopDemo",b_gopdemo) + b.demo //@godef("demo",b_demo__0) + b.demo 100 //@godef("demo",b_demo__1) + b.demo 100, 200 //@godef("demo",b_demo__2) + b.demo "hello" //@godef("demo",b_demo__3) +} + +func _() { + var n b.N + n.add 100 //@godef("add",b_add__0) + n.add "hello" //@godef("add",b_add__1) +} diff --git a/gopls/internal/goxls/testdata/overload/overload.gop.golden b/gopls/internal/goxls/testdata/overload/godef.gop.golden similarity index 78% rename from gopls/internal/goxls/testdata/overload/overload.gop.golden rename to gopls/internal/goxls/testdata/overload/godef.gop.golden index c190441b1a6..1819749445c 100644 --- a/gopls/internal/goxls/testdata/overload/overload.gop.golden +++ b/gopls/internal/goxls/testdata/overload/godef.gop.golden @@ -66,6 +66,18 @@ func a.Demo__3(s ...string) ``` [`a.Demo__3` on pkg.go.dev](https://pkg.go.dev/golang.org/lsptests/overload/a#Demo__3) +-- addFloat-hoverdef -- +```go +func addFloat(n1 float64, n2 float64, n3 float64) float64 +``` +-- addInt-hoverdef -- +```go +func addInt(n1 int, n2 int) int +``` +-- addString-hoverdef -- +```go +func addString(s ...string) string +``` -- b_add__0-hoverdef -- ```go func (*b.N).Add__0(a int) @@ -108,33 +120,15 @@ func b.GopDemo() ``` [`b.GopDemo` on pkg.go.dev](https://pkg.go.dev/golang.org/lsptests/overload/b#GopDemo) --- pkg_add__0-hoverdef -- +-- m_addFloat-hoverdef -- ```go -func (*pkg.N).Add__0(a int) +func (*M).addFloat(n1 float64, n2 float64, n3 float64) float64 ``` - -[`(pkg.N).Add__0` on pkg.go.dev](https://pkg.go.dev/golang.org/lsptests/overload/pkg#N.Add__0) --- pkg_add__1-hoverdef -- +-- m_addInt-hoverdef -- ```go -func (*pkg.N).Add__1(a ...string) +func (*M).addInt(n1 int, n2 int) int ``` - -[`(pkg.N).Add__1` on pkg.go.dev](https://pkg.go.dev/golang.org/lsptests/overload/pkg#N.Add__1) --- pkg_demo__0-hoverdef -- +-- m_addString-hoverdef -- ```go -func pkg.Demo__0() +func (*M).addString(s ...string) string ``` - -[`pkg.Demo__0` on pkg.go.dev](https://pkg.go.dev/golang.org/lsptests/overload/pkg#Demo__0) --- pkg_demo__1-hoverdef -- -```go -func pkg.Demo__1(n int) int -``` - -[`pkg.Demo__1` on pkg.go.dev](https://pkg.go.dev/golang.org/lsptests/overload/pkg#Demo__1) --- pkg_demo__3-hoverdef -- -```go -func pkg.Demo__3(s ...string) -``` - -[`pkg.Demo__3` on pkg.go.dev](https://pkg.go.dev/golang.org/lsptests/overload/pkg#Demo__3) diff --git a/gopls/internal/goxls/testdata/overload/gop_autogen.go b/gopls/internal/goxls/testdata/overload/gop_autogen.go index 26751cdc203..4b02dc8c06e 100644 --- a/gopls/internal/goxls/testdata/overload/gop_autogen.go +++ b/gopls/internal/goxls/testdata/overload/gop_autogen.go @@ -3,6 +3,8 @@ package main import ( + "strings" + "golang.org/lsptests/overload/a" "golang.org/lsptests/overload/b" ) diff --git a/gopls/internal/goxls/testdata/overload/overload.gop b/gopls/internal/goxls/testdata/overload/overload.gop index 32a305086a3..635eac1eeff 100644 --- a/gopls/internal/goxls/testdata/overload/overload.gop +++ b/gopls/internal/goxls/testdata/overload/overload.gop @@ -1,32 +1,42 @@ import ( - "golang.org/lsptests/overload/a" - "golang.org/lsptests/overload/b" + "strings" ) -Demo //@godef("Demo",Demo__0) -Demo 100 //@godef("Demo",Demo__1) -Demo 100, 200 //@godef("Demo",Demo__2) -Demo "hello" //@godef("Demo",Demo__3) - -var n N -n.add 100 //@godef("add",Add__0) -n.add "hello" //@godef("add",Add__1) - -a.demo //@godef("demo",a_demo__0) -a.demo 100 //@godef("demo",a_demo__1) -a.demo 100, 200 //@godef("demo",a_demo__2) -a.demo "hello" //@godef("demo",a_demo__3) - -var n1 a.N -n1.add 100 //@godef("add",a_add__0) -n1.add "hello" //@godef("add",a_add__1) - -b.gopDemo //@godef("gopDemo",b_gopdemo) -b.demo //@godef("demo",b_demo__0) -b.demo 100 //@godef("demo",b_demo__1) -b.demo 100, 200 //@godef("demo",b_demo__2) -b.demo "hello" //@godef("demo",b_demo__3) - -var n2 b.N -n2.add 100 //@godef("add",b_add__0) -n2.add "hello" //@godef("add",b_add__1) +func addInt(n1, n2 int) int { //@addInt + return n1 + n2 +} + +func addFloat(n1, n2, n3 float64) float64 { //@addFloat + return n1 + n2 + n3 +} + +func addString(s ...string) string { //@addString + return strings.Join(s, "") +} + +func add = ( + addInt + addFloat + addString +) + +type M struct { +} + +func (m *M) addInt(n1, n2 int) int { //@mark(m_addInt,"addInt") + return n1 + n2 +} + +func (m *M) addFloat(n1, n2, n3 float64) float64 { //@mark(m_addFloat,"addFloat") + return n1 + n2 + n3 +} + +func (m *M) addString(s ...string) string { //@mark(m_addString,"addString") + return strings.Join(s, "") +} + +func (M).add = ( + (M).addInt + (M).addFloat + (M).addString +) diff --git a/gopls/internal/goxls/testdata/overload/signature.gop b/gopls/internal/goxls/testdata/overload/signature.gop new file mode 100644 index 00000000000..300c44f202a --- /dev/null +++ b/gopls/internal/goxls/testdata/overload/signature.gop @@ -0,0 +1,29 @@ +func _() { + Demo(100) //@signature_overload("100", "Demo(n int) int", 1, 0) + Demo(100, 200) //@signature_overload("100", "Demo(n1 int, n2 int)", 2, 0) + Demo(100, 200) //@signature_overload("200", "Demo(n1 int, n2 int)", 2, 1) +} + +func _() { + Demo 100 //@signature_overload("100", "Demo(n int) int", 1, 0) + Demo 100, 200 //@signature_overload("100", "Demo(n1 int, n2 int)", 2, 0) + Demo 100, 200 //@signature_overload("200", "Demo(n1 int, n2 int)", 2, 1) +} + +func _() { + add(100, 200) //@signature_overload("100", "add(n1 int, n2 int) int", 0, 0) + add(100, 200) //@signature_overload("200", "add(n1 int, n2 int) int", 0, 1) + add(100, 200, 300) //@signature_overload("100", "add(n1 float64, n2 float64, n3 float64) float64", 1, 0) + add(100, 200, 300) //@signature_overload(", 3", "add(n1 float64, n2 float64, n3 float64) float64", 1, 1) + add(100, 200, 300) //@signature_overload(" 3", "add(n1 float64, n2 float64, n3 float64) float64", 1, 2) + add(100, 200, 300) //@signature_overload(")", "add(n1 float64, n2 float64, n3 float64) float64", 1, 2) +} + +func _() { + add 100, 200 //@signature_overload("100", "add(n1 int, n2 int) int", 0, 0) + add 100, 200 //@signature_overload("200", "add(n1 int, n2 int) int", 0, 1) + add 100, 200, 300 //@signature_overload("100", "add(n1 float64, n2 float64, n3 float64) float64", 1, 0) + add 100, 200, 300 //@signature_overload(", 3", "add(n1 float64, n2 float64, n3 float64) float64", 1, 1) + add 100, 200, 300 //@signature_overload(" 3", "add(n1 float64, n2 float64, n3 float64) float64", 1, 2) + add 100, 200, 300 //@signature_overload(" //", "add(n1 float64, n2 float64, n3 float64) float64", 1, 2) +} diff --git a/gopls/internal/goxls/testdata/overload/signature2.gop.in b/gopls/internal/goxls/testdata/overload/signature2.gop.in new file mode 100644 index 00000000000..68ed92b57d1 --- /dev/null +++ b/gopls/internal/goxls/testdata/overload/signature2.gop.in @@ -0,0 +1,3 @@ +func _() { + Demo //@signature_overload(" //", "Demo()", 0, 0) +} diff --git a/gopls/internal/goxls/testdata/overload/signature3.gop.in b/gopls/internal/goxls/testdata/overload/signature3.gop.in new file mode 100644 index 00000000000..d95c8c81c51 --- /dev/null +++ b/gopls/internal/goxls/testdata/overload/signature3.gop.in @@ -0,0 +1,3 @@ +func _() { + Demo 100,//@signature_overload("//", "Demo(n1 int, n2 int)", 2, 1) +} diff --git a/gopls/internal/goxls/testdata/summary.txt.golden b/gopls/internal/goxls/testdata/summary.txt.golden index 70d47218cfc..c91dfa1884f 100644 --- a/gopls/internal/goxls/testdata/summary.txt.golden +++ b/gopls/internal/goxls/testdata/summary.txt.golden @@ -13,7 +13,7 @@ FoldingRangesCount = 0 SemanticTokenCount = 0 SuggestedFixCount = 0 MethodExtractionCount = 0 -DefinitionsCount = 23 +DefinitionsCount = 29 TypeDefinitionsCount = 1 HighlightsCount = 0 InlayHintsCount = 0 diff --git a/gopls/internal/goxls/testdata/summary_go1.18.txt.golden b/gopls/internal/goxls/testdata/summary_go1.18.txt.golden index 70d47218cfc..c91dfa1884f 100644 --- a/gopls/internal/goxls/testdata/summary_go1.18.txt.golden +++ b/gopls/internal/goxls/testdata/summary_go1.18.txt.golden @@ -13,7 +13,7 @@ FoldingRangesCount = 0 SemanticTokenCount = 0 SuggestedFixCount = 0 MethodExtractionCount = 0 -DefinitionsCount = 23 +DefinitionsCount = 29 TypeDefinitionsCount = 1 HighlightsCount = 0 InlayHintsCount = 0 diff --git a/gopls/internal/goxls/testdata/summary_go1.21.txt.golden b/gopls/internal/goxls/testdata/summary_go1.21.txt.golden index 70d47218cfc..c91dfa1884f 100644 --- a/gopls/internal/goxls/testdata/summary_go1.21.txt.golden +++ b/gopls/internal/goxls/testdata/summary_go1.21.txt.golden @@ -13,7 +13,7 @@ FoldingRangesCount = 0 SemanticTokenCount = 0 SuggestedFixCount = 0 MethodExtractionCount = 0 -DefinitionsCount = 23 +DefinitionsCount = 29 TypeDefinitionsCount = 1 HighlightsCount = 0 InlayHintsCount = 0 diff --git a/gopls/internal/lsp/goxls_test.go b/gopls/internal/lsp/goxls_test.go index 5226f335aa0..bf1160f413e 100644 --- a/gopls/internal/lsp/goxls_test.go +++ b/gopls/internal/lsp/goxls_test.go @@ -1,13 +1,17 @@ package lsp import ( + "fmt" "path/filepath" + "strings" "testing" "golang.org/x/tools/gopls/internal/lsp/cache" "golang.org/x/tools/gopls/internal/lsp/debug" + "golang.org/x/tools/gopls/internal/lsp/protocol" "golang.org/x/tools/gopls/internal/lsp/source" "golang.org/x/tools/gopls/internal/lsp/tests" + "golang.org/x/tools/gopls/internal/lsp/tests/compare" "golang.org/x/tools/gopls/internal/span" "golang.org/x/tools/internal/testenv" ) @@ -98,4 +102,102 @@ func testXLS(t *testing.T, datum *tests.Data) { } r.server = NewServer(session, testClient{runner: r}) tests.Run(t, r, datum) + // test signature_overload + testSignatureHelpOverload(t, r, datum) +} + +func testSignatureHelpOverload(t *testing.T, r *runner, datum *tests.Data) { + // Collect names for the entries that require golden files. + signatures := make(map[span.Span]*protocol.SignatureHelp) + collectSignatures := func(spn span.Span, signature string, activeSignature int64, activeParam int64) { + signatures[spn] = &protocol.SignatureHelp{ + Signatures: []protocol.SignatureInformation{ + { + Label: signature, + }, + }, + ActiveSignature: uint32(activeSignature), + ActiveParameter: uint32(activeParam), + } + // Hardcode special case to test the lack of a signature. + if signature == "" && activeParam == 0 { + signatures[spn] = nil + } + } + + if err := datum.Exported.Expect(map[string]interface{}{ + "signature_overload": collectSignatures, + }); err != nil { + t.Fatal(err) + } + + t.Run("SignatureHelpOverload", func(t *testing.T) { + t.Helper() + for spn, expectedSignature := range signatures { + t.Run(tests.SpanName(spn), func(t *testing.T) { + t.Helper() + r.SignatureHelpOverload(t, spn, expectedSignature) + }) + } + }) +} + +func (r *runner) SignatureHelpOverload(t *testing.T, spn span.Span, want *protocol.SignatureHelp) { + m, err := r.data.Mapper(spn.URI()) + if err != nil { + t.Fatal(err) + } + loc, err := m.SpanLocation(spn) + if err != nil { + t.Fatalf("failed for %v: %v", loc, err) + } + params := &protocol.SignatureHelpParams{ + TextDocumentPositionParams: protocol.LocationTextDocumentPositionParams(loc), + } + got, err := r.server.SignatureHelp(r.ctx, params) + if err != nil { + // Only fail if we got an error we did not expect. + if want != nil { + t.Fatal(err) + } + return + } + if want == nil { + if got != nil { + t.Errorf("expected no signature, got %v", got) + } + return + } + if got == nil { + t.Fatalf("expected %v, got nil", want) + } + if diff := DiffSignaturesOverload(spn, want, got); diff != "" { + t.Error(diff) + } +} + +func DiffSignaturesOverload(spn span.Span, want, got *protocol.SignatureHelp) string { + decorate := func(f string, args ...interface{}) string { + return fmt.Sprintf("invalid signature at %s: %s", spn, fmt.Sprintf(f, args...)) + } + if want.ActiveSignature != got.ActiveSignature { + return decorate("wanted active signature of %d, got %d", want.ActiveSignature, int(got.ActiveSignature)) + } + if want.ActiveParameter != got.ActiveParameter { + return decorate("wanted active parameter of %d, got %d", want.ActiveParameter, int(got.ActiveParameter)) + } + g := got.Signatures[got.ActiveSignature] + w := want.Signatures[0] + if diff := compare.Text(tests.NormalizeAny(w.Label), tests.NormalizeAny(g.Label)); diff != "" { + return decorate("mismatched labels:\n%s", diff) + } + var paramParts []string + for _, p := range g.Parameters { + paramParts = append(paramParts, p.Label) + } + paramsStr := strings.Join(paramParts, ", ") + if !strings.Contains(g.Label, paramsStr) { + return decorate("expected signature %q to contain params %q", g.Label, paramsStr) + } + return "" } diff --git a/gopls/internal/lsp/source/signature_help_gox.go b/gopls/internal/lsp/source/signature_help_gox.go index c0fdecaf021..c8aaba54300 100644 --- a/gopls/internal/lsp/source/signature_help_gox.go +++ b/gopls/internal/lsp/source/signature_help_gox.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "go/types" + "strings" "github.com/goplus/gop/ast" "github.com/goplus/gop/token" @@ -37,7 +38,7 @@ func GopSignatureHelp(ctx context.Context, snapshot Snapshot, fh FileHandle, pos } var offset = int(pos - start) npos := pos - if len(pgf.File.Code) >= offset { + if len(pgf.File.Code) > offset { if pgf.File.Code[offset] == '\n' { offset-- npos-- @@ -124,8 +125,9 @@ FindCall: var sigType types.Type if callExpr != nil { sigType = pkg.GopTypesInfo().TypeOf(callExpr.Fun) - } else { - sigType = cmdObj.Type() + } + if sigType == nil && obj != nil { + sigType = obj.Type() } if sigType == nil { return nil, 0, 0, fmt.Errorf("cannot get type for Fun %[1]T (%[1]v)", callExpr.Fun) @@ -145,12 +147,14 @@ FindCall: comment *ast.CommentGroup ) if obj != nil { - d, err := HoverDocForObject(ctx, snapshot, pkg.FileSet(), obj) - if err != nil && !overloads { - return nil, 0, 0, err - } name = obj.Name() - comment = d + if !overloads { + d, err := HoverDocForObject(ctx, snapshot, pkg.FileSet(), obj) + if err != nil { + return nil, 0, 0, err + } + comment = d + } } else { name = "func" } @@ -171,21 +175,62 @@ FindCall: Parameters: paramInfo, }, nil } + makeInfoEx := func(name string, obj types.Object, sig *types.Signature) (*protocol.SignatureInformation, string, error) { + d, err := HoverDocForObject(ctx, snapshot, pkg.FileSet(), obj) + if err != nil { + return nil, "", err + } + s, err := NewSignature(ctx, snapshot, pkg, sig, d, qf, mq) + if err != nil { + return nil, "", err + } + paramInfo := make([]protocol.ParameterInformation, 0, len(s.params)) + for _, p := range s.params { + paramInfo = append(paramInfo, protocol.ParameterInformation{Label: p}) + } + return &protocol.SignatureInformation{ + Label: name + s.Format(), + Documentation: stringToSigInfoDocumentation(s.doc, snapshot.View().Options()), + Parameters: paramInfo, + }, s.doc, nil + } if overloads { - activeSignature := 0 + var activeSignature int + var matchSignature []int infos := make([]protocol.SignatureInformation, len(objs)) + docs := make([]string, len(objs)) for i, o := range objs { + sig := o.Type().(*types.Signature) + info, doc, err := makeInfoEx(ident.Name, o, sig) + if err != nil { + return nil, 0, 0, nil + } if o.Name() == obj.Name() { activeSignature = i } - info, err := makeInfo(o.Name(), o.Type().(*types.Signature)) - if err != nil { - return nil, 0, 0, nil + if sig.Variadic() || (sig.Params() != nil && sig.Params().Len() > activeParam) || (sig.Params() == nil && activeParam == 0) { + matchSignature = append(matchSignature, i) } infos[i] = *info + docs[i] = doc } - return infos, activeSignature, activeParam, nil + for i := 0; i < len(infos); i++ { + var doc string + for j, v := range infos { + if i == j { + doc += "```doc\n* " + v.Label + "\n```\n" + } else { + doc += "```doc\n " + v.Label + "\n```\n" + } + } + if s := docs[i]; s != "" { + doc += "---\n" + doc += stringToDocumentation(s, snapshot.View().Options()) + } + infos[i].Documentation = stringToMarkDown(doc) + } + return infos, checkBestSignature(activeSignature, matchSignature), activeParam, nil } info, err := makeInfo(name, sig) if err != nil { @@ -239,3 +284,34 @@ func gopActiveParameter(callExpr *ast.CallExpr, numParams int, variadic bool, po } return activeParam } + +func checkBestSignature(active int, matches []int) int { + if len(matches) == 0 { + return active + } + for _, n := range matches { + if active == n { + return active + } + } + return matches[0] +} + +func stringToMarkDown(s string) *protocol.Or_SignatureInformation_documentation { + k := protocol.Markdown + return &protocol.Or_SignatureInformation_documentation{ + Value: protocol.MarkupContent{ + Kind: k, + Value: s, + }, + } +} + +func stringToDocumentation(v string, options *Options) string { + if options.PreferredContentFormat == protocol.Markdown { + v = CommentToMarkdown(v, options) + v = strings.TrimSuffix(v, "\n") // TODO(pjw): change the golden files + return v + } + return v +}