From a7be2b0f24db732ef40a2317b4d784f3beb8896f Mon Sep 17 00:00:00 2001 From: GSmithApps Date: Mon, 13 Jan 2025 21:19:15 -0600 Subject: [PATCH 1/3] gopls/internal/golang/hover: added hover support for pkg idents --- gopls/internal/golang/hover.go | 79 ++++++++++++++--- .../test/integration/misc/hover_test.go | 88 +++++++++++++++++++ 2 files changed, 153 insertions(+), 14 deletions(-) diff --git a/gopls/internal/golang/hover.go b/gopls/internal/golang/hover.go index 80c47470215..26b344e1262 100644 --- a/gopls/internal/golang/hover.go +++ b/gopls/internal/golang/hover.go @@ -258,6 +258,18 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro // The general case: compute hover information for the object referenced by // the identifier at pos. ident, obj, selectedType := referencedObject(pkg, pgf, pos) + + if pkgName, ok := obj.(*types.PkgName); ok { + rng, hoverRes, err := hoverPackageIdent(ctx, snapshot, pkg, pgf, ident, pkgName.Imported().Path()) + if err != nil { + return protocol.Range{}, nil, err + } + if hoverRange == nil { + hoverRange = &rng + } + return *hoverRange, hoverRes, nil // (hoverRes may be nil) + } + if obj == nil || ident == nil { return protocol.Range{}, nil, nil // no object to hover } @@ -691,27 +703,22 @@ func hoverBuiltin(ctx context.Context, snapshot *cache.Snapshot, obj types.Objec }, nil } -// hoverImport computes hover information when hovering over the import path of -// imp in the file pgf of pkg. +// hoverPackageRef computes hover information when hovering over the import path or ident of +// imp in the file pgf of pkg or over the identifier for an imported pkg. // // If we do not have metadata for the hovered import, it returns _ -func hoverImport(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, imp *ast.ImportSpec) (protocol.Range, *hoverResult, error) { - rng, err := pgf.NodeRange(imp.Path) - if err != nil { - return protocol.Range{}, nil, err - } - +func hoverPackageRef(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, imp *ast.ImportSpec) (*hoverResult, error) { importPath := metadata.UnquoteImportPath(imp) if importPath == "" { - return protocol.Range{}, nil, fmt.Errorf("invalid import path") + return nil, fmt.Errorf("invalid import path") } impID := pkg.Metadata().DepsByImpPath[importPath] if impID == "" { - return protocol.Range{}, nil, fmt.Errorf("no package data for import %q", importPath) + return nil, fmt.Errorf("no package data for import %q", importPath) } impMetadata := snapshot.Metadata(impID) if impMetadata == nil { - return protocol.Range{}, nil, bug.Errorf("failed to resolve import ID %q", impID) + return nil, bug.Errorf("failed to resolve import ID %q", impID) } // Find the first file with a package doc comment. @@ -720,14 +727,14 @@ func hoverImport(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Packa fh, err := snapshot.ReadFile(ctx, f) if err != nil { if ctx.Err() != nil { - return protocol.Range{}, nil, ctx.Err() + return nil, ctx.Err() } continue } pgf, err := snapshot.ParseGo(ctx, fh, parsego.Header) if err != nil { if ctx.Err() != nil { - return protocol.Range{}, nil, ctx.Err() + return nil, ctx.Err() } continue } @@ -738,13 +745,57 @@ func hoverImport(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Packa } docText := comment.Text() - return rng, &hoverResult{ + return &hoverResult{ signature: "package " + string(impMetadata.Name), synopsis: doc.Synopsis(docText), fullDocumentation: docText, }, nil } +// hoverImport computes hover information when hovering over the import path of +// imp in the file pgf of pkg. +// +// If we do not have metadata for the hovered import, it returns _ +func hoverImport(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, imp *ast.ImportSpec) (protocol.Range, *hoverResult, error) { + rng, err := pgf.NodeRange(imp.Path) + if err != nil { + return protocol.Range{}, nil, err + } + hoverRes, err := hoverPackageRef(ctx, snapshot, pkg, imp) + if err != nil { + return protocol.Range{}, nil, err + } + return rng, hoverRes, err +} + +// hoverPackageIdent computes hover information when hovering over the identifier +// of an imported pkg. +// +// If we do not have metadata for the hovered import, it returns _ +func hoverPackageIdent(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, ident *ast.Ident, path string) (protocol.Range, *hoverResult, error) { + + for _, spec := range pgf.File.Imports { + importPathString, err := strconv.Unquote(spec.Path.Value) + if err != nil { + return protocol.Range{}, nil, err + } + if importPathString != path { + continue + } + rng, err := pgf.NodeRange(ident) + if err != nil { + return protocol.Range{}, nil, err + } + hoverRes, err := hoverPackageRef(ctx, snapshot, pkg, spec) + if err != nil { + return protocol.Range{}, nil, err + } + return rng, hoverRes, nil // (hoverRes may be nil) + } + + return protocol.Range{}, nil, fmt.Errorf("invalid import path") +} + // hoverPackageName computes hover information for the package name of the file // pgf in pkg. func hoverPackageName(pkg *cache.Package, pgf *parsego.File) (protocol.Range, *hoverResult, error) { diff --git a/gopls/internal/test/integration/misc/hover_test.go b/gopls/internal/test/integration/misc/hover_test.go index 1592b899b1d..ed5603d7448 100644 --- a/gopls/internal/test/integration/misc/hover_test.go +++ b/gopls/internal/test/integration/misc/hover_test.go @@ -230,6 +230,94 @@ func main() { }) } +func TestHoverPackageIdent(t *testing.T) { + const packageDoc1 = "Package lib1 hover documentation" + const packageDoc2 = "Package lib2 hover documentation" + tests := []struct { + hoverIdent string + want string + wantError bool + }{ + { + "lib1", + packageDoc1, + false, + }, + { + "lib2", + packageDoc2, + false, + }, + { + "lib3", + "", + false, + }, + { + "lib4", + "", + true, + }, + } + source := fmt.Sprintf(` +-- go.mod -- +module mod.com + +go 1.12 +-- lib1/a.go -- +// %s +package lib1 + +const C = 1 + +-- lib1/b.go -- +package lib1 + +const D = 1 + +-- lib2/a.go -- +// %s +package lib2 + +const E = 1 + +-- lib3/a.go -- +package lib3 + +const F = 1 + +-- main.go -- +package main + +import ( + "mod.com/lib1" + "mod.com/lib2" + "mod.com/lib3" + "mod.com/lib4" +) + +func main() { + println(lib1.C) + println(lib2.E) + println(lib3.F) + println(lib4.Z) +} + `, packageDoc1, packageDoc2) + Run(t, source, func(t *testing.T, env *Env) { + env.OpenFile("main.go") + for _, test := range tests { + got, _, err := env.Editor.Hover(env.Ctx, env.RegexpSearch("main.go", "("+test.hoverIdent+")\\.")) + if test.wantError { + if err == nil { + t.Errorf("Hover(%q) succeeded unexpectedly", test.hoverIdent) + } + } else if !strings.Contains(got.Value, test.want) { + t.Errorf("Hover(%q): got:\n%q\nwant:\n%q", test.hoverIdent, got.Value, test.want) + } + } + }) +} + // for x/tools/gopls: unhandled named anchor on the hover #57048 func TestHoverTags(t *testing.T) { const source = ` From c1d90f190bd35823ca26cbd8b5d9b1bf36f3af85 Mon Sep 17 00:00:00 2001 From: GSmithApps Date: Mon, 3 Feb 2025 22:49:52 -0600 Subject: [PATCH 2/3] small fix --- gopls/internal/golang/hover.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/gopls/internal/golang/hover.go b/gopls/internal/golang/hover.go index 6b029fe92aa..e4bb19fd50e 100644 --- a/gopls/internal/golang/hover.go +++ b/gopls/internal/golang/hover.go @@ -258,6 +258,9 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro // The general case: compute hover information for the object referenced by // the identifier at pos. ident, obj, selectedType := referencedObject(pkg, pgf, pos) + if obj == nil || ident == nil { + return protocol.Range{}, nil, nil // no object to hover + } if pkgName, ok := obj.(*types.PkgName); ok { rng, hoverRes, err := hoverPackageIdent(ctx, snapshot, pkg, pgf, ident, pkgName.Imported().Path()) @@ -270,10 +273,6 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro return *hoverRange, hoverRes, nil // (hoverRes may be nil) } - if obj == nil || ident == nil { - return protocol.Range{}, nil, nil // no object to hover - } - // Unless otherwise specified, rng covers the ident being hovered. if hoverRange == nil { rng, err := pgf.NodeRange(ident) From a743363bb2997bd7685a5ee9dd38304b6d78aaec Mon Sep 17 00:00:00 2001 From: GSmithApps Date: Mon, 3 Feb 2025 23:01:47 -0600 Subject: [PATCH 3/3] fixed test --- gopls/internal/test/integration/misc/link_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gopls/internal/test/integration/misc/link_test.go b/gopls/internal/test/integration/misc/link_test.go index 53b0f0818f3..4d91b333288 100644 --- a/gopls/internal/test/integration/misc/link_test.go +++ b/gopls/internal/test/integration/misc/link_test.go @@ -36,6 +36,7 @@ module import.test go 1.12 -- import.test@v1.2.3/pkg/const.go -- +// package documentation package pkg const Hello = "Hello" @@ -49,10 +50,11 @@ const Hello = "Hello" modLink := "https://pkg.go.dev/mod/import.test@v1.2.3" pkgLink := "https://pkg.go.dev/import.test@v1.2.3/pkg" + pkgDoc := "package documentation" // First, check that we get the expected links via hover and documentLink. content, _ := env.Hover(env.RegexpSearch("main.go", "pkg.Hello")) - if content == nil || !strings.Contains(content.Value, pkgLink) { + if content == nil || !strings.Contains(content.Value, pkgDoc) { t.Errorf("hover: got %v in main.go, want contains %q", content, pkgLink) } content, _ = env.Hover(env.RegexpSearch("go.mod", "import.test"))