From 1e6c1e2be244e26174a939f328b6eb01e7d25213 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 24 Jun 2024 15:29:36 -0400 Subject: [PATCH] gopls/internal/golang: strip @v1.2.3 suffix from pkgdoc URLs The package path in a /pkg URL does not want a module version suffix: the view specifies the versions of all packages. Remove them. Fixes golang/go#68116 Change-Id: Icbe7a6e7346d12456724d005fe8f755872657055 Reviewed-on: https://go-review.googlesource.com/c/tools/+/594556 LUCI-TryBot-Result: Go LUCI Auto-Submit: Alan Donovan Reviewed-by: Robert Findley --- gopls/internal/golang/hover.go | 7 ++- .../internal/test/integration/fake/editor.go | 3 +- .../test/integration/misc/hover_test.go | 50 +++++++++++++++++++ gopls/internal/test/integration/wrappers.go | 1 + 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/gopls/internal/golang/hover.go b/gopls/internal/golang/hover.go index 81594a6bf39..5e912285323 100644 --- a/gopls/internal/golang/hover.go +++ b/gopls/internal/golang/hover.go @@ -70,6 +70,7 @@ type hoverJSON struct { // LinkPath is the pkg.go.dev link for the given symbol. // For example, the "go/ast" part of "pkg.go.dev/go/ast#Node". + // It may have a module version suffix "@v1.2.3". LinkPath string `json:"linkPath"` // LinkAnchor is the pkg.go.dev link anchor for the given symbol. @@ -98,6 +99,7 @@ type hoverJSON struct { } // Hover implements the "textDocument/hover" RPC for Go files. +// It may return nil even on success. // // If pkgURL is non-nil, it should be used to generate doc links. func Hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, position protocol.Position, pkgURL func(path PackagePath, fragment string) protocol.URI) (*protocol.Hover, error) { @@ -200,7 +202,7 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro if hoverRange == nil { hoverRange = &rng } - return *hoverRange, hoverJSON, nil + return *hoverRange, hoverJSON, nil // (hoverJSON may be nil) } } // Handle hovering over (non-import-path) literals. @@ -1159,7 +1161,8 @@ func formatLink(h *hoverJSON, options *settings.Options, pkgURL func(path Packag var url protocol.URI var caption string if pkgURL != nil { // LinksInHover == "gopls" - url = pkgURL(PackagePath(h.LinkPath), h.LinkAnchor) + path, _, _ := strings.Cut(h.LinkPath, "@") // remove optional module version suffix + url = pkgURL(PackagePath(path), h.LinkAnchor) caption = "in gopls doc viewer" } else { if options.LinkTarget == "" { diff --git a/gopls/internal/test/integration/fake/editor.go b/gopls/internal/test/integration/fake/editor.go index 487a255da3d..383f047aeab 100644 --- a/gopls/internal/test/integration/fake/editor.go +++ b/gopls/internal/test/integration/fake/editor.go @@ -1595,6 +1595,7 @@ func (e *Editor) EditResolveSupport() (bool, error) { } // Hover triggers a hover at the given position in an open buffer. +// It may return (nil, zero) if no symbol was selected. func (e *Editor) Hover(ctx context.Context, loc protocol.Location) (*protocol.MarkupContent, protocol.Location, error) { if err := e.checkBufferLocation(loc); err != nil { return nil, protocol.Location{}, err @@ -1608,7 +1609,7 @@ func (e *Editor) Hover(ctx context.Context, loc protocol.Location) (*protocol.Ma return nil, protocol.Location{}, fmt.Errorf("hover: %w", err) } if resp == nil { - return nil, protocol.Location{}, nil + return nil, protocol.Location{}, nil // e.g. no selected symbol } return &resp.Contents, protocol.Location{URI: loc.URI, Range: resp.Range}, nil } diff --git a/gopls/internal/test/integration/misc/hover_test.go b/gopls/internal/test/integration/misc/hover_test.go index d3445ddec4a..2d5687ed847 100644 --- a/gopls/internal/test/integration/misc/hover_test.go +++ b/gopls/internal/test/integration/misc/hover_test.go @@ -553,3 +553,53 @@ func main() { }) } } + +func TestHoverInternalLinksIssue68116(t *testing.T) { + // Links for the internal viewer should not include a module version suffix: + // the package path and the view are an unambiguous key; see #68116. + + const proxy = ` +-- example.com@v1.2.3/go.mod -- +module example.com + +go 1.12 + +-- example.com@v1.2.3/a/a.go -- +package a + +// F is a function. +func F() +` + + const mod = ` +-- go.mod -- +module main + +go 1.12 + +require example.com v1.2.3 + +-- main.go -- +package main + +import "example.com/a" + +func main() { + a.F() +} +` + WithOptions( + ProxyFiles(proxy), + Settings{"linksInHover": "gopls"}, + WriteGoSum("."), + ).Run(t, mod, func(t *testing.T, env *Env) { + env.OpenFile("main.go") + got, _ := env.Hover(env.RegexpSearch("main.go", "F")) + const wantRE = "\\[`a.F` in gopls doc viewer\\]\\(http://127.0.0.1:[0-9]+/gopls/[^/]+/pkg/example.com\\?view=[0-9]+#F\\)" // no version + if m, err := regexp.MatchString(wantRE, got.Value); err != nil { + t.Fatalf("bad regexp in test: %v", err) + } else if !m { + t.Fatalf("hover output does not match %q; got:\n\n%s", wantRE, got.Value) + } + }) +} diff --git a/gopls/internal/test/integration/wrappers.go b/gopls/internal/test/integration/wrappers.go index 8c5e35e93c0..88145e73209 100644 --- a/gopls/internal/test/integration/wrappers.go +++ b/gopls/internal/test/integration/wrappers.go @@ -233,6 +233,7 @@ func (e *Env) GetQuickFixes(path string, diagnostics []protocol.Diagnostic) []pr } // Hover in the editor, calling t.Fatal on any error. +// It may return (nil, zero) even on success. func (e *Env) Hover(loc protocol.Location) (*protocol.MarkupContent, protocol.Location) { e.T.Helper() c, loc, err := e.Editor.Hover(e.Ctx, loc)