go/buildutil: FakeContext: create build.Context of fake file tree, for testing

This refactoring of the tests of go/loader and refactor/rename made it
possible to write some loader tests I wanted, but the new tests reveal
bugs so they're commented out for now.  I will fix them in a follow-up.

Change-Id: Iae3a20681a0a0791076debd4b82bb5ed74b0c577
Reviewed-on: https://go-review.googlesource.com/2825
Reviewed-by: Robert Griesemer <gri@golang.org>

go/buildutil/fakecontext.go

@@ -0,0 +1,108 @@
+package buildutil
+
+import (
+	"fmt"
+	"go/build"
+	"io"
+	"io/ioutil"
+	"os"
+	"path"
+	"path/filepath"
+	"sort"
+	"strings"
+	"time"
+)
+
+// FakeContext returns a build.Context for the fake file tree specified
+// by pkgs, which maps package import paths to a mapping from file base
+// names to contents.
+//
+// The fake Context has a GOROOT of "/go" and no GOPATH, and overrides
+// the necessary file access methods to read from memory instead of the
+// real file system.
+//
+// Unlike a real file tree, the fake one has only two levels---packages
+// and files---so ReadDir("/go/src/") returns all packages under
+// /go/src/ including, for instance, "math" and "math/big".
+// ReadDir("/go/src/math/big") would return all the files in the
+// "math/big" package.
+//
+func FakeContext(pkgs map[string]map[string]string) *build.Context {
+	clean := func(filename string) string {
+		f := path.Clean(filepath.ToSlash(filename))
+		// Removing "/go/src" while respecting segment
+		// boundaries has this unfortunate corner case:
+		if f == "/go/src" {
+			return ""
+		}
+		return strings.TrimPrefix(f, "/go/src/")
+	}
+
+	ctxt := build.Default // copy
+	ctxt.GOROOT = "/go"
+	ctxt.GOPATH = ""
+	ctxt.IsDir = func(dir string) bool {
+		dir = clean(dir)
+		if dir == "" {
+			return true // needed by (*build.Context).SrcDirs
+		}
+		return pkgs[dir] != nil
+	}
+	ctxt.ReadDir = func(dir string) ([]os.FileInfo, error) {
+		dir = clean(dir)
+		var fis []os.FileInfo
+		if dir == "" {
+			// enumerate packages
+			for importPath := range pkgs {
+				fis = append(fis, fakeDirInfo(importPath))
+			}
+		} else {
+			// enumerate files of package
+			for basename := range pkgs[dir] {
+				fis = append(fis, fakeFileInfo(basename))
+			}
+		}
+		sort.Sort(byName(fis))
+		return fis, nil
+	}
+	ctxt.OpenFile = func(filename string) (io.ReadCloser, error) {
+		filename = clean(filename)
+		dir, base := filepath.Split(filename)
+		content, ok := pkgs[filepath.Clean(dir)][base]
+		if !ok {
+			return nil, fmt.Errorf("file not found: %s", filename)
+		}
+		return ioutil.NopCloser(strings.NewReader(content)), nil
+	}
+	ctxt.IsAbsPath = func(path string) bool {
+		path = filepath.ToSlash(path)
+		// Don't rely on the default (filepath.Path) since on
+		// Windows, it reports virtual paths as non-absolute.
+		return strings.HasPrefix(path, "/")
+	}
+	return &ctxt
+}
+
+type byName []os.FileInfo
+
+func (s byName) Len() int           { return len(s) }
+func (s byName) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
+func (s byName) Less(i, j int) bool { return s[i].Name() < s[j].Name() }
+
+type fakeFileInfo string
+
+func (fi fakeFileInfo) Name() string    { return string(fi) }
+func (fakeFileInfo) Sys() interface{}   { return nil }
+func (fakeFileInfo) ModTime() time.Time { return time.Time{} }
+func (fakeFileInfo) IsDir() bool        { return false }
+func (fakeFileInfo) Size() int64        { return 0 }
+func (fakeFileInfo) Mode() os.FileMode  { return 0644 }
+
+type fakeDirInfo string
+
+func (fd fakeDirInfo) Name() string    { return string(fd) }
+func (fakeDirInfo) Sys() interface{}   { return nil }
+func (fakeDirInfo) ModTime() time.Time { return time.Time{} }
+func (fakeDirInfo) IsDir() bool        { return true }
+func (fakeDirInfo) Size() int64        { return 0 }
+func (fakeDirInfo) Mode() os.FileMode  { return 0755 }

go/loader/loader_test.go

@@ -5,17 +5,14 @@
 package loader_test
 
 import (
-	"bytes"
 	"fmt"
 	"go/build"
-	"io"
-	"io/ioutil"
-	"os"
 	"sort"
 	"strings"
+	"sync"
 	"testing"
-	"time"
 
+	"golang.org/x/tools/go/buildutil"
 	"golang.org/x/tools/go/loader"
 )
 
@@ -116,29 +113,13 @@ func TestLoadFromArgsSource(t *testing.T) {
 	}
 }
 
-type fakeFileInfo struct{}
+// Simplifying wrapper around buildutil.FakeContext for single-file packages.
-
-func (fakeFileInfo) Name() string       { return "x.go" }
-func (fakeFileInfo) Sys() interface{}   { return nil }
-func (fakeFileInfo) ModTime() time.Time { return time.Time{} }
-func (fakeFileInfo) IsDir() bool        { return false }
-func (fakeFileInfo) Size() int64        { return 0 }
-func (fakeFileInfo) Mode() os.FileMode  { return 0644 }
-
-var justXgo = [1]os.FileInfo{fakeFileInfo{}} // ["x.go"]
-
 func fakeContext(pkgs map[string]string) *build.Context {
-	ctxt := build.Default // copy
+	pkgs2 := make(map[string]map[string]string)
-	ctxt.GOROOT = "/go"
+	for path, content := range pkgs {
-	ctxt.GOPATH = ""
+		pkgs2[path] = map[string]string{"x.go": content}
-	ctxt.IsDir = func(path string) bool { return true }
-	ctxt.ReadDir = func(dir string) ([]os.FileInfo, error) { return justXgo[:], nil }
-	ctxt.OpenFile = func(path string) (io.ReadCloser, error) {
-		path = path[len("/go/src/"):]
-		importPath := path[:strings.IndexByte(path, '/')]
-		return ioutil.NopCloser(bytes.NewBufferString(pkgs[importPath])), nil
 	}
-	return &ctxt
+	return buildutil.FakeContext(pkgs2)
 }
 
 func TestTransitivelyErrorFreeFlag(t *testing.T) {
@@ -222,9 +203,12 @@ func TestErrorReporting(t *testing.T) {
 		SourceImports: true,
 		Build:         fakeContext(pkgs),
 	}
+	var mu sync.Mutex
 	var allErrors []error
 	conf.TypeChecker.Error = func(err error) {
+		mu.Lock()
 		allErrors = append(allErrors, err)
+		mu.Unlock()
 	}
 	conf.Import("a")
 
@@ -261,10 +245,12 @@ func TestErrorReporting(t *testing.T) {
 
 func TestCycles(t *testing.T) {
 	for _, test := range []struct {
+		descr   string
 		ctxt    *build.Context
 		wantErr string
 	}{
 		{
+			"self-cycle",
 			fakeContext(map[string]string{
 				"main":      `package main; import _ "selfcycle"`,
 				"selfcycle": `package selfcycle; import _ "selfcycle"`,
@@ -272,6 +258,7 @@ func TestCycles(t *testing.T) {
 			`import cycle: selfcycle -> selfcycle`,
 		},
 		{
+			"three-package cycle",
 			fakeContext(map[string]string{
 				"main": `package main; import _ "a"`,
 				"a":    `package a; import _ "b"`,
@@ -280,35 +267,73 @@ func TestCycles(t *testing.T) {
 			}),
 			`import cycle: c -> a -> b -> c`,
 		},
+		{
+			"self-cycle in dependency of test file",
+			buildutil.FakeContext(map[string]map[string]string{
+				"main": {
+					"main.go":      `package main`,
+					"main_test.go": `package main; import _ "a"`,
+				},
+				"a": {
+					"a.go": `package a; import _ "a"`,
+				},
+			}),
+			`import cycle: a -> a`,
+		},
+		// TODO(adonovan): fix: these fail
+		// {
+		// 	"two-package cycle in dependency of test file",
+		// 	buildutil.FakeContext(map[string]map[string]string{
+		// 		"main": {
+		// 			"main.go":      `package main`,
+		// 			"main_test.go": `package main; import _ "a"`,
+		// 		},
+		// 		"a": {
+		// 			"a.go": `package a; import _ "main"`,
+		// 		},
+		// 	}),
+		// 	`import cycle: main -> a -> main`,
+		// },
+		// {
+		// 	"self-cycle in augmented package",
+		// 	buildutil.FakeContext(map[string]map[string]string{
+		// 		"main": {
+		// 			"main.go":      `package main`,
+		// 			"main_test.go": `package main; import _ "main"`,
+		// 		},
+		// 	}),
+		// 	`import cycle: main -> main`,
+		// },
 	} {
 		conf := loader.Config{
 			AllowErrors:   true,
 			SourceImports: true,
 			Build:         test.ctxt,
 		}
+		var mu sync.Mutex
 		var allErrors []error
 		conf.TypeChecker.Error = func(err error) {
+			mu.Lock()
 			allErrors = append(allErrors, err)
+			mu.Unlock()
 		}
-		conf.Import("main")
+		conf.ImportWithTests("main")
 
 		prog, err := conf.Load()
 		if err != nil {
-			t.Errorf("Load failed: %s", err)
+			t.Errorf("%s: Load failed: %s", test.descr, err)
 		}
 		if prog == nil {
-			t.Fatalf("Load returned nil *Program")
+			t.Fatalf("%s: Load returned nil *Program", test.descr)
 		}
 
 		if !hasError(allErrors, test.wantErr) {
-			t.Errorf("Load() errors = %q, want %q", allErrors, test.wantErr)
+			t.Errorf("%s: Load() errors = %q, want %q",
+				test.descr, allErrors, test.wantErr)
 		}
 	}
 
 	// TODO(adonovan):
 	// - Test that in a legal test cycle, none of the symbols
 	//   defined by augmentation are visible via import.
-	// - Test when augmentation discovers a wholly new cycle among the deps.
-	//
-	// These tests require that fakeContext let us control the filenames.
 }

refactor/rename/rename_test.go

@@ -11,15 +11,12 @@ import (
 	"go/build"
 	"go/format"
 	"go/token"
-	"io"
-	"io/ioutil"
-	"os"
 	"path/filepath"
 	"regexp"
-	"strconv"
 	"strings"
 	"testing"
-	"time"
+
+	"golang.org/x/tools/go/buildutil"
 )
 
 // TODO(adonovan): test reported source positions, somehow.
@@ -1022,84 +1019,22 @@ var _ = I(C(0)).(J)
 
 // ---------------------------------------------------------------------
 
-// Plundered/adapted from go/loader/loader_test.go
+// Simplifying wrapper around buildutil.FakeContext for packages whose
-
+// filenames are sequentially numbered (%d.go).  pkgs maps a package
-// TODO(adonovan): make this into a nice testing utility within go/buildutil.
+// import path to its list of file contents.
-
-// pkgs maps the import path of a fake package to a list of its file contents;
-// file names are synthesized, e.g. %d.go.
 func fakeContext(pkgs map[string][]string) *build.Context {
-	ctxt := build.Default // copy
+	pkgs2 := make(map[string]map[string]string)
-	ctxt.GOROOT = "/go"
+	for path, files := range pkgs {
-	ctxt.GOPATH = ""
+		filemap := make(map[string]string)
-	ctxt.IsDir = func(path string) bool {
+		for i, contents := range files {
-		path = filepath.ToSlash(path)
+			filemap[fmt.Sprintf("%d.go", i)] = contents
-		if path == "/go/src" {
-			return true // needed by (*build.Context).SrcDirs
 		}
-		if p := strings.TrimPrefix(path, "/go/src/"); p == path {
+		pkgs2[path] = filemap
-			return false
-		} else {
-			path = p
-		}
-		_, ok := pkgs[path]
-		return ok
 	}
-	ctxt.ReadDir = func(dir string) ([]os.FileInfo, error) {
+	return buildutil.FakeContext(pkgs2)
-		dir = filepath.ToSlash(dir)
-		dir = dir[len("/go/src/"):]
-		var fis []os.FileInfo
-		if dir == "" {
-			// Assumes keys of pkgs are single-segment.
-			for p := range pkgs {
-				fis = append(fis, fakeDirInfo(p))
-			}
-		} else {
-			for i := range pkgs[dir] {
-				fis = append(fis, fakeFileInfo(i))
-			}
-		}
-		return fis, nil
-	}
-	ctxt.OpenFile = func(path string) (io.ReadCloser, error) {
-		path = filepath.ToSlash(path)
-		path = path[len("/go/src/"):]
-		dir, base := filepath.Split(path)
-		dir = filepath.Clean(dir)
-		index, _ := strconv.Atoi(strings.TrimSuffix(base, ".go"))
-		if _, ok := pkgs[dir]; !ok || index >= len(pkgs[dir]) {
-			return nil, fmt.Errorf("file does not exist")
-		}
-		return ioutil.NopCloser(bytes.NewBufferString(pkgs[dir][index])), nil
-	}
-	ctxt.IsAbsPath = func(path string) bool {
-		path = filepath.ToSlash(path)
-		// Don't rely on the default (filepath.Path) since on
-		// Windows, it reports our virtual paths as non-absolute.
-		return strings.HasPrefix(path, "/")
-	}
-	return &ctxt
 }
 
 // helper for single-file main packages with no imports.
 func main(content string) *build.Context {
 	return fakeContext(map[string][]string{"main": {content}})
 }
-
-type fakeFileInfo int
-
-func (fi fakeFileInfo) Name() string    { return fmt.Sprintf("%d.go", fi) }
-func (fakeFileInfo) Sys() interface{}   { return nil }
-func (fakeFileInfo) ModTime() time.Time { return time.Time{} }
-func (fakeFileInfo) IsDir() bool        { return false }
-func (fakeFileInfo) Size() int64        { return 0 }
-func (fakeFileInfo) Mode() os.FileMode  { return 0644 }
-
-type fakeDirInfo string
-
-func (fd fakeDirInfo) Name() string    { return string(fd) }
-func (fakeDirInfo) Sys() interface{}   { return nil }
-func (fakeDirInfo) ModTime() time.Time { return time.Time{} }
-func (fakeDirInfo) IsDir() bool        { return true }
-func (fakeDirInfo) Size() int64        { return 0 }
-func (fakeDirInfo) Mode() os.FileMode  { return 0755 }