diff --git a/internal/lsp/cmd/test/signature.go b/internal/lsp/cmd/test/signature.go index 8f55aeaa35f..4e2726cfa65 100644 --- a/internal/lsp/cmd/test/signature.go +++ b/internal/lsp/cmd/test/signature.go @@ -25,8 +25,7 @@ func (r *runner) SignatureHelp(t *testing.T, spn span.Span, expectedSignature *s expect := string(r.data.Golden(goldenTag, filename, func() ([]byte, error) { return []byte(got), nil })) - if expect != got { - t.Errorf("signature failed failed for %s expected:\n%s\ngot:\n%s", filename, expect, got) + t.Errorf("signature failed for %s expected:\n%q\ngot:\n%q'", filename, expect, got) } } diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index 67148c91f9d..dc5d245bcc0 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -47,7 +47,7 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { // expandFuncCall mutates the completion label, detail, and snippet // to that of an invocation of sig. expandFuncCall := func(sig *types.Signature) { - params := formatParams(sig.Params(), sig.Variadic(), c.qf) + params := formatParams(c.snapshot, c.pkg, sig, c.qf) snip = c.functionCallSnippet(label, params) results, writeParens := formatResults(sig.Results(), c.qf) detail = "func" + formatFunction(params, results, writeParens) @@ -66,6 +66,12 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { case *types.Var: if _, ok := obj.Type().(*types.Struct); ok { detail = "struct{...}" // for anonymous structs + } else if obj.IsField() { + var err error + detail, err = formatFieldType(c.snapshot, c.pkg, obj, c.qf) + if err != nil { + detail = types.TypeString(obj.Type(), c.qf) + } } if obj.IsField() { kind = protocol.FieldCompletion @@ -156,12 +162,10 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { } file, _, pkg, err := c.snapshot.View().FindPosInPackage(searchPkg, obj.Pos()) if err != nil { - log.Error(c.ctx, "error finding file in package", err, telemetry.URI.Of(uri), telemetry.Package.Of(searchPkg.ID())) return item, nil } - ident, err := findIdentifier(c.ctx, c.snapshot, pkg, file, obj.Pos()) + ident, err := findIdentifier(c.snapshot, pkg, file, obj.Pos()) if err != nil { - log.Error(c.ctx, "failed to findIdentifier", err, telemetry.URI.Of(uri)) return item, nil } hover, err := ident.Hover(c.ctx) diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go index fef7e94b887..f9aec593bdc 100644 --- a/internal/lsp/source/highlight.go +++ b/internal/lsp/source/highlight.go @@ -87,7 +87,7 @@ Outer: var result []protocol.Range // Add the for statement. - forStmt, err := posToRange(ctx, snapshot.View(), m, loop.Pos(), loop.Pos()+3) + forStmt, err := posToRange(snapshot.View(), m, loop.Pos(), loop.Pos()+3) if err != nil { return nil, err } diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index 5e5f4c84283..d2761dae6b7 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -47,6 +47,9 @@ type Declaration struct { // Identifier returns identifier information for a position // in a file, accounting for a potentially incomplete selector. func Identifier(ctx context.Context, snapshot Snapshot, f File, pos protocol.Position) (*IdentifierInfo, error) { + ctx, done := trace.StartSpan(ctx, "source.Identifier") + defer done() + cphs, err := snapshot.PackageHandles(ctx, f) if err != nil { return nil, err @@ -75,17 +78,17 @@ func Identifier(ctx context.Context, snapshot Snapshot, f File, pos protocol.Pos if err != nil { return nil, err } - return findIdentifier(ctx, snapshot, pkg, file, rng.Start) + return findIdentifier(snapshot, pkg, file, rng.Start) } -func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { - if result, err := identifier(ctx, snapshot, pkg, file, pos); err != nil || result != nil { +func findIdentifier(snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { + if result, err := identifier(snapshot, pkg, file, pos); err != nil || result != nil { return result, err } // If the position is not an identifier but immediately follows // an identifier or selector period (as is common when // requesting a completion), use the path to the preceding node. - ident, err := identifier(ctx, snapshot, pkg, file, pos-1) + ident, err := identifier(snapshot, pkg, file, pos-1) if ident == nil && err == nil { err = errors.New("no identifier found") } @@ -93,21 +96,18 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, file *a } // identifier checks a single position for a potential identifier. -func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { - ctx, done := trace.StartSpan(ctx, "source.identifier") - defer done() - +func identifier(s Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { var err error // Handle import specs separately, as there is no formal position for a package declaration. - if result, err := importSpec(ctx, snapshot, pkg, file, pos); result != nil || err != nil { + if result, err := importSpec(s, pkg, file, pos); result != nil || err != nil { return result, err } path, _ := astutil.PathEnclosingInterval(file, pos, pos) if path == nil { return nil, errors.Errorf("can't find node enclosing position") } - view := snapshot.View() + view := s.View() uri := span.FileURI(view.Session().Cache().FileSet().Position(pos).Filename) var ph ParseGoHandle for _, h := range pkg.CompiledGoFiles() { @@ -117,7 +117,7 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F } result := &IdentifierInfo{ File: ph, - Snapshot: snapshot, + Snapshot: s, qf: qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo()), pkg: pkg, ident: searchForIdent(path[0]), @@ -134,7 +134,7 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F } } result.Name = result.ident.Name - if result.mappedRange, err = posToMappedRange(ctx, view, pkg, result.ident.Pos(), result.ident.End()); err != nil { + if result.mappedRange, err = posToMappedRange(view, pkg, result.ident.Pos(), result.ident.End()); err != nil { return nil, err } result.Declaration.obj = pkg.GetTypesInfo().ObjectOf(result.ident) @@ -165,7 +165,7 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F return nil, errors.Errorf("no declaration for %s", result.Name) } result.Declaration.node = decl - if result.Declaration.mappedRange, err = nameToMappedRange(ctx, view, pkg, decl.Pos(), result.Name); err != nil { + if result.Declaration.mappedRange, err = nameToMappedRange(view, pkg, decl.Pos(), result.Name); err != nil { return nil, err } return result, nil @@ -190,10 +190,10 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F } } - if result.Declaration.mappedRange, err = objToMappedRange(ctx, view, pkg, result.Declaration.obj); err != nil { + if result.Declaration.mappedRange, err = objToMappedRange(view, pkg, result.Declaration.obj); err != nil { return nil, err } - if result.Declaration.node, err = objToNode(ctx, snapshot.View(), pkg, result.Declaration.obj); err != nil { + if result.Declaration.node, err = objToNode(s.View(), pkg, result.Declaration.obj); err != nil { return nil, err } typ := pkg.GetTypesInfo().TypeOf(result.ident) @@ -207,7 +207,7 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F if hasErrorType(result.Type.Object) { return result, nil } - if result.Type.mappedRange, err = objToMappedRange(ctx, view, pkg, result.Type.Object); err != nil { + if result.Type.mappedRange, err = objToMappedRange(view, pkg, result.Type.Object); err != nil { return nil, err } } @@ -241,7 +241,7 @@ func hasErrorType(obj types.Object) bool { return types.IsInterface(obj.Type()) && obj.Pkg() == nil && obj.Name() == "error" } -func objToNode(ctx context.Context, v View, pkg Package, obj types.Object) (ast.Decl, error) { +func objToNode(v View, pkg Package, obj types.Object) (ast.Decl, error) { declAST, _, _, err := v.FindPosInPackage(pkg, obj.Pos()) if err != nil { return nil, err @@ -269,7 +269,7 @@ func objToNode(ctx context.Context, v View, pkg Package, obj types.Object) (ast. } // importSpec handles positions inside of an *ast.ImportSpec. -func importSpec(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { +func importSpec(s Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { var imp *ast.ImportSpec for _, spec := range file.Imports { if spec.Path.Pos() <= pos && pos < spec.Path.End() { @@ -283,7 +283,7 @@ func importSpec(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F if err != nil { return nil, errors.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err) } - uri := span.FileURI(snapshot.View().Session().Cache().FileSet().Position(pos).Filename) + uri := span.FileURI(s.View().Session().Cache().FileSet().Position(pos).Filename) var ph ParseGoHandle for _, h := range pkg.CompiledGoFiles() { if h.File().Identity().URI == uri { @@ -292,11 +292,11 @@ func importSpec(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F } result := &IdentifierInfo{ File: ph, - Snapshot: snapshot, + Snapshot: s, Name: importPath, pkg: pkg, } - if result.mappedRange, err = posToMappedRange(ctx, snapshot.View(), pkg, imp.Path.Pos(), imp.Path.End()); err != nil { + if result.mappedRange, err = posToMappedRange(s.View(), pkg, imp.Path.Pos(), imp.Path.End()); err != nil { return nil, err } // Consider the "declaration" of an import spec to be the imported package. @@ -317,7 +317,7 @@ func importSpec(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F if dest == nil { return nil, errors.Errorf("package %q has no files", importPath) } - if result.Declaration.mappedRange, err = posToMappedRange(ctx, snapshot.View(), pkg, dest.Pos(), dest.End()); err != nil { + if result.Declaration.mappedRange, err = posToMappedRange(s.View(), pkg, dest.Pos(), dest.End()); err != nil { return nil, err } result.Declaration.node = imp diff --git a/internal/lsp/source/implementation.go b/internal/lsp/source/implementation.go index 4a4452f1d77..855f314ccfd 100644 --- a/internal/lsp/source/implementation.go +++ b/internal/lsp/source/implementation.go @@ -69,7 +69,7 @@ func (i *IdentifierInfo) Implementation(ctx context.Context) ([]protocol.Locatio if err != nil { return nil, err } - ident, err := findIdentifier(ctx, i.Snapshot, pkg, file, obj.Pos()) + ident, err := findIdentifier(i.Snapshot, pkg, file, obj.Pos()) if err != nil { return nil, err } diff --git a/internal/lsp/source/references.go b/internal/lsp/source/references.go index 1351dfc6c36..921a7564450 100644 --- a/internal/lsp/source/references.go +++ b/internal/lsp/source/references.go @@ -56,7 +56,7 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro if obj == nil || !sameObj(obj, i.Declaration.obj) { continue } - rng, err := posToMappedRange(ctx, i.Snapshot.View(), i.pkg, ident.Pos(), ident.End()) + rng, err := posToMappedRange(i.Snapshot.View(), i.pkg, ident.Pos(), ident.End()) if err != nil { return nil, err } @@ -84,7 +84,7 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro if obj == nil || !(sameObj(obj, i.Declaration.obj)) { continue } - rng, err := posToMappedRange(ctx, i.Snapshot.View(), pkg, ident.Pos(), ident.End()) + rng, err := posToMappedRange(i.Snapshot.View(), pkg, ident.Pos(), ident.End()) if err != nil { return nil, err } diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 312cf7c8831..1df62d001ba 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -222,10 +222,10 @@ func getPkgNameIdentifier(ctx context.Context, ident *IdentifierInfo, pkgName *t wasImplicit: true, } var err error - if decl.mappedRange, err = objToMappedRange(ctx, ident.Snapshot.View(), ident.pkg, decl.obj); err != nil { + if decl.mappedRange, err = objToMappedRange(ident.Snapshot.View(), ident.pkg, decl.obj); err != nil { return nil, err } - if decl.node, err = objToNode(ctx, ident.Snapshot.View(), ident.pkg, decl.obj); err != nil { + if decl.node, err = objToNode(ident.Snapshot.View(), ident.pkg, decl.obj); err != nil { return nil, err } return &IdentifierInfo{ diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index 2b2582a6aae..3e422d016da 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -112,7 +112,7 @@ FindCall: } qf := qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo()) - params := formatParams(sig.Params(), sig.Variadic(), qf) + params := formatParams(snapshot, pkg, sig, qf) results, writeResultParens := formatResults(sig.Results(), qf) activeParam := activeParameter(callExpr, sig.Params().Len(), sig.Variadic(), rng.Start) @@ -121,11 +121,11 @@ FindCall: comment *ast.CommentGroup ) if obj != nil { - node, err := objToNode(ctx, snapshot.View(), pkg, obj) + node, err := objToNode(snapshot.View(), pkg, obj) if err != nil { return nil, err } - rng, err := objToMappedRange(ctx, snapshot.View(), pkg, obj) + rng, err := objToMappedRange(snapshot.View(), pkg, obj) if err != nil { return nil, err } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 57ad8d92a79..7ad9625fc87 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -5,9 +5,11 @@ package source import ( + "bytes" "context" "fmt" "go/ast" + "go/printer" "go/token" "go/types" "path/filepath" @@ -120,14 +122,14 @@ func IsGenerated(ctx context.Context, view View, uri span.URI) bool { } func nodeToProtocolRange(ctx context.Context, view View, m *protocol.ColumnMapper, n ast.Node) (protocol.Range, error) { - mrng, err := nodeToMappedRange(ctx, view, m, n) + mrng, err := nodeToMappedRange(view, m, n) if err != nil { return protocol.Range{}, err } return mrng.Range() } -func objToMappedRange(ctx context.Context, v View, pkg Package, obj types.Object) (mappedRange, error) { +func objToMappedRange(v View, pkg Package, obj types.Object) (mappedRange, error) { if pkgName, ok := obj.(*types.PkgName); ok { // An imported Go package has a package-local, unqualified name. // When the name matches the imported package name, there is no @@ -140,29 +142,29 @@ func objToMappedRange(ctx context.Context, v View, pkg Package, obj types.Object // When the identifier does not appear in the source, have the range // of the object be the import path, including quotes. if pkgName.Imported().Name() == pkgName.Name() { - return posToMappedRange(ctx, v, pkg, obj.Pos(), obj.Pos()+token.Pos(len(pkgName.Imported().Path())+2)) + return posToMappedRange(v, pkg, obj.Pos(), obj.Pos()+token.Pos(len(pkgName.Imported().Path())+2)) } } - return nameToMappedRange(ctx, v, pkg, obj.Pos(), obj.Name()) + return nameToMappedRange(v, pkg, obj.Pos(), obj.Name()) } -func nameToMappedRange(ctx context.Context, v View, pkg Package, pos token.Pos, name string) (mappedRange, error) { - return posToMappedRange(ctx, v, pkg, pos, pos+token.Pos(len(name))) +func nameToMappedRange(v View, pkg Package, pos token.Pos, name string) (mappedRange, error) { + return posToMappedRange(v, pkg, pos, pos+token.Pos(len(name))) } -func nodeToMappedRange(ctx context.Context, view View, m *protocol.ColumnMapper, n ast.Node) (mappedRange, error) { - return posToRange(ctx, view, m, n.Pos(), n.End()) +func nodeToMappedRange(view View, m *protocol.ColumnMapper, n ast.Node) (mappedRange, error) { + return posToRange(view, m, n.Pos(), n.End()) } -func posToMappedRange(ctx context.Context, v View, pkg Package, pos, end token.Pos) (mappedRange, error) { +func posToMappedRange(v View, pkg Package, pos, end token.Pos) (mappedRange, error) { _, m, _, err := v.FindPosInPackage(pkg, pos) if err != nil { return mappedRange{}, err } - return posToRange(ctx, v, m, pos, end) + return posToRange(v, m, pos, end) } -func posToRange(ctx context.Context, view View, m *protocol.ColumnMapper, pos, end token.Pos) (mappedRange, error) { +func posToRange(view View, m *protocol.ColumnMapper, pos, end token.Pos) (mappedRange, error) { if !pos.IsValid() { return mappedRange{}, errors.Errorf("invalid position for %v", pos) } @@ -378,14 +380,17 @@ func typeConversion(call *ast.CallExpr, info *types.Info) types.Type { return nil } -func formatParams(tup *types.Tuple, variadic bool, qf types.Qualifier) []string { - params := make([]string, 0, tup.Len()) - for i := 0; i < tup.Len(); i++ { - el := tup.At(i) - typ := types.TypeString(el.Type(), qf) +func formatParams(s Snapshot, pkg Package, sig *types.Signature, qf types.Qualifier) []string { + params := make([]string, 0, sig.Params().Len()) + for i := 0; i < sig.Params().Len(); i++ { + el := sig.Params().At(i) + typ, err := formatFieldType(s, pkg, el, qf) + if err != nil { + typ = types.TypeString(el.Type(), qf) + } // Handle a variadic parameter (can only be the final parameter). - if variadic && i == tup.Len()-1 { + if sig.Variadic() && i == sig.Params().Len()-1 { typ = strings.Replace(typ, "[]", "...", 1) } @@ -398,6 +403,30 @@ func formatParams(tup *types.Tuple, variadic bool, qf types.Qualifier) []string return params } +func formatFieldType(s Snapshot, srcpkg Package, obj types.Object, qf types.Qualifier) (string, error) { + file, _, pkg, err := s.View().FindPosInPackage(srcpkg, obj.Pos()) + if err != nil { + return "", err + } + ident, err := findIdentifier(s, pkg, file, obj.Pos()) + if err != nil { + return "", err + } + if i := ident.ident; i == nil || i.Obj == nil || i.Obj.Decl == nil { + return "", errors.Errorf("no object for ident %v", i.Name) + } + f, ok := ident.ident.Obj.Decl.(*ast.Field) + if !ok { + return "", errors.Errorf("ident %s is not a field type", ident.Name) + } + var typeNameBuf bytes.Buffer + fset := s.View().Session().Cache().FileSet() + if err := printer.Fprint(&typeNameBuf, fset, f.Type); err != nil { + return "", err + } + return typeNameBuf.String(), nil +} + func formatResults(tup *types.Tuple, qf types.Qualifier) ([]string, bool) { var writeResultParens bool results := make([]string, 0, tup.Len()) diff --git a/internal/lsp/testdata/snippets/snippets.go.golden b/internal/lsp/testdata/snippets/snippets.go.golden new file mode 100644 index 00000000000..34b919e6d1a --- /dev/null +++ b/internal/lsp/testdata/snippets/snippets.go.golden @@ -0,0 +1,5 @@ +-- -signature -- + +-- baz(at AliasType, b bool)-signature -- +baz(at AliasType, b bool) + diff --git a/internal/lsp/testdata/snippets/snippets.go.in b/internal/lsp/testdata/snippets/snippets.go.in index bba8c39fd76..35a9295a9e9 100644 --- a/internal/lsp/testdata/snippets/snippets.go.in +++ b/internal/lsp/testdata/snippets/snippets.go.in @@ -1,20 +1,28 @@ package snippets +type AliasType = int //@item(sigAliasType, "AliasType", "AliasType", "type") + func foo(i int, b bool) {} //@item(snipFoo, "foo", "func(i int, b bool)", "func") func bar(fn func()) func() {} //@item(snipBar, "bar", "func(fn func())", "func") +func baz(at AliasType, b bool) {} //@item(snipBaz, "baz", "func(at AliasType, b bool)", "func") type Foo struct { Bar int //@item(snipFieldBar, "Bar", "int", "field") + Func func(at AliasType) error //@item(snipFieldFunc, "Func", "func(at AliasType) error", "field") } func (Foo) Baz() func() {} //@item(snipMethodBaz, "Baz", "func() func()", "method") func (Foo) BazBar() func() {} //@item(snipMethodBazBar, "BazBar", "func() func()", "method") +func (Foo) BazBaz(at AliasType) func() {} //@item(snipMethodBazBaz, "BazBaz", "func(at AliasType) func()", "method") func _() { f //@snippet(" //", snipFoo, "foo(${1:})", "foo(${1:i int}, ${2:b bool})") bar //@snippet(" //", snipBar, "bar(${1:})", "bar(${1:fn func()})") + baz //@snippet(" //", snipBaz, "baz(${1:})", "baz(${1:at AliasType}, ${2:b bool})") + baz() //@signature("(", "baz(at AliasType, b bool)", 0) + bar(nil) //@snippet("(", snipBar, "bar", "bar") bar(ba) //@snippet(")", snipBar, "bar(${1:})", "bar(${1:fn func()})") var f Foo @@ -26,6 +34,10 @@ func _() { B //@snippet(" //", snipFieldBar, "Bar: ${1:},", "Bar: ${1:int},") } + Foo{ + F //@snippet(" //", snipFieldFunc, "Func: ${1:},", "Func: ${1:func(at AliasType) error},") + } + Foo{B} //@snippet("}", snipFieldBar, "Bar: ${1:}", "Bar: ${1:int}") Foo{} //@snippet("}", snipFieldBar, "Bar: ${1:}", "Bar: ${1:int}") @@ -36,4 +48,6 @@ func _() { f.Baz() //@snippet("B", snipMethodBaz, "Baz()", "Baz()") f.Baz() //@snippet("(", snipMethodBazBar, "BazBar", "BazBar") + + f.Baz() //@snippet("B", snipMethodBazBaz, "BazBaz(${1:})", "BazBaz(${1:at AliasType})") } diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index 743cd38823a..ae1a2631797 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -1,6 +1,6 @@ -- summary -- CompletionsCount = 215 -CompletionSnippetCount = 47 +CompletionSnippetCount = 50 UnimportedCompletionsCount = 3 DeepCompletionsCount = 5 FuzzyCompletionsCount = 7 @@ -18,6 +18,6 @@ ReferencesCount = 7 RenamesCount = 22 PrepareRenamesCount = 8 SymbolsCount = 1 -SignaturesCount = 21 +SignaturesCount = 22 LinksCount = 6