Madeline Kalil (Gerrit)

unread,

Dec 12, 2025, 2:49:09 PM (4 days ago) Dec 12

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Madeline Kalil has uploaded the change for review

Commit message

gopls/internal/golang: implement type definition for builtin types

Currently, type definition queries on builtin types such as int
and bool return no results.
Meanwhile, type definition queries on the builtin "error" return
the same location as a normal definition query - its
fake declaration in {builtin,unsafe}.go.
We should make this behavior consistent across all builtin types
by returning the source declaration for both definition and type
definition queries.

Change-Id: Ifb3cea6bd679b6d445afa5caa62799a6bc88160b

Change diff

diff --git a/gopls/internal/golang/type_definition.go b/gopls/internal/golang/type_definition.go
index 48b62fe..1e3d2c3 100644
--- a/gopls/internal/golang/type_definition.go
+++ b/gopls/internal/golang/type_definition.go
@@ -69,6 +69,25 @@
 	if t == nil {
 		return nil, fmt.Errorf("no enclosing expression has a type")
 	}
+	if _, ok := t.(*types.Basic); ok {
+		objects, err := objectsAt(pkg.TypesInfo(), cur)
+		if err != nil {
+			return nil, err
+		}
+		obj := objects[0].obj
+		if isBuiltin(obj) {
+			// Returns fake source declaration in {builtin,unsafe}.go.
+			pgf, ident, err := builtinDecl(ctx, snapshot, obj)
+			if err != nil {
+				return nil, err
+			}
+			loc, err := pgf.NodeLocation(ident)
+			if err != nil {
+				return nil, err
+			}
+			return []protocol.Location{loc}, nil
+		}
+	}
 	tname := typeToObject(t)
 	if tname == nil {
 		return nil, fmt.Errorf("cannot find type name from type %s", t)
diff --git a/gopls/internal/test/marker/testdata/typedef/typedef.txt b/gopls/internal/test/marker/testdata/typedef/typedef.txt
index 940b275..1245892 100644
--- a/gopls/internal/test/marker/testdata/typedef/typedef.txt
+++ b/gopls/internal/test/marker/testdata/typedef/typedef.txt
@@ -45,7 +45,7 @@
 func F3() (Struct, int, bool, error)       { return Struct{}, 0, false, nil }
 func F4() (**int, Int, bool, *error)       { return nil, 0, false, nil }
 func F5() (int, float64, error, Struct)    { return 0, 0, nil, Struct{} }
-func F6() (int, float64, ***Struct, error) { return 0, 0, nil, nil }
+func F6() (int, float64, ***Struct, error) { return 0, 0, nil, nil } //@ typedef("int", BUILTIN), typedef("float64", BUILTIN), typedef("error", BUILTIN)
 
 func _() {
 	F1() //@typedef("F1", Int)
@@ -57,6 +57,7 @@
 
 	f := func() Int { return 0 }
 	f() //@typedef("f", Int)
+	_ = append([]int{1, 2}, 3) //@ typedef("append", BUILTIN)
 }
 
 // https://github.com/golang/go/issues/38589#issuecomment-620350922
@@ -77,7 +78,7 @@
 
 	// And in this one, it's the composite literal enclosing the
 	// KeyValueExpr denoted by the colon (which must not be adjacent
-	// to either they key or the value!).
+	// to either the key or the value!).
 	_ = Struct{Field  :  ""} //@typedef(":", Struct)
 }

Change information

Files:

M gopls/internal/golang/type_definition.go
M gopls/internal/test/marker/testdata/typedef/typedef.txt

Change size: S

Delta: 2 files changed, 22 insertions(+), 2 deletions(-)

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Madeline Kalil (Gerrit)

unread,

Dec 12, 2025, 2:49:24 PM (4 days ago) Dec 12

to goph...@pubsubhelper.golang.org, Alan Donovan, golang-co...@googlegroups.com

Attention needed from Alan Donovan

Madeline Kalil voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

Alan Donovan

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Alan Donovan (Gerrit)

unread,

Dec 12, 2025, 2:55:12 PM (4 days ago) Dec 12

to Madeline Kalil, goph...@pubsubhelper.golang.org, Hongxiang Jiang, Go LUCI, golang-co...@googlegroups.com

Attention needed from Madeline Kalil

Alan Donovan added 2 comments

File gopls/internal/golang/type_definition.go

Line 72, Patchset 1 (Latest): if _, ok := t.(*types.Basic); ok {

Alan Donovan . unresolved

If `typeToObject` also reported TypeNames for Basic types (currently it does not), would the existing logic do the right thing? ObjectLocation already knows how to handle built-ins.

Line 73, Patchset 1 (Latest): objects, err := objectsAt(pkg.TypesInfo(), cur)

Alan Donovan . unresolved

This value of cur comes from L34, not the assignments to the loop variable (also named cur) at L45, so I think the behavior will be inconsistent with other types. I think using typeToObject might be a better solution.

Open in Gerrit

Related details

Attention is currently required from:

Madeline Kalil

Submit Requirements:

Code-Review

No-Unresolved-Comments

Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Madeline Kalil (Gerrit)

unread,

Dec 15, 2025, 12:55:16 PM (yesterday) Dec 15

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention needed from Madeline Kalil

Madeline Kalil uploaded new patchset

Madeline Kalil uploaded patch set #2 to this change.

Following approvals got outdated and were removed:

TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI

Open in Gerrit

Related details

Attention is currently required from:

Madeline Kalil

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Madeline Kalil (Gerrit)

unread,

Dec 15, 2025, 12:56:47 PM (yesterday) Dec 15

to goph...@pubsubhelper.golang.org, Hongxiang Jiang, Go LUCI, Alan Donovan, golang-co...@googlegroups.com

Attention needed from Alan Donovan and Hongxiang Jiang

Madeline Kalil voted and added 2 comments

Votes added by Madeline Kalil

Commit-Queue

+1

2 comments

File gopls/internal/golang/type_definition.go

Line 72, Patchset 1: if _, ok := t.(*types.Basic); ok {

Alan Donovan . resolved

If `typeToObject` also reported TypeNames for Basic types (currently it does not), would the existing logic do the right thing? ObjectLocation already knows how to handle built-ins.

Madeline Kalil

Added the *types.Basic case to typeToObject

Line 73, Patchset 1: objects, err := objectsAt(pkg.TypesInfo(), cur)

Alan Donovan . resolved

This value of cur comes from L34, not the assignments to the loop variable (also named cur) at L45, so I think the behavior will be inconsistent with other types. I think using typeToObject might be a better solution.

Madeline Kalil

Done

Open in Gerrit

Related details

Attention is currently required from:

Alan Donovan
Hongxiang Jiang

Submit Requirements:

Code-Review

No-Unresolved-Comments

Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Alan Donovan (Gerrit)

unread,

Dec 15, 2025, 1:25:20 PM (yesterday) Dec 15

to Madeline Kalil, goph...@pubsubhelper.golang.org, Go LUCI, Hongxiang Jiang, golang-co...@googlegroups.com

Attention needed from Hongxiang Jiang and Madeline Kalil

Alan Donovan added 3 comments

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Alan Donovan . resolved

Nice.

File gopls/internal/golang/identifier.go

Line 91, Patchset 2 (Latest): obj := types.Universe.Lookup(typ.Name())

Alan Donovan . unresolved

You can kill two stones with one bird: if Lookup returns nil, then the type test will (safely) fail:
```
tname, ok := types.Universe.Lookup(typ.Name()).(*types.TypeName)
```

File gopls/internal/test/marker/testdata/typedef/typedef.txt

Line 64, Patchset 2 (Latest): F5() //@typedef(re"F5()", Struct, BUILTIN, BUILTIN, BUILTIN)

Alan Donovan . resolved

This reads nicely, but the marker test currently only checks for set-equality, so neither the order nor cardinality of each item actually matters. (There's an argument that since the LSP result is an ordered list, our tests should use list semantics, at least optionally.)

Open in Gerrit

Related details

Attention is currently required from:

Hongxiang Jiang
Madeline Kalil

Submit Requirements:

Code-Review

No-Unresolved-Comments

Review-Enforcement

TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Madeline Kalil (Gerrit)

unread,

Dec 15, 2025, 2:08:59 PM (yesterday) Dec 15

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention needed from Hongxiang Jiang and Madeline Kalil

Madeline Kalil uploaded new patchset

Madeline Kalil uploaded patch set #3 to this change.

Following approvals got outdated and were removed:

TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI

Open in Gerrit

Related details

Attention is currently required from:

Hongxiang Jiang
Madeline Kalil

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement

TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Madeline Kalil (Gerrit)

unread,

Dec 15, 2025, 2:10:16 PM (yesterday) Dec 15

to goph...@pubsubhelper.golang.org, Go LUCI, Hongxiang Jiang, Alan Donovan, golang-co...@googlegroups.com

Attention needed from Alan Donovan and Hongxiang Jiang

Madeline Kalil added 2 comments

File gopls/internal/golang/identifier.go

Line 91, Patchset 2: obj := types.Universe.Lookup(typ.Name())

Alan Donovan . resolved

You can kill two stones with one bird: if Lookup returns nil, then the type test will (safely) fail:
```
tname, ok := types.Universe.Lookup(typ.Name()).(*types.TypeName)
```

Madeline Kalil

Done

File gopls/internal/test/marker/testdata/typedef/typedef.txt

Line 64, Patchset 2: F5() //@typedef(re"F5()", Struct, BUILTIN, BUILTIN, BUILTIN)

Alan Donovan . resolved

This reads nicely, but the marker test currently only checks for set-equality, so neither the order nor cardinality of each item actually matters. (There's an argument that since the LSP result is an ordered list, our tests should use list semantics, at least optionally.)

Madeline Kalil

Good to know! I updated the marker test docs as well (a "set" or "list" of locations was actually already specified for every marker except typedef)

Open in Gerrit

Related details

Attention is currently required from:

Alan Donovan
Hongxiang Jiang

Submit Requirements:

Code-Review

No-Unresolved-Comments

Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Alan Donovan (Gerrit)

unread,

Dec 15, 2025, 4:15:40 PM (22 hours ago) Dec 15

to Madeline Kalil, goph...@pubsubhelper.golang.org, Go LUCI, Hongxiang Jiang, golang-co...@googlegroups.com

Attention needed from Hongxiang Jiang and Madeline Kalil

Alan Donovan voted and added 2 comments

Votes added by Alan Donovan

Code-Review

+2

2 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Alan Donovan . resolved

LGTM

File gopls/internal/test/marker/doc.go

Line 339, Patchset 3 (Latest):for set-equality, so the order and cardinality of locations does not matter. Of

Alan Donovan . unresolved

space, not hyphen

Open in Gerrit

Related details

Attention is currently required from:

Hongxiang Jiang
Madeline Kalil

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement

TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Hongxiang Jiang (Gerrit)

unread,

Dec 15, 2025, 7:15:40 PM (19 hours ago) Dec 15

to Madeline Kalil, goph...@pubsubhelper.golang.org, Alan Donovan, Go LUCI, golang-co...@googlegroups.com

Attention needed from Madeline Kalil

Hongxiang Jiang added 1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Hongxiang Jiang . resolved

Could you delete the TODO from

`gopls/internal/golang/type_definition.go`

Open in Gerrit

Related details

Attention is currently required from:

Madeline Kalil

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Hongxiang Jiang (Gerrit)

unread,

Dec 15, 2025, 7:15:54 PM (19 hours ago) Dec 15

to Madeline Kalil, goph...@pubsubhelper.golang.org, Alan Donovan, Go LUCI, golang-co...@googlegroups.com

Attention needed from Madeline Kalil

Hongxiang Jiang voted and added 1 comment

Votes added by Hongxiang Jiang

Code-Review

+2

1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Hongxiang Jiang . unresolved

Could you delete the TODO from
`gopls/internal/golang/type_definition.go`

Hongxiang Jiang

Unresolved.

Open in Gerrit

Related details

Attention is currently required from:

Madeline Kalil

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Madeline Kalil (Gerrit)

unread,

1:13 PM (1 hour ago) 1:13 PM

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention needed from Alan Donovan, Hongxiang Jiang and Madeline Kalil

Madeline Kalil uploaded new patchset

Madeline Kalil uploaded patch set #4 to this change.

Following approvals got outdated and were removed:

Code-Review: +2 by Alan Donovan, +2 by Hongxiang Jiang

Open in Gerrit

Related details

Attention is currently required from:

Alan Donovan
Hongxiang Jiang
Madeline Kalil

Submit Requirements:

Code-Review

No-Unresolved-Comments

Review-Enforcement

TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Madeline Kalil (Gerrit)

unread,

1:14 PM (1 hour ago) 1:14 PM

to goph...@pubsubhelper.golang.org, Hongxiang Jiang, Alan Donovan, Go LUCI, golang-co...@googlegroups.com

Attention needed from Alan Donovan and Hongxiang Jiang

Madeline Kalil voted and added 2 comments

Votes added by Madeline Kalil

Commit-Queue

+1

2 comments

Patchset-level comments

File-level comment, Patchset 3:

Hongxiang Jiang . resolved

Could you delete the TODO from
`gopls/internal/golang/type_definition.go`

Hongxiang Jiang

Unresolved.

Madeline Kalil

Done

File gopls/internal/test/marker/doc.go

Line 339, Patchset 3:for set-equality, so the order and cardinality of locations does not matter. Of

Alan Donovan . resolved

space, not hyphen

Madeline Kalil

Done

Open in Gerrit

Related details

Attention is currently required from:

Alan Donovan
Hongxiang Jiang

Submit Requirements:

Code-Review

No-Unresolved-Comments

Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Hongxiang Jiang (Gerrit)

unread,

2:19 PM (17 minutes ago) 2:19 PM

to Madeline Kalil, goph...@pubsubhelper.golang.org, Go LUCI, Alan Donovan, golang-co...@googlegroups.com

Attention needed from Alan Donovan and Madeline Kalil

Hongxiang Jiang voted Code-Review+2

Code-Review

+2

Open in Gerrit

Related details

Attention is currently required from:

Alan Donovan
Madeline Kalil

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[tools] gopls/internal/golang: implement type definition for builtin types

Madeline Kalil (Gerrit)

Madeline Kalil has uploaded the change for review

Commit message

Change diff

Change information

Related details

Madeline Kalil (Gerrit)

Madeline Kalil voted Commit-Queue+1

Related details

Alan Donovan (Gerrit)

Alan Donovan added 2 comments

Related details

Madeline Kalil (Gerrit)

Madeline Kalil uploaded new patchset

Related details

Madeline Kalil (Gerrit)

Madeline Kalil voted and added 2 comments

Votes added by Madeline Kalil

2 comments

Related details

Alan Donovan (Gerrit)

Alan Donovan added 3 comments

Related details

Madeline Kalil (Gerrit)

Madeline Kalil uploaded new patchset

Related details

Madeline Kalil (Gerrit)

Madeline Kalil added 2 comments

Related details

Alan Donovan (Gerrit)

Alan Donovan voted and added 2 comments

Votes added by Alan Donovan

2 comments

Related details

Hongxiang Jiang (Gerrit)

Hongxiang Jiang added 1 comment

Related details

Hongxiang Jiang (Gerrit)

Hongxiang Jiang voted and added 1 comment

Votes added by Hongxiang Jiang

1 comment

Related details

Madeline Kalil (Gerrit)

Madeline Kalil uploaded new patchset

Related details

Madeline Kalil (Gerrit)

Madeline Kalil voted and added 2 comments

Votes added by Madeline Kalil

2 comments

Related details

Hongxiang Jiang (Gerrit)

Hongxiang Jiang voted Code-Review+2

Related details