gopls/internal/golang: typeDefinition: allow any expression
Previously, typeDefinition required an identifier with a type.
Now it permits any expression.
Also
- fix the bug in the type-switch edge case in the TODO comment.
- fix bug in marker test of @typedef; now it follows the shape
of ordinary @def, and supports multiple locations.
And is documented.
(The real motive for this change was to remove a use of
referencedObject, which needs to be melted down for golang/go#75975.)
+ tests
For golang/go#69058
For golang/go#75975
Change-Id: If8d29d3b920a51f12624ed512c29292bdbb4fbb4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/713600
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Hongxiang Jiang <hxjiang@golang.org>
diff --git a/gopls/internal/golang/call_hierarchy.go b/gopls/internal/golang/call_hierarchy.go
index 0a82308..31389a4 100644
--- a/gopls/internal/golang/call_hierarchy.go
+++ b/gopls/internal/golang/call_hierarchy.go
@@ -39,7 +39,7 @@
return nil, err
}
- _, obj, _ := referencedObject(pkg, pgf, pos)
+ _, obj, _ := referencedObject(pkg.TypesInfo(), pgf, pos)
if obj == nil {
return nil, nil
}
@@ -202,7 +202,7 @@
return nil, err
}
- _, obj, _ := referencedObject(pkg, pgf, pos)
+ _, obj, _ := referencedObject(pkg.TypesInfo(), pgf, pos)
if obj == nil {
return nil, nil
}
@@ -271,7 +271,7 @@
outgoingCalls := make(map[protocol.Location]*protocol.CallHierarchyOutgoingCall)
for _, callRange := range callRanges {
- _, obj, _ := referencedObject(declPkg, declPGF, callRange.start)
+ _, obj, _ := referencedObject(declPkg.TypesInfo(), declPGF, callRange.start)
if obj == nil {
continue
}
diff --git a/gopls/internal/golang/definition.go b/gopls/internal/golang/definition.go
index 5c5a076..cace355 100644
--- a/gopls/internal/golang/definition.go
+++ b/gopls/internal/golang/definition.go
@@ -165,7 +165,7 @@
}
// The general case: the cursor is on an identifier.
- _, obj, _ := referencedObject(pkg, pgf, pos)
+ _, obj, _ := referencedObject(pkg.TypesInfo(), pgf, pos)
if obj == nil {
return nil, nil
}
@@ -321,16 +321,13 @@
// TODO(rfindley): this function exists to preserve the pre-existing behavior
// of golang.Identifier. Eliminate this helper in favor of sharing
// functionality with objectsAt, after choosing suitable primitives.
-func referencedObject(pkg *cache.Package, pgf *parsego.File, pos token.Pos) (*ast.Ident, types.Object, types.Type) {
+func referencedObject(info *types.Info, pgf *parsego.File, pos token.Pos) (*ast.Ident, types.Object, types.Type) {
path := pathEnclosingObjNode(pgf.File, pos)
if len(path) == 0 {
return nil, nil, nil
}
- var obj types.Object
- info := pkg.TypesInfo()
- switch n := path[0].(type) {
- case *ast.Ident:
- obj = info.ObjectOf(n)
+ if id, ok := path[0].(*ast.Ident); ok {
+ obj := info.ObjectOf(id)
// If n is the var's declaring ident in a type switch
// [i.e. the x in x := foo.(type)], it will not have an object. In this
// case, set obj to the first implicit object (if any), and return the type
@@ -340,7 +337,7 @@
// implicit objects; this is a type error ("unused x"),
if obj == nil {
if implicits, typ := typeSwitchImplicits(info, path); len(implicits) > 0 {
- return n, implicits[0], typ
+ return id, implicits[0], typ
}
}
@@ -348,11 +345,11 @@
// to the field's type definition, not the field's definition.
if v, ok := obj.(*types.Var); ok && v.Embedded() {
// types.Info.Uses contains the embedded field's *types.TypeName.
- if typeName := info.Uses[n]; typeName != nil {
+ if typeName := info.Uses[id]; typeName != nil {
obj = typeName
}
}
- return n, obj, nil
+ return id, obj, nil
}
return nil, nil, nil
}
diff --git a/gopls/internal/golang/hover.go b/gopls/internal/golang/hover.go
index 0620d74..a1ba81f 100644
--- a/gopls/internal/golang/hover.go
+++ b/gopls/internal/golang/hover.go
@@ -301,7 +301,7 @@
// The general case: compute hover information for the object referenced by
// the identifier at pos.
- ident, obj, selectedType := referencedObject(pkg, pgf, pos)
+ ident, obj, selectedType := referencedObject(pkg.TypesInfo(), pgf, pos)
if obj == nil || ident == nil {
return protocol.Range{}, nil, nil // no object to hover
}
diff --git a/gopls/internal/golang/type_definition.go b/gopls/internal/golang/type_definition.go
index 3b6587d..04b32e6 100644
--- a/gopls/internal/golang/type_definition.go
+++ b/gopls/internal/golang/type_definition.go
@@ -7,10 +7,14 @@
import (
"context"
"fmt"
+ "go/ast"
+ "go/types"
+ "golang.org/x/tools/go/ast/edge"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
+ astutil "golang.org/x/tools/internal/astutil"
"golang.org/x/tools/internal/event"
)
@@ -27,18 +31,47 @@
if err != nil {
return nil, err
}
-
- // TODO(rfindley): handle type switch implicits correctly here: if the user
- // jumps to the type definition of x in x := y.(type), it makes sense to jump
- // to the type of y.
- _, obj, _ := referencedObject(pkg, pgf, pos)
- if obj == nil {
- return nil, nil
+ cur, ok := pgf.Cursor.FindByPos(pos, pos)
+ if !ok {
+ return nil, fmt.Errorf("no enclosing syntax") // can't happen
}
- tname := typeToObject(obj.Type())
+ // Find innermost enclosing expression that has a type.
+ // It needn't be an identifier.
+ var (
+ info = pkg.TypesInfo()
+ t types.Type
+ )
+ for cur := range cur.Enclosing() {
+ expr, ok := cur.Node().(ast.Expr)
+ if !ok {
+ continue
+ }
+
+ // edge case: switch id := expr.(type) {}
+ // id has no type; use expr instead.
+ if astutil.IsChildOf(cur, edge.AssignStmt_Lhs) &&
+ astutil.IsChildOf(cur.Parent(), edge.TypeSwitchStmt_Assign) {
+ expr = cur.Parent().Node().(*ast.AssignStmt).Rhs[0].(*ast.TypeAssertExpr).X
+ }
+
+ if id, ok := expr.(*ast.Ident); ok {
+ if obj := info.ObjectOf(id); obj != nil {
+ t = obj.Type()
+ }
+ } else {
+ t = info.TypeOf(expr)
+ }
+ if t != nil {
+ break
+ }
+ }
+ if t == nil {
+ return nil, fmt.Errorf("no enclosing expression has a type")
+ }
+ tname := typeToObject(t)
if tname == nil {
- return nil, fmt.Errorf("no type definition for %s", obj.Name())
+ return nil, fmt.Errorf("cannot find type name from type %s", t)
}
loc, err := ObjectLocation(ctx, pkg.FileSet(), snapshot, tname)
if err != nil {
diff --git a/gopls/internal/golang/type_hierarchy.go b/gopls/internal/golang/type_hierarchy.go
index 6352c45..c07ba21 100644
--- a/gopls/internal/golang/type_hierarchy.go
+++ b/gopls/internal/golang/type_hierarchy.go
@@ -43,7 +43,7 @@
}
// For now, we require that the selection be a type name.
- _, obj, _ := referencedObject(pkg, pgf, pos)
+ _, obj, _ := referencedObject(pkg.TypesInfo(), pgf, pos)
if obj == nil {
return nil, fmt.Errorf("not a symbol")
}
diff --git a/gopls/internal/test/marker/doc.go b/gopls/internal/test/marker/doc.go
index 8b4c457..4f7ef28 100644
--- a/gopls/internal/test/marker/doc.go
+++ b/gopls/internal/test/marker/doc.go
@@ -301,6 +301,9 @@
request at the given location, and asserts that the result includes
exactly one token with the given token type and modifier string.
+ - typedef(src, want ...location): performs a textDocument/typeDefinition request
+ at the src location, and checks that the results equals want.
+
- workspacesymbol(query, golden): makes a workspace/symbol request for the
given query, formats the response with one symbol per line, and compares
against the named golden file. As workspace symbols are by definition a
diff --git a/gopls/internal/test/marker/marker_test.go b/gopls/internal/test/marker/marker_test.go
index 690d1bf..8e1382f 100644
--- a/gopls/internal/test/marker/marker_test.go
+++ b/gopls/internal/test/marker/marker_test.go
@@ -1723,11 +1723,16 @@
}
}
-func typedefMarker(mark marker, src, dst protocol.Location) {
- got := mark.run.env.FirstTypeDefinition(src)
- if got != dst {
- mark.errorf("type definition location does not match:\n\tgot: %s\n\twant %s",
- mark.run.fmtLoc(got), mark.run.fmtLoc(dst))
+func typedefMarker(mark marker, loc protocol.Location, want ...protocol.Location) {
+ env := mark.run.env
+ got, err := env.Editor.TypeDefinitions(env.Ctx, loc)
+ if err != nil {
+ mark.errorf("typeDefinition request failed: %v", err)
+ return
+ }
+
+ if err := compareLocations(mark, got, want); err != nil {
+ mark.errorf("typedef failed: %v", err)
}
}
diff --git a/gopls/internal/test/marker/testdata/typedef/typedef.txt b/gopls/internal/test/marker/testdata/typedef/typedef.txt
index 3bc9dab..940b275 100644
--- a/gopls/internal/test/marker/testdata/typedef/typedef.txt
+++ b/gopls/internal/test/marker/testdata/typedef/typedef.txt
@@ -66,3 +66,28 @@
var foo myFunc
_ = foo() //@typedef("foo", myFunc), diag(")", re"not enough arguments")
}
+
+func _() {
+ // Any expression is fair game for typeDefinition;
+ // it needn't be a referring identifier.
+ // In this example it's the r-paren of a call expr.
+ func() Struct {
+ panic(0)
+ }() //@typedef(")", Struct)
+
+ // 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!).
+ _ = Struct{Field : ""} //@typedef(":", Struct)
+}
+
+// edge case: type-switch implicits.
+// y in "switch y" has no type; use x instead.
+func _(x any) {
+ switch y := x.(type) { //@typedef("y", BUILTIN)
+ case Int:
+ _ = y //@typedef("y", Int)
+ case Struct:
+ _ = y //@typedef("y", Struct)
+ }
+}
