From 707862fa78b87f8fb5d33b3920ff5662a607f1cf Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 14 May 2019 21:20:41 -0400 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/cmd/cmd_test.go | 2 +- internal/lsp/lsp_test.go | 2 +- internal/lsp/source/completion_format.go | 39 ++++--- internal/lsp/source/signature_help.go | 116 ++++++++++++------- internal/lsp/source/source_test.go | 2 +- internal/lsp/testdata/signature/signature.go | 5 +- internal/lsp/tests/tests.go | 8 +- 7 files changed, 105 insertions(+), 69 deletions(-) diff --git a/internal/lsp/cmd/cmd_test.go b/internal/lsp/cmd/cmd_test.go index 54742380..316b46e7 100644 --- a/internal/lsp/cmd/cmd_test.go +++ b/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 } diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 05bae658..7c4943b5 100644 --- a/internal/lsp/lsp_test.go +++ b/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) diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index 68942548..da3774e8 100644 --- a/internal/lsp/source/completion_format.go +++ b/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() - if builtinPkg == nil || builtinPkg.Scope == nil { + decl := lookupBuiltin(c.view, obj.Name()) + if decl == nil { break } - fn := builtinPkg.Scope.Lookup(obj.Name()) - if fn == nil { - break - } - decl, ok := fn.Decl.(*ast.FuncDecl) - if !ok { - break - } - params, _ := c.formatFieldList(decl.Type.Params) - results, writeResultParens := c.formatFieldList(decl.Type.Results) + params, _ := formatFieldList(c.ctx, c.view, decl.Type.Params) + results, writeResultParens := formatFieldList(c.ctx, c.view, 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 { - c.view.Logger().Errorf(c.ctx, "unable to print type %v", p.Type) + if err := cfg.Fprint(b, v.FileSet(), p.Type); err != nil { + v.Logger().Errorf(ctx, "unable to print type %v", p.Type) continue } typ := replacer.Replace(b.String()) diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index 1d9f81f4..ea5a644c 100644 --- a/internal/lsp/source/signature_help.go +++ b/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 - for i := 0; i < sig.Params().Len(); i++ { - param := sig.Params().At(i) - label := types.TypeString(param.Type(), qf) - if sig.Variadic() && i == sig.Params().Len()-1 { - label = strings.Replace(label, "[]", "...", 1) - } - if param.Name() != "" { - label = fmt.Sprintf("%s %s", param.Name(), label) - } - paramInfo = append(paramInfo, ParameterInformation{ - Label: label, - }) - } + params := formatParams(sig.Params(), sig.Variadic(), qf) + results, writeResultParens := formatResults(sig.Results(), qf) + activeParam := activeParameter(callExpr, sig.Params().Len(), sig.Variadic(), pos) + var name string + if obj != nil { + name = obj.Name() + } else { + name = "func" + } + return signatureInformation(name, params, results, writeResultParens, activeParam), nil +} + +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 } - - // 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 + return activeParam } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 7913b9cd..70874309 100644 --- a/internal/lsp/source/source_test.go +++ b/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()) diff --git a/internal/lsp/testdata/signature/signature.go b/internal/lsp/testdata/signature/signature.go index 7e826a04..dd500350 100644 --- a/internal/lsp/testdata/signature/signature.go +++ b/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) } diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index 5ffe3acf..a2631601 100644 --- a/internal/lsp/tests/tests.go +++ b/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) {