internal/typeparams: guard against generics in stdlib tests

Add a new package internal/typeparams/genericfeatures, factoring out the
check for usage of generics from the usesgenerics checker. For tests
operating on the standard library, this allows us to explicitly detect
usage of generics, rather than relying on a hard-coded list.

To make this work, I switched the go/ssa standard library test to use
go/packages rather than go/loader. This resulted in finding 457 packages
versus 676 (likely due to tests), but my assumption is that this is
still enough coverage.

Change-Id: I6ba32fb4068dd7f5eedb55c2081c642665ed124a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/355972
Trust: Robert Findley <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Tim King <[email protected]>

Author: findleyr

go/analysis/passes/usesgenerics/usesgenerics.go

@@ -7,15 +7,12 @@
 package usesgenerics
 
 import (
-	"go/ast"
-	"go/types"
 	"reflect"
-	"strings"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
-	"golang.org/x/tools/internal/typeparams"
+	"golang.org/x/tools/internal/typeparams/genericfeatures"
 )
 
 var Analyzer = &analysis.Analyzer{
@@ -32,6 +29,16 @@ const Doc = `detect whether a package uses generics features
 The usesgenerics analysis reports whether a package directly or transitively
 uses certain features associated with generic programming in Go.`
 
+type Features = genericfeatures.Features
+
+const (
+	GenericTypeDecls  = genericfeatures.GenericTypeDecls
+	GenericFuncDecls  = genericfeatures.GenericFuncDecls
+	EmbeddedTypeSets  = genericfeatures.EmbeddedTypeSets
+	TypeInstantiation = genericfeatures.TypeInstantiation
+	FuncInstantiation = genericfeatures.FuncInstantiation
+)
+
 // Result is the usesgenerics analyzer result type. The Direct field records
 // features used directly by the package being analyzed (i.e. contained in the
 // package source code). The Transitive field records any features used by the
@@ -40,53 +47,6 @@ type Result struct {
 	Direct, Transitive Features
 }
 
-// Features is a set of flags reporting which features of generic Go code a
-// package uses, or 0.
-type Features int
-
-const (
-	// GenericTypeDecls indicates whether the package declares types with type
-	// parameters.
-	GenericTypeDecls Features = 1 << iota
-
-	// GenericFuncDecls indicates whether the package declares functions with
-	// type parameters.
-	GenericFuncDecls
-
-	// EmbeddedTypeSets indicates whether the package declares interfaces that
-	// contain structural type restrictions, i.e. are not fully described by
-	// their method sets.
-	EmbeddedTypeSets
-
-	// TypeInstantiation indicates whether the package instantiates any generic
-	// types.
-	TypeInstantiation
-
-	// FuncInstantiation indicates whether the package instantiates any generic
-	// functions.
-	FuncInstantiation
-)
-
-func (f Features) String() string {
-	var feats []string
-	if f&GenericTypeDecls != 0 {
-		feats = append(feats, "typeDecl")
-	}
-	if f&GenericFuncDecls != 0 {
-		feats = append(feats, "funcDecl")
-	}
-	if f&EmbeddedTypeSets != 0 {
-		feats = append(feats, "typeSet")
-	}
-	if f&TypeInstantiation != 0 {
-		feats = append(feats, "typeInstance")
-	}
-	if f&FuncInstantiation != 0 {
-		feats = append(feats, "funcInstance")
-	}
-	return "features{" + strings.Join(feats, ",") + "}"
-}
-
 type featuresFact struct {
 	Features Features
 }
@@ -95,7 +55,9 @@ func (f *featuresFact) AFact()         {}
 func (f *featuresFact) String() string { return f.Features.String() }
 
 func run(pass *analysis.Pass) (interface{}, error) {
-	direct := directFeatures(pass)
+	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+	direct := genericfeatures.ForPackage(inspect, pass.TypesInfo)
 
 	transitive := direct | importedTransitiveFeatures(pass)
 	if transitive != 0 {
@@ -108,50 +70,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	}, nil
 }
 
-// directFeatures computes which generic features are used directly by the
-// package being analyzed.
-func directFeatures(pass *analysis.Pass) Features {
-	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
-
-	nodeFilter := []ast.Node{
-		(*ast.FuncType)(nil),
-		(*ast.InterfaceType)(nil),
-		(*ast.ImportSpec)(nil),
-		(*ast.TypeSpec)(nil),
-	}
-
-	var direct Features
-
-	inspect.Preorder(nodeFilter, func(node ast.Node) {
-		switch n := node.(type) {
-		case *ast.FuncType:
-			if tparams := typeparams.ForFuncType(n); tparams != nil {
-				direct |= GenericFuncDecls
-			}
-		case *ast.InterfaceType:
-			tv := pass.TypesInfo.Types[n]
-			if iface, _ := tv.Type.(*types.Interface); iface != nil && !typeparams.IsMethodSet(iface) {
-				direct |= EmbeddedTypeSets
-			}
-		case *ast.TypeSpec:
-			if tparams := typeparams.ForTypeSpec(n); tparams != nil {
-				direct |= GenericTypeDecls
-			}
-		}
-	})
-
-	instances := typeparams.GetInstances(pass.TypesInfo)
-	for _, inst := range instances {
-		switch inst.Type.(type) {
-		case *types.Named:
-			direct |= TypeInstantiation
-		case *types.Signature:
-			direct |= FuncInstantiation
-		}
-	}
-	return direct
-}
-
 // importedTransitiveFeatures computes features that are used transitively via
 // imports.
 func importedTransitiveFeatures(pass *analysis.Pass) Features {

go/internal/gcimporter/bexport_test.go

@@ -19,10 +19,12 @@ import (
 	"strings"
 	"testing"
 
+	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/buildutil"
 	"golang.org/x/tools/go/internal/gcimporter"
 	"golang.org/x/tools/go/loader"
 	"golang.org/x/tools/internal/typeparams"
+	"golang.org/x/tools/internal/typeparams/genericfeatures"
 )
 
 var isRace = false
@@ -66,14 +68,22 @@ type UnknownType undefined
 	}
 
 	numPkgs := len(prog.AllPackages)
-	if want := 248; numPkgs < want {
+	if want := minStdlibPackages; numPkgs < want {
 		t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want)
 	}
 
+	checked := 0
 	for pkg, info := range prog.AllPackages {
 		if info.Files == nil {
 			continue // empty directory
 		}
+		// Binary export does not support generic code.
+		inspect := inspector.New(info.Files)
+		if genericfeatures.ForPackage(inspect, &info.Info) != 0 {
+			t.Logf("skipping package %q which uses generics", pkg.Path())
+			continue
+		}
+		checked++
 		exportdata, err := gcimporter.BExportData(conf.Fset, pkg)
 		if err != nil {
 			t.Fatal(err)
@@ -116,6 +126,9 @@ type UnknownType undefined
 			}
 		}
 	}
+	if want := minStdlibPackages; checked < want {
+		t.Errorf("Checked only %d packages, want at least %d", checked, want)
+	}
 }
 
 func fileLine(fset *token.FileSet, obj types.Object) string {

go/internal/gcimporter/iexport_test.go

@@ -28,10 +28,11 @@ import (
 	"strings"
 	"testing"
 
+	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/buildutil"
 	"golang.org/x/tools/go/internal/gcimporter"
 	"golang.org/x/tools/go/loader"
-	"golang.org/x/tools/internal/testenv"
+	"golang.org/x/tools/internal/typeparams/genericfeatures"
 )
 
 func readExportFile(filename string) ([]byte, error) {
@@ -69,6 +70,8 @@ func isUnifiedBuilder() bool {
 	return os.Getenv("GO_BUILDER_NAME") == "linux-amd64-unified"
 }
 
+const minStdlibPackages = 248
+
 func TestIExportData_stdlib(t *testing.T) {
 	if runtime.Compiler == "gccgo" {
 		t.Skip("gccgo standard library is inaccessible")
@@ -90,15 +93,8 @@ func TestIExportData_stdlib(t *testing.T) {
 			Sizes: types.SizesFor(ctxt.Compiler, ctxt.GOARCH),
 		},
 	}
-	// Temporarily skip packages that use generics on the unified builder, to fix
-	// TryBots.
-	//
-	// TODO(#48595): fix this test with GOEXPERIMENT=unified.
-	isUnified := isUnifiedBuilder()
 	for _, path := range buildutil.AllPackages(conf.Build) {
-		if !(isUnified && testenv.UsesGenerics(path)) {
-			conf.Import(path)
-		}
+		conf.Import(path)
 	}
 
 	// Create a package containing type and value errors to ensure
@@ -117,13 +113,19 @@ type UnknownType undefined
 		t.Fatalf("Load failed: %v", err)
 	}
 
-	numPkgs := len(prog.AllPackages)
-	if want := 248; numPkgs < want {
-		t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want)
-	}
-
 	var sorted []*types.Package
+	isUnified := isUnifiedBuilder()
 	for pkg, info := range prog.AllPackages {
+		// Temporarily skip packages that use generics on the unified builder, to
+		// fix TryBots.
+		//
+		// TODO(#48595): fix this test with GOEXPERIMENT=unified.
+		inspect := inspector.New(info.Files)
+		features := genericfeatures.ForPackage(inspect, &info.Info)
+		if isUnified && features != 0 {
+			t.Logf("skipping package %q which uses generics", pkg.Path())
+			continue
+		}
 		if info.Files != nil { // non-empty directory
 			sorted = append(sorted, pkg)
 		}
@@ -133,6 +135,11 @@ type UnknownType undefined
 	})
 
 	version := gcimporter.IExportVersion
+	numPkgs := len(sorted)
+	if want := minStdlibPackages; numPkgs < want {
+		t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want)
+	}
+
 	for _, pkg := range sorted {
 		if exportdata, err := iexport(conf.Fset, version, pkg); err != nil {
 			t.Error(err)

go/ssa/stdlib_test.go

@@ -16,17 +16,17 @@ package ssa_test
 
 import (
 	"go/ast"
-	"go/build"
 	"go/token"
 	"runtime"
 	"testing"
 	"time"
 
-	"golang.org/x/tools/go/buildutil"
-	"golang.org/x/tools/go/loader"
+	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/go/ssa"
 	"golang.org/x/tools/go/ssa/ssautil"
 	"golang.org/x/tools/internal/testenv"
+	"golang.org/x/tools/internal/typeparams/genericfeatures"
 )
 
 func bytesAllocated() uint64 {
@@ -46,23 +46,27 @@ func TestStdlib(t *testing.T) {
 	t0 := time.Now()
 	alloc0 := bytesAllocated()
 
-	// Load, parse and type-check the program.
-	ctxt := build.Default // copy
-	ctxt.GOPATH = ""      // disable GOPATH
-	conf := loader.Config{Build: &ctxt}
-	for _, path := range buildutil.AllPackages(conf.Build) {
-		// Temporarily skip packages that use generics until supported by go/ssa.
-		//
-		// TODO(#48595): revert this once go/ssa supports generics.
-		if !testenv.UsesGenerics(path) {
-			conf.ImportWithTests(path)
-		}
-	}
-
-	iprog, err := conf.Load()
+	cfg := &packages.Config{Mode: packages.LoadSyntax}
+	pkgs, err := packages.Load(cfg, "std", "cmd")
 	if err != nil {
-		t.Fatalf("Load failed: %v", err)
+		t.Fatal(err)
+	}
+	var nonGeneric int
+	for i := 0; i < len(pkgs); i++ {
+		pkg := pkgs[i]
+		inspect := inspector.New(pkg.Syntax)
+		features := genericfeatures.ForPackage(inspect, pkg.TypesInfo)
+		// Skip standard library packages that use generics. This won't be
+		// sufficient if any standard library packages start _importing_ packages
+		// that use generics.
+		if features != 0 {
+			t.Logf("skipping package %q which uses generics", pkg.PkgPath)
+			continue
+		}
+		pkgs[nonGeneric] = pkg
+		nonGeneric++
 	}
+	pkgs = pkgs[:nonGeneric]
 
 	t1 := time.Now()
 	alloc1 := bytesAllocated()
@@ -72,7 +76,7 @@ func TestStdlib(t *testing.T) {
 	// Comment out these lines during benchmarking.  Approx SSA build costs are noted.
 	mode |= ssa.SanityCheckFunctions // + 2% space, + 4% time
 	mode |= ssa.GlobalDebug          // +30% space, +18% time
-	prog := ssautil.CreateProgram(iprog, mode)
+	prog, _ := ssautil.Packages(pkgs, mode)
 
 	t2 := time.Now()
 
@@ -87,8 +91,8 @@ func TestStdlib(t *testing.T) {
 		t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want)
 	}
 
-	// Keep iprog reachable until after we've measured memory usage.
-	if len(iprog.AllPackages) == 0 {
+	// Keep pkgs reachable until after we've measured memory usage.
+	if len(pkgs) == 0 {
 		panic("unreachable")
 	}
 

internal/testenv/testenv.go

@@ -297,9 +297,3 @@ func SkipAfterGo1Point(t Testing, x int) {
 		t.Skipf("running Go version %q is version 1.%d, newer than maximum 1.%d", runtime.Version(), Go1Point(), x)
 	}
 }
-
-// UsesGenerics reports if the standard library package stdlibPkg uses
-// generics.
-func UsesGenerics(stdlibPkg string) bool {
-	return stdlibPkg == "constraints"
-}