gopls/internal/golang/completion: avoid crash in comment completion

Avoid an observed crash in comment completion, as well as another nearby
crash that was discovered in the course of debugging. Add debugging bug
reports for the case of a missing function definition.

Fixes golang/go#71273

Change-Id: Ibac993e0cf041fc074e5f00a7ce6d5718d77052f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/642877
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: findleyr

gopls/internal/golang/completion/completion.go

@@ -31,6 +31,7 @@ import (
 	"golang.org/x/tools/go/ast/astutil"
 	"golang.org/x/tools/gopls/internal/cache"
 	"golang.org/x/tools/gopls/internal/cache/metadata"
+	"golang.org/x/tools/gopls/internal/cache/parsego"
 	"golang.org/x/tools/gopls/internal/file"
 	"golang.org/x/tools/gopls/internal/fuzzy"
 	"golang.org/x/tools/gopls/internal/golang"
@@ -218,17 +219,16 @@ type completer struct {
 	// filename is the name of the file associated with this completion request.
 	filename string
 
-	// file is the AST of the file associated with this completion request.
-	file *ast.File
+	// pgf is the AST of the file associated with this completion request.
+	pgf *parsego.File // debugging
 
 	// goversion is the version of Go in force in the file, as
 	// defined by x/tools/internal/versions. Empty if unknown.
 	// Since go1.22 it should always be known.
 	goversion string
 
-	// (tokFile, pos) is the position at which the request was triggered.
-	tokFile *token.File
-	pos     token.Pos
+	// pos is the position at which the request was triggered.
+	pos token.Pos
 
 	// path is the path of AST nodes enclosing the position.
 	path []ast.Node
@@ -410,7 +410,7 @@ func (c *completer) setSurrounding(ident *ast.Ident) {
 		content: ident.Name,
 		cursor:  c.pos,
 		// Overwrite the prefix only.
-		tokFile: c.tokFile,
+		tokFile: c.pgf.Tok,
 		start:   ident.Pos(),
 		end:     ident.End(),
 		mapper:  c.mapper,
@@ -435,7 +435,7 @@ func (c *completer) getSurrounding() *Selection {
 		c.surrounding = &Selection{
 			content: "",
 			cursor:  c.pos,
-			tokFile: c.tokFile,
+			tokFile: c.pgf.Tok,
 			start:   c.pos,
 			end:     c.pos,
 			mapper:  c.mapper,
@@ -609,8 +609,7 @@ func Completion(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p
 		},
 		fh:                        fh,
 		filename:                  fh.URI().Path(),
-		tokFile:                   pgf.Tok,
-		file:                      pgf.File,
+		pgf:                       pgf,
 		goversion:                 goversion,
 		path:                      path,
 		pos:                       pos,
@@ -711,15 +710,15 @@ func Completion(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p
 // search queue or completion items directly for different completion contexts.
 func (c *completer) collectCompletions(ctx context.Context) error {
 	// Inside import blocks, return completions for unimported packages.
-	for _, importSpec := range c.file.Imports {
+	for _, importSpec := range c.pgf.File.Imports {
 		if !(importSpec.Path.Pos() <= c.pos && c.pos <= importSpec.Path.End()) {
 			continue
 		}
 		return c.populateImportCompletions(importSpec)
 	}
 
 	// Inside comments, offer completions for the name of the relevant symbol.
-	for _, comment := range c.file.Comments {
+	for _, comment := range c.pgf.File.Comments {
 		if comment.Pos() < c.pos && c.pos <= comment.End() {
 			c.populateCommentCompletions(comment)
 			return nil
@@ -749,7 +748,7 @@ func (c *completer) collectCompletions(ctx context.Context) error {
 
 	switch n := c.path[0].(type) {
 	case *ast.Ident:
-		if c.file.Name == n {
+		if c.pgf.File.Name == n {
 			return c.packageNameCompletions(ctx, c.fh.URI(), n)
 		} else if sel, ok := c.path[1].(*ast.SelectorExpr); ok && sel.Sel == n {
 			// Is this the Sel part of a selector?
@@ -921,14 +920,14 @@ func (c *completer) populateImportCompletions(searchImport *ast.ImportSpec) erro
 	c.surrounding = &Selection{
 		content: content,
 		cursor:  c.pos,
-		tokFile: c.tokFile,
+		tokFile: c.pgf.Tok,
 		start:   start,
 		end:     end,
 		mapper:  c.mapper,
 	}
 
 	seenImports := make(map[string]struct{})
-	for _, importSpec := range c.file.Imports {
+	for _, importSpec := range c.pgf.File.Imports {
 		if importSpec.Path.Value == importPath {
 			continue
 		}
@@ -1024,7 +1023,7 @@ func (c *completer) populateCommentCompletions(comment *ast.CommentGroup) {
 	c.setSurroundingForComment(comment)
 
 	// Using the next line pos, grab and parse the exported symbol on that line
-	for _, n := range c.file.Decls {
+	for _, n := range c.pgf.File.Decls {
 		declLine := safetoken.Line(file, n.Pos())
 		// if the comment is not in, directly above or on the same line as a declaration
 		if declLine != commentLine && declLine != commentLine+1 &&
@@ -1080,8 +1079,33 @@ func (c *completer) populateCommentCompletions(comment *ast.CommentGroup) {
 
 			// collect receiver struct fields
 			if node.Recv != nil {
-				sig := c.pkg.TypesInfo().Defs[node.Name].(*types.Func).Signature()
-				_, named := typesinternal.ReceiverNamed(sig.Recv()) // may be nil if ill-typed
+				obj := c.pkg.TypesInfo().Defs[node.Name]
+				switch obj.(type) {
+				case nil:
+					report := func() {
+						bug.Reportf("missing def for func %s", node.Name)
+					}
+					// Debugging golang/go#71273.
+					if !slices.Contains(c.pkg.CompiledGoFiles(), c.pgf) {
+						if c.snapshot.View().Type() == cache.GoPackagesDriverView {
+							report()
+						} else {
+							report()
+						}
+					} else {
+						report()
+					}
+					continue
+				case *types.Func:
+				default:
+					bug.Reportf("unexpected func obj type %T for %s", obj, node.Name)
+				}
+				sig := obj.(*types.Func).Signature()
+				recv := sig.Recv()
+				if recv == nil {
+					continue // may be nil if ill-typed
+				}
+				_, named := typesinternal.ReceiverNamed(recv)
 				if named != nil {
 					if recvStruct, ok := named.Underlying().(*types.Struct); ok {
 						for i := 0; i < recvStruct.NumFields(); i++ {
@@ -1133,7 +1157,7 @@ func (c *completer) setSurroundingForComment(comments *ast.CommentGroup) {
 	c.surrounding = &Selection{
 		content: cursorComment.Text[start:end],
 		cursor:  c.pos,
-		tokFile: c.tokFile,
+		tokFile: c.pgf.Tok,
 		start:   token.Pos(int(cursorComment.Slash) + start),
 		end:     token.Pos(int(cursorComment.Slash) + end),
 		mapper:  c.mapper,
@@ -1437,7 +1461,7 @@ func (c *completer) selector(ctx context.Context, sel *ast.SelectorExpr) error {
 		return nil
 	}
 
-	goversion := c.pkg.TypesInfo().FileVersions[c.file]
+	goversion := c.pkg.TypesInfo().FileVersions[c.pgf.File]
 
 	// Extract the package-level candidates using a quick parse.
 	var g errgroup.Group
@@ -1694,7 +1718,7 @@ func (c *completer) lexical(ctx context.Context) error {
 				// Make sure the package name isn't already in use by another
 				// object, and that this file doesn't import the package yet.
 				// TODO(adonovan): what if pkg.Path has vendor/ prefix?
-				if _, ok := seen[pkg.Name()]; !ok && pkg != c.pkg.Types() && !alreadyImports(c.file, golang.ImportPath(pkg.Path())) {
+				if _, ok := seen[pkg.Name()]; !ok && pkg != c.pkg.Types() && !alreadyImports(c.pgf.File, golang.ImportPath(pkg.Path())) {
 					seen[pkg.Name()] = struct{}{}
 					obj := types.NewPkgName(0, nil, pkg.Name(), pkg)
 					imp := &importInfo{

gopls/internal/golang/completion/postfix_snippets.go

@@ -653,7 +653,7 @@ func (c *completer) importIfNeeded(pkgPath string, scope *types.Scope) (string,
 	defaultName := imports.ImportPathToAssumedName(pkgPath)
 
 	// Check if file already imports pkgPath.
-	for _, s := range c.file.Imports {
+	for _, s := range c.pgf.File.Imports {
 		// TODO(adonovan): what if pkgPath has a vendor/ suffix?
 		// This may be the cause of go.dev/issue/56291.
 		if string(metadata.UnquoteImportPath(s)) == pkgPath {

gopls/internal/golang/completion/util.go

@@ -284,7 +284,7 @@ func isBasicKind(t types.Type, k types.BasicInfo) bool {
 }
 
 func (c *completer) editText(from, to token.Pos, newText string) ([]protocol.TextEdit, error) {
-	start, end, err := safetoken.Offsets(c.tokFile, from, to)
+	start, end, err := safetoken.Offsets(c.pgf.Tok, from, to)
 	if err != nil {
 		return nil, err // can't happen: from/to came from c
 	}

gopls/internal/test/marker/testdata/completion/comment.txt

@@ -79,3 +79,9 @@ func Multiline() int { //@item(multiline, "Multiline", "func() int", "func")
 	// //@complete(" ", multiline)
 	return 0
 }
+
+// This test checks that gopls does not panic if the receiver is syntactically
+// present but empty.
+//
+// //@complete(" ")
+func () _() {}

internal/packagesinternal/packages.go

@@ -5,7 +5,7 @@
 // Package packagesinternal exposes internal-only fields from go/packages.
 package packagesinternal
 
-var GetDepsErrors = func(p interface{}) []*PackageError { return nil }
+var GetDepsErrors = func(p any) []*PackageError { return nil }
 
 type PackageError struct {
 	ImportStack []string // shortest path from package named on command line to this one
@@ -16,5 +16,5 @@ type PackageError struct {
 var TypecheckCgo int
 var DepsErrors int // must be set as a LoadMode to call GetDepsErrors
 
-var SetModFlag = func(config interface{}, value string) {}
+var SetModFlag = func(config any, value string) {}
 var SetModFile = func(config interface{}, value string) {}