gopls/internal/lsp/source: fix panic in typeDefinition on comparable

comparable is also permitted to have no position.

Updates golang/go#60544

Change-Id: Ic0694796432ab8b3271a60e4f4f649a1657d462b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/499986
gopls-CI: kokoro <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Robert Findley <[email protected]>

Author: findleyr

gopls/internal/lsp/source/identifier.go

@@ -56,7 +56,7 @@ func searchForEnclosing(info *types.Info, path []ast.Node) *types.TypeName {
 
 // typeToObject returns the relevant type name for the given type, after
 // unwrapping pointers, arrays, slices, channels, and function signatures with
-// a single non-error result.
+// a single non-error result, and ignoring built-in named types.
 func typeToObject(typ types.Type) *types.TypeName {
 	switch typ := typ.(type) {
 	case *types.Named:
@@ -79,7 +79,7 @@ func typeToObject(typ types.Type) *types.TypeName {
 		for i := 0; i < results.Len(); i++ {
 			obj := typeToObject(results.At(i).Type())
 			if obj == nil || hasErrorType(obj) {
-				// Skip builtins.
+				// Skip builtins. TODO(rfindley): should comparable be handled here as well?
 				continue
 			}
 			if res != nil {

gopls/internal/lsp/source/type_definition.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"go/token"
 
+	"golang.org/x/tools/gopls/internal/bug"
 	"golang.org/x/tools/gopls/internal/lsp/protocol"
 	"golang.org/x/tools/internal/event"
 )
@@ -35,19 +36,20 @@ func TypeDefinition(ctx context.Context, snapshot Snapshot, fh FileHandle, posit
 		return nil, nil
 	}
 
-	typObj := typeToObject(obj.Type())
-	if typObj == nil {
+	tname := typeToObject(obj.Type())
+	if tname == nil {
 		return nil, fmt.Errorf("no type definition for %s", obj.Name())
 	}
 
-	// Identifiers with the type "error" are a special case with no position.
-	if hasErrorType(typObj) {
-		// TODO(rfindley): we can do better here, returning a link to the builtin
-		// file.
+	if !tname.Pos().IsValid() {
+		// The only defined types with no position are error and comparable.
+		if tname.Name() != "error" && tname.Name() != "comparable" {
+			bug.Reportf("unexpected type name with no position: %s", tname)
+		}
 		return nil, nil
 	}
 
-	loc, err := mapPosition(ctx, pkg.FileSet(), snapshot, typObj.Pos(), typObj.Pos()+token.Pos(len(typObj.Name())))
+	loc, err := mapPosition(ctx, pkg.FileSet(), snapshot, tname.Pos(), tname.Pos()+token.Pos(len(tname.Name())))
 	if err != nil {
 		return nil, err
 	}

gopls/internal/regtest/misc/definition_test.go

@@ -371,6 +371,28 @@ func main() {}
 	}
 }
 
+func TestGoToTypeDefinition_Issue60544(t *testing.T) {
+	const mod = `
+-- go.mod --
+module mod.com
+
+go 1.19
+-- main.go --
+package main
+
+func F[T comparable]() {}
+`
+
+	Run(t, mod, func(t *testing.T, env *Env) {
+		env.OpenFile("main.go")
+
+		_, err := env.Editor.GoToTypeDefinition(env.Ctx, env.RegexpSearch("main.go", "comparable")) // must not panic
+		if err != nil {
+			t.Fatal(err)
+		}
+	})
+}
+
 // Test for golang/go#47825.
 func TestImportTestVariant(t *testing.T) {
 	const mod = `