Skip to content

Commit

Permalink
gopls/internal/lsp/source: test references bug on struct{p.T}
Browse files Browse the repository at this point in the history
This change adds a regression test for bug in gopls' references
operation applied to the T identifier in an embedded struct field
such as struct{p.T): instead of reporting references to T,
it reports references to package name p.

This is a consequence of go/types bug golang/go#60372,
which sets the position of the struct field types.Var to
that of the ast.Field syntax (p) not the type name (T).
The bug was fixed in go1.21.

Updates golang/go#60372
Fixes golang/go#60369

Change-Id: Ibabe885ea689b30d966dbf7e51f8c25e44a6ce1c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/497495
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
  • Loading branch information
adonovan committed May 25, 2023
1 parent a12e1a6 commit 827f5aa
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 0 deletions.
8 changes: 8 additions & 0 deletions gopls/internal/lsp/source/references.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ func ordinaryReferences(ctx context.Context, snapshot Snapshot, uri span.URI, pp
}

// Find the selected object (declaration or reference).
// For struct{T}, we choose the field (Def) over the type (Use).
pos, err := pgf.PositionPos(pp)
if err != nil {
return nil, err
Expand Down Expand Up @@ -649,6 +650,13 @@ func objectsAt(info *types.Info, file *ast.File, pos token.Pos) (map[types.Objec
targets[obj] = leaf
}
} else {
// Note: prior to go1.21, go/types issue #60372 causes the position
// a field Var T created for struct{*p.T} to be recorded at the
// start of the field type ("*") not the location of the T.
// This affects references and other gopls operations (issue #60369).
// TODO(adonovan): delete this comment when we drop support for go1.20.

// For struct{T}, we prefer the defined field Var over the used TypeName.
obj := info.ObjectOf(leaf)
if obj == nil {
return nil, nil, fmt.Errorf("%w for %q", errNoObjectFound, leaf.Name)
Expand Down
27 changes: 27 additions & 0 deletions gopls/internal/regtest/marker/testdata/references/issue60369.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
Regression test for 'references' bug golang/go#60369: a references
query on the embedded type name T in struct{p.T} instead reports all
references to the package name p.

The bug was fixed in release go1.21 of go/types.

-- flags --
-min_go=go1.21

-- go.mod --
module example.com
go 1.12

-- a/a.go --
package a

type A struct{}
const C = 0

-- b/b.go --
package b

import a "example.com/a" //@loc(adef, "a")
type s struct { a.A } //@loc(Aref1, "A"), loc(aref1, "a"), refs(Aref1, Aref1, Aref3), refs(aref1, adef, aref1, aref2, aref3)
var _ a.A //@loc(aref2, re" (a)"), loc(Aref2, "A")
var _ = s{}.A //@loc(Aref3, "A")
const c = a.C //@loc(aref3, "a")

0 comments on commit 827f5aa

Please sign in to comment.