wire: use go/packages for analysis (google/go-cloud#623)

Unfortunately, this does come with a ~4x slowdown to Wire, as it is now pulling source for all transitively depended packages, but not trimming comments or function bodies. This is due to limitations with the ParseFile callback in go/packages. This comes with a single semantic change: when performing analysis, Wire will now evaluate everything with the wireinject build tag. I updated the build tags tests accordingly. Prior to this PR, only the packages directly named by the package patterns would be evaluated with the wireinject build tag. Dependencies would not have the wireinject build tag applied. There isn't a way to selectively apply build tags in go/packages, and there isn't a clear benefit to applying it selectively. Being consistent with other Go tooling provides greater benefit. I deleted the vendoring test, as non-top-level vendoring becomes obsolete with modules. go/packages now parses comments by default, so now the generated code includes comments for non-injector declarations. Fixes google/go-cloud#78
2018-11-07 14:44:15 -08:00
parent ce30a430c6
commit 282105c273
26 changed files with 198 additions and 391 deletions
--- a/internal/wire/parse.go
+++ b/internal/wire/parse.go
@@ -19,14 +19,13 @@ import (
 	"errors"
 	"fmt"
 	"go/ast"
-	"go/build"
 	"go/token"
 	"go/types"
 	"strconv"
 	"strings"

 	"golang.org/x/tools/go/ast/astutil"
-	"golang.org/x/tools/go/loader"
+	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/go/types/typeutil"
 )

@@ -135,8 +134,8 @@ type IfaceBinding struct {
 // Provider records the signature of a provider. A provider is a
 // single Go object, either a function or a named struct type.
 type Provider struct {
-	// ImportPath is the package path that the Go object resides in.
-	ImportPath string
+	// Pkg is the package that the Go object resides in.
+	Pkg *types.Package

 	// Name is the name of the Go object.
 	Name string
@@ -203,22 +202,26 @@ type Value struct {
 // In case of duplicate environment variables, the last one in the list
 // takes precedence.
 func Load(ctx context.Context, wd string, env []string, patterns []string) (*Info, []error) {
-	prog, errs := load(ctx, wd, env, patterns)
+	pkgs, errs := load(ctx, wd, env, patterns)
 	if len(errs) > 0 {
 		return nil, errs
 	}
+	if len(pkgs) == 0 {
+		return new(Info), nil
+	}
+	fset := pkgs[0].Fset
 	info := &Info{
-		Fset: prog.Fset,
+		Fset: fset,
 		Sets: make(map[ProviderSetID]*ProviderSet),
 	}
-	oc := newObjectCache(prog)
+	oc := newObjectCache(pkgs)
 	ec := new(errorCollector)
-	for _, pkgInfo := range prog.InitialPackages() {
-		if isWireImport(pkgInfo.Pkg.Path()) {
+	for _, pkg := range pkgs {
+		if isWireImport(pkg.PkgPath) {
 			// The marker function package confuses analysis.
 			continue
 		}
-		scope := pkgInfo.Pkg.Scope()
+		scope := pkg.Types.Scope()
 		for _, name := range scope.Names() {
 			obj := scope.Lookup(name)
 			if !isProviderSetType(obj.Type()) {
@@ -226,7 +229,7 @@ func Load(ctx context.Context, wd string, env []string, patterns []string) (*Inf
 			}
 			item, errs := oc.get(obj)
 			if len(errs) > 0 {
-				ec.add(notePositionAll(prog.Fset.Position(obj.Pos()), errs)...)
+				ec.add(notePositionAll(fset.Position(obj.Pos()), errs)...)
 				continue
 			}
 			pset := item.(*ProviderSet)
@@ -235,47 +238,47 @@ func Load(ctx context.Context, wd string, env []string, patterns []string) (*Inf
 			id := ProviderSetID{ImportPath: pset.PkgPath, VarName: name}
 			info.Sets[id] = pset
 		}
-		for _, f := range pkgInfo.Files {
+		for _, f := range pkg.Syntax {
 			for _, decl := range f.Decls {
 				fn, ok := decl.(*ast.FuncDecl)
 				if !ok {
 					continue
 				}
-				buildCall, err := findInjectorBuild(&pkgInfo.Info, fn)
+				buildCall, err := findInjectorBuild(pkg.TypesInfo, fn)
 				if err != nil {
-					ec.add(notePosition(prog.Fset.Position(fn.Pos()), fmt.Errorf("inject %s: %v", fn.Name.Name, err)))
+					ec.add(notePosition(fset.Position(fn.Pos()), fmt.Errorf("inject %s: %v", fn.Name.Name, err)))
 					continue
 				}
 				if buildCall == nil {
 					continue
 				}
-				set, errs := oc.processNewSet(pkgInfo, buildCall, "")
+				set, errs := oc.processNewSet(pkg.TypesInfo, pkg.PkgPath, buildCall, "")
 				if len(errs) > 0 {
-					ec.add(notePositionAll(prog.Fset.Position(fn.Pos()), errs)...)
+					ec.add(notePositionAll(fset.Position(fn.Pos()), errs)...)
 					continue
 				}
-				sig := pkgInfo.ObjectOf(fn.Name).Type().(*types.Signature)
+				sig := pkg.TypesInfo.ObjectOf(fn.Name).Type().(*types.Signature)
 				ins, out, err := injectorFuncSignature(sig)
 				if err != nil {
 					if w, ok := err.(*wireErr); ok {
 						ec.add(notePosition(w.position, fmt.Errorf("inject %s: %v", fn.Name.Name, w.error)))
 					} else {
-						ec.add(notePosition(prog.Fset.Position(fn.Pos()), fmt.Errorf("inject %s: %v", fn.Name.Name, err)))
+						ec.add(notePosition(fset.Position(fn.Pos()), fmt.Errorf("inject %s: %v", fn.Name.Name, err)))
 					}
 					continue
 				}
-				_, errs = solve(prog.Fset, out.out, ins, set)
+				_, errs = solve(fset, out.out, ins, set)
 				if len(errs) > 0 {
 					ec.add(mapErrors(errs, func(e error) error {
 						if w, ok := e.(*wireErr); ok {
 							return notePosition(w.position, fmt.Errorf("inject %s: %v", fn.Name.Name, w.error))
 						}
-						return notePosition(prog.Fset.Position(fn.Pos()), fmt.Errorf("inject %s: %v", fn.Name.Name, e))
+						return notePosition(fset.Position(fn.Pos()), fmt.Errorf("inject %s: %v", fn.Name.Name, e))
 					})...)
 					continue
 				}
 				info.Injectors = append(info.Injectors, &Injector{
-					ImportPath: pkgInfo.Pkg.Path(),
+					ImportPath: pkg.PkgPath,
 					FuncName:   fn.Name.Name,
 				})
 			}
@@ -284,111 +287,43 @@ func Load(ctx context.Context, wd string, env []string, patterns []string) (*Inf
 	return info, ec.errors
 }

-// load typechecks the packages that match the given patterns, including
-// function body type checking for the packages that directly match. The
-// patterns are defined by the underlying build system. For the go tool,
-// this is described at
-// https://golang.org/cmd/go/#hdr-Package_lists_and_patterns
+// load typechecks the packages that match the given patterns and
+// includes source for all transitive dependencies.  The patterns are
+// defined by the underlying build system. For the go tool, this is
+// described at https://golang.org/cmd/go/#hdr-Package_lists_and_patterns
 //
 // wd is the working directory and env is the set of environment
 // variables to use when loading the packages specified by patterns. If
 // env is nil or empty, it is interpreted as an empty set of variables.
 // In case of duplicate environment variables, the last one in the list
 // takes precedence.
-func load(ctx context.Context, wd string, env []string, patterns []string) (*loader.Program, []error) {
-	bctx := buildContextFromEnv(env)
-	var foundPkgs []*build.Package
-	ec := new(errorCollector)
-	for _, name := range patterns {
-		p, err := bctx.Import(name, wd, build.FindOnly)
-		if err != nil {
-			ec.add(err)
-			continue
-		}
-		foundPkgs = append(foundPkgs, p)
+func load(ctx context.Context, wd string, env []string, patterns []string) ([]*packages.Package, []error) {
+	cfg := &packages.Config{
+		Context:    ctx,
+		Mode:       packages.LoadAllSyntax,
+		Dir:        wd,
+		Env:        env,
+		BuildFlags: []string{"-tags=wireinject"},
+		// TODO(light): Use ParseFile to skip function bodies and comments in indirect packages.
 	}
-	if len(ec.errors) > 0 {
-		return nil, ec.errors
-	}
-	conf := &loader.Config{
-		Build: bctx,
-		Cwd:   wd,
-		TypeChecker: types.Config{
-			Error: func(err error) {
-				ec.add(err)
-			},
-		},
-		TypeCheckFuncBodies: func(path string) bool {
-			return importPathInPkgList(foundPkgs, path)
-		},
-		FindPackage: func(bctx *build.Context, importPath, fromDir string, mode build.ImportMode) (*build.Package, error) {
-			// Optimistically try to load in the package with normal build tags.
-			pkg, err := bctx.Import(importPath, fromDir, mode)
-
-			// If this is the generated package, then load it in with the
-			// wireinject build tag to pick up the injector template. Since
-			// the *build.Context is shared between calls to FindPackage, this
-			// uses a copy.
-			if pkg != nil && importPathInPkgList(foundPkgs, pkg.ImportPath) {
-				bctx2 := new(build.Context)
-				*bctx2 = *bctx
-				n := len(bctx2.BuildTags)
-				bctx2.BuildTags = append(bctx2.BuildTags[:n:n], "wireinject")
-				pkg, err = bctx2.Import(importPath, fromDir, mode)
-			}
-			return pkg, err
-		},
-	}
-	for _, name := range patterns {
-		conf.Import(name)
-	}
-
-	prog, err := conf.Load()
-	if len(ec.errors) > 0 {
-		return nil, ec.errors
+	escaped := make([]string, len(patterns))
+	for i := range patterns {
+		escaped[i] = "pattern=" + patterns[i]
 	}
+	pkgs, err := packages.Load(cfg, escaped...)
 	if err != nil {
 		return nil, []error{err}
 	}
-	return prog, nil
-}
-
-func buildContextFromEnv(env []string) *build.Context {
-	// TODO(#78): Remove this function in favor of using go/packages,
-	// which does not need a *build.Context.
-
-	getenv := func(name string) string {
-		for i := len(env) - 1; i >= 0; i-- {
-			if strings.HasPrefix(env[i], name+"=") {
-				return env[i][len(name)+1:]
-			}
-		}
-		return ""
-	}
-	bctx := new(build.Context)
-	*bctx = build.Default
-	if v := getenv("GOARCH"); v != "" {
-		bctx.GOARCH = v
-	}
-	if v := getenv("GOOS"); v != "" {
-		bctx.GOOS = v
-	}
-	if v := getenv("GOROOT"); v != "" {
-		bctx.GOROOT = v
-	}
-	if v := getenv("GOPATH"); v != "" {
-		bctx.GOPATH = v
-	}
-	return bctx
-}
-
-func importPathInPkgList(pkgs []*build.Package, path string) bool {
+	var errs []error
 	for _, p := range pkgs {
-		if path == p.ImportPath {
-			return true
+		for _, e := range p.Errors {
+			errs = append(errs, e)
 		}
 	}
-	return false
+	if len(errs) > 0 {
+		return nil, errs
+	}
+	return pkgs, nil
 }

 // Info holds the result of Load.
@@ -427,9 +362,10 @@ func (in *Injector) String() string {

 // objectCache is a lazily evaluated mapping of objects to Wire structures.
 type objectCache struct {
-	prog    *loader.Program
-	objects map[objRef]objCacheEntry
-	hasher  typeutil.Hasher
+	fset     *token.FileSet
+	packages map[string]*packages.Package
+	objects  map[objRef]objCacheEntry
+	hasher   typeutil.Hasher
 }

 type objRef struct {
@@ -442,12 +378,33 @@ type objCacheEntry struct {
 	errs []error
 }

-func newObjectCache(prog *loader.Program) *objectCache {
-	return &objectCache{
-		prog:    prog,
-		objects: make(map[objRef]objCacheEntry),
-		hasher:  typeutil.MakeHasher(),
+func newObjectCache(pkgs []*packages.Package) *objectCache {
+	if len(pkgs) == 0 {
+		panic("object cache must have packages to draw from")
 	}
+	oc := &objectCache{
+		fset:     pkgs[0].Fset,
+		packages: make(map[string]*packages.Package),
+		objects:  make(map[objRef]objCacheEntry),
+		hasher:   typeutil.MakeHasher(),
+	}
+	// Depth-first search of all dependencies to gather import path to
+	// packages.Package mapping. go/packages guarantees that for a single
+	// call to packages.Load and an import path X, there will exist only
+	// one *packages.Package value with PkgPath X.
+	stk := append([]*packages.Package(nil), pkgs...)
+	for len(stk) > 0 {
+		p := stk[len(stk)-1]
+		stk = stk[:len(stk)-1]
+		if oc.packages[p.PkgPath] != nil {
+			continue
+		}
+		oc.packages[p.PkgPath] = p
+		for _, imp := range p.Imports {
+			stk = append(stk, imp)
+		}
+	}
+	return oc
 }

 // get converts a Go object into a Wire structure. It may return a
@@ -478,9 +435,10 @@ func (oc *objectCache) get(obj types.Object) (val interface{}, errs []error) {
 				break
 			}
 		}
-		return oc.processExpr(oc.prog.Package(obj.Pkg().Path()), spec.Values[i], obj.Name())
+		pkgPath := obj.Pkg().Path()
+		return oc.processExpr(oc.packages[pkgPath].TypesInfo, pkgPath, spec.Values[i], obj.Name())
 	case *types.Func:
-		return processFuncProvider(oc.prog.Fset, obj)
+		return processFuncProvider(oc.fset, obj)
 	default:
 		return nil, []error{fmt.Errorf("%v is not a provider or a provider set", obj)}
 	}
@@ -490,10 +448,10 @@ func (oc *objectCache) get(obj types.Object) (val interface{}, errs []error) {
 func (oc *objectCache) varDecl(obj *types.Var) *ast.ValueSpec {
 	// TODO(light): Walk files to build object -> declaration mapping, if more performant.
 	// Recommended by https://golang.org/s/types-tutorial
-	pkg := oc.prog.Package(obj.Pkg().Path())
+	pkg := oc.packages[obj.Pkg().Path()]
 	pos := obj.Pos()
-	for _, f := range pkg.Files {
-		tokenFile := oc.prog.Fset.File(f.Pos())
+	for _, f := range pkg.Syntax {
+		tokenFile := oc.fset.File(f.Pos())
 		if base := tokenFile.Base(); base <= int(pos) && int(pos) < base+tokenFile.Size() {
 			path, _ := astutil.PathEnclosingInterval(f, pos, pos)
 			for _, node := range path {
@@ -508,38 +466,38 @@ func (oc *objectCache) varDecl(obj *types.Var) *ast.ValueSpec {

 // processExpr converts an expression into a Wire structure. It may
 // return a *Provider, an *IfaceBinding, a *ProviderSet, or a *Value.
-func (oc *objectCache) processExpr(pkg *loader.PackageInfo, expr ast.Expr, varName string) (interface{}, []error) {
-	exprPos := oc.prog.Fset.Position(expr.Pos())
+func (oc *objectCache) processExpr(info *types.Info, pkgPath string, expr ast.Expr, varName string) (interface{}, []error) {
+	exprPos := oc.fset.Position(expr.Pos())
 	expr = astutil.Unparen(expr)
-	if obj := qualifiedIdentObject(&pkg.Info, expr); obj != nil {
+	if obj := qualifiedIdentObject(info, expr); obj != nil {
 		item, errs := oc.get(obj)
 		return item, mapErrors(errs, func(err error) error {
 			return notePosition(exprPos, err)
 		})
 	}
 	if call, ok := expr.(*ast.CallExpr); ok {
-		fnObj := qualifiedIdentObject(&pkg.Info, call.Fun)
+		fnObj := qualifiedIdentObject(info, call.Fun)
 		if fnObj == nil || !isWireImport(fnObj.Pkg().Path()) {
 			return nil, []error{notePosition(exprPos, errors.New("unknown pattern"))}
 		}
 		switch fnObj.Name() {
 		case "NewSet":
-			pset, errs := oc.processNewSet(pkg, call, varName)
+			pset, errs := oc.processNewSet(info, pkgPath, call, varName)
 			return pset, notePositionAll(exprPos, errs)
 		case "Bind":
-			b, err := processBind(oc.prog.Fset, &pkg.Info, call)
+			b, err := processBind(oc.fset, info, call)
 			if err != nil {
 				return nil, []error{notePosition(exprPos, err)}
 			}
 			return b, nil
 		case "Value":
-			v, err := processValue(oc.prog.Fset, &pkg.Info, call)
+			v, err := processValue(oc.fset, info, call)
 			if err != nil {
 				return nil, []error{notePosition(exprPos, err)}
 			}
 			return v, nil
 		case "InterfaceValue":
-			v, err := processInterfaceValue(oc.prog.Fset, &pkg.Info, call)
+			v, err := processInterfaceValue(oc.fset, info, call)
 			if err != nil {
 				return nil, []error{notePosition(exprPos, err)}
 			}
@@ -548,8 +506,8 @@ func (oc *objectCache) processExpr(pkg *loader.PackageInfo, expr ast.Expr, varNa
 			return nil, []error{notePosition(exprPos, errors.New("unknown pattern"))}
 		}
 	}
-	if tn := structArgType(&pkg.Info, expr); tn != nil {
-		p, errs := processStructProvider(oc.prog.Fset, tn)
+	if tn := structArgType(info, expr); tn != nil {
+		p, errs := processStructProvider(oc.fset, tn)
 		if len(errs) > 0 {
 			return nil, notePositionAll(exprPos, errs)
 		}
@@ -558,17 +516,17 @@ func (oc *objectCache) processExpr(pkg *loader.PackageInfo, expr ast.Expr, varNa
 	return nil, []error{notePosition(exprPos, errors.New("unknown pattern"))}
 }

-func (oc *objectCache) processNewSet(pkg *loader.PackageInfo, call *ast.CallExpr, varName string) (*ProviderSet, []error) {
+func (oc *objectCache) processNewSet(info *types.Info, pkgPath string, call *ast.CallExpr, varName string) (*ProviderSet, []error) {
 	// Assumes that call.Fun is wire.NewSet or wire.Build.

 	pset := &ProviderSet{
 		Pos:     call.Pos(),
-		PkgPath: pkg.Pkg.Path(),
+		PkgPath: pkgPath,
 		VarName: varName,
 	}
 	ec := new(errorCollector)
 	for _, arg := range call.Args {
-		item, errs := oc.processExpr(pkg, arg, "")
+		item, errs := oc.processExpr(info, pkgPath, arg, "")
 		if len(errs) > 0 {
 			ec.add(errs...)
 			continue
@@ -590,7 +548,7 @@ func (oc *objectCache) processNewSet(pkg *loader.PackageInfo, call *ast.CallExpr
 		return nil, ec.errors
 	}
 	var errs []error
-	pset.providerMap, pset.srcMap, errs = buildProviderMap(oc.prog.Fset, oc.hasher, pset)
+	pset.providerMap, pset.srcMap, errs = buildProviderMap(oc.fset, oc.hasher, pset)
 	if len(errs) > 0 {
 		return nil, errs
 	}
@@ -647,7 +605,7 @@ func processFuncProvider(fset *token.FileSet, fn *types.Func) (*Provider, []erro
 	}
 	params := sig.Params()
 	provider := &Provider{
-		ImportPath: fn.Pkg().Path(),
+		Pkg:        fn.Pkg(),
 		Name:       fn.Name(),
 		Pos:        fn.Pos(),
 		Args:       make([]ProviderInput, params.Len()),
@@ -733,13 +691,13 @@ func processStructProvider(fset *token.FileSet, typeName *types.TypeName) (*Prov

 	pos := typeName.Pos()
 	provider := &Provider{
-		ImportPath: typeName.Pkg().Path(),
-		Name:       typeName.Name(),
-		Pos:        pos,
-		Args:       make([]ProviderInput, st.NumFields()),
-		Fields:     make([]string, st.NumFields()),
-		IsStruct:   true,
-		Out:        []types.Type{out, types.NewPointer(out)},
+		Pkg:      typeName.Pkg(),
+		Name:     typeName.Name(),
+		Pos:      pos,
+		Args:     make([]ProviderInput, st.NumFields()),
+		Fields:   make([]string, st.NumFields()),
+		IsStruct: true,
+		Out:      []types.Type{out, types.NewPointer(out)},
 	}
 	for i := 0; i < st.NumFields(); i++ {
 		f := st.Field(i)