From bffc5affc6df36a7c1fee87811e47b69912e721f Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Wed, 15 May 2019 17:58:16 -0400 Subject: [PATCH] internal/lsp: support definitions and hover for builtins This change adds support for definitions and hover for builtin types and functions. It also includes some small (non-logic) changes to the import spec definition function. Additionally, there are some resulting changes in diagnostics to ignore the builtin file but also use it for definitions (Ian, you were right with your comment on my earlier review...). Fixes golang/go#31696 Change-Id: I52d43d010a5ca8359b539c33e40782877eb730d0 Reviewed-on: https://go-review.googlesource.com/c/tools/+/177517 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/cache/file.go | 4 +- internal/lsp/cache/session.go | 4 + internal/lsp/cache/view.go | 30 ++--- internal/lsp/cmd/definition_test.go | 4 +- internal/lsp/lsp_test.go | 65 ++++++---- internal/lsp/source/completion_format.go | 20 +-- internal/lsp/source/diagnostics.go | 12 +- internal/lsp/source/hover.go | 12 +- internal/lsp/source/identifier.go | 143 ++++++++++++---------- internal/lsp/source/signature_help.go | 4 +- internal/lsp/source/source_test.go | 13 +- internal/lsp/source/util.go | 12 ++ internal/lsp/source/view.go | 1 + internal/lsp/testdata/godef/a/a.go | 3 + internal/lsp/testdata/godef/a/a.go.golden | 5 + internal/lsp/tests/tests.go | 23 +++- 16 files changed, 208 insertions(+), 147 deletions(-) diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go index 9aecfa4a..bd8fb054 100644 --- a/internal/lsp/cache/file.go +++ b/internal/lsp/cache/file.go @@ -174,12 +174,12 @@ func (f *goFile) GetActiveReverseDeps(ctx context.Context) []source.GoFile { results := make(map[*goFile]struct{}) f.view.reverseDeps(ctx, seen, results, pkg.PkgPath()) - files := make([]source.GoFile, 0, len(results)) + var files []source.GoFile for rd := range results { if rd == nil { continue } - // Don't return any of the active file's in this package. + // Don't return any of the active files in this package. if rd.pkg != nil && rd.pkg == pkg { continue } diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 7335b617..4ebe57ec 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -63,6 +63,10 @@ func (s *session) NewView(name string, folder span.URI, config *packages.Config) pcache: &packageCache{ packages: make(map[string]*entry), }, + ignoredURIs: make(map[span.URI]struct{}), + } + for filename := range v.builtinPkg.Files { + v.ignoredURIs[span.NewURI(filename)] = struct{}{} } s.views = append(s.views, v) // we always need to drop the view map diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index cf4227c8..f64f7a37 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -6,7 +6,6 @@ package cache import ( "context" - "fmt" "go/ast" "go/parser" "go/token" @@ -67,6 +66,9 @@ type view struct { // builtinPkg is the AST package used to resolve builtin types. builtinPkg *ast.Package + + // ignoredURIs is the set of URIs of files that we ignore. + ignoredURIs map[span.URI]struct{} } type metadataCache struct { @@ -290,18 +292,15 @@ func (v *view) GetFile(ctx context.Context, uri span.URI) (source.File, error) { // getFile is the unlocked internal implementation of GetFile. func (v *view) getFile(uri span.URI) (viewFile, error) { - filename, err := uri.Filename() - if err != nil { - return nil, err - } - if v.isIgnored(filename) { - return nil, fmt.Errorf("%s is ignored", filename) - } if f, err := v.findFile(uri); err != nil { return nil, err } else if f != nil { return f, nil } + filename, err := uri.Filename() + if err != nil { + return nil, err + } f := &goFile{ fileBase: fileBase{ view: v, @@ -312,18 +311,11 @@ func (v *view) getFile(uri span.URI) (viewFile, error) { return f, nil } -// isIgnored checks if the given filename is a file we ignore. +// Ignore checks if the given URI is a URI we ignore. // As of right now, we only ignore files in the "builtin" package. -func (v *view) isIgnored(filename string) bool { - bpkg := v.BuiltinPackage() - if bpkg != nil { - for builtinFilename := range bpkg.Files { - if filename == builtinFilename { - return true - } - } - } - return false +func (v *view) Ignore(uri span.URI) bool { + _, ok := v.ignoredURIs[uri] + return ok } // findFile checks the cache for any file matching the given uri. diff --git a/internal/lsp/cmd/definition_test.go b/internal/lsp/cmd/definition_test.go index 033c4185..6eacc18d 100644 --- a/internal/lsp/cmd/definition_test.go +++ b/internal/lsp/cmd/definition_test.go @@ -64,8 +64,8 @@ func TestDefinitionHelpExample(t *testing.T) { func (r *runner) Definition(t *testing.T, data tests.Definitions) { for _, d := range data { - if d.IsType { - // TODO: support type definition queries + if d.IsType || d.OnlyHover { + // TODO: support type definition, hover queries continue } d.Src = span.New(d.Src.URI(), span.NewPoint(0, 0, d.Src.Start().Offset()), span.Point{}) diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index c550ada6..172f77c2 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -312,7 +312,10 @@ func (r *runner) Format(t *testing.T, data tests.Formats) { func (r *runner) Definition(t *testing.T, data tests.Definitions) { for _, d := range data { - sm := r.mapper(d.Src.URI()) + sm, err := r.mapper(d.Src.URI()) + if err != nil { + t.Fatal(err) + } loc, err := sm.Location(d.Src) if err != nil { t.Fatalf("failed for %v: %v", d.Src, err) @@ -338,13 +341,6 @@ func (r *runner) Definition(t *testing.T, data tests.Definitions) { if len(locs) != 1 { t.Errorf("got %d locations for definition, expected 1", len(locs)) } - locURI := span.NewURI(locs[0].URI) - lm := r.mapper(locURI) - if def, err := lm.Span(locs[0]); err != nil { - t.Fatalf("failed for %v: %v", locs[0], err) - } else if def != d.Def { - t.Errorf("for %v got %v want %v", d.Src, def, d.Def) - } if hover != nil { tag := fmt.Sprintf("%s-hover", d.Name) filename, err := d.Src.URI().Filename() @@ -357,13 +353,29 @@ func (r *runner) Definition(t *testing.T, data tests.Definitions) { if hover.Contents.Value != expectHover { t.Errorf("for %v got %q want %q", d.Src, hover.Contents.Value, expectHover) } + } else if !d.OnlyHover { + locURI := span.NewURI(locs[0].URI) + lm, err := r.mapper(locURI) + if err != nil { + t.Fatal(err) + } + if def, err := lm.Span(locs[0]); err != nil { + t.Fatalf("failed for %v: %v", locs[0], err) + } else if def != d.Def { + t.Errorf("for %v got %v want %v", d.Src, def, d.Def) + } + } else { + t.Errorf("no tests ran for %s", d.Src.URI()) } } } func (r *runner) Highlight(t *testing.T, data tests.Highlights) { for name, locations := range data { - m := r.mapper(locations[0].URI()) + m, err := r.mapper(locations[0].URI()) + if err != nil { + t.Fatal(err) + } loc, err := m.Location(locations[0]) if err != nil { t.Fatalf("failed for %v: %v", locations[0], err) @@ -405,16 +417,19 @@ func (r *runner) Symbol(t *testing.T, data tests.Symbols) { t.Errorf("want %d top-level symbols in %v, got %d", len(expectedSymbols), uri, len(symbols)) continue } - if diff := r.diffSymbols(uri, expectedSymbols, symbols); diff != "" { + if diff := r.diffSymbols(t, uri, expectedSymbols, symbols); diff != "" { t.Error(diff) } } } -func (r *runner) diffSymbols(uri span.URI, want []source.Symbol, got []protocol.DocumentSymbol) string { +func (r *runner) diffSymbols(t *testing.T, uri span.URI, want []source.Symbol, got []protocol.DocumentSymbol) string { sort.Slice(want, func(i, j int) bool { return want[i].Name < want[j].Name }) sort.Slice(got, func(i, j int) bool { return got[i].Name < got[j].Name }) - m := r.mapper(uri) + m, err := r.mapper(uri) + if err != nil { + t.Fatal(err) + } if len(got) != len(want) { return summarizeSymbols(-1, want, got, "different lengths got %v want %v", len(got), len(want)) } @@ -433,7 +448,7 @@ func (r *runner) diffSymbols(uri span.URI, want []source.Symbol, got []protocol. if w.SelectionSpan != spn { return summarizeSymbols(i, want, got, "incorrect span got %v want %v", spn, w.SelectionSpan) } - if msg := r.diffSymbols(uri, w.Children, g.Children); msg != "" { + if msg := r.diffSymbols(t, uri, w.Children, g.Children); msg != "" { return fmt.Sprintf("children of %s: %s", w.Name, msg) } } @@ -461,7 +476,10 @@ func summarizeSymbols(i int, want []source.Symbol, got []protocol.DocumentSymbol func (r *runner) SignatureHelp(t *testing.T, data tests.Signatures) { for spn, expectedSignatures := range data { - m := r.mapper(spn.URI()) + m, err := r.mapper(spn.URI()) + if err != nil { + t.Fatal(err) + } loc, err := m.Location(spn) if err != nil { t.Fatalf("failed for %v: %v", loc, err) @@ -519,7 +537,10 @@ func diffSignatures(spn span.Span, want source.SignatureInformation, got *protoc func (r *runner) Link(t *testing.T, data tests.Links) { for uri, wantLinks := range data { - m := r.mapper(uri) + m, err := r.mapper(uri) + if err != nil { + t.Fatal(err) + } gotLinks, err := r.server.DocumentLink(context.Background(), &protocol.DocumentLinkParams{ TextDocument: protocol.TextDocumentIdentifier{ URI: protocol.NewURI(uri), @@ -552,28 +573,28 @@ func (r *runner) Link(t *testing.T, data tests.Links) { } } -func (r *runner) mapper(uri span.URI) *protocol.ColumnMapper { - fname, err := uri.Filename() +func (r *runner) mapper(uri span.URI) (*protocol.ColumnMapper, error) { + filename, err := uri.Filename() if err != nil { - return nil + return nil, err } fset := r.data.Exported.ExpectFileSet var f *token.File fset.Iterate(func(check *token.File) bool { - if check.Name() == fname { + if check.Name() == filename { f = check return false } return true }) if f == nil { - return nil + return nil, fmt.Errorf("no token.File for %s", uri) } content, err := r.data.Exported.FileContents(f.Name()) if err != nil { - return nil + return nil, err } - return protocol.NewColumnMapper(uri, fset, f, content) + return protocol.NewColumnMapper(uri, fset, f, content), nil } func TestBytesOffset(t *testing.T) { diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index ad7f15be..d2128687 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -107,8 +107,8 @@ func (c *completer) formatBuiltin(obj types.Object, score float64) CompletionIte item.Kind = ConstantCompletionItem case *types.Builtin: item.Kind = FunctionCompletionItem - decl := lookupBuiltin(c.view, obj.Name()) - if decl == nil { + decl, ok := lookupBuiltinDecl(c.view, obj.Name()).(*ast.FuncDecl) + if !ok { break } params, _ := formatFieldList(c.ctx, c.view, decl.Type.Params) @@ -127,22 +127,6 @@ 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`, diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 667f07df..f6ae6755 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -66,17 +66,25 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag // Prepare the reports we will send for this package. reports := make(map[span.URI][]Diagnostic) for _, filename := range pkg.GetFilenames() { - reports[span.FileURI(filename)] = []Diagnostic{} + uri := span.FileURI(filename) + if v.Ignore(uri) { + continue + } + reports[uri] = []Diagnostic{} } // Run diagnostics for the package that this URI belongs to. if !diagnostics(ctx, v, pkg, reports) { // If we don't have any list, parse, or type errors, run analyses. if err := analyses(ctx, v, pkg, reports); err != nil { - return singleDiagnostic(uri, "failed to run analyses for %s", uri), nil + return singleDiagnostic(uri, "failed to run analyses for %s: %v", uri, err), nil } } // Updates to the diagnostics for this package may need to be propagated. for _, f := range gof.GetActiveReverseDeps(ctx) { + if f == nil { + v.Session().Logger().Errorf(ctx, "nil file in reverse active dependencies for %s", f.URI()) + continue + } pkg := f.GetPackage(ctx) if pkg == nil { continue diff --git a/internal/lsp/source/hover.go b/internal/lsp/source/hover.go index 931489a7..d4bb08ec 100644 --- a/internal/lsp/source/hover.go +++ b/internal/lsp/source/hover.go @@ -38,9 +38,19 @@ func (i *IdentifierInfo) Hover(ctx context.Context, qf types.Qualifier, markdown case *types.TypeName, *types.Var, *types.Const, *types.Func: return formatGenDecl(node, obj, obj.Type(), f) } + case *ast.TypeSpec: + if obj.Parent() == types.Universe { + if obj.Name() == "error" { + return f(node, nil) + } + return f(node.Name, nil) // comments not needed for builtins + } case *ast.FuncDecl: - if _, ok := obj.(*types.Func); ok { + switch obj.(type) { + case *types.Func: return f(obj, node.Doc) + case *types.Builtin: + return f(node.Type, node.Doc) } } return f(obj, nil) diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index 6e43fccd..4e870af4 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -27,7 +27,7 @@ type IdentifierInfo struct { } Declaration struct { Range span.Range - Node ast.Decl + Node ast.Node Object types.Object } @@ -64,15 +64,12 @@ func identifier(ctx context.Context, v View, f GoFile, pos token.Pos) (*Identifi return nil, fmt.Errorf("can't find node enclosing position") } - // Handle import specs first because they can contain *ast.Idents, and - // we don't want the default *ast.Ident behavior below. - if result, err := checkImportSpec(f, fAST, pkg, pos); result != nil || err != nil { + // Handle import specs separately, as there is no formal position for a package declaration. + if result, err := importSpec(f, fAST, pkg, pos); result != nil || err != nil { return result, err } - result := &IdentifierInfo{ - File: f, - } + result := &IdentifierInfo{File: f} switch node := path[0].(type) { case *ast.Ident: @@ -95,6 +92,22 @@ func identifier(ctx context.Context, v View, f GoFile, pos token.Pos) (*Identifi if result.Declaration.Object == nil { return nil, fmt.Errorf("no object for ident %v", result.Name) } + + var err error + + // Handle builtins separately. + if result.Declaration.Object.Parent() == types.Universe { + decl, ok := lookupBuiltinDecl(f.View(), result.Name).(ast.Node) + if !ok { + return nil, fmt.Errorf("no declaration for %s", result.Name) + } + result.Declaration.Node = decl + if result.Declaration.Range, err = posToRange(ctx, v.FileSet(), result.Name, decl.Pos()); err != nil { + return nil, err + } + return result, nil + } + if result.wasEmbeddedField { // The original position was on the embedded field declaration, so we // try to dig out the type and jump to that instead. @@ -104,8 +117,8 @@ func identifier(ctx context.Context, v View, f GoFile, pos token.Pos) (*Identifi } } } - var err error - if result.Declaration.Range, err = objToRange(ctx, v, result.Declaration.Object); err != nil { + + if result.Declaration.Range, err = objToRange(ctx, v.FileSet(), result.Declaration.Object); err != nil { return nil, err } if result.Declaration.Node, err = objToNode(ctx, v, result.Declaration.Object, result.Declaration.Range); err != nil { @@ -118,68 +131,16 @@ func identifier(ctx context.Context, v View, f GoFile, pos token.Pos) (*Identifi result.Type.Object = typeToObject(typ) if result.Type.Object != nil { // Identifiers with the type "error" are a special case with no position. - if types.IsInterface(result.Type.Object.Type()) && result.Type.Object.Pkg() == nil && result.Type.Object.Name() == "error" { + if hasErrorType(result.Type.Object) { return result, nil } - if result.Type.Range, err = objToRange(ctx, v, result.Type.Object); err != nil { + if result.Type.Range, err = objToRange(ctx, v.FileSet(), result.Type.Object); err != nil { return nil, err } } return result, nil } -func checkImportSpec(f GoFile, fAST *ast.File, pkg Package, pos token.Pos) (*IdentifierInfo, error) { - // Check if pos is in an *ast.ImportSpec. - for _, imp := range fAST.Imports { - if imp.Pos() <= pos && pos < imp.End() { - pkgPath, err := strconv.Unquote(imp.Path.Value) - if err != nil { - return nil, fmt.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err) - } - - result := &IdentifierInfo{ - File: f, - Name: pkgPath, - Range: span.NewRange(f.View().FileSet(), imp.Pos(), imp.End()), - } - - // Consider the definition of an import spec to be the imported package. - result.Declaration.Range, err = importedPkg(f.View(), pkg, pkgPath) - if err != nil { - return nil, err - } - - return result, nil - } - } - - return nil, nil -} - -func importedPkg(v View, pkg Package, importPath string) (span.Range, error) { - otherPkg := pkg.GetImport(importPath) - if otherPkg == nil { - return span.Range{}, fmt.Errorf("no import for %q", importPath) - } - if otherPkg.GetSyntax() == nil { - return span.Range{}, fmt.Errorf("no syntax for for %q", importPath) - } - - // Heuristic: Jump to the longest file of the package, assuming it's the most "interesting." - // TODO: Consider alternative approaches, if necessary. - var longest *ast.File - for _, astFile := range otherPkg.GetSyntax() { - if longest == nil || astFile.End()-astFile.Pos() > longest.End()-longest.Pos() { - longest = astFile - } - } - if longest == nil { - return span.Range{}, fmt.Errorf("package %q has no files", importPath) - } - - return span.NewRange(v.FileSet(), longest.Name.Pos(), longest.Name.End()), nil -} - func typeToObject(typ types.Type) types.Object { switch typ := typ.(type) { case *types.Named: @@ -191,12 +152,19 @@ func typeToObject(typ types.Type) types.Object { } } -func objToRange(ctx context.Context, v View, obj types.Object) (span.Range, error) { - p := obj.Pos() - if !p.IsValid() { - return span.Range{}, fmt.Errorf("invalid position for %v", obj.Name()) +func hasErrorType(obj types.Object) bool { + return types.IsInterface(obj.Type()) && obj.Pkg() == nil && obj.Name() == "error" +} + +func objToRange(ctx context.Context, fset *token.FileSet, obj types.Object) (span.Range, error) { + return posToRange(ctx, fset, obj.Name(), obj.Pos()) +} + +func posToRange(ctx context.Context, fset *token.FileSet, name string, pos token.Pos) (span.Range, error) { + if !pos.IsValid() { + return span.Range{}, fmt.Errorf("invalid position for %v", name) } - return span.NewRange(v.FileSet(), p, p+token.Pos(len(obj.Name()))), nil + return span.NewRange(fset, pos, pos+token.Pos(len(name))), nil } func objToNode(ctx context.Context, v View, obj types.Object, rng span.Range) (ast.Decl, error) { @@ -234,3 +202,42 @@ func objToNode(ctx context.Context, v View, obj types.Object, rng span.Range) (a } return nil, nil // didn't find a node, but don't fail } + +// importSpec handles positions inside of an *ast.ImportSpec. +func importSpec(f GoFile, fAST *ast.File, pkg Package, pos token.Pos) (*IdentifierInfo, error) { + for _, imp := range fAST.Imports { + if !(imp.Pos() <= pos && pos < imp.End()) { + continue + } + importPath, err := strconv.Unquote(imp.Path.Value) + if err != nil { + return nil, fmt.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err) + } + result := &IdentifierInfo{ + File: f, + Name: importPath, + Range: span.NewRange(f.View().FileSet(), imp.Pos(), imp.End()), + } + // Consider the "declaration" of an import spec to be the imported package. + importedPkg := pkg.GetImport(importPath) + if importedPkg == nil { + return nil, fmt.Errorf("no import for %q", importPath) + } + if importedPkg.GetSyntax() == nil { + return nil, fmt.Errorf("no syntax for for %q", importPath) + } + // Heuristic: Jump to the longest (most "interesting") file of the package. + var dest *ast.File + for _, f := range importedPkg.GetSyntax() { + if dest == nil || f.End()-f.Pos() > dest.End()-dest.Pos() { + dest = f + } + } + if dest == nil { + return nil, fmt.Errorf("package %q has no files", importPath) + } + result.Declaration.Range = span.NewRange(f.View().FileSet(), dest.Name.Pos(), dest.Name.End()) + return result, nil + } + return nil, nil +} diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index 250d4281..5a4500ab 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -89,8 +89,8 @@ func SignatureHelp(ctx context.Context, f GoFile, pos token.Pos) (*SignatureInfo } func builtinSignature(ctx context.Context, v View, callExpr *ast.CallExpr, name string, pos token.Pos) (*SignatureInformation, error) { - decl := lookupBuiltin(v, name) - if decl == nil { + decl, ok := lookupBuiltinDecl(v, name).(*ast.FuncDecl) + if !ok { return nil, fmt.Errorf("no function declaration for builtin: %s", name) } params, _ := formatFieldList(ctx, v, decl.Type.Params) diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 263ba080..9041160b 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -310,11 +310,6 @@ func (r *runner) Definition(t *testing.T, data tests.Definitions) { rng = ident.Type.Range hover = "" } - if def, err := rng.Span(); err != nil { - t.Fatalf("failed for %v: %v", rng, err) - } else if def != d.Def { - t.Errorf("for %v got %v want %v", d.Src, def, d.Def) - } if hover != "" { tag := fmt.Sprintf("%s-hover", d.Name) filename, err := d.Src.URI().Filename() @@ -327,6 +322,14 @@ func (r *runner) Definition(t *testing.T, data tests.Definitions) { if hover != expectHover { t.Errorf("for %v got %q want %q", d.Src, hover, expectHover) } + } else if !d.OnlyHover { + if def, err := rng.Span(); err != nil { + t.Fatalf("failed for %v: %v", rng, err) + } else if def != d.Def { + t.Errorf("for %v got %v want %v", d.Src, def, d.Def) + } + } else { + t.Errorf("no tests ran for %s", d.Src.URI()) } } } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 66002ccd..81299b96 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -102,6 +102,18 @@ func resolveInvalid(obj types.Object, node ast.Node, info *types.Info) types.Obj return formatResult(resultExpr) } +func lookupBuiltinDecl(v View, name string) interface{} { + builtinPkg := v.BuiltinPackage() + if builtinPkg == nil || builtinPkg.Scope == nil { + return nil + } + obj := builtinPkg.Scope.Lookup(name) + if obj == nil { + return nil + } + return obj.Decl +} + func isPointer(T types.Type) bool { _, ok := T.(*types.Pointer) return ok diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 57a87071..f9802c17 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -67,6 +67,7 @@ type View interface { Config() packages.Config SetEnv([]string) Shutdown(ctx context.Context) + Ignore(span.URI) bool } // File represents a source file of any type. diff --git a/internal/lsp/testdata/godef/a/a.go b/internal/lsp/testdata/godef/a/a.go index a927333e..6f7ecb3a 100644 --- a/internal/lsp/testdata/godef/a/a.go +++ b/internal/lsp/testdata/godef/a/a.go @@ -13,4 +13,7 @@ func Stuff() { //@Stuff var err error //@err fmt.Printf("%v", err) //@godef("err", err) + + var y string //@string,hover("string", string) + _ = make([]int, 0) //@make,hover("make", make) } diff --git a/internal/lsp/testdata/godef/a/a.go.golden b/internal/lsp/testdata/godef/a/a.go.golden index a8a81085..41ae6652 100644 --- a/internal/lsp/testdata/godef/a/a.go.golden +++ b/internal/lsp/testdata/godef/a/a.go.golden @@ -67,3 +67,8 @@ godef/a/a.go:14:6-9: defined here as var err error -- err-hover -- var err error +-- string-hover -- +string +-- make-hover -- +The make built-in function allocates and initializes an object of type slice, map, or chan (only). +func(t Type, size ...IntegerType) Type diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index 15286777..ae5cfaa1 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -32,7 +32,7 @@ const ( ExpectedCompletionSnippetCount = 13 ExpectedDiagnosticsCount = 17 ExpectedFormatCount = 5 - ExpectedDefinitionsCount = 33 + ExpectedDefinitionsCount = 35 ExpectedTypeDefinitionsCount = 2 ExpectedHighlightsCount = 2 ExpectedSymbolsCount = 1 @@ -94,10 +94,11 @@ type Tests interface { } type Definition struct { - Name string - Src span.Span - IsType bool - Def span.Span + Name string + Src span.Span + IsType bool + OnlyHover bool + Def span.Span } type CompletionSnippet struct { @@ -203,6 +204,7 @@ func Load(t testing.TB, exporter packagestest.Exporter, dir string) *Data { "format": data.collectFormats, "godef": data.collectDefinitions, "typdef": data.collectTypeDefinitions, + "hover": data.collectHoverDefinitions, "highlight": data.collectHighlights, "symbol": data.collectSymbols, "signature": data.collectSignatures, @@ -217,9 +219,10 @@ func Load(t testing.TB, exporter packagestest.Exporter, dir string) *Data { symbols[i].Children = children } } - // run a second pass to collect names for some entries. + // Collect names for the entries that require golden files. if err := data.Exported.Expect(map[string]interface{}{ "godef": data.collectDefinitionNames, + "hover": data.collectDefinitionNames, }); err != nil { t.Fatal(err) } @@ -420,6 +423,14 @@ func (data *Data) collectDefinitions(src, target span.Span) { } } +func (data *Data) collectHoverDefinitions(src, target span.Span) { + data.Definitions[src] = Definition{ + Src: src, + Def: target, + OnlyHover: true, + } +} + func (data *Data) collectTypeDefinitions(src, target span.Span) { data.Definitions[src] = Definition{ Src: src,