From e3dc58c6e083234b1749a446dce4d9a56075d46a Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 14 Mar 2014 16:17:53 -0400 Subject: [PATCH] go.tools/go/loader: use the build.Context's I/O hooks, if any. Also, add loader.Config.DisplayPath hook, which allows the filename returned by build.Context.Import() to be transformed prior to attaching to the AST. This allows a virtual file system to be used without leaking into the user interface. Eliminate parsePackageFiles hook; I don't think we need it any more. The test that was using it has been rewritten to use the build.Context hooks. LGTM=gri R=gri, crawshaw CC=daniel.morsing, golang-codereviews, rsc https://golang.org/cl/75520046 --- go/loader/backdoor_test.go | 17 -------- go/loader/importer_test.go | 73 +++++++++++++++++++-------------- go/loader/loader.go | 51 +++++++++++++++++++++-- go/loader/util.go | 84 ++++++++++++++++---------------------- 4 files changed, 124 insertions(+), 101 deletions(-) delete mode 100644 go/loader/backdoor_test.go diff --git a/go/loader/backdoor_test.go b/go/loader/backdoor_test.go deleted file mode 100644 index 918546a0..00000000 --- a/go/loader/backdoor_test.go +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright 2014 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. - -package loader - -import ( - "go/ast" - "go/build" - "go/token" -) - -// PackageLocatorFunc exposes the address of parsePackageFiles to tests. -// This is a temporary hack until we expose a proper PackageLocator interface. -func PackageLocatorFunc() *func(ctxt *build.Context, fset *token.FileSet, path string, which rune) ([]*ast.File, error) { - return &parsePackageFiles -} diff --git a/go/loader/importer_test.go b/go/loader/importer_test.go index be35825d..f984f93b 100644 --- a/go/loader/importer_test.go +++ b/go/loader/importer_test.go @@ -5,12 +5,14 @@ package loader_test import ( + "bytes" "fmt" - "go/ast" "go/build" - "go/token" + "io" + "os" "sort" "testing" + "time" "code.google.com/p/go.tools/go/loader" ) @@ -107,52 +109,61 @@ func TestLoadFromArgsSource(t *testing.T) { } } +type nopCloser struct{ *bytes.Buffer } + +func (nopCloser) Close() error { return nil } + +type fakeFileInfo struct{} + +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 TestTransitivelyErrorFreeFlag(t *testing.T) { conf := loader.Config{ AllowTypeErrors: true, SourceImports: true, } - conf.Import("a") - // Fake the following packages: + // Create an minimal custom build.Context + // that fakes the following packages: // // a --> b --> c! c has a TypeError // \ d and e are transitively error free. // e --> d - - // Temporary hack until we expose a principled PackageLocator. - pfn := loader.PackageLocatorFunc() - saved := *pfn - *pfn = func(_ *build.Context, fset *token.FileSet, path string, which rune) (files []*ast.File, err error) { - if which != 'g' { - return nil, nil // no test/xtest files - } - var contents string - switch path { - case "a": - contents = `package a; import (_ "b"; _ "e")` - case "b": - contents = `package b; import _ "c"` - case "c": - contents = `package c; func f() { _ = int(false) }` // type error within function body - case "d": - contents = `package d;` - case "e": - contents = `package e; import _ "d"` - default: - return nil, fmt.Errorf("no such package %q", path) - } - f, err := conf.ParseFile(fmt.Sprintf("%s/x.go", path), contents, 0) - return []*ast.File{f}, err + // + // Each package [a-e] consists of one file, x.go. + pkgs := map[string]string{ + "a": `package a; import (_ "b"; _ "e")`, + "b": `package b; import _ "c"`, + "c": `package c; func f() { _ = int(false) }`, // type error within function body + "d": `package d;`, + "e": `package e; import _ "d"`, } - defer func() { *pfn = saved }() + ctxt := build.Default // copy + ctxt.GOROOT = "/go" + ctxt.GOPATH = "" + 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/pkg/"):] + return nopCloser{bytes.NewBufferString(pkgs[path[0:1]])}, nil + } + conf.Build = &ctxt + + conf.Import("a") prog, err := conf.Load() if err != nil { t.Errorf("Load failed: %s", err) } if prog == nil { - t.Fatalf("Load returnd nil *Program") + t.Fatalf("Load returned nil *Program") } for pkg, info := range prog.AllPackages { diff --git a/go/loader/loader.go b/go/loader/loader.go index c3db31f2..970fd6df 100644 --- a/go/loader/loader.go +++ b/go/loader/loader.go @@ -184,6 +184,12 @@ type Config struct { // Otherwise &build.Default is used. Build *build.Context + // If DisplayPath is non-nil, it is used to transform each + // file name obtained from Build.Import(). This can be used + // to prevent a virtualized build.Config's file names from + // leaking into the user interface. + DisplayPath func(path string) string + // If AllowTypeErrors is true, Load will return a Program even // if some of the its packages contained type errors; such // errors are accessible via PackageInfo.TypeError. @@ -334,7 +340,7 @@ func (conf *Config) FromArgs(args []string, xtest bool) (rest []string, err erro // conf.CreatePkgs. // func (conf *Config) CreateFromFilenames(path string, filenames ...string) error { - files, err := parseFiles(conf.fset(), ".", filenames...) + files, err := parseFiles(conf.fset(), conf.build(), nil, ".", filenames...) if err != nil { return err } @@ -373,7 +379,7 @@ func (conf *Config) ImportWithTests(path string) error { conf.Import(path) // Load the external test package. - xtestFiles, err := parsePackageFiles(conf.build(), conf.fset(), path, 'x') + xtestFiles, err := conf.parsePackageFiles(path, 'x') if err != nil { return err } @@ -496,7 +502,7 @@ func (conf *Config) Load() (*Program, error) { ii := imp.imported[path] // Find and create the actual package. - files, err := parsePackageFiles(imp.conf.build(), imp.conf.fset(), path, 't') + files, err := imp.conf.parsePackageFiles(path, 't') // Prefer the earlier error, if any. if err != nil && ii.err == nil { ii.err = err // e.g. parse error. @@ -598,6 +604,43 @@ func (conf *Config) build() *build.Context { return &build.Default } +// parsePackageFiles enumerates the files belonging to package path, +// then loads, parses and returns them. +// +// 'which' indicates which files to include: +// 'g': include non-test *.go source files (GoFiles) +// 't': include in-package *_test.go source files (TestGoFiles) +// 'x': include external *_test.go source files. (XTestGoFiles) +// +func (conf *Config) parsePackageFiles(path string, which rune) ([]*ast.File, error) { + // Set the "!cgo" go/build tag, preferring (dummy) Go to + // native C implementations of net.cgoLookupHost et al. + ctxt := *conf.build() // copy + ctxt.CgoEnabled = false + + // Import(srcDir="") disables local imports, e.g. import "./foo". + bp, err := ctxt.Import(path, "", 0) + if _, ok := err.(*build.NoGoError); ok { + return nil, nil // empty directory + } + if err != nil { + return nil, err // import failed + } + + var filenames []string + switch which { + case 'g': + filenames = bp.GoFiles + case 't': + filenames = bp.TestGoFiles + case 'x': + filenames = bp.XTestGoFiles + default: + panic(which) + } + return parseFiles(conf.fset(), &ctxt, conf.DisplayPath, bp.Dir, filenames...) +} + // doImport imports the package denoted by path. // It implements the types.Importer signature. // @@ -677,7 +720,7 @@ func (imp *importer) importFromBinary(path string) (*PackageInfo, error) { // located by go/build. // func (imp *importer) importFromSource(path string) (*PackageInfo, error) { - files, err := parsePackageFiles(imp.conf.build(), imp.conf.fset(), path, 'g') + files, err := imp.conf.parsePackageFiles(path, 'g') if err != nil { return nil, err } diff --git a/go/loader/util.go b/go/loader/util.go index 14bde951..a772f02b 100644 --- a/go/loader/util.go +++ b/go/loader/util.go @@ -9,68 +9,54 @@ import ( "go/build" "go/parser" "go/token" + "io" + "os" "path/filepath" "sync" ) -// parsePackageFiles enumerates the files belonging to package path, -// then loads, parses and returns them. -// -// 'which' is a list of flags indicating which files to include: -// 'g': include non-test *.go source files (GoFiles) -// 't': include in-package *_test.go source files (TestGoFiles) -// 'x': include external *_test.go source files. (XTestGoFiles) -// -// This function is stored in a var as a concession to testing. -// TODO(adonovan): we plan to replace this hook with an exposed -// "PackageLocator" interface for use proprietary build sytems that -// are incompatible with "go test", and also for testing. -// -var parsePackageFiles = func(ctxt *build.Context, fset *token.FileSet, path string, which rune) ([]*ast.File, error) { - // Set the "!cgo" go/build tag, preferring (dummy) Go to - // native C implementations of net.cgoLookupHost et al. - ctxt2 := *ctxt - ctxt2.CgoEnabled = false - - // Import(srcDir="") disables local imports, e.g. import "./foo". - bp, err := ctxt2.Import(path, "", 0) - if _, ok := err.(*build.NoGoError); ok { - return nil, nil // empty directory - } - if err != nil { - return nil, err // import failed - } - - var filenames []string - switch which { - case 'g': - filenames = bp.GoFiles - case 't': - filenames = bp.TestGoFiles - case 'x': - filenames = bp.XTestGoFiles - default: - panic(which) - } - return parseFiles(fset, bp.Dir, filenames...) -} - // parseFiles parses the Go source files files within directory dir // and returns their ASTs, or the first parse error if any. // -func parseFiles(fset *token.FileSet, dir string, files ...string) ([]*ast.File, error) { +// I/O is done via ctxt, which may specify a virtual file system. +// displayPath is used to transform the filenames attached to the ASTs. +// +func parseFiles(fset *token.FileSet, ctxt *build.Context, displayPath func(string) string, dir string, files ...string) ([]*ast.File, error) { + if displayPath == nil { + displayPath = func(path string) string { return path } + } + isAbs := filepath.IsAbs + if ctxt.IsAbsPath != nil { + isAbs = ctxt.IsAbsPath + } + joinPath := filepath.Join + if ctxt.JoinPath != nil { + joinPath = ctxt.JoinPath + } var wg sync.WaitGroup n := len(files) - parsed := make([]*ast.File, n, n) - errors := make([]error, n, n) + parsed := make([]*ast.File, n) + errors := make([]error, n) for i, file := range files { - if !filepath.IsAbs(file) { - file = filepath.Join(dir, file) + if !isAbs(file) { + file = joinPath(dir, file) } wg.Add(1) go func(i int, file string) { - parsed[i], errors[i] = parser.ParseFile(fset, file, nil, 0) - wg.Done() + defer wg.Done() + var rd io.ReadCloser + var err error + if ctxt.OpenFile != nil { + rd, err = ctxt.OpenFile(file) + } else { + rd, err = os.Open(file) + } + defer rd.Close() + if err != nil { + errors[i] = err + return + } + parsed[i], errors[i] = parser.ParseFile(fset, displayPath(file), rd, 0) }(i, file) } wg.Wait()