internal/lsp: honor GOPRIVATE in documentLinks and go.mod hovers

Several fixes related to GOPRIVATE handling and links:
 + In Go source, fix links matching GOPRIVATE for external modules.
   Previously, in these cases we'd try to match <mod>@v1.2.3/<suffix>,
   which wasn't the correct input into the GOPRIVATE matching algorithm.
 + Similarly check GOPRIVATE for go.mod require statement hovers.
 + Likewise, for documentLink requests (both mod and source).
 + Move the existing hover regtest to link_test.go, and expand to cover
   all these cases.

Along the way, I encountered a couple apparent bugs, which I fixed:
 + Correctly handle the case where there is only one require in a go.mod
   file. This was exercised by the regtest, so took some debugging.
 + Only format links [like](this) if the requested format is actually
   markdown.

Fixes golang/go#36998

Change-Id: I92011821f646f2a7449dcca619483f83bdeb54b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/238029
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/cache/mod.go

@@ -200,7 +200,7 @@ func goModWhy(ctx context.Context, cfg *packages.Config, modFH, sumFH source.Fil
 		return err
 	}
 	whyList := strings.Split(stdout.String(), "\n\n")
-	if len(whyList) <= 1 || len(whyList) != len(data.parsed.Require) {
+	if len(whyList) != len(data.parsed.Require) {
 		return nil
 	}
 	data.why = make(map[string]string, len(data.parsed.Require))

internal/lsp/fake/editor.go

@@ -760,3 +760,12 @@ func (e *Editor) Hover(ctx context.Context, path string, pos Pos) (*protocol.Mar
 	}
 	return &resp.Contents, fromProtocolPosition(resp.Range.Start), nil
 }
+
+func (e *Editor) DocumentLink(ctx context.Context, path string) ([]protocol.DocumentLink, error) {
+	if e.Server == nil {
+		return nil, nil
+	}
+	params := &protocol.DocumentLinkParams{}
+	params.TextDocument.URI = e.sandbox.Workdir.URI(path)
+	return e.Server.DocumentLink(ctx, params)
+}

internal/lsp/link.go

@@ -56,6 +56,10 @@ func modLinks(ctx context.Context, snapshot source.Snapshot, fh source.FileHandl
 	}
 	var links []protocol.DocumentLink
 	for _, req := range file.Require {
+		// See golang/go#36998: don't link to modules matching GOPRIVATE.
+		if snapshot.View().IsGoPrivatePath(req.Mod.Path) {
+			continue
+		}
 		dep := []byte(req.Mod.Path)
 		s, e := req.Syntax.Start.Byte, req.Syntax.End.Byte
 		i := bytes.Index(m.Content[s:e], dep)
@@ -132,6 +136,10 @@ func goLinks(ctx context.Context, view source.View, fh source.FileHandle) ([]pro
 		if err != nil {
 			continue
 		}
+		// See golang/go#36998: don't link to modules matching GOPRIVATE.
+		if view.IsGoPrivatePath(target) {
+			continue
+		}
 		if mod, version, ok := moduleAtVersion(ctx, target, ph); ok && strings.ToLower(view.Options().LinkTarget) == "pkg.go.dev" {
 			target = strings.Replace(target, mod, mod+"@"+version, 1)
 		}

internal/lsp/mod/hover.go

@@ -26,20 +26,20 @@ func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle,
 
 	mh, err := snapshot.ModHandle(ctx, fh)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("getting modfile handle: %w", err)
 	}
 	file, m, why, err := mh.Why(ctx)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("running go mod why: %w", err)
 	}
 	// Get the position of the cursor.
 	spn, err := m.PointSpan(position)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("computing cursor position: %w", err)
 	}
 	hoverRng, err := spn.Range(m.Converter)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("computing hover range: %w", err)
 	}
 
 	var req *modfile.Require
@@ -85,7 +85,8 @@ func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle,
 		return nil, err
 	}
 	options := snapshot.View().Options()
-	explanation = formatExplanation(explanation, req, options)
+	isPrivate := snapshot.View().IsGoPrivatePath(req.Mod.Path)
+	explanation = formatExplanation(explanation, req, options, isPrivate)
 	return &protocol.Hover{
 		Contents: protocol.MarkupContent{
 			Kind:  options.PreferredContentFormat,
@@ -95,7 +96,7 @@ func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle,
 	}, nil
 }
 
-func formatExplanation(text string, req *modfile.Require, options source.Options) string {
+func formatExplanation(text string, req *modfile.Require, options source.Options, isPrivate bool) string {
 	text = strings.TrimSuffix(text, "\n")
 	splt := strings.Split(text, "\n")
 	length := len(splt)
@@ -117,16 +118,17 @@ func formatExplanation(text string, req *modfile.Require, options source.Options
 		return b.String()
 	}
 
-	imp := splt[length-1]
-	target := imp
-	if strings.ToLower(options.LinkTarget) == "pkg.go.dev" {
-		target = strings.Replace(target, req.Mod.Path, req.Mod.String(), 1)
+	imp := splt[length-1] // import path
+	reference := imp
+	// See golang/go#36998: don't link to modules matching GOPRIVATE.
+	if !isPrivate && options.PreferredContentFormat == protocol.Markdown {
+		target := imp
+		if strings.ToLower(options.LinkTarget) == "pkg.go.dev" {
+			target = strings.Replace(target, req.Mod.Path, req.Mod.String(), 1)
+		}
+		reference = fmt.Sprintf("[%s](https://%s/%s)", imp, options.LinkTarget, target)
 	}
-	target = fmt.Sprintf("https://%s/%s", options.LinkTarget, target)
-
-	b.WriteString("This module is necessary because ")
-	msg := fmt.Sprintf("[%s](%s) is imported in", imp, target)
-	b.WriteString(msg)
+	b.WriteString("This module is necessary because " + reference + " is imported in")
 
 	// If the explanation is 3 lines, then it is of the form:
 	// # golang.org/x/tools

internal/lsp/regtest/link_test.go

@@ -0,0 +1,85 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package regtest
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestHoverAndDocumentLink(t *testing.T) {
+	const program = `
+-- go.mod --
+module mod.test
+
+go 1.12
+
+require import.test v1.2.3
+-- main.go --
+package main
+
+import "import.test/pkg"
+
+func main() {
+	println(pkg.Hello)
+}`
+
+	const proxy = `
+-- import.test@v1.2.3/go.mod --
+module import.test
+
+go 1.12
+-- import.test@v1.2.3/pkg/const.go --
+package pkg
+
+const Hello = "Hello"
+`
+	runner.Run(t, program, func(t *testing.T, env *Env) {
+		env.OpenFile("main.go")
+		env.OpenFile("go.mod")
+
+		modLink := "https://pkg.go.dev/mod/import.test@v1.2.3"
+		pkgLink := "https://pkg.go.dev/import.test@v1.2.3/pkg"
+
+		// First, check that we get the expected links via hover and documentLink.
+		content, _ := env.Hover("main.go", env.RegexpSearch("main.go", "pkg.Hello"))
+		if content == nil || !strings.Contains(content.Value, pkgLink) {
+			t.Errorf("hover: got %v in main.go, want contains %q", content, pkgLink)
+		}
+		content, _ = env.Hover("go.mod", env.RegexpSearch("go.mod", "import.test"))
+		if content == nil || !strings.Contains(content.Value, pkgLink) {
+			t.Errorf("hover: got %v in go.mod, want contains %q", content, pkgLink)
+		}
+		links := env.DocumentLink("main.go")
+		if len(links) != 1 || links[0].Target != pkgLink {
+			t.Errorf("documentLink: got %v for main.go, want link to %q", links, pkgLink)
+		}
+		links = env.DocumentLink("go.mod")
+		if len(links) != 1 || links[0].Target != modLink {
+			t.Errorf("documentLink: got %v for go.mod, want link to %q", links, modLink)
+		}
+
+		// Then change the environment to make these links private.
+		env.ChangeEnv("GOPRIVATE=import.test")
+
+		// Finally, verify that the links are gone.
+		content, _ = env.Hover("main.go", env.RegexpSearch("main.go", "pkg.Hello"))
+		if content == nil || strings.Contains(content.Value, pkgLink) {
+			t.Errorf("hover: got %v in main.go, want non-empty hover without %q", content, pkgLink)
+		}
+		content, _ = env.Hover("go.mod", env.RegexpSearch("go.mod", "import.test"))
+		if content == nil || strings.Contains(content.Value, modLink) {
+			t.Errorf("hover: got %v in go.mod, want contains %q", content, modLink)
+		}
+		links = env.DocumentLink("main.go")
+		if len(links) != 0 {
+			t.Errorf("documentLink: got %d document links for main.go, want 0\nlinks: %v", len(links), links)
+		}
+		links = env.DocumentLink("go.mod")
+		if len(links) != 0 {
+			t.Errorf("documentLink: got %d document links for go.mod, want 0\nlinks: %v", len(links), links)
+		}
+	}, WithProxy(proxy))
+}

internal/lsp/regtest/wrappers.go

@@ -164,6 +164,15 @@ func (e *Env) Hover(name string, pos fake.Pos) (*protocol.MarkupContent, fake.Po
 	return c, p
 }
 
+func (e *Env) DocumentLink(name string) []protocol.DocumentLink {
+	e.T.Helper()
+	links, err := e.Editor.DocumentLink(e.Ctx, name)
+	if err != nil {
+		e.T.Fatal(err)
+	}
+	return links
+}
+
 func checkIsFatal(t *testing.T, err error) {
 	t.Helper()
 	if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrClosedPipe) {

internal/lsp/source/completion_format.go

@@ -195,7 +195,7 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e
 	if err != nil {
 		return item, nil
 	}
-	hover, err := ident.Hover(ctx)
+	hover, err := HoverIdentifier(ctx, ident)
 	if err != nil {
 		event.Error(ctx, "failed to find Hover", err, tag.URI.Of(uri))
 		return item, nil