Skip to content

Commit ec0831a

Browse files
committed
refactor/satisfy: don't crash on type parameters
This change causes the satisfy constraint pass to correctly handle type parameters. In nearly all cases this means calling coreType(T) instead of T.Underlying(). This, and the addition of cases for C[T] and C[X, Y], should make the code robust to generic syntax. However, it is still not clear what the semantics of constraints are for the renaming tool. That work is left to a follow-up. Also, add a test suite that exercises all the basic operators, using generics in each case. Fixes golang/go#52940 Change-Id: Ic1261eb551c99b582c35fadaa148b979532588df Reviewed-on: https://go-review.googlesource.com/c/tools/+/413690 Reviewed-by: Robert Findley <[email protected]>
1 parent 66bbba3 commit ec0831a

File tree

2 files changed

+277
-27
lines changed

2 files changed

+277
-27
lines changed

refactor/satisfy/find.go

Lines changed: 51 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,8 @@
1010
//
1111
// THIS PACKAGE IS EXPERIMENTAL AND MAY CHANGE AT ANY TIME.
1212
//
13-
// It is provided only for the gorename tool. Ideally this
14-
// functionality will become part of the type-checker in due course,
15-
// since it is computing it anyway, and it is robust for ill-typed
16-
// inputs, which this package is not.
13+
// It is provided only for the gopls tool. It requires well-typed inputs.
14+
//
1715
package satisfy // import "golang.org/x/tools/refactor/satisfy"
1816

1917
// NOTES:
@@ -25,9 +23,6 @@ package satisfy // import "golang.org/x/tools/refactor/satisfy"
2523
// ...
2624
// }})
2725
//
28-
// TODO(adonovan): make this robust against ill-typed input.
29-
// Or move it into the type-checker.
30-
//
3126
// Assignability conversions are possible in the following places:
3227
// - in assignments y = x, y := x, var y = x.
3328
// - from call argument types to formal parameter types
@@ -51,11 +46,15 @@ import (
5146

5247
"golang.org/x/tools/go/ast/astutil"
5348
"golang.org/x/tools/go/types/typeutil"
49+
"golang.org/x/tools/internal/typeparams"
5450
)
5551

5652
// A Constraint records the fact that the RHS type does and must
5753
// satisfy the LHS type, which is an interface.
5854
// The names are suggestive of an assignment statement LHS = RHS.
55+
//
56+
// The constraint is implicitly universally quantified over any type
57+
// parameters appearing within the two types.
5958
type Constraint struct {
6059
LHS, RHS types.Type
6160
}
@@ -129,13 +128,13 @@ func (f *Finder) exprN(e ast.Expr) types.Type {
129128

130129
case *ast.CallExpr:
131130
// x, err := f(args)
132-
sig := f.expr(e.Fun).Underlying().(*types.Signature)
131+
sig := coreType(f.expr(e.Fun)).(*types.Signature)
133132
f.call(sig, e.Args)
134133

135134
case *ast.IndexExpr:
136135
// y, ok := x[i]
137136
x := f.expr(e.X)
138-
f.assign(f.expr(e.Index), x.Underlying().(*types.Map).Key())
137+
f.assign(f.expr(e.Index), coreType(x).(*types.Map).Key())
139138

140139
case *ast.TypeAssertExpr:
141140
// y, ok := x.(T)
@@ -215,7 +214,7 @@ func (f *Finder) builtin(obj *types.Builtin, sig *types.Signature, args []ast.Ex
215214
f.expr(args[1])
216215
} else {
217216
// append(x, y, z)
218-
tElem := s.Underlying().(*types.Slice).Elem()
217+
tElem := coreType(s).(*types.Slice).Elem()
219218
for _, arg := range args[1:] {
220219
f.assign(tElem, f.expr(arg))
221220
}
@@ -224,7 +223,7 @@ func (f *Finder) builtin(obj *types.Builtin, sig *types.Signature, args []ast.Ex
224223
case "delete":
225224
m := f.expr(args[0])
226225
k := f.expr(args[1])
227-
f.assign(m.Underlying().(*types.Map).Key(), k)
226+
f.assign(coreType(m).(*types.Map).Key(), k)
228227

229228
default:
230229
// ordinary call
@@ -358,6 +357,7 @@ func (f *Finder) expr(e ast.Expr) types.Type {
358357
f.sig = saved
359358

360359
case *ast.CompositeLit:
360+
// No need for coreType here: go1.18 disallows P{...} for type param P.
361361
switch T := deref(tv.Type).Underlying().(type) {
362362
case *types.Struct:
363363
for i, elem := range e.Elts {
@@ -403,12 +403,20 @@ func (f *Finder) expr(e ast.Expr) types.Type {
403403
}
404404

405405
case *ast.IndexExpr:
406-
x := f.expr(e.X)
407-
i := f.expr(e.Index)
408-
if ux, ok := x.Underlying().(*types.Map); ok {
409-
f.assign(ux.Key(), i)
406+
if instance(f.info, e.X) {
407+
// f[T] or C[T] -- generic instantiation
408+
} else {
409+
// x[i] or m[k] -- index or lookup operation
410+
x := f.expr(e.X)
411+
i := f.expr(e.Index)
412+
if ux, ok := coreType(x).(*types.Map); ok {
413+
f.assign(ux.Key(), i)
414+
}
410415
}
411416

417+
case *typeparams.IndexListExpr:
418+
// f[X, Y] -- generic instantiation
419+
412420
case *ast.SliceExpr:
413421
f.expr(e.X)
414422
if e.Low != nil {
@@ -439,7 +447,7 @@ func (f *Finder) expr(e ast.Expr) types.Type {
439447
}
440448
}
441449
// ordinary call
442-
f.call(f.expr(e.Fun).Underlying().(*types.Signature), e.Args)
450+
f.call(coreType(f.expr(e.Fun)).(*types.Signature), e.Args)
443451
}
444452

445453
case *ast.StarExpr:
@@ -499,7 +507,7 @@ func (f *Finder) stmt(s ast.Stmt) {
499507
case *ast.SendStmt:
500508
ch := f.expr(s.Chan)
501509
val := f.expr(s.Value)
502-
f.assign(ch.Underlying().(*types.Chan).Elem(), val)
510+
f.assign(coreType(ch).(*types.Chan).Elem(), val)
503511

504512
case *ast.IncDecStmt:
505513
f.expr(s.X)
@@ -647,35 +655,35 @@ func (f *Finder) stmt(s ast.Stmt) {
647655
if s.Key != nil {
648656
k := f.expr(s.Key)
649657
var xelem types.Type
650-
// keys of array, *array, slice, string aren't interesting
651-
switch ux := x.Underlying().(type) {
658+
// Keys of array, *array, slice, string aren't interesting
659+
// since the RHS key type is just an int.
660+
switch ux := coreType(x).(type) {
652661
case *types.Chan:
653662
xelem = ux.Elem()
654663
case *types.Map:
655664
xelem = ux.Key()
656665
}
657666
if xelem != nil {
658-
f.assign(xelem, k)
667+
f.assign(k, xelem)
659668
}
660669
}
661670
if s.Value != nil {
662671
val := f.expr(s.Value)
663672
var xelem types.Type
664-
// values of strings aren't interesting
665-
switch ux := x.Underlying().(type) {
673+
// Values of type strings aren't interesting because
674+
// the RHS value type is just a rune.
675+
switch ux := coreType(x).(type) {
666676
case *types.Array:
667677
xelem = ux.Elem()
668-
case *types.Chan:
669-
xelem = ux.Elem()
670678
case *types.Map:
671679
xelem = ux.Elem()
672680
case *types.Pointer: // *array
673-
xelem = deref(ux).(*types.Array).Elem()
681+
xelem = coreType(deref(ux)).(*types.Array).Elem()
674682
case *types.Slice:
675683
xelem = ux.Elem()
676684
}
677685
if xelem != nil {
678-
f.assign(xelem, val)
686+
f.assign(val, xelem)
679687
}
680688
}
681689
}
@@ -690,7 +698,7 @@ func (f *Finder) stmt(s ast.Stmt) {
690698

691699
// deref returns a pointer's element type; otherwise it returns typ.
692700
func deref(typ types.Type) types.Type {
693-
if p, ok := typ.Underlying().(*types.Pointer); ok {
701+
if p, ok := coreType(typ).(*types.Pointer); ok {
694702
return p.Elem()
695703
}
696704
return typ
@@ -699,3 +707,19 @@ func deref(typ types.Type) types.Type {
699707
func unparen(e ast.Expr) ast.Expr { return astutil.Unparen(e) }
700708

701709
func isInterface(T types.Type) bool { return types.IsInterface(T) }
710+
711+
func coreType(T types.Type) types.Type { return typeparams.CoreType(T) }
712+
713+
func instance(info *types.Info, expr ast.Expr) bool {
714+
var id *ast.Ident
715+
switch x := expr.(type) {
716+
case *ast.Ident:
717+
id = x
718+
case *ast.SelectorExpr:
719+
id = x.Sel
720+
default:
721+
return false
722+
}
723+
_, ok := typeparams.GetInstances(info)[id]
724+
return ok
725+
}

0 commit comments

Comments
 (0)