internal/lsp: don't show links in hover for test functions

source.Identifier previously was used for references and rename, so it
needed to take a package policy. Now, it's only used for definition and
hover, so it should always be the narrowest package handle. We can use
this fact to determine if the identifier is located in its declaring
package, and if that package is a test variant, we don't link to the
documentation on pkg.go.dev, since it doesn't exist.

Change-Id: I5686828858a3feafb8ff2e4c5964b562f66db9fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

internal/lsp/cache/check.go

@@ -256,6 +256,7 @@ func typeCheck(ctx context.Context, fset *token.FileSet, m *metadata, mode sourc
 			Selections: make(map[*ast.SelectorExpr]*types.Selection),
 			Scopes:     make(map[ast.Node]*types.Scope),
 		},
+		forTest: m.forTest,
 	}
 	var (
 		files        = make([]*ast.File, len(pkg.compiledGoFiles))

internal/lsp/cache/pkg.go

@@ -18,10 +18,10 @@ import (
 // pkg contains the type information needed by the source package.
 type pkg struct {
 	// ID and package path have their own types to avoid being used interchangeably.
-	id      packageID
+	id              packageID
-	pkgPath packagePath
+	pkgPath         packagePath
-	mode    source.ParseMode
+	mode            source.ParseMode
-
+	forTest         packagePath
 	goFiles         []source.ParseGoHandle
 	compiledGoFiles []source.ParseGoHandle
 	errors          []*source.Error
@@ -99,6 +99,10 @@ func (p *pkg) IsIllTyped() bool {
 	return p.types == nil || p.typesInfo == nil || p.typesSizes == nil
 }
 
+func (p *pkg) ForTest() string {
+	return string(p.forTest)
+}
+
 func (p *pkg) GetImport(pkgPath string) (source.Package, error) {
 	if imp := p.imports[packagePath(pkgPath)]; imp != nil {
 		return imp, nil

internal/lsp/definition.go

@@ -26,7 +26,7 @@ func (s *Server) definition(ctx context.Context, params *protocol.DefinitionPara
 	if fh.Identity().Kind != source.Go {
 		return nil, nil
 	}
-	ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.WidestPackageHandle)
+	ident, err := source.Identifier(ctx, snapshot, fh, params.Position)
 	if err != nil {
 		return nil, err
 	}
@@ -56,7 +56,7 @@ func (s *Server) typeDefinition(ctx context.Context, params *protocol.TypeDefini
 	if fh.Identity().Kind != source.Go {
 		return nil, nil
 	}
-	ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.WidestPackageHandle)
+	ident, err := source.Identifier(ctx, snapshot, fh, params.Position)
 	if err != nil {
 		return nil, err
 	}

internal/lsp/hover.go

@@ -26,7 +26,7 @@ func (s *Server) hover(ctx context.Context, params *protocol.HoverParams) (*prot
 	if fh.Identity().Kind != source.Go {
 		return nil, nil
 	}
-	ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.WidestPackageHandle)
+	ident, err := source.Identifier(ctx, snapshot, fh, params.Position)
 	if err != nil {
 		return nil, nil
 	}

internal/lsp/source/hover.go

@@ -85,6 +85,14 @@ func (i *IdentifierInfo) linkAndSymbolName() (string, string) {
 	case *types.Builtin:
 		return fmt.Sprintf("builtin#%s", obj.Name()), obj.Name()
 	}
+	// Check if the identifier is test-only (and is therefore not part of a
+	// package's API). This is true if the request originated in a test package,
+	// and if the declaration is also found in the same test package.
+	if i.pkg != nil && obj.Pkg() != nil && i.pkg.ForTest() != "" {
+		if _, pkg, _ := findFileInPackage(i.pkg, i.Declaration.URI()); i.pkg == pkg {
+			return "", ""
+		}
+	}
 	// Don't return links for other unexported types.
 	if !obj.Exported() {
 		return "", ""

internal/lsp/source/identifier.go

@@ -48,11 +48,11 @@ 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, fh FileHandle, pos protocol.Position, selectPackage PackagePolicy) (*IdentifierInfo, error) {
+func Identifier(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protocol.Position) (*IdentifierInfo, error) {
 	ctx, done := trace.StartSpan(ctx, "source.Identifier")
 	defer done()
 
-	pkg, pgh, err := getParsedFile(ctx, snapshot, fh, selectPackage)
+	pkg, pgh, err := getParsedFile(ctx, snapshot, fh, NarrowestPackageHandle)
 	if err != nil {
 		return nil, fmt.Errorf("getting file for Identifier: %v", err)
 	}

internal/lsp/source/source_test.go

@@ -484,7 +484,7 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	ident, err := source.Identifier(r.ctx, r.view.Snapshot(), fh, srcRng.Start, source.WidestPackageHandle)
+	ident, err := source.Identifier(r.ctx, r.view.Snapshot(), fh, srcRng.Start)
 	if err != nil {
 		t.Fatalf("failed for %v: %v", d.Src, err)
 	}
@@ -768,7 +768,7 @@ func (r *runner) PrepareRename(t *testing.T, src span.Span, want *source.Prepare
 		}
 		return
 	}
-	if want.Text == "" && item != nil {
+	if want.Text == "" {
 		t.Errorf("prepare rename failed for %v: expected nil, got %v", src, item)
 		return
 	}

internal/lsp/source/view.go

@@ -341,6 +341,7 @@ type Package interface {
 	GetTypesInfo() *types.Info
 	GetTypesSizes() types.Sizes
 	IsIllTyped() bool
+	ForTest() string
 	GetImport(pkgPath string) (Package, error)
 	Imports() []Package
 }

internal/lsp/testdata/godef/a/a_test.go

@@ -0,0 +1,8 @@
+package a
+
+import (
+	"testing"
+)
+
+func TestA(t *testing.T) { //@TestA,godef(TestA, TestA)
+}

internal/lsp/testdata/godef/a/a_test.go.golden

@@ -0,0 +1,26 @@
+-- TestA-definition --
+godef/a/a_test.go:7:6-11: defined here as ```go
+func TestA(t *testing.T)
+```
+-- TestA-definition-json --
+{
+	"span": {
+		"uri": "file://godef/a/a_test.go",
+		"start": {
+			"line": 7,
+			"column": 6,
+			"offset": 39
+		},
+		"end": {
+			"line": 7,
+			"column": 11,
+			"offset": 44
+		}
+	},
+	"description": "```go\nfunc TestA(t *testing.T)\n```"
+}
+
+-- TestA-hover --
+```go
+func TestA(t *testing.T)
+```

internal/lsp/testdata/godef/a/a_x_test.go

@@ -0,0 +1,8 @@
+package a_test
+
+import (
+	"testing"
+)
+
+func TestA2(t *testing.T) { //@TestA2,godef(TestA2, TestA2)
+}

internal/lsp/testdata/godef/a/a_x_test.go.golden

@@ -0,0 +1,26 @@
+-- TestA2-definition --
+godef/a/a_x_test.go:7:6-12: defined here as ```go
+func TestA2(t *testing.T)
+```
+-- TestA2-definition-json --
+{
+	"span": {
+		"uri": "file://godef/a/a_x_test.go",
+		"start": {
+			"line": 7,
+			"column": 6,
+			"offset": 44
+		},
+		"end": {
+			"line": 7,
+			"column": 12,
+			"offset": 50
+		}
+	},
+	"description": "```go\nfunc TestA2(t *testing.T)\n```"
+}
+
+-- TestA2-hover --
+```go
+func TestA2(t *testing.T)
+```

internal/lsp/testdata/summary.txt.golden

@@ -11,7 +11,7 @@ FoldingRangesCount = 2
 FormatCount = 6
 ImportCount = 7
 SuggestedFixCount = 1
-DefinitionsCount = 43
+DefinitionsCount = 45
 TypeDefinitionsCount = 2
 HighlightsCount = 52
 ReferencesCount = 8