go/packages: split LoadMode into fine-grained options

The options are all unexported, and this CL is (almost) a no-op:
the one difference is that since needImports and needSyntax are now
independently specified, LoadSyntax and LoadAllSyntax are equivalent,
because LoadSyntax needs both the needImports and needSyntax bits.

I want to pin down the options that we want to split into, and
future CLs can allow the options to be used individually...

Updates golang/go#29429
Updates golang/go#29427

Change-Id: I5b2913e2c53e7ade56905e46912b076ccc339827
Reviewed-on: https://go-review.googlesource.com/c/tools/+/162140
Run-TryBot: Michael Matloob <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Cottrell <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>

Author: matloob

go/packages/golist.go

@@ -78,7 +78,7 @@ func goListDriver(cfg *Config, patterns ...string) (*driverResponse, error) {
 	var sizes types.Sizes
 	var sizeserr error
 	var sizeswg sync.WaitGroup
-	if cfg.Mode >= LoadTypes {
+	if cfg.Mode&NeedTypesSizes != 0 {
 		sizeswg.Add(1)
 		go func() {
 			sizes, sizeserr = getSizes(cfg)
@@ -701,14 +701,16 @@ func absJoin(dir string, fileses ...[]string) (res []string) {
 }
 
 func golistargs(cfg *Config, words []string) []string {
+	const findFlags = NeedImports | NeedTypes | NeedSyntax | NeedTypesInfo
 	fullargs := []string{
-		"list", "-e", "-json", "-compiled",
+		"list", "-e", "-json",
+		fmt.Sprintf("-compiled=%t", cfg.Mode&(NeedCompiledGoFiles|NeedSyntax|NeedTypesInfo|NeedTypesSizes) != 0),
 		fmt.Sprintf("-test=%t", cfg.Tests),
 		fmt.Sprintf("-export=%t", usesExportData(cfg)),
-		fmt.Sprintf("-deps=%t", cfg.Mode >= LoadImports),
+		fmt.Sprintf("-deps=%t", cfg.Mode&NeedDeps != 0),
 		// go list doesn't let you pass -test and -find together,
 		// probably because you'd just get the TestMain.
-		fmt.Sprintf("-find=%t", cfg.Mode < LoadImports && !cfg.Tests),
+		fmt.Sprintf("-find=%t", !cfg.Tests && cfg.Mode&findFlags == 0),
 	}
 	fullargs = append(fullargs, cfg.BuildFlags...)
 	fullargs = append(fullargs, "--")

go/packages/packages.go

@@ -30,32 +30,78 @@ import (
 // but may be slower. Load may return more information than requested.
 type LoadMode int
 
+const (
+	// The following constants are used to specify which fields of the Package
+	// should be filled when loading is done. As a special case to provide
+	// backwards compatibility, a LoadMode of 0 is equivalent to LoadFiles.
+	// For all other LoadModes, the bits below specify which fields will be filled
+	// in the result packages.
+	// WARNING: This part of the go/packages API is EXPERIMENTAL. It might
+	// be changed or removed up until April 15 2019. After that date it will
+	// be frozen.
+	// TODO(matloob): Remove this comment on April 15.
+
+	// ID and Errors (if present) will always be filled.
+
+	// NeedName adds Name and PkgPath.
+	NeedName LoadMode = 1 << iota
+
+	// NeedFiles adds GoFiles and OtherFiles.
+	NeedFiles
+
+	// NeedCompiledGoFiles adds CompiledGoFiles.
+	NeedCompiledGoFiles
+
+	// NeedImports adds Imports. If NeedDeps is not set, the Imports field will contain
+	// "placeholder" Packages with only the ID set.
+	NeedImports
+
+	// NeedDeps adds the fields requested by the LoadMode in the packages in Imports. If NeedImports
+	// is not set NeedDeps has no effect.
+	NeedDeps
+
+	// NeedExportsFile adds ExportsFile.
+	NeedExportsFile
+
+	// NeedTypes adds Types, Fset, and IllTyped.
+	NeedTypes
+
+	// NeedSyntax adds Syntax.
+	NeedSyntax
+
+	// NeedTypesInfo adds TypesInfo.
+	NeedTypesInfo
+
+	// NeedTypesSizes adds TypesSizes.
+	NeedTypesSizes
+)
+
 const (
 	// LoadFiles finds the packages and computes their source file lists.
-	// Package fields: ID, Name, Errors, GoFiles, and OtherFiles.
-	LoadFiles LoadMode = iota
+	// Package fields: ID, Name, Errors, GoFiles, CompiledGoFiles, and OtherFiles.
+	LoadFiles = NeedName | NeedFiles | NeedCompiledGoFiles
 
 	// LoadImports adds import information for each package
 	// and its dependencies.
 	// Package fields added: Imports.
-	LoadImports
+	LoadImports = LoadFiles | NeedImports | NeedDeps
 
 	// LoadTypes adds type information for package-level
 	// declarations in the packages matching the patterns.
 	// Package fields added: Types, Fset, and IllTyped.
 	// This mode uses type information provided by the build system when
 	// possible, and may fill in the ExportFile field.
-	LoadTypes
+	LoadTypes = LoadImports | NeedTypes
 
 	// LoadSyntax adds typed syntax trees for the packages matching the patterns.
 	// Package fields added: Syntax, and TypesInfo, for direct pattern matches only.
-	LoadSyntax
+	LoadSyntax = LoadTypes | NeedSyntax | NeedTypesInfo | NeedTypesSizes
 
 	// LoadAllSyntax adds typed syntax trees for the packages matching the patterns
 	// and all dependencies.
 	// Package fields added: Types, Fset, IllTyped, Syntax, and TypesInfo,
 	// for all packages in the import graph.
-	LoadAllSyntax
+	LoadAllSyntax = LoadSyntax
 )
 
 // A Config specifies details about how packages should be loaded.
@@ -381,6 +427,9 @@ func newLoader(cfg *Config) *loader {
 	if cfg != nil {
 		ld.Config = *cfg
 	}
+	if ld.Config.Mode == 0 {
+		ld.Config.Mode = LoadFiles // Preserve zero behavior of Mode for backwards compatibility.
+	}
 	if ld.Config.Env == nil {
 		ld.Config.Env = os.Environ()
 	}
@@ -393,7 +442,7 @@ func newLoader(cfg *Config) *loader {
 		}
 	}
 
-	if ld.Mode >= LoadTypes {
+	if ld.Mode&NeedTypes != 0 {
 		if ld.Fset == nil {
 			ld.Fset = token.NewFileSet()
 		}
@@ -430,11 +479,9 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
 			rootIndex = i
 		}
 		lpkg := &loaderPackage{
-			Package: pkg,
-			needtypes: ld.Mode >= LoadAllSyntax ||
-				ld.Mode >= LoadTypes && rootIndex >= 0,
-			needsrc: ld.Mode >= LoadAllSyntax ||
-				ld.Mode >= LoadSyntax && rootIndex >= 0 ||
+			Package:   pkg,
+			needtypes: (ld.Mode&(NeedTypes|NeedTypesInfo) != 0 && rootIndex < 0) || rootIndex >= 0,
+			needsrc: (ld.Mode&(NeedSyntax|NeedTypesInfo) != 0 && rootIndex < 0) || rootIndex >= 0 ||
 				len(ld.Overlay) > 0 || // Overlays can invalidate export data. TODO(matloob): make this check fine-grained based on dependencies on overlaid files
 				pkg.ExportFile == "" && pkg.PkgPath != "unsafe",
 		}
@@ -513,8 +560,8 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
 		return lpkg.needsrc
 	}
 
-	if ld.Mode < LoadImports {
-		//we do this to drop the stub import packages that we are not even going to try to resolve
+	if ld.Mode&NeedImports == 0 {
+		// We do this to drop the stub import packages that we are not even going to try to resolve.
 		for _, lpkg := range initial {
 			lpkg.Imports = nil
 		}
@@ -524,17 +571,19 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
 			visit(lpkg)
 		}
 	}
-	for _, lpkg := range srcPkgs {
-		// Complete type information is required for the
-		// immediate dependencies of each source package.
-		for _, ipkg := range lpkg.Imports {
-			imp := ld.pkgs[ipkg.ID]
-			imp.needtypes = true
+	if ld.Mode&NeedDeps != 0 {
+		for _, lpkg := range srcPkgs {
+			// Complete type information is required for the
+			// immediate dependencies of each source package.
+			for _, ipkg := range lpkg.Imports {
+				imp := ld.pkgs[ipkg.ID]
+				imp.needtypes = true
+			}
 		}
 	}
 	// Load type data if needed, starting at
 	// the initial packages (roots of the import DAG).
-	if ld.Mode >= LoadTypes {
+	if ld.Mode&NeedTypes != 0 {
 		var wg sync.WaitGroup
 		for _, lpkg := range initial {
 			wg.Add(1)
@@ -547,16 +596,59 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
 	}
 
 	result := make([]*Package, len(initial))
+	importPlaceholders := make(map[string]*Package)
 	for i, lpkg := range initial {
 		result[i] = lpkg.Package
+		// Clear all unrequested fields, for extra de-Hyrum-ization.
+		if ld.Mode&NeedName == 0 {
+			result[i].Name = ""
+			result[i].PkgPath = ""
+		}
+		if ld.Mode&NeedFiles == 0 {
+			result[i].GoFiles = nil
+			result[i].OtherFiles = nil
+		}
+		if ld.Mode&NeedCompiledGoFiles == 0 {
+			result[i].CompiledGoFiles = nil
+		}
+		if ld.Mode&NeedImports == 0 {
+			result[i].Imports = nil
+		}
+		if ld.Mode&NeedExportsFile == 0 {
+			result[i].ExportFile = ""
+		}
+		if ld.Mode&NeedTypes == 0 {
+			result[i].Types = nil
+			result[i].Fset = nil
+			result[i].IllTyped = false
+		}
+		if ld.Mode&NeedSyntax == 0 {
+			result[i].Syntax = nil
+		}
+		if ld.Mode&NeedTypesInfo == 0 {
+			result[i].TypesInfo = nil
+		}
+		if ld.Mode&NeedTypesSizes == 0 {
+			result[i].TypesSizes = nil
+		}
+		if ld.Mode&NeedDeps == 0 {
+			for j, pkg := range result[i].Imports {
+				ph, ok := importPlaceholders[pkg.ID]
+				if !ok {
+					ph = &Package{ID: pkg.ID}
+					importPlaceholders[pkg.ID] = ph
+				}
+				result[i].Imports[j] = ph
+			}
+		}
 	}
 	return result, nil
 }
 
 // loadRecursive loads the specified package and its dependencies,
 // recursively, in parallel, in topological order.
 // It is atomic and idempotent.
-// Precondition: ld.Mode >= LoadTypes.
+// Precondition: ld.Mode&NeedTypes.
 func (ld *loader) loadRecursive(lpkg *loaderPackage) {
 	lpkg.loadOnce.Do(func() {
 		// Load the direct dependencies, in parallel.
@@ -708,7 +800,7 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) {
 		// Type-check bodies of functions only in non-initial packages.
 		// Example: for import graph A->B->C and initial packages {A,C},
 		// we can ignore function bodies in B.
-		IgnoreFuncBodies: ld.Mode < LoadAllSyntax && !lpkg.initial,
+		IgnoreFuncBodies: (ld.Mode&(NeedDeps|NeedTypesInfo) == 0) && !lpkg.initial,
 
 		Error: appendError,
 		Sizes: ld.sizes,
@@ -952,5 +1044,5 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error
 }
 
 func usesExportData(cfg *Config) bool {
-	return LoadTypes <= cfg.Mode && cfg.Mode < LoadAllSyntax
+	return cfg.Mode&NeedExportsFile != 0 || cfg.Mode&NeedTypes != 0 && cfg.Mode&NeedTypesInfo == 0
 }

go/packages/packages_test.go

@@ -554,17 +554,14 @@ func testLoadTypes(t *testing.T, exporter packagestest.Exporter) {
 		t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantGraph)
 	}
 
-	for _, test := range []struct {
-		id         string
-		wantSyntax bool
-	}{
-		{"golang.org/fake/a", true},  // need src, no export data for c
-		{"golang.org/fake/b", false}, // use export data
-		{"golang.org/fake/c", true},  // need src, no export data for c
+	for _, id := range []string{
+		"golang.org/fake/a",
+		"golang.org/fake/b",
+		"golang.org/fake/c",
 	} {
-		p := all[test.id]
+		p := all[id]
 		if p == nil {
-			t.Errorf("missing package: %s", test.id)
+			t.Errorf("missing package: %s", id)
 			continue
 		}
 		if p.Types == nil {
@@ -573,13 +570,7 @@ func testLoadTypes(t *testing.T, exporter packagestest.Exporter) {
 		} else if !p.Types.Complete() {
 			t.Errorf("incomplete types.Package for %s", p)
 		}
-		if (p.Syntax != nil) != test.wantSyntax {
-			if test.wantSyntax {
-				t.Errorf("missing ast.Files for %s", p)
-			} else {
-				t.Errorf("unexpected ast.Files for for %s", p)
-			}
-		}
+
 	}
 }
 
@@ -622,17 +613,18 @@ func testLoadSyntaxOK(t *testing.T, exporter packagestest.Exporter) {
 	}
 
 	for _, test := range []struct {
-		id           string
-		wantSyntax   bool
-		wantComplete bool
+		id string
 	}{
-		{"golang.org/fake/a", true, true},   // source package
-		{"golang.org/fake/b", true, true},   // source package
-		{"golang.org/fake/c", true, true},   // source package
-		{"golang.org/fake/d", false, true},  // export data package
-		{"golang.org/fake/e", false, false}, // export data package
-		{"golang.org/fake/f", false, false}, // export data package
+		{"golang.org/fake/a"}, // source package
+		{"golang.org/fake/b"}, // source package
+		{"golang.org/fake/c"}, // source package
+		{"golang.org/fake/d"}, // export data package
+		{"golang.org/fake/e"}, // export data package
+		{"golang.org/fake/f"}, // export data package
 	} {
+		// TODO(matloob): LoadSyntax and LoadAllSyntax are now equivalent, wantSyntax and wantComplete
+		// are true for all packages in the transitive dependency set. Add test cases on the individual
+		// Need* fields to check the equivalents on the new API.
 		p := all[test.id]
 		if p == nil {
 			t.Errorf("missing package: %s", test.id)
@@ -641,19 +633,9 @@ func testLoadSyntaxOK(t *testing.T, exporter packagestest.Exporter) {
 		if p.Types == nil {
 			t.Errorf("missing types.Package for %s", p)
 			continue
-		} else if p.Types.Complete() != test.wantComplete {
-			if test.wantComplete {
-				t.Errorf("incomplete types.Package for %s", p)
-			} else {
-				t.Errorf("unexpected complete types.Package for %s", p)
-			}
 		}
-		if (p.Syntax != nil) != test.wantSyntax {
-			if test.wantSyntax {
-				t.Errorf("missing ast.Files for %s", p)
-			} else {
-				t.Errorf("unexpected ast.Files for for %s", p)
-			}
+		if p.Syntax == nil {
+			t.Errorf("missing ast.Files for %s", p)
 		}
 		if p.Errors != nil {
 			t.Errorf("errors in package: %s: %s", p, p.Errors)
@@ -749,7 +731,7 @@ func testLoadSyntaxError(t *testing.T, exporter packagestest.Exporter) {
 		{"golang.org/fake/c", true, true},
 		{"golang.org/fake/d", true, true},
 		{"golang.org/fake/e", true, true},
-		{"golang.org/fake/f", false, false},
+		{"golang.org/fake/f", true, false},
 	} {
 		p := all[test.id]
 		if p == nil {