From b8b8e7f4c083082b8672c32be919747ae146795d Mon Sep 17 00:00:00 2001 From: Tim King Date: Thu, 28 Oct 2021 10:20:43 -0700 Subject: [PATCH] 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 gopls-CI: kokoro TryBot-Result: Go Bot Reviewed-by: Robert Findley Trust: Tim King --- go/analysis/internal/facts/facts_test.go | 260 ++++++++++++++++++----- go/analysis/internal/facts/imports.go | 33 ++- 2 files changed, 241 insertions(+), 52 deletions(-) diff --git a/go/analysis/internal/facts/facts_test.go b/go/analysis/internal/facts/facts_test.go index 971334e22d9..13c358230f0 100644 --- a/go/analysis/internal/facts/facts_test.go +++ b/go/analysis/internal/facts/facts_test.go @@ -17,6 +17,7 @@ import ( "golang.org/x/tools/go/analysis/internal/facts" "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/testenv" + "golang.org/x/tools/internal/typeparams" ) type myFact struct { @@ -26,23 +27,209 @@ type myFact struct { func (f *myFact) String() string { return fmt.Sprintf("myFact(%s)", f.S) } func (f *myFact) AFact() {} -func TestEncodeDecode(t *testing.T) { +func init() { gob.Register(new(myFact)) +} - // c -> b -> a, a2 - // c does not directly depend on a, but it indirectly uses a.T. - // - // Package a2 is never loaded directly so it is incomplete. - // - // We use only types in this example because we rely on - // types.Eval to resolve the lookup expressions, and it only - // works for types. This is a definite gap in the typechecker API. - files := map[string]string{ - "a/a.go": `package a; type A int; type T int`, - "a2/a.go": `package a2; type A2 int; type Unneeded int`, - "b/b.go": `package b; import ("a"; "a2"); type B chan a2.A2; type F func() a.T`, - "c/c.go": `package c; import "b"; type C []b.B`, +func TestEncodeDecode(t *testing.T) { + tests := []struct { + name string + typeparams bool // requires typeparams to be enabled + files map[string]string + plookups []pkgLookups // see testEncodeDecode for details + }{ + { + name: "loading-order", + // c -> b -> a, a2 + // c does not directly depend on a, but it indirectly uses a.T. + // + // Package a2 is never loaded directly so it is incomplete. + // + // We use only types in this example because we rely on + // types.Eval to resolve the lookup expressions, and it only + // works for types. This is a definite gap in the typechecker API. + files: map[string]string{ + "a/a.go": `package a; type A int; type T int`, + "a2/a.go": `package a2; type A2 int; type Unneeded int`, + "b/b.go": `package b; import ("a"; "a2"); type B chan a2.A2; type F func() a.T`, + "c/c.go": `package c; import "b"; type C []b.B`, + }, + // In the following table, we analyze packages (a, b, c) in order, + // look up various objects accessible within each package, + // and see if they have a fact. The "analysis" exports a fact + // for every object at package level. + // + // Note: Loop iterations are not independent test cases; + // order matters, as we populate factmap. + plookups: []pkgLookups{ + {"a", []lookup{ + {"A", "myFact(a.A)"}, + }}, + {"b", []lookup{ + {"a.A", "myFact(a.A)"}, + {"a.T", "myFact(a.T)"}, + {"B", "myFact(b.B)"}, + {"F", "myFact(b.F)"}, + {"F(nil)()", "myFact(a.T)"}, // (result type of b.F) + }}, + {"c", []lookup{ + {"b.B", "myFact(b.B)"}, + {"b.F", "myFact(b.F)"}, + //{"b.F(nil)()", "myFact(a.T)"}, // no fact; TODO(adonovan): investigate + {"C", "myFact(c.C)"}, + {"C{}[0]", "myFact(b.B)"}, + {"<-(C{}[0])", "no fact"}, // object but no fact (we never "analyze" a2) + }}, + }, + }, + { + name: "globals", + files: map[string]string{ + "a/a.go": `package a; + type T1 int + type T2 int + type T3 int + type T4 int + type T5 int + type K int; type V string + `, + "b/b.go": `package b + import "a" + var ( + G1 []a.T1 + G2 [7]a.T2 + G3 chan a.T3 + G4 *a.T4 + G5 struct{ F a.T5 } + G6 map[a.K]a.V + ) + `, + "c/c.go": `package c; import "b"; + var ( + v1 = b.G1 + v2 = b.G2 + v3 = b.G3 + v4 = b.G4 + v5 = b.G5 + v6 = b.G6 + ) + `, + }, + plookups: []pkgLookups{ + {"a", []lookup{}}, + {"b", []lookup{}}, + {"c", []lookup{ + {"v1[0]", "myFact(a.T1)"}, + {"v2[0]", "myFact(a.T2)"}, + {"<-v3", "myFact(a.T3)"}, + {"*v4", "myFact(a.T4)"}, + {"v5.F", "myFact(a.T5)"}, + {"v6[0]", "myFact(a.V)"}, + }}, + }, + }, + { + name: "typeparams", + typeparams: true, + files: map[string]string{ + "a/a.go": `package a + type T1 int + type T2 int + type T3 interface{Foo()} + type T4 int + type T5 int + type T6 interface{Foo()} + `, + "b/b.go": `package b + import "a" + type N1[T a.T1|int8] func() T + type N2[T any] struct{ F T } + type N3[T a.T3] func() T + type N4[T a.T4|int8] func() T + type N5[T interface{Bar() a.T5} ] func() T + + type t5 struct{}; func (t5) Bar() a.T5 + + var G1 N1[a.T1] + var G2 func() N2[a.T2] + var G3 N3[a.T3] + var G4 N4[a.T4] + var G5 N5[t5] + + func F6[T a.T6]() T { var x T; return x } + `, + "c/c.go": `package c; import "b"; + var ( + v1 = b.G1 + v2 = b.G2 + v3 = b.G3 + v4 = b.G4 + v5 = b.G5 + v6 = b.F6[t6] + ) + + type t6 struct{}; func (t6) Foo() {} + `, + }, + plookups: []pkgLookups{ + {"a", []lookup{}}, + {"b", []lookup{}}, + {"c", []lookup{ + {"v1", "myFact(b.N1)"}, + {"v1()", "myFact(a.T1)"}, + {"v2()", "myFact(b.N2)"}, + {"v2().F", "myFact(a.T2)"}, + {"v3", "myFact(b.N3)"}, + {"v4", "myFact(b.N4)"}, + {"v4()", "myFact(a.T4)"}, + {"v5", "myFact(b.N5)"}, + {"v5()", "myFact(b.t5)"}, + {"v6()", "myFact(c.t6)"}, + }}, + }, + }, + } + + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + t.Parallel() + if test.typeparams && !typeparams.Enabled { + t.Skip("type parameters are not enabled") + } + testEncodeDecode(t, test.files, test.plookups) + }) } +} + +type lookup struct { + objexpr string + want string +} + +type pkgLookups struct { + path string + lookups []lookup +} + +// testEncodeDecode tests fact encoding and decoding and simulates how package facts +// are passed during analysis. It operates on a group of Go file contents. Then +// for each in tests it does the following: +// 1) loads and type checks the package, +// 2) calls facts.Decode to loads the facts exported by its imports, +// 3) exports a myFact Fact for all of package level objects, +// 4) For each lookup for the current package: +// 4.a) lookup the types.Object for an Go source expression in the curent package +// (or confirms one is not expected want=="no object"), +// 4.b) finds a Fact for the object (or confirms one is not expected want=="no fact"), +// 4.c) compares the content of the Fact to want. +// 5) encodes the Facts of the package. +// +// Note: tests are not independent test cases; order matters (as does a package being +// skipped). It changes what Facts can be imported. +// +// Failures are reported on t. +func testEncodeDecode(t *testing.T, files map[string]string, tests []pkgLookups) { dir, cleanup, err := analysistest.WriteFiles(files) if err != nil { t.Fatal(err) @@ -54,40 +241,13 @@ func TestEncodeDecode(t *testing.T) { factmap := make(map[string][]byte) read := func(path string) ([]byte, error) { return factmap[path], nil } - // In the following table, we analyze packages (a, b, c) in order, - // look up various objects accessible within each package, - // and see if they have a fact. The "analysis" exports a fact - // for every object at package level. + // Analyze packages in order, look up various objects accessible within + // each package, and see if they have a fact. The "analysis" exports a + // fact for every object at package level. // // Note: Loop iterations are not independent test cases; // order matters, as we populate factmap. - type lookups []struct { - objexpr string - want string - } - for _, test := range []struct { - path string - lookups lookups - }{ - {"a", lookups{ - {"A", "myFact(a.A)"}, - }}, - {"b", lookups{ - {"a.A", "myFact(a.A)"}, - {"a.T", "myFact(a.T)"}, - {"B", "myFact(b.B)"}, - {"F", "myFact(b.F)"}, - {"F(nil)()", "myFact(a.T)"}, // (result type of b.F) - }}, - {"c", lookups{ - {"b.B", "myFact(b.B)"}, - {"b.F", "myFact(b.F)"}, - //{"b.F(nil)()", "myFact(a.T)"}, // no fact; TODO(adonovan): investigate - {"C", "myFact(c.C)"}, - {"C{}[0]", "myFact(b.B)"}, - {"<-(C{}[0])", "no fact"}, // object but no fact (we never "analyze" a2) - }}, - } { + for _, test := range tests { // load package pkg, err := load(t, dir, test.path) if err != nil { @@ -99,18 +259,16 @@ func TestEncodeDecode(t *testing.T) { if err != nil { t.Fatalf("Decode failed: %v", err) } - if true { - t.Logf("decode %s facts = %v", pkg.Path(), facts) // show all facts - } + t.Logf("decode %s facts = %v", pkg.Path(), facts) // show all facts // export // (one fact for each package-level object) - scope := pkg.Scope() - for _, name := range scope.Names() { - obj := scope.Lookup(name) + for _, name := range pkg.Scope().Names() { + obj := pkg.Scope().Lookup(name) fact := &myFact{obj.Pkg().Name() + "." + obj.Name()} facts.ExportObjectFact(obj, fact) } + t.Logf("exported %s facts = %v", pkg.Path(), facts) // show all facts // import // (after export, because an analyzer may import its own facts) diff --git a/go/analysis/internal/facts/imports.go b/go/analysis/internal/facts/imports.go index 34740f48e04..ade0cc6fab4 100644 --- a/go/analysis/internal/facts/imports.go +++ b/go/analysis/internal/facts/imports.go @@ -4,7 +4,11 @@ package facts -import "go/types" +import ( + "go/types" + + "golang.org/x/tools/internal/typeparams" +) // importMap computes the import map for a package by traversing the // entire exported API each of its imports. @@ -42,9 +46,20 @@ func importMap(imports []*types.Package) map[string]*types.Package { // nop case *types.Named: if addObj(T.Obj()) { + // TODO(taking): Investigate why the Underlying type is not added here. for i := 0; i < T.NumMethods(); i++ { addObj(T.Method(i)) } + if tparams := typeparams.ForNamed(T); tparams != nil { + for i := 0; i < tparams.Len(); i++ { + addType(tparams.At(i)) + } + } + if targs := typeparams.NamedTypeArgs(T); targs != nil { + for i := 0; i < targs.Len(); i++ { + addType(targs.At(i)) + } + } } case *types.Pointer: addType(T.Elem()) @@ -60,6 +75,11 @@ func importMap(imports []*types.Package) map[string]*types.Package { case *types.Signature: addType(T.Params()) addType(T.Results()) + if tparams := typeparams.ForSignature(T); tparams != nil { + for i := 0; i < tparams.Len(); i++ { + addType(tparams.At(i)) + } + } case *types.Struct: for i := 0; i < T.NumFields(); i++ { addObj(T.Field(i)) @@ -72,6 +92,17 @@ func importMap(imports []*types.Package) map[string]*types.Package { for i := 0; i < T.NumMethods(); i++ { addObj(T.Method(i)) } + for i := 0; i < T.NumEmbeddeds(); i++ { + addType(T.EmbeddedType(i)) // walk Embedded for implicits + } + case *typeparams.Union: + for i := 0; i < T.Len(); i++ { + addType(T.Term(i).Type()) + } + case *typeparams.TypeParam: + if addObj(T.Obj()) { + addType(T.Constraint()) + } } }