gopls/internal/golang: reproduce various edge cases of issue 69616

For some reason, I finally realized how the "impossible" edge cases of
golang/go#69616 could occur: in one case, there are scenarios where the
type checker doesn't actually type-check a compiled go file. In another,
there is exactly one package with a Go file that isn't compiled: unsafe.

These reproduced the two bug reports in golang/go#69616. Add a test and
remove the bug reports with an explanation.

Fixes golang/go#69616

Change-Id: Id73aab618880acf85b3c63e5b2275ab01ac2c1e9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/684615
Reviewed-by: Madeline Kalil <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Robert Findley <[email protected]>

Author: findleyr

gopls/internal/golang/comment.go

@@ -13,15 +13,13 @@ import (
 	"go/token"
 	"go/types"
 	pathpkg "path"
-	"slices"
 	"strings"
 
 	"golang.org/x/tools/gopls/internal/cache"
 	"golang.org/x/tools/gopls/internal/cache/parsego"
 	"golang.org/x/tools/gopls/internal/protocol"
 	"golang.org/x/tools/gopls/internal/settings"
 	"golang.org/x/tools/gopls/internal/util/astutil"
-	"golang.org/x/tools/gopls/internal/util/bug"
 	"golang.org/x/tools/gopls/internal/util/safetoken"
 )
 
@@ -61,7 +59,7 @@ func DocCommentToMarkdown(text string, options *settings.Options) string {
 // docLinkDefinition finds the definition of the doc link in comments at pos.
 // If there is no reference at pos, returns errNoCommentReference.
 func docLinkDefinition(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, pos token.Pos) ([]protocol.Location, error) {
-	obj, _, err := parseDocLink(pkg, pgf, pos)
+	obj, _, err := resolveDocLink(pkg, pgf, pos)
 	if err != nil {
 		return nil, err
 	}
@@ -72,9 +70,9 @@ func docLinkDefinition(ctx context.Context, snapshot *cache.Snapshot, pkg *cache
 	return []protocol.Location{loc}, nil
 }
 
-// parseDocLink parses a doc link in a comment such as [fmt.Println]
+// resolveDocLink parses a doc link in a comment such as [fmt.Println]
 // and returns the symbol at pos, along with the link's start position.
-func parseDocLink(pkg *cache.Package, pgf *parsego.File, pos token.Pos) (types.Object, protocol.Range, error) {
+func resolveDocLink(pkg *cache.Package, pgf *parsego.File, pos token.Pos) (types.Object, protocol.Range, error) {
 	var comment *ast.Comment
 	for _, cg := range pgf.File.Comments {
 		for _, c := range cg.List {
@@ -154,15 +152,10 @@ func lookupDocLinkSymbol(pkg *cache.Package, pgf *parsego.File, name string) typ
 	// allowing for non-renaming and renaming imports.
 	fileScope := pkg.TypesInfo().Scopes[pgf.File]
 	if fileScope == nil {
-		// This is theoretically possible if pgf is a GoFile but not a
-		// CompiledGoFile. However, we do not know how to produce such a package
-		// without using an external GoPackagesDriver.
-		// See if this is the source of golang/go#70635
-		if slices.Contains(pkg.CompiledGoFiles(), pgf) {
-			bug.Reportf("missing file scope for compiled file")
-		} else {
-			bug.Reportf("missing file scope for non-compiled file")
-		}
+		// As we learned in golang/go#69616, any file may not be Scopes!
+		//  - A non-compiled Go file (such as unsafe.go) won't be in Scopes.
+		//  - A (technically) compiled go file with the wrong package name won't be
+		//    in Scopes, as it will be skipped by go/types.
 		return nil
 	}
 	pkgname, ok := fileScope.Lookup(prefix).(*types.PkgName) // ok => prefix is imported name

gopls/internal/golang/hover.go

@@ -246,7 +246,7 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro
 	}
 
 	// Handle hovering over a doc link
-	if obj, rng, _ := parseDocLink(pkg, pgf, pos); obj != nil {
+	if obj, rng, _ := resolveDocLink(pkg, pgf, pos); obj != nil {
 		// Built-ins have no position.
 		if isBuiltin(obj) {
 			h, err := hoverBuiltin(ctx, snapshot, obj)

gopls/internal/test/integration/misc/definition_test.go

@@ -699,3 +699,51 @@ func Foo() {
 		}
 	})
 }
+
+func TestCommentDefinition_Issue69616(t *testing.T) {
+	// This test exercises a few edge cases discovered by telemetry in
+	// golang/go#69616, namely situations where a parsed Go file might
+	// not have an associated scope in the package types.Info.
+	//
+	// The files below set up two types of edge cases:
+	//  - a 'compiled' Go file that isn't actually type-checked, because it has
+	//    the wrong package name
+	//  - a non-compiled Go file (unsafe.go).
+	const src = `
+-- go.mod --
+module mod.com
+
+go 1.21
+-- cmd/main.go --
+package main
+
+import "unsafe"
+
+var _  = unsafe.Offsetof
+
+func Foo() {}
+-- cmd/x.go --
+package x
+
+// Bar is like [Foo]
+func Bar() {}
+`
+
+	Run(t, src, func(t *testing.T, env *Env) {
+		// First, check that we don't produce a crash or bug when
+		// finding definitions in a 'compiled' go file that isn't actually type
+		// checked.
+		env.OpenFile("cmd/x.go")
+		_, _ = env.Editor.Definitions(env.Ctx, env.RegexpSearch("cmd/x.go", "()Foo"))
+
+		// Next, go to the unsafe package, and find the doc link to [Sizeof].
+		// It will also fail to resolve, because unsafe.go isn't compiled, but
+		// again should not panic or result in a bug.
+		env.OpenFile("cmd/main.go")
+		loc := env.FirstDefinition(env.RegexpSearch("cmd/main.go", `unsafe\.(Offsetof)`))
+		unsafePath := loc.URI.Path()
+		env.OpenFile(unsafePath)
+		_, _ = env.Editor.Definitions(env.Ctx,
+			env.RegexpSearch(unsafePath, `\[()Sizeof\]`))
+	})
+}