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 <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/lsp/source/identifier.go b/gopls/internal/lsp/source/identifier.go
index 15fe13a..57001af 100644
--- a/gopls/internal/lsp/source/identifier.go
+++ b/gopls/internal/lsp/source/identifier.go
@@ -56,7 +56,7 @@
// 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 @@
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 {
diff --git a/gopls/internal/lsp/source/type_definition.go b/gopls/internal/lsp/source/type_definition.go
index 73fc965..6c26b16 100644
--- a/gopls/internal/lsp/source/type_definition.go
+++ b/gopls/internal/lsp/source/type_definition.go
@@ -9,6 +9,7 @@
"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 @@
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
}
diff --git a/gopls/internal/regtest/misc/definition_test.go b/gopls/internal/regtest/misc/definition_test.go
index 9f24ef6..0a36336 100644
--- a/gopls/internal/regtest/misc/definition_test.go
+++ b/gopls/internal/regtest/misc/definition_test.go
@@ -371,6 +371,28 @@
}
}
+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 = `
