Skip to content

Commit b8b8e7f

Browse files
timothy-kingtimothy-king
committed
analysis/internal/facts: Updating facts for generics.
Updates importMap() to consider generics. For golang/go#48704 Change-Id: Ic5dafc644c4e81a64df338a165ee8e20627c6113 Reviewed-on: https://go-review.googlesource.com/c/tools/+/360295 Run-TryBot: Tim King <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Robert Findley <[email protected]> Trust: Tim King <[email protected]>
1 parent 4ab7496 commit b8b8e7f

File tree

2 files changed

+241
-52
lines changed

2 files changed

+241
-52
lines changed

Diff for: ‎go/analysis/internal/facts/facts_test.go

+209-51
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"golang.org/x/tools/go/analysis/internal/facts"
1818
"golang.org/x/tools/go/packages"
1919
"golang.org/x/tools/internal/testenv"
20+
"golang.org/x/tools/internal/typeparams"
2021
)
2122

2223
type myFact struct {
@@ -26,23 +27,209 @@ type myFact struct {
2627
func (f *myFact) String() string { return fmt.Sprintf("myFact(%s)", f.S) }
2728
func (f *myFact) AFact() {}
2829

29-
func TestEncodeDecode(t *testing.T) {
30+
func init() {
3031
gob.Register(new(myFact))
32+
}
3133

32-
// c -> b -> a, a2
33-
// c does not directly depend on a, but it indirectly uses a.T.
34-
//
35-
// Package a2 is never loaded directly so it is incomplete.
36-
//
37-
// We use only types in this example because we rely on
38-
// types.Eval to resolve the lookup expressions, and it only
39-
// works for types. This is a definite gap in the typechecker API.
40-
files := map[string]string{
41-
"a/a.go": `package a; type A int; type T int`,
42-
"a2/a.go": `package a2; type A2 int; type Unneeded int`,
43-
"b/b.go": `package b; import ("a"; "a2"); type B chan a2.A2; type F func() a.T`,
44-
"c/c.go": `package c; import "b"; type C []b.B`,
34+
func TestEncodeDecode(t *testing.T) {
35+
tests := []struct {
36+
name string
37+
typeparams bool // requires typeparams to be enabled
38+
files map[string]string
39+
plookups []pkgLookups // see testEncodeDecode for details
40+
}{
41+
{
42+
name: "loading-order",
43+
// c -> b -> a, a2
44+
// c does not directly depend on a, but it indirectly uses a.T.
45+
//
46+
// Package a2 is never loaded directly so it is incomplete.
47+
//
48+
// We use only types in this example because we rely on
49+
// types.Eval to resolve the lookup expressions, and it only
50+
// works for types. This is a definite gap in the typechecker API.
51+
files: map[string]string{
52+
"a/a.go": `package a; type A int; type T int`,
53+
"a2/a.go": `package a2; type A2 int; type Unneeded int`,
54+
"b/b.go": `package b; import ("a"; "a2"); type B chan a2.A2; type F func() a.T`,
55+
"c/c.go": `package c; import "b"; type C []b.B`,
56+
},
57+
// In the following table, we analyze packages (a, b, c) in order,
58+
// look up various objects accessible within each package,
59+
// and see if they have a fact. The "analysis" exports a fact
60+
// for every object at package level.
61+
//
62+
// Note: Loop iterations are not independent test cases;
63+
// order matters, as we populate factmap.
64+
plookups: []pkgLookups{
65+
{"a", []lookup{
66+
{"A", "myFact(a.A)"},
67+
}},
68+
{"b", []lookup{
69+
{"a.A", "myFact(a.A)"},
70+
{"a.T", "myFact(a.T)"},
71+
{"B", "myFact(b.B)"},
72+
{"F", "myFact(b.F)"},
73+
{"F(nil)()", "myFact(a.T)"}, // (result type of b.F)
74+
}},
75+
{"c", []lookup{
76+
{"b.B", "myFact(b.B)"},
77+
{"b.F", "myFact(b.F)"},
78+
//{"b.F(nil)()", "myFact(a.T)"}, // no fact; TODO(adonovan): investigate
79+
{"C", "myFact(c.C)"},
80+
{"C{}[0]", "myFact(b.B)"},
81+
{"<-(C{}[0])", "no fact"}, // object but no fact (we never "analyze" a2)
82+
}},
83+
},
84+
},
85+
{
86+
name: "globals",
87+
files: map[string]string{
88+
"a/a.go": `package a;
89+
type T1 int
90+
type T2 int
91+
type T3 int
92+
type T4 int
93+
type T5 int
94+
type K int; type V string
95+
`,
96+
"b/b.go": `package b
97+
import "a"
98+
var (
99+
G1 []a.T1
100+
G2 [7]a.T2
101+
G3 chan a.T3
102+
G4 *a.T4
103+
G5 struct{ F a.T5 }
104+
G6 map[a.K]a.V
105+
)
106+
`,
107+
"c/c.go": `package c; import "b";
108+
var (
109+
v1 = b.G1
110+
v2 = b.G2
111+
v3 = b.G3
112+
v4 = b.G4
113+
v5 = b.G5
114+
v6 = b.G6
115+
)
116+
`,
117+
},
118+
plookups: []pkgLookups{
119+
{"a", []lookup{}},
120+
{"b", []lookup{}},
121+
{"c", []lookup{
122+
{"v1[0]", "myFact(a.T1)"},
123+
{"v2[0]", "myFact(a.T2)"},
124+
{"<-v3", "myFact(a.T3)"},
125+
{"*v4", "myFact(a.T4)"},
126+
{"v5.F", "myFact(a.T5)"},
127+
{"v6[0]", "myFact(a.V)"},
128+
}},
129+
},
130+
},
131+
{
132+
name: "typeparams",
133+
typeparams: true,
134+
files: map[string]string{
135+
"a/a.go": `package a
136+
type T1 int
137+
type T2 int
138+
type T3 interface{Foo()}
139+
type T4 int
140+
type T5 int
141+
type T6 interface{Foo()}
142+
`,
143+
"b/b.go": `package b
144+
import "a"
145+
type N1[T a.T1|int8] func() T
146+
type N2[T any] struct{ F T }
147+
type N3[T a.T3] func() T
148+
type N4[T a.T4|int8] func() T
149+
type N5[T interface{Bar() a.T5} ] func() T
150+
151+
type t5 struct{}; func (t5) Bar() a.T5
152+
153+
var G1 N1[a.T1]
154+
var G2 func() N2[a.T2]
155+
var G3 N3[a.T3]
156+
var G4 N4[a.T4]
157+
var G5 N5[t5]
158+
159+
func F6[T a.T6]() T { var x T; return x }
160+
`,
161+
"c/c.go": `package c; import "b";
162+
var (
163+
v1 = b.G1
164+
v2 = b.G2
165+
v3 = b.G3
166+
v4 = b.G4
167+
v5 = b.G5
168+
v6 = b.F6[t6]
169+
)
170+
171+
type t6 struct{}; func (t6) Foo() {}
172+
`,
173+
},
174+
plookups: []pkgLookups{
175+
{"a", []lookup{}},
176+
{"b", []lookup{}},
177+
{"c", []lookup{
178+
{"v1", "myFact(b.N1)"},
179+
{"v1()", "myFact(a.T1)"},
180+
{"v2()", "myFact(b.N2)"},
181+
{"v2().F", "myFact(a.T2)"},
182+
{"v3", "myFact(b.N3)"},
183+
{"v4", "myFact(b.N4)"},
184+
{"v4()", "myFact(a.T4)"},
185+
{"v5", "myFact(b.N5)"},
186+
{"v5()", "myFact(b.t5)"},
187+
{"v6()", "myFact(c.t6)"},
188+
}},
189+
},
190+
},
191+
}
192+
193+
for i := range tests {
194+
test := tests[i]
195+
t.Run(test.name, func(t *testing.T) {
196+
t.Parallel()
197+
if test.typeparams && !typeparams.Enabled {
198+
t.Skip("type parameters are not enabled")
199+
}
200+
testEncodeDecode(t, test.files, test.plookups)
201+
})
45202
}
203+
}
204+
205+
type lookup struct {
206+
objexpr string
207+
want string
208+
}
209+
210+
type pkgLookups struct {
211+
path string
212+
lookups []lookup
213+
}
214+
215+
// testEncodeDecode tests fact encoding and decoding and simulates how package facts
216+
// are passed during analysis. It operates on a group of Go file contents. Then
217+
// for each <package, []lookup> in tests it does the following:
218+
// 1) loads and type checks the package,
219+
// 2) calls facts.Decode to loads the facts exported by its imports,
220+
// 3) exports a myFact Fact for all of package level objects,
221+
// 4) For each lookup for the current package:
222+
// 4.a) lookup the types.Object for an Go source expression in the curent package
223+
// (or confirms one is not expected want=="no object"),
224+
// 4.b) finds a Fact for the object (or confirms one is not expected want=="no fact"),
225+
// 4.c) compares the content of the Fact to want.
226+
// 5) encodes the Facts of the package.
227+
//
228+
// Note: tests are not independent test cases; order matters (as does a package being
229+
// skipped). It changes what Facts can be imported.
230+
//
231+
// Failures are reported on t.
232+
func testEncodeDecode(t *testing.T, files map[string]string, tests []pkgLookups) {
46233
dir, cleanup, err := analysistest.WriteFiles(files)
47234
if err != nil {
48235
t.Fatal(err)
@@ -54,40 +241,13 @@ func TestEncodeDecode(t *testing.T) {
54241
factmap := make(map[string][]byte)
55242
read := func(path string) ([]byte, error) { return factmap[path], nil }
56243

57-
// In the following table, we analyze packages (a, b, c) in order,
58-
// look up various objects accessible within each package,
59-
// and see if they have a fact. The "analysis" exports a fact
60-
// for every object at package level.
244+
// Analyze packages in order, look up various objects accessible within
245+
// each package, and see if they have a fact. The "analysis" exports a
246+
// fact for every object at package level.
61247
//
62248
// Note: Loop iterations are not independent test cases;
63249
// order matters, as we populate factmap.
64-
type lookups []struct {
65-
objexpr string
66-
want string
67-
}
68-
for _, test := range []struct {
69-
path string
70-
lookups lookups
71-
}{
72-
{"a", lookups{
73-
{"A", "myFact(a.A)"},
74-
}},
75-
{"b", lookups{
76-
{"a.A", "myFact(a.A)"},
77-
{"a.T", "myFact(a.T)"},
78-
{"B", "myFact(b.B)"},
79-
{"F", "myFact(b.F)"},
80-
{"F(nil)()", "myFact(a.T)"}, // (result type of b.F)
81-
}},
82-
{"c", lookups{
83-
{"b.B", "myFact(b.B)"},
84-
{"b.F", "myFact(b.F)"},
85-
//{"b.F(nil)()", "myFact(a.T)"}, // no fact; TODO(adonovan): investigate
86-
{"C", "myFact(c.C)"},
87-
{"C{}[0]", "myFact(b.B)"},
88-
{"<-(C{}[0])", "no fact"}, // object but no fact (we never "analyze" a2)
89-
}},
90-
} {
250+
for _, test := range tests {
91251
// load package
92252
pkg, err := load(t, dir, test.path)
93253
if err != nil {
@@ -99,18 +259,16 @@ func TestEncodeDecode(t *testing.T) {
99259
if err != nil {
100260
t.Fatalf("Decode failed: %v", err)
101261
}
102-
if true {
103-
t.Logf("decode %s facts = %v", pkg.Path(), facts) // show all facts
104-
}
262+
t.Logf("decode %s facts = %v", pkg.Path(), facts) // show all facts
105263

106264
// export
107265
// (one fact for each package-level object)
108-
scope := pkg.Scope()
109-
for _, name := range scope.Names() {
110-
obj := scope.Lookup(name)
266+
for _, name := range pkg.Scope().Names() {
267+
obj := pkg.Scope().Lookup(name)
111268
fact := &myFact{obj.Pkg().Name() + "." + obj.Name()}
112269
facts.ExportObjectFact(obj, fact)
113270
}
271+
t.Logf("exported %s facts = %v", pkg.Path(), facts) // show all facts
114272

115273
// import
116274
// (after export, because an analyzer may import its own facts)

Diff for: ‎go/analysis/internal/facts/imports.go

+32-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@
44

55
package facts
66

7-
import "go/types"
7+
import (
8+
"go/types"
9+
10+
"golang.org/x/tools/internal/typeparams"
11+
)
812

913
// importMap computes the import map for a package by traversing the
1014
// entire exported API each of its imports.
@@ -42,9 +46,20 @@ func importMap(imports []*types.Package) map[string]*types.Package {
4246
// nop
4347
case *types.Named:
4448
if addObj(T.Obj()) {
49+
// TODO(taking): Investigate why the Underlying type is not added here.
4550
for i := 0; i < T.NumMethods(); i++ {
4651
addObj(T.Method(i))
4752
}
53+
if tparams := typeparams.ForNamed(T); tparams != nil {
54+
for i := 0; i < tparams.Len(); i++ {
55+
addType(tparams.At(i))
56+
}
57+
}
58+
if targs := typeparams.NamedTypeArgs(T); targs != nil {
59+
for i := 0; i < targs.Len(); i++ {
60+
addType(targs.At(i))
61+
}
62+
}
4863
}
4964
case *types.Pointer:
5065
addType(T.Elem())
@@ -60,6 +75,11 @@ func importMap(imports []*types.Package) map[string]*types.Package {
6075
case *types.Signature:
6176
addType(T.Params())
6277
addType(T.Results())
78+
if tparams := typeparams.ForSignature(T); tparams != nil {
79+
for i := 0; i < tparams.Len(); i++ {
80+
addType(tparams.At(i))
81+
}
82+
}
6383
case *types.Struct:
6484
for i := 0; i < T.NumFields(); i++ {
6585
addObj(T.Field(i))
@@ -72,6 +92,17 @@ func importMap(imports []*types.Package) map[string]*types.Package {
7292
for i := 0; i < T.NumMethods(); i++ {
7393
addObj(T.Method(i))
7494
}
95+
for i := 0; i < T.NumEmbeddeds(); i++ {
96+
addType(T.EmbeddedType(i)) // walk Embedded for implicits
97+
}
98+
case *typeparams.Union:
99+
for i := 0; i < T.Len(); i++ {
100+
addType(T.Term(i).Type())
101+
}
102+
case *typeparams.TypeParam:
103+
if addObj(T.Obj()) {
104+
addType(T.Constraint())
105+
}
75106
}
76107
}
77108

0 commit comments

Comments
 (0)