internal/lsp: use builtin package for signature help

This change uses the builtin package to derive the signature help for
builtin functions.

Updates golang/go#31696

Change-Id: I458b3a89bdf143e7018e8be7cb6a5e8c068a47c2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/176922
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

internal/lsp/cmd/cmd_test.go

@@ -55,7 +55,7 @@ func (r *runner) Symbol(t *testing.T, data tests.Symbols) {
 	//TODO: add command line symbol tests when it works
 }
 
-func (r *runner) Signature(t *testing.T, data tests.Signatures) {
+func (r *runner) SignatureHelp(t *testing.T, data tests.Signatures) {
 	//TODO: add command line signature tests when it works
 }
 

internal/lsp/lsp_test.go

@@ -471,7 +471,7 @@ func summarizeSymbols(i int, want []source.Symbol, got []protocol.DocumentSymbol
 	return msg.String()
 }
 
-func (r *runner) Signature(t *testing.T, data tests.Signatures) {
+func (r *runner) SignatureHelp(t *testing.T, data tests.Signatures) {
 	for spn, expectedSignatures := range data {
 		m := r.mapper(spn.URI())
 		loc, err := m.Location(spn)

internal/lsp/source/completion_format.go

@@ -6,6 +6,7 @@ package source
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"go/ast"
 	"go/printer"
@@ -111,20 +112,12 @@ func (c *completer) formatBuiltin(obj types.Object, score float64) CompletionIte
 		item.Kind = ConstantCompletionItem
 	case *types.Builtin:
 		item.Kind = FunctionCompletionItem
-		builtinPkg := c.view.BuiltinPackage()
+		decl := lookupBuiltin(c.view, obj.Name())
-		if builtinPkg == nil || builtinPkg.Scope == nil {
+		if decl == nil {
 			break
 		}
-		fn := builtinPkg.Scope.Lookup(obj.Name())
+		params, _ := formatFieldList(c.ctx, c.view, decl.Type.Params)
-		if fn == nil {
+		results, writeResultParens := formatFieldList(c.ctx, c.view, decl.Type.Results)
-			break
-		}
-		decl, ok := fn.Decl.(*ast.FuncDecl)
-		if !ok {
-			break
-		}
-		params, _ := c.formatFieldList(decl.Type.Params)
-		results, writeResultParens := c.formatFieldList(decl.Type.Results)
 		item.Label, item.Detail = formatFunction(obj.Name(), params, results, writeResultParens)
 		item.plainSnippet, item.placeholderSnippet = c.functionCallSnippets(obj.Name(), params)
 	case *types.TypeName:
@@ -139,13 +132,29 @@ func (c *completer) formatBuiltin(obj types.Object, score float64) CompletionIte
 	return item
 }
 
+func lookupBuiltin(v View, name string) *ast.FuncDecl {
+	builtinPkg := v.BuiltinPackage()
+	if builtinPkg == nil || builtinPkg.Scope == nil {
+		return nil
+	}
+	fn := builtinPkg.Scope.Lookup(name)
+	if fn == nil {
+		return nil
+	}
+	decl, ok := fn.Decl.(*ast.FuncDecl)
+	if !ok {
+		return nil
+	}
+	return decl
+}
+
 var replacer = strings.NewReplacer(
 	`ComplexType`, `complex128`,
 	`FloatType`, `float64`,
 	`IntegerType`, `int`,
 )
 
-func (c *completer) formatFieldList(list *ast.FieldList) ([]string, bool) {
+func formatFieldList(ctx context.Context, v View, list *ast.FieldList) ([]string, bool) {
 	if list == nil {
 		return nil, false
 	}
@@ -158,8 +167,8 @@ func (c *completer) formatFieldList(list *ast.FieldList) ([]string, bool) {
 		p := list.List[i]
 		cfg := printer.Config{Mode: printer.UseSpaces | printer.TabIndent, Tabwidth: 4}
 		b := &bytes.Buffer{}
-		if err := cfg.Fprint(b, c.view.FileSet(), p.Type); err != nil {
+		if err := cfg.Fprint(b, v.FileSet(), p.Type); err != nil {
-			c.view.Logger().Errorf(c.ctx, "unable to print type %v", p.Type)
+			v.Logger().Errorf(ctx, "unable to print type %v", p.Type)
 			continue
 		}
 		typ := replacer.Replace(b.String())

internal/lsp/source/signature_help.go

@@ -10,7 +10,6 @@ import (
 	"go/ast"
 	"go/token"
 	"go/types"
-	"strings"
 
 	"golang.org/x/tools/go/ast/astutil"
 )
@@ -48,6 +47,22 @@ func SignatureHelp(ctx context.Context, f File, pos token.Pos) (*SignatureInform
 		return nil, fmt.Errorf("cannot find an enclosing function")
 	}
 
+	// Get the object representing the function, if available.
+	// There is no object in certain cases such as calling a function returned by
+	// a function (e.g. "foo()()").
+	var obj types.Object
+	switch t := callExpr.Fun.(type) {
+	case *ast.Ident:
+		obj = pkg.GetTypesInfo().ObjectOf(t)
+	case *ast.SelectorExpr:
+		obj = pkg.GetTypesInfo().ObjectOf(t.Sel)
+	}
+
+	// Handle builtin functions separately.
+	if obj, ok := obj.(*types.Builtin); ok {
+		return builtinSignature(ctx, f.View(), callExpr, obj.Name(), pos)
+	}
+
 	// Get the type information for the function being called.
 	sigType := pkg.GetTypesInfo().TypeOf(callExpr.Fun)
 	if sigType == nil {
@@ -60,21 +75,60 @@ func SignatureHelp(ctx context.Context, f File, pos token.Pos) (*SignatureInform
 	}
 
 	qf := qualifier(fAST, pkg.GetTypes(), pkg.GetTypesInfo())
-	var paramInfo []ParameterInformation
+	params := formatParams(sig.Params(), sig.Variadic(), qf)
-	for i := 0; i < sig.Params().Len(); i++ {
+	results, writeResultParens := formatResults(sig.Results(), qf)
-		param := sig.Params().At(i)
+	activeParam := activeParameter(callExpr, sig.Params().Len(), sig.Variadic(), pos)
-		label := types.TypeString(param.Type(), qf)
+
-		if sig.Variadic() && i == sig.Params().Len()-1 {
+	var name string
-			label = strings.Replace(label, "[]", "...", 1)
+	if obj != nil {
+		name = obj.Name()
+	} else {
+		name = "func"
 	}
-		if param.Name() != "" {
+	return signatureInformation(name, params, results, writeResultParens, activeParam), nil
-			label = fmt.Sprintf("%s %s", param.Name(), label)
-		}
-		paramInfo = append(paramInfo, ParameterInformation{
-			Label: label,
-		})
 }
 
+func builtinSignature(ctx context.Context, v View, callExpr *ast.CallExpr, name string, pos token.Pos) (*SignatureInformation, error) {
+	decl := lookupBuiltin(v, name)
+	if decl == nil {
+		return nil, fmt.Errorf("no function declaration for builtin: %s", name)
+	}
+	params, _ := formatFieldList(ctx, v, decl.Type.Params)
+	results, writeResultParens := formatFieldList(ctx, v, decl.Type.Results)
+
+	var (
+		numParams int
+		variadic  bool
+	)
+	if decl.Type.Params.List != nil {
+		numParams = len(decl.Type.Params.List)
+		lastParam := decl.Type.Params.List[numParams-1]
+		if _, ok := lastParam.Type.(*ast.Ellipsis); ok {
+			variadic = true
+		}
+	}
+	activeParam := activeParameter(callExpr, numParams, variadic, pos)
+	return signatureInformation(name, params, results, writeResultParens, activeParam), nil
+}
+
+func signatureInformation(name string, params, results []string, writeResultParens bool, activeParam int) *SignatureInformation {
+	paramInfo := make([]ParameterInformation, 0, len(params))
+	for _, p := range params {
+		paramInfo = append(paramInfo, ParameterInformation{Label: p})
+	}
+	label, detail := formatFunction(name, params, results, writeResultParens)
+	// Show return values of the function in the label.
+	if detail != "" {
+		label += " " + detail
+	}
+	return &SignatureInformation{
+		Label:           label,
+		Parameters:      paramInfo,
+		ActiveParameter: activeParam,
+	}
+}
+
+func activeParameter(callExpr *ast.CallExpr, numParams int, variadic bool, pos token.Pos) int {
 	// Determine the query position relative to the number of parameters in the function.
 	var activeParam int
 	var start, end token.Pos
@@ -88,38 +142,10 @@ func SignatureHelp(ctx context.Context, f File, pos token.Pos) (*SignatureInform
 		}
 
 		// Don't advance the active parameter for the last parameter of a variadic function.
-		if !sig.Variadic() || activeParam < sig.Params().Len()-1 {
+		if !variadic || activeParam < numParams-1 {
 			activeParam++
 		}
 		start = expr.Pos() + 1 // to account for commas
 	}
-
+	return activeParam
-	// Get the object representing the function, if available.
-	// There is no object in certain cases such as calling a function returned by
-	// a function (e.g. "foo()()").
-	var obj types.Object
-	switch t := callExpr.Fun.(type) {
-	case *ast.Ident:
-		obj = pkg.GetTypesInfo().ObjectOf(t)
-	case *ast.SelectorExpr:
-		obj = pkg.GetTypesInfo().ObjectOf(t.Sel)
-	}
-
-	var name string
-	if obj != nil {
-		name = obj.Name()
-	} else {
-		name = "func"
-	}
-
-	results, writeResultParens := formatResults(sig.Results(), qf)
-	label, detail := formatFunction(name, formatParams(sig.Params(), sig.Variadic(), qf), results, writeResultParens)
-	if sig.Results().Len() > 0 {
-		label += " " + detail
-	}
-	return &SignatureInformation{
-		Label:           label,
-		Parameters:      paramInfo,
-		ActiveParameter: activeParam,
-	}, nil
 }

internal/lsp/source/source_test.go

@@ -420,7 +420,7 @@ func summarizeSymbols(i int, want []source.Symbol, got []source.Symbol, reason s
 	return msg.String()
 }
 
-func (r *runner) Signature(t *testing.T, data tests.Signatures) {
+func (r *runner) SignatureHelp(t *testing.T, data tests.Signatures) {
 	ctx := context.Background()
 	for spn, expectedSignatures := range data {
 		f, err := r.view.GetFile(ctx, spn.URI())

internal/lsp/testdata/signature/signature.go

@@ -53,10 +53,11 @@ func Qux() {
 	var ms myStruct
 	ms.foo(nil) //@signature("nil", "foo(e *json.Decoder) (*big.Int, error)", 0)
 
-	_ = make([]int, 1, 2) //@signature("2", "make([]int, int, int) []int", 2)
+	_ = make([]int, 1, 2) //@signature("2", "make(t Type, size ...int) Type", 1)
 
 	Foo(myFunc(123), 456) //@signature("myFunc", "Foo(a string, b int) (c bool)", 0)
 	Foo(myFunc(123), 456) //@signature("123", "myFunc(foo int) string", 0)
 
-	panic("oops!") //@signature("oops", "panic(interface{})", 0)
+	panic("oops!")            //@signature("oops", "panic(v interface{})", 0)
+	println("hello", "world") //@signature("world", "println(args ...Type)", 0)
 }

internal/lsp/tests/tests.go

@@ -35,7 +35,7 @@ const (
 	ExpectedTypeDefinitionsCount   = 2
 	ExpectedHighlightsCount        = 2
 	ExpectedSymbolsCount           = 1
-	ExpectedSignaturesCount        = 19
+	ExpectedSignaturesCount        = 20
 	ExpectedCompletionSnippetCount = 11
 	ExpectedLinksCount             = 2
 )
@@ -89,7 +89,7 @@ type Tests interface {
 	Definition(*testing.T, Definitions)
 	Highlight(*testing.T, Highlights)
 	Symbol(*testing.T, Symbols)
-	Signature(*testing.T, Signatures)
+	SignatureHelp(*testing.T, Signatures)
 	Link(*testing.T, Links)
 }
 
@@ -291,12 +291,12 @@ func Run(t *testing.T, tests Tests, data *Data) {
 		tests.Symbol(t, data.Symbols)
 	})
 
-	t.Run("Signatures", func(t *testing.T) {
+	t.Run("SignatureHelp", func(t *testing.T) {
 		t.Helper()
 		if len(data.Signatures) != ExpectedSignaturesCount {
 			t.Errorf("got %v signatures expected %v", len(data.Signatures), ExpectedSignaturesCount)
 		}
-		tests.Signature(t, data.Signatures)
+		tests.SignatureHelp(t, data.Signatures)
 	})
 
 	t.Run("Links", func(t *testing.T) {