internal/typeparams: update x/tools for recent typeparams changes
Recent changes to the go/ast APIs for type parameters have broken the
internal/typeparams package when built with -tags=typeparams.
Fix this by adjusting the internal/typeparams API. Also update a few
tests accordingly.
Bump the build constraint used by the internal/typeparams package to
go1.18, as we are no longer compatible with the 1.17 typeparams API. It
is no long possible to opt in to type parameter specific functionality
1.17, which is fine as the dev.typeparams branch has moved to 1.18.
Even after these fixes, not all x/tools tests pass with go1.18. Some
completion tests are failing due to finding 'any' in types.Universe.
Change-Id: I5f92870aaf7853e531e3a154987f98520a52d70c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339349
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
diff --git a/go/ast/astutil/rewrite.go b/go/ast/astutil/rewrite.go
index b949fc8..5fe75b1 100644
--- a/go/ast/astutil/rewrite.go
+++ b/go/ast/astutil/rewrite.go
@@ -439,8 +439,10 @@
}
default:
- if typeparams.IsListExpr(n) {
- a.applyList(n, "ElemList")
+ if ix := typeparams.GetIndexExprData(n); ix != nil {
+ a.apply(n, "X", nil, ix.X)
+ // *ast.IndexExpr was handled above, so n must be an *ast.MultiIndexExpr.
+ a.applyList(n, "Indices")
} else {
panic(fmt.Sprintf("Apply: unexpected node type %T", n))
}
diff --git a/go/ast/astutil/rewrite_test.go b/go/ast/astutil/rewrite_test.go
index 3a74afa..9d23170 100644
--- a/go/ast/astutil/rewrite_test.go
+++ b/go/ast/astutil/rewrite_test.go
@@ -205,15 +205,18 @@
`,
want: `package p
-type T[P1, P2 any] int32
+type S[P1, P2 any] int32
-type R T[int32, string]
+type R S[int32, string]
`,
post: func(c *astutil.Cursor) bool {
if ident, ok := c.Node().(*ast.Ident); ok {
if ident.Name == "int" {
c.Replace(ast.NewIdent("int32"))
}
+ if ident.Name == "T" {
+ c.Replace(ast.NewIdent("S"))
+ }
}
return true
},
diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go
index 36eacf4..2ab6cfd 100644
--- a/internal/lsp/source/identifier.go
+++ b/internal/lsp/source/identifier.go
@@ -371,22 +371,25 @@
case *ast.CallExpr:
_, sig := typeparams.GetInferred(info, n)
return sig
- case *ast.IndexExpr:
- // If the IndexExpr is fully instantiated, we consider that 'inference' for
- // gopls' purposes.
- sig, _ := info.TypeOf(n).(*types.Signature)
- if sig != nil && len(typeparams.ForSignature(sig)) == 0 {
- return sig
- }
- _, sig = typeparams.GetInferred(info, n)
- if sig != nil {
- return sig
- }
- if len(path) >= 2 {
- if call, _ := path[2].(*ast.CallExpr); call != nil {
- _, sig := typeparams.GetInferred(info, call)
+ default:
+ if ix := typeparams.GetIndexExprData(n); ix != nil {
+ e := n.(ast.Expr)
+ // If the IndexExpr is fully instantiated, we consider that 'inference' for
+ // gopls' purposes.
+ sig, _ := info.TypeOf(e).(*types.Signature)
+ if sig != nil && len(typeparams.ForSignature(sig)) == 0 {
return sig
}
+ _, sig = typeparams.GetInferred(info, e)
+ if sig != nil {
+ return sig
+ }
+ if len(path) >= 2 {
+ if call, _ := path[2].(*ast.CallExpr); call != nil {
+ _, sig := typeparams.GetInferred(info, call)
+ return sig
+ }
+ }
}
}
return nil
diff --git a/internal/lsp/testdata/summary_generics.txt.golden b/internal/lsp/testdata/summary_generics.txt.golden
index 3128120..f93110a 100644
--- a/internal/lsp/testdata/summary_generics.txt.golden
+++ b/internal/lsp/testdata/summary_generics.txt.golden
@@ -14,16 +14,17 @@
ImportCount = 8
SemanticTokenCount = 3
SuggestedFixCount = 40
-FunctionExtractionCount = 18
+FunctionExtractionCount = 24
+MethodExtractionCount = 6
DefinitionsCount = 99
TypeDefinitionsCount = 18
HighlightsCount = 69
ReferencesCount = 27
-RenamesCount = 33
+RenamesCount = 37
PrepareRenamesCount = 7
SymbolsCount = 5
WorkspaceSymbolsCount = 20
-SignaturesCount = 32
+SignaturesCount = 33
LinksCount = 7
ImplementationsCount = 14
diff --git a/internal/typeparams/common.go b/internal/typeparams/common.go
new file mode 100644
index 0000000..9fc6b4b
--- /dev/null
+++ b/internal/typeparams/common.go
@@ -0,0 +1,25 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package typeparams provides functions to work indirectly with type parameter
+// data stored in go/ast and go/types objects, while these API are guarded by a
+// build constraint.
+//
+// This package exists to make it easier for tools to work with generic code,
+// while also compiling against older Go versions.
+package typeparams
+
+import (
+ "go/ast"
+ "go/token"
+)
+
+// A IndexExprData holds data from both ast.IndexExpr and the new
+// ast.MultiIndexExpr, which was introduced in Go 1.18.
+type IndexExprData struct {
+ X ast.Expr // expression
+ Lbrack token.Pos // position of "["
+ Indices []ast.Expr // index expressions
+ Rbrack token.Pos // position of "]"
+}
diff --git a/internal/typeparams/common_test.go b/internal/typeparams/common_test.go
new file mode 100644
index 0000000..e15c297
--- /dev/null
+++ b/internal/typeparams/common_test.go
@@ -0,0 +1,36 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package typeparams_test
+
+import (
+ "go/ast"
+ "testing"
+
+ "golang.org/x/tools/internal/typeparams"
+)
+
+func TestGetIndexExprData(t *testing.T) {
+ x := &ast.Ident{}
+ i := &ast.Ident{}
+
+ tests := map[ast.Node]bool{
+ &ast.IndexExpr{X: x, Lbrack: 1, Index: i, Rbrack: 2}: true,
+ &ast.Ident{}: false,
+ }
+ want := &typeparams.IndexExprData{X: x, Lbrack: 1, Indices: []ast.Expr{i}, Rbrack: 2}
+
+ for n, isIndexExpr := range tests {
+ ix := typeparams.GetIndexExprData(n)
+ if got := ix != nil; got != isIndexExpr {
+ t.Errorf("GetIndexExprData(%+v) = %+v, want nil: %t", n, ix, !isIndexExpr)
+ }
+ if ix == nil {
+ continue
+ }
+ if ix.X != x || ix.Lbrack != 1 || ix.Indices[0] != i || ix.Rbrack != 2 {
+ t.Errorf("GetIndexExprData(%+v) = %+v, want %+v", n, ix, want)
+ }
+ }
+}
diff --git a/internal/typeparams/doc.go b/internal/typeparams/doc.go
deleted file mode 100644
index 5583947..0000000
--- a/internal/typeparams/doc.go
+++ /dev/null
@@ -1,11 +0,0 @@
-// Copyright 2021 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// Package typeparams provides functions to work indirectly with type parameter
-// data stored in go/ast and go/types objects, while these API are guarded by a
-// build constraint.
-//
-// This package exists to make it easier for tools to work with generic code,
-// while also compiling against older Go versions.
-package typeparams
diff --git a/internal/typeparams/notypeparams.go b/internal/typeparams/notypeparams.go
index 3a0abc7..e975e47 100644
--- a/internal/typeparams/notypeparams.go
+++ b/internal/typeparams/notypeparams.go
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
-//go:build !typeparams || !go1.17
-// +build !typeparams !go1.17
+//go:build !typeparams || !go1.18
+// +build !typeparams !go1.18
package typeparams
@@ -18,17 +18,25 @@
// environment.
const Enabled = false
-// UnpackIndex extracts all index expressions from e. For non-generic code this
-// is always one expression: e.Index, but may be more than one expression for
-// generic type instantiation.
-func UnpackIndex(e *ast.IndexExpr) []ast.Expr {
- return []ast.Expr{e.Index}
-}
-
-// IsListExpr reports whether n is an *ast.ListExpr, which is a new node type
-// introduced to hold type arguments for generic type instantiation.
-func IsListExpr(n ast.Node) bool {
- return false
+// GetIndexExprData extracts data from AST nodes that represent index
+// expressions.
+//
+// For an ast.IndexExpr, the resulting IndexExprData will have exactly one
+// index expression. For an ast.MultiIndexExpr (go1.18+), it may have a
+// variable number of index expressions.
+//
+// For nodes that don't represent index expressions, GetIndexExprData returns
+// nil.
+func GetIndexExprData(n ast.Node) *IndexExprData {
+ if e, _ := n.(*ast.IndexExpr); e != nil {
+ return &IndexExprData{
+ X: e.X,
+ Lbrack: e.Lbrack,
+ Indices: []ast.Expr{e.Index},
+ Rbrack: e.Rbrack,
+ }
+ }
+ return nil
}
// ForTypeDecl extracts the (possibly nil) type parameter node list from n.
@@ -47,11 +55,6 @@
return nil
}
-// HasTypeSet reports if iface has a type set.
-func HasTypeSet(*types.Interface) bool {
- return false
-}
-
// IsComparable reports if iface is the comparable interface.
func IsComparable(*types.Interface) bool {
return false
diff --git a/internal/typeparams/typeparams.go b/internal/typeparams/typeparams.go
index 6b7958a..be6b052 100644
--- a/internal/typeparams/typeparams.go
+++ b/internal/typeparams/typeparams.go
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
-//go:build typeparams && go1.17
-// +build typeparams,go1.17
+//go:build typeparams && go1.18
+// +build typeparams,go1.18
package typeparams
@@ -18,26 +18,30 @@
// environment.
const Enabled = true
-// UnpackIndex extracts all index expressions from e. For non-generic code this
-// is always one expression: e.Index, but may be more than one expression for
-// generic type instantiation.
-func UnpackIndex(e *ast.IndexExpr) []ast.Expr {
- if x, _ := e.Index.(*ast.ListExpr); x != nil {
- return x.ElemList
- }
- if e.Index != nil {
- return []ast.Expr{e.Index}
+// GetIndexExprData extracts data from AST nodes that represent index
+// expressions.
+//
+// For an ast.IndexExpr, the resulting IndexExprData will have exactly one
+// index expression. For an ast.MultiIndexExpr (go1.18+), it may have a
+// variable number of index expressions.
+//
+// For nodes that don't represent index expressions, GetIndexExprData returns
+// nil.
+func GetIndexExprData(n ast.Node) *IndexExprData {
+ switch e := n.(type) {
+ case *ast.IndexExpr:
+ return &IndexExprData{
+ X: e.X,
+ Lbrack: e.Lbrack,
+ Indices: []ast.Expr{e.Index},
+ Rbrack: e.Rbrack,
+ }
+ case *ast.MultiIndexExpr:
+ return (*IndexExprData)(e)
}
return nil
}
-// IsListExpr reports whether n is an *ast.ListExpr, which is a new node type
-// introduced to hold type arguments for generic type instantiation.
-func IsListExpr(n ast.Node) bool {
- _, ok := n.(*ast.ListExpr)
- return ok
-}
-
// ForTypeDecl extracts the (possibly nil) type parameter node list from n.
func ForTypeDecl(n *ast.TypeSpec) *ast.FieldList {
return n.TParams
@@ -54,12 +58,7 @@
// ForSignature extracts the (possibly empty) type parameter object list from
// sig.
func ForSignature(sig *types.Signature) []*types.TypeName {
- return sig.TParams()
-}
-
-// HasTypeSet reports if iface has a type set.
-func HasTypeSet(iface *types.Interface) bool {
- return iface.HasTypeList()
+ return tparamsSlice(sig.TParams())
}
// IsComparable reports if iface is the comparable interface.
@@ -76,7 +75,18 @@
// ForNamed extracts the (possibly empty) type parameter object list from
// named.
func ForNamed(named *types.Named) []*types.TypeName {
- return named.TParams()
+ return tparamsSlice(named.TParams())
+}
+
+func tparamsSlice(tparams *types.TypeParams) []*types.TypeName {
+ if tparams.Len() == 0 {
+ return nil
+ }
+ result := make([]*types.TypeName, tparams.Len())
+ for i := 0; i < tparams.Len(); i++ {
+ result[i] = tparams.At(i)
+ }
+ return result
}
// NamedTArgs extracts the (possibly empty) type argument list from named.
