go/packages: switch fallback implementation to use go list

The go/loader based implementation was too different. Instead
Just call go list twice to get all the dependencies. This immediately
enables more functionality.

Change-Id: I22883492e996133bc61664314318c5e0a9cc10dc
Reviewed-on: https://go-review.googlesource.com/125939
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

go/packages/golist.go

@@ -27,10 +27,6 @@ type GoTooOldError struct {
 	error
 }
 
-// golistPackages uses the "go list" command to expand the
-// pattern words and return metadata for the specified packages.
-// dir may be "" and env may be nil, as per os/exec.Command.
-func golistPackages(cfg *rawConfig, words ...string) ([]*rawPackage, error) {
 // Fields must match go list;
 // see $GOROOT/src/cmd/go/internal/load/pkg.go.
 type jsonPackage struct {
@@ -52,6 +48,11 @@ func golistPackages(cfg *rawConfig, words ...string) ([]*rawPackage, error) {
 	ForTest      string // q in a "p [q.test]" package, else ""
 	DepOnly      bool
 }
+
+// golistPackages uses the "go list" command to expand the
+// pattern words and return metadata for the specified packages.
+// dir may be "" and env may be nil, as per os/exec.Command.
+func golistPackages(cfg *rawConfig, words ...string) ([]*rawPackage, error) {
 	// go list uses the following identifiers in ImportPath and Imports:
 	//
 	// 	"p"			-- importable package or main (command)
@@ -66,11 +67,11 @@ func golistPackages(cfg *rawConfig, words ...string) ([]*rawPackage, error) {
 	// Run "go list" for complete
 	// information on the specified packages.
 
-	buf, err := golist(cfg, words)
+	buf, err := golist(cfg, golistargs(cfg, words))
 	if err != nil {
 		return nil, err
 	}
-	// Decode the JSON and convert it to Package form.
+	// Decode the JSON and convert it to rawPackage form.
 	var result []*rawPackage
 	for dec := json.NewDecoder(buf); dec.More(); {
 		p := new(jsonPackage)
@@ -179,14 +180,18 @@ func absJoin(dir string, fileses ...[]string) (res []string) {
 	return res
 }
 
-// golist returns the JSON-encoded result of a "go list args..." query.
+func golistargs(cfg *rawConfig, words []string) []string {
-func golist(cfg *rawConfig, args []string) (*bytes.Buffer, error) {
-	out := new(bytes.Buffer)
 	fullargs := []string{"list", "-e", "-json", "-cgo=true"}
 	fullargs = append(fullargs, cfg.Flags()...)
 	fullargs = append(fullargs, "--")
-	fullargs = append(fullargs, args...)
+	fullargs = append(fullargs, words...)
-	cmd := exec.CommandContext(cfg.Context, "go", fullargs...)
+	return fullargs
+}
+
+// golist returns the JSON-encoded result of a "go list args..." query.
+func golist(cfg *rawConfig, args []string) (*bytes.Buffer, error) {
+	out := new(bytes.Buffer)
+	cmd := exec.CommandContext(cfg.Context, "go", args...)
 	cmd.Env = cfg.Env
 	cmd.Dir = cfg.Dir
 	cmd.Stdout = out

go/packages/golist_fallback.go

@@ -0,0 +1,147 @@
+package packages
+
+import (
+	"encoding/json"
+	"fmt"
+	"golang.org/x/tools/imports"
+)
+
+// TODO(matloob): Delete this file once Go 1.12 is released.
+
+// This file provides backwards compatibility support for
+// loading for versions of Go earlier than 1.10.4. This support is meant to
+// assist with migration to the Package API until there's
+// widespread adoption of these newer Go versions.
+// This support will be removed once Go 1.12 is released
+// in Q1 2019.
+
+// TODO(matloob): Support cgo. Copy code from the loader that runs cgo.
+
+func golistPackagesFallback(cfg *rawConfig, words ...string) ([]*rawPackage, error) {
+	original, deps, err := getDeps(cfg, words...)
+	if err != nil {
+		return nil, err
+	}
+
+	var result []*rawPackage
+	addPackage := func(p *jsonPackage) {
+		if p.Name == "" {
+			return
+		}
+
+		id := p.ImportPath
+		pkgpath := id
+
+		if pkgpath == "unsafe" {
+			p.GoFiles = nil // ignore fake unsafe.go file
+		}
+
+		importMap := func(importlist []string) map[string]string {
+			importMap := make(map[string]string)
+			for _, id := range importlist {
+
+				if id == "C" {
+					importMap["unsafe"] = "unsafe"
+					importMap["syscall"] = "syscall"
+					if pkgpath != "runtime/cgo" {
+						importMap["runtime/cgo"] = "runtime/cgo"
+					}
+					continue
+				}
+				importMap[imports.VendorlessPath(id)] = id
+			}
+			return importMap
+		}
+
+		result = append(result, &rawPackage{
+			ID:         id,
+			Name:       p.Name,
+			GoFiles:    absJoin(p.Dir, p.GoFiles, p.CgoFiles),
+			OtherFiles: absJoin(p.Dir, p.SFiles, p.CFiles),
+			PkgPath:    pkgpath,
+			Imports:    importMap(p.Imports),
+			DepOnly:    original[id] == nil,
+		})
+		if cfg.Tests {
+			testID := fmt.Sprintf("%s [%s.test]", id, id)
+			if len(p.TestGoFiles) > 0 || len(p.XTestGoFiles) > 0 {
+				result = append(result, &rawPackage{
+					ID:         testID,
+					Name:       p.Name,
+					GoFiles:    absJoin(p.Dir, p.GoFiles, p.TestGoFiles, p.CgoFiles),
+					OtherFiles: absJoin(p.Dir, p.SFiles, p.CFiles),
+					PkgPath:    pkgpath,
+					Imports:    importMap(append(p.Imports, p.TestImports)),
+					DepOnly:    original[id] == nil,
+				})
+			}
+			if len(p.XTestGoFiles) > 0 {
+				result = append(result, &rawPackage{
+					ID:      fmt.Sprintf("%s_test [%s.test]", id, id),
+					Name:    p.Name + "_test",
+					GoFiles: absJoin(p.Dir, p.XTestGoFiles),
+					PkgPath: pkgpath,
+					Imports: importMap(append(p.XTestImports, testID)),
+					DepOnly: original[id] == nil,
+				})
+			}
+		}
+	}
+
+	for _, pkg := range original {
+		addPackage(pkg)
+	}
+	if !cfg.Deps || len(deps) == 0 {
+		return result, nil
+	}
+
+	buf, err := golist(cfg, append([]string{"list", "-e", "-json", "--"}, deps...))
+	if err != nil {
+		return nil, err
+	}
+
+	// Decode the JSON and convert it to rawPackage form.
+	for dec := json.NewDecoder(buf); dec.More(); {
+		p := new(jsonPackage)
+		if err := dec.Decode(p); err != nil {
+			return nil, fmt.Errorf("JSON decoding failed: %v", err)
+		}
+
+		addPackage(p)
+	}
+
+	return result, nil
+}
+
+// getDeps runs an initial go list to determine all the dependency packages.
+func getDeps(cfg *rawConfig, words ...string) (originalSet map[string]*jsonPackage, deps []string, err error) {
+	buf, err := golist(cfg, append([]string{"list", "-e", "-json", "--"}, words...))
+	if err != nil {
+		return nil, nil, err
+	}
+
+	depsSet := make(map[string]bool)
+	originalSet = make(map[string]*jsonPackage)
+
+	// Extract deps from the JSON.
+	for dec := json.NewDecoder(buf); dec.More(); {
+		p := new(jsonPackage)
+		if err := dec.Decode(p); err != nil {
+			return nil, nil, fmt.Errorf("JSON decoding failed: %v", err)
+		}
+
+		originalSet[p.ImportPath] = p
+		for _, dep := range p.Deps {
+			depsSet[dep] = true
+		}
+	}
+	for orig := range originalSet {
+		delete(depsSet, orig)
+	}
+
+	deps = make([]string, 0, len(depsSet))
+	for dep := range depsSet {
+		deps = append(deps, dep)
+	}
+	return originalSet, deps, nil
+}

go/packages/loader_fallback.go

@@ -1,130 +0,0 @@
-// Copyright 2018 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// TODO(matloob): Delete this file once Go 1.12 is released.
-
-// This file provides backwards compatibility support for
-// loading for versions of Go earlier than 1.10.4. This support is meant to
-// assist with migration to the Package API until there's
-// widespread adoption of these newer Go versions.
-// This support will be removed once Go 1.12 is released
-// in Q1 2019.
-
-// The support is incomplete. These are some of the missing
-// features:
-// - the Tests option has no behavior, and test packages are
-//   never returned.
-// - Package.OtherFiles are always missing even if the package
-//   contains non-go sources.
-
-package packages
-
-import (
-	"fmt"
-	"go/build"
-	"strings"
-
-	legacy "golang.org/x/tools/go/loader"
-	"golang.org/x/tools/imports"
-)
-
-func loaderFallback(dir string, env []string, patterns []string) ([]*Package, error) {
-	cfg := legacy.Config{}
-	cfg.Cwd = dir
-	cfg.AllowErrors = true
-	cfg.FromArgs(patterns, false) // test packages are not supported
-
-	// Set build ctx
-	buildCtx := build.Default
-	for _, ev := range env {
-		sp := strings.Split(ev, "=")
-		if len(sp) != 2 {
-			continue
-		}
-		evar, val := sp[0], sp[1]
-		switch evar {
-		case "GOPATH":
-			buildCtx.GOPATH = val
-		case "GOROOT":
-			buildCtx.GOROOT = val
-		case "GOARCH":
-			buildCtx.GOARCH = val
-		case "GOOS":
-			buildCtx.GOOS = val
-		}
-	}
-	cfg.Build = &buildCtx
-
-	lprog, err := cfg.Load()
-	if err != nil {
-		if err.Error() == "no initial packages were loaded" {
-			return nil, fmt.Errorf("packages not found") // Return same error as golist-based code
-		}
-		return nil, fmt.Errorf("failed to load packages with legacy loader: %v", err)
-	}
-
-	allpkgs := make(map[string]*loaderPackage)
-
-	initial := make(map[*legacy.PackageInfo]bool)
-	for _, lpkg := range lprog.InitialPackages() {
-		initial[lpkg] = true
-	}
-	for _, lpkg := range lprog.AllPackages {
-		id := lpkg.Pkg.Path()
-
-		var goFiles []string
-		for _, f := range lpkg.Files {
-			goFiles = append(goFiles, lprog.Fset.File(f.Pos()).Name())
-		}
-
-		pkgimports := make(map[string]string)
-		for _, imppkg := range lpkg.Pkg.Imports() {
-			// TODO(matloob): Is the import path of a package always VendorlessPath(path)?
-			pkgimports[imports.VendorlessPath(imppkg.Path())] = imppkg.Path()
-		}
-
-		allpkgs[id] = &loaderPackage{
-			raw: &rawPackage{
-				ID:      id,
-				Name:    lpkg.Pkg.Name(),
-				Imports: pkgimports,
-				GoFiles: goFiles,
-				DepOnly: !initial[lpkg],
-			},
-			Package: &Package{
-				ID:         id,
-				Name:       lpkg.Pkg.Name(),
-				GoFiles:    goFiles,
-				Fset:       lprog.Fset,
-				Syntax:     lpkg.Files,
-				Errors:     lpkg.Errors,
-				Types:      lpkg.Pkg,
-				TypesInfo:  &lpkg.Info,
-				IllTyped:   !lpkg.TransitivelyErrorFree,
-				OtherFiles: nil, // Never set for the fallback, because we can't extract from loader.
-			},
-		}
-	}
-
-	// Do a second pass to populate imports.
-	for _, pkg := range allpkgs {
-		pkg.Imports = make(map[string]*Package)
-		for imppath, impid := range pkg.raw.Imports {
-			target, ok := allpkgs[impid]
-			if !ok {
-				// return nil, fmt.Errorf("could not load package: %v", impid)
-				continue
-			}
-			pkg.Imports[imppath] = target.Package
-		}
-	}
-
-	// Grab the initial set of packages.
-	var packages []*Package
-	for _, lpkg := range lprog.InitialPackages() {
-		packages = append(packages, allpkgs[lpkg.Pkg.Path()].Package)
-	}
-
-	return packages, nil
-}

go/packages/packages.go

@@ -262,7 +262,12 @@ func (ld *loader) load(patterns ...string) ([]*Package, error) {
 	rawCfg := newRawConfig(&ld.Config)
 	list, err := golistPackages(rawCfg, patterns...)
 	if _, ok := err.(GoTooOldError); ok {
-		return loaderFallback(ld.Dir, ld.Env, patterns)
+		if ld.Config.Mode >= LoadTypes {
+			// Upgrade to LoadAllSyntax because we can't depend on the existance
+			// of export data. We can remove this once iancottrell's cl is in.
+			ld.Config.Mode = LoadAllSyntax
+		}
+		list, err = golistPackagesFallback(rawCfg, patterns...)
 	}
 	if err != nil {
 		return nil, err

go/packages/packages110_test.go

@@ -7,5 +7,5 @@
 package packages_test
 
 func init() {
-	usesLegacyLoader = true
+	usesOldGolist = true
 }

go/packages/packages_test.go

@@ -25,8 +25,8 @@ import (
 )
 
 // TODO(matloob): remove this once Go 1.12 is released as we will end support
-// for the loader-backed implementation then.
+// for versions of go list before Go 1.10.4.
-var usesLegacyLoader = false
+var usesOldGolist = false
 
 // TODO(adonovan): more test cases to write:
 //
@@ -133,7 +133,7 @@ func TestMetadataImportGraph(t *testing.T) {
   subdir/d_test [subdir/d.test] -> subdir/d [subdir/d.test]
 `[1:]
 
-	if graph != wantGraph && !usesLegacyLoader {
+	if graph != wantGraph && !usesOldGolist {
 		t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantGraph)
 	}
 
@@ -153,8 +153,8 @@ func TestMetadataImportGraph(t *testing.T) {
 		{"subdir/d.test", "main", "command", "0.go"},
 		{"unsafe", "unsafe", "package", ""},
 	} {
-		if usesLegacyLoader && test.id == "subdir/d.test" {
+		if usesOldGolist && test.id == "subdir/d.test" {
-			// Legacy Loader does not support tests.
+			// Legacy go list support does not create test main package.
 			continue
 		}
 		p, ok := all[test.id]
@@ -187,12 +187,12 @@ func TestMetadataImportGraph(t *testing.T) {
 		t.Errorf("failed to obtain metadata for ad-hoc package: %s", err)
 	} else {
 		got := fmt.Sprintf("%s %s", initial[0].ID, srcs(initial[0]))
-		if want := "command-line-arguments [c.go]"; got != want && !usesLegacyLoader {
+		if want := "command-line-arguments [c.go]"; got != want && !usesOldGolist {
 			t.Errorf("oops: got %s, want %s", got, want)
 		}
 	}
 
-	if usesLegacyLoader {
+	if usesOldGolist {
 		// TODO(matloob): Wildcards are not yet supported.
 		return
 	}
@@ -213,7 +213,7 @@ func TestMetadataImportGraph(t *testing.T) {
 	}
 }
 
-func TestOptionsDir_Go110(t *testing.T) {
+func TestOptionsDir(t *testing.T) {
 	tmp, cleanup := makeTree(t, map[string]string{
 		"src/a/a.go":   `package a; const Name = "a" `,
 		"src/a/b/b.go": `package b; const Name = "a/b"`,
@@ -271,7 +271,7 @@ func (ec *errCollector) add(err error) {
 	ec.mu.Unlock()
 }
 
-func TestTypeCheckOK_Go110(t *testing.T) {
+func TestTypeCheckOK(t *testing.T) {
 	tmp, cleanup := makeTree(t, map[string]string{
 		"src/a/a.go": `package a; import "b"; const A = "a" + b.B`,
 		"src/b/b.go": `package b; import "c"; const B = "b" + c.C`,
@@ -307,7 +307,7 @@ func TestTypeCheckOK_Go110(t *testing.T) {
 		t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantGraph)
 	}
 
-	// TODO(matloob): The go/loader based support loads everything from source
+	// TODO(matloob): The legacy go list based support loads everything from source
 	// because it doesn't do a build and the .a files don't exist.
 	// Can we simulate its existance?
 
@@ -322,8 +322,8 @@ func TestTypeCheckOK_Go110(t *testing.T) {
 		{"d", true, false},  // export data package
 		{"e", false, false}, // no package
 	} {
-		if usesLegacyLoader && test.id == "d" || test.id == "e" {
+		if usesOldGolist && test.id == "d" || test.id == "e" {
-			// legacyLoader always does a whole-program load.
+			// go list always upgrades  whole-program load.
 			continue
 		}
 		p := all[test.id]
@@ -411,8 +411,8 @@ func TestTypeCheckError(t *testing.T) {
 		{"d", false, false, true, nil},  // missing export data
 		{"e", false, false, false, nil}, // type info not requested (despite type error)
 	} {
-		if usesLegacyLoader && test.id == "c" || test.id == "d" || test.id == "e" {
+		if usesOldGolist && test.id == "c" || test.id == "d" || test.id == "e" {
-			// Behavior is different for legacy loader because it always loads wholeProgram.
+			// Behavior is different for old golist because it upgrades to wholeProgram.
 			// TODO(matloob): can we run more of this test? Can we put export data into the test GOPATH?
 			continue
 		}
@@ -453,10 +453,6 @@ func TestTypeCheckError(t *testing.T) {
 // This function tests use of the ParseFile hook to supply
 // alternative file contents to the parser and type-checker.
 func TestWholeProgramOverlay(t *testing.T) {
-	if usesLegacyLoader {
-		t.Skip("not yet supported in go/loader based implementation")
-	}
-
 	type M = map[string]string
 
 	tmp, cleanup := makeTree(t, M{
@@ -517,8 +513,8 @@ func TestWholeProgramOverlay(t *testing.T) {
 }
 
 func TestWholeProgramImportErrors(t *testing.T) {
-	if usesLegacyLoader {
+	if usesOldGolist {
-		t.Skip("not yet supported in go/loader based implementation")
+		t.Skip("not yet supported in pre-Go 1.10.4 golist fallback implementation")
 	}
 
 	tmp, cleanup := makeTree(t, map[string]string{