Skip to content

Commit 4ce49b4

Browse files
committed
go/types: support type parameters in NewMethodSet
Add handling for TypeParams in NewMethodSet, to bring it in sync with lookupFieldOrMethod. Also add a test, since we had none. I wanted this fix to get gopls completion working with type params, but due to the subtlety of lookupFieldOrMethod, I left a TODO to confirm that there are no behavioral differences between the APIs. Updates #45639 Change-Id: I16723e16d4d944ca4ecb4d87fc196815abb6fcff Reviewed-on: https://go-review.googlesource.com/c/go/+/311455 Trust: Robert Findley <[email protected]> Trust: Robert Griesemer <[email protected]> Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
1 parent af8a176 commit 4ce49b4

File tree

4 files changed

+129
-4
lines changed

4 files changed

+129
-4
lines changed

src/go/types/api_test.go

+7
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ import (
2121
. "go/types"
2222
)
2323

24+
// pkgFor parses and type checks the package specified by path and source,
25+
// populating info if provided.
26+
//
27+
// If source begins with "package generic_" and type parameters are enabled,
28+
// generic code is permitted.
2429
func pkgFor(path, source string, info *Info) (*Package, error) {
2530
fset := token.NewFileSet()
2631
mode := modeForSource(source)
@@ -1213,6 +1218,8 @@ func TestLookupFieldOrMethod(t *testing.T) {
12131218
// Test cases assume a lookup of the form a.f or x.f, where a stands for an
12141219
// addressable value, and x for a non-addressable value (even though a variable
12151220
// for ease of test case writing).
1221+
//
1222+
// Should be kept in sync with TestMethodSet.
12161223
var tests = []struct {
12171224
src string
12181225
found bool

src/go/types/lookup.go

+2
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,8 @@ func (check *Checker) rawLookupFieldOrMethod(T Type, addressable bool, pkg *Pack
142142

143143
// continue with underlying type, but only if it's not a type parameter
144144
// TODO(gri) is this what we want to do for type parameters? (spec question)
145+
// TODO(#45639) the error message produced as a result of skipping an
146+
// underlying type parameter should be improved.
145147
typ = named.under()
146148
if asTypeParam(typ) != nil {
147149
continue

src/go/types/methodset.go

+11-4
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func (s *MethodSet) Lookup(pkg *Package, name string) *Selection {
6363
var emptyMethodSet MethodSet
6464

6565
// Note: NewMethodSet is intended for external use only as it
66-
// requires interfaces to be complete. If may be used
66+
// requires interfaces to be complete. It may be used
6767
// internally if LookupFieldOrMethod completed the same
6868
// interfaces beforehand.
6969

@@ -73,8 +73,8 @@ func NewMethodSet(T Type) *MethodSet {
7373
// WARNING: The code in this function is extremely subtle - do not modify casually!
7474
// This function and lookupFieldOrMethod should be kept in sync.
7575

76-
// TODO(gri) This code is out-of-sync with the lookup code at this point.
77-
// Need to update.
76+
// TODO(rfindley) confirm that this code is in sync with lookupFieldOrMethod
77+
// with respect to type params.
7878

7979
// method set up to the current depth, allocated lazily
8080
var base methodSet
@@ -127,8 +127,12 @@ func NewMethodSet(T Type) *MethodSet {
127127

128128
mset = mset.add(named.methods, e.index, e.indirect, e.multiples)
129129

130-
// continue with underlying type
130+
// continue with underlying type, but only if it's not a type parameter
131+
// TODO(rFindley): should this use named.under()? Can there be a difference?
131132
typ = named.underlying
133+
if _, ok := typ.(*_TypeParam); ok {
134+
continue
135+
}
132136
}
133137

134138
switch t := typ.(type) {
@@ -154,6 +158,9 @@ func NewMethodSet(T Type) *MethodSet {
154158

155159
case *Interface:
156160
mset = mset.add(t.allMethods, e.index, true, e.multiples)
161+
162+
case *_TypeParam:
163+
mset = mset.add(t.Bound().allMethods, e.index, true, e.multiples)
157164
}
158165
}
159166

src/go/types/methodset_test.go

+109
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
// Copyright 2021 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package types_test
6+
7+
import (
8+
"testing"
9+
10+
"go/internal/typeparams"
11+
. "go/types"
12+
)
13+
14+
func TestNewMethodSet(t *testing.T) {
15+
type method struct {
16+
name string
17+
index []int
18+
indirect bool
19+
}
20+
21+
// Tests are expressed src -> methods, for simplifying the composite literal.
22+
// Should be kept in sync with TestLookupFieldOrMethod.
23+
tests := map[string][]method{
24+
// Named types
25+
"var a T; type T struct{}; func (T) f() {}": {{"f", []int{0}, false}},
26+
"var a *T; type T struct{}; func (T) f() {}": {{"f", []int{0}, true}},
27+
"var a T; type T struct{}; func (*T) f() {}": {},
28+
"var a *T; type T struct{}; func (*T) f() {}": {{"f", []int{0}, true}},
29+
30+
// Interfaces
31+
"var a T; type T interface{ f() }": {{"f", []int{0}, true}},
32+
"var a T1; type ( T1 T2; T2 interface{ f() } )": {{"f", []int{0}, true}},
33+
"var a T1; type ( T1 interface{ T2 }; T2 interface{ f() } )": {{"f", []int{0}, true}},
34+
35+
// Embedding
36+
"var a struct{ E }; type E interface{ f() }": {{"f", []int{0, 0}, true}},
37+
"var a *struct{ E }; type E interface{ f() }": {{"f", []int{0, 0}, true}},
38+
"var a struct{ E }; type E struct{}; func (E) f() {}": {{"f", []int{0, 0}, false}},
39+
"var a struct{ *E }; type E struct{}; func (E) f() {}": {{"f", []int{0, 0}, true}},
40+
"var a struct{ E }; type E struct{}; func (*E) f() {}": {},
41+
"var a struct{ *E }; type E struct{}; func (*E) f() {}": {{"f", []int{0, 0}, true}},
42+
43+
// collisions
44+
"var a struct{ E1; *E2 }; type ( E1 interface{ f() }; E2 struct{ f int })": {},
45+
"var a struct{ E1; *E2 }; type ( E1 struct{ f int }; E2 struct{} ); func (E2) f() {}": {},
46+
}
47+
48+
genericTests := map[string][]method{
49+
// By convention, look up a in the scope of "g"
50+
"type C interface{ f() }; func g[T C](a T){}": {{"f", []int{0}, true}},
51+
"type C interface{ f() }; func g[T C]() { var a T; _ = a }": {{"f", []int{0}, true}},
52+
"type C interface{ f() }; func g[T C]() { var a struct{T}; _ = a }": {{"f", []int{0, 0}, true}},
53+
54+
// Issue #45639.
55+
"type C interface{ f() }; func g[T C]() { type Y T; var a Y; _ = a }": {},
56+
}
57+
58+
check := func(src string, methods []method, generic bool) {
59+
pkgName := "p"
60+
if generic {
61+
// The generic_ prefix causes pkgFor to allow generic code.
62+
pkgName = "generic_p"
63+
}
64+
pkg, err := pkgFor("test", "package "+pkgName+";"+src, nil)
65+
if err != nil {
66+
t.Errorf("%s: incorrect test case: %s", src, err)
67+
return
68+
}
69+
70+
scope := pkg.Scope()
71+
if generic {
72+
fn := pkg.Scope().Lookup("g").(*Func)
73+
scope = fn.Scope()
74+
}
75+
obj := scope.Lookup("a")
76+
if obj == nil {
77+
t.Errorf("%s: incorrect test case - no object a", src)
78+
return
79+
}
80+
81+
ms := NewMethodSet(obj.Type())
82+
if got, want := ms.Len(), len(methods); got != want {
83+
t.Errorf("%s: got %d methods, want %d", src, got, want)
84+
return
85+
}
86+
for i, m := range methods {
87+
sel := ms.At(i)
88+
if got, want := sel.Obj().Name(), m.name; got != want {
89+
t.Errorf("%s [method %d]: got name = %q at, want %q", src, i, got, want)
90+
}
91+
if got, want := sel.Index(), m.index; !sameSlice(got, want) {
92+
t.Errorf("%s [method %d]: got index = %v, want %v", src, i, got, want)
93+
}
94+
if got, want := sel.Indirect(), m.indirect; got != want {
95+
t.Errorf("%s [method %d]: got indirect = %v, want %v", src, i, got, want)
96+
}
97+
}
98+
}
99+
100+
for src, methods := range tests {
101+
check(src, methods, false)
102+
}
103+
104+
if typeparams.Enabled {
105+
for src, methods := range genericTests {
106+
check(src, methods, true)
107+
}
108+
}
109+
}

0 commit comments

Comments
 (0)