go/loader: improve robustness in face of input errors
Before this change, many kinds of error would cause the loader to stop.
making it brittle when analyzing large codebases, as in "godoc -analysis".
This change moves operations that used to occur during
configuration---(*build.Context).Import, loading, and parsing of
initial packages---into the Load call, and ensures that all failures
during Loading are reported at the end so that the maximum amount of
progress is made.
Also: redesign the tests and add many new cases.
Change-Id: Ia8cd99416af7c5d4a5fe133908adfa83676d401f
Reviewed-on: https://go-review.googlesource.com/3626
Reviewed-by: Robert Griesemer <gri@golang.org>
diff --git a/cmd/eg/eg.go b/cmd/eg/eg.go
index 0d9cf91..cc3ea64 100644
--- a/cmd/eg/eg.go
+++ b/cmd/eg/eg.go
@@ -61,9 +61,7 @@
}
// The first Created package is the template.
- if err := conf.CreateFromFilenames("template", *templateFlag); err != nil {
- return err // e.g. "foo.go:1: syntax error"
- }
+ conf.CreateFromFilenames("template", *templateFlag)
if len(args) == 0 {
fmt.Fprint(os.Stderr, usage)
diff --git a/cmd/ssadump/main.go b/cmd/ssadump/main.go
index 2050780..1901b30 100644
--- a/cmd/ssadump/main.go
+++ b/cmd/ssadump/main.go
@@ -203,7 +203,7 @@
}
if runtime.GOARCH != build.Default.GOARCH {
- return fmt.Errorf("cross-interpretation is not yet supported (target has GOARCH %s, interpreter has %s)",
+ return fmt.Errorf("cross-interpretation is not supported (target has GOARCH %s, interpreter has %s)",
build.Default.GOARCH, runtime.GOARCH)
}
diff --git a/go/loader/loader.go b/go/loader/loader.go
index 26a6ae2..0fdda05 100644
--- a/go/loader/loader.go
+++ b/go/loader/loader.go
@@ -27,19 +27,19 @@
//
// // Parse the specified files and create an ad-hoc package with path "foo".
// // All files must have the same 'package' declaration.
-// err := conf.CreateFromFilenames("foo", "foo.go", "bar.go")
+// conf.CreateFromFilenames("foo", "foo.go", "bar.go")
//
// // Create an ad-hoc package with path "foo" from
// // the specified already-parsed files.
// // All ASTs must have the same 'package' declaration.
-// err := conf.CreateFromFiles("foo", parsedFiles)
+// conf.CreateFromFiles("foo", parsedFiles)
//
// // Add "runtime" to the set of packages to be loaded.
// conf.Import("runtime")
//
// // Adds "fmt" and "fmt_test" to the set of packages
// // to be loaded. "fmt" will include *_test.go files.
-// err := conf.ImportWithTests("fmt")
+// conf.ImportWithTests("fmt")
//
// // Finally, load all the packages specified by the configuration.
// prog, err := conf.Load()
@@ -182,15 +182,9 @@
// The result of using concurrency is about a 2.5x speedup for stdlib_test.
// TODO(adonovan):
-// - (*Config).ParseFile is very handy, but feels like feature creep.
-// (*Config).CreateFromFiles has a nasty precondition.
// - cache the calls to build.Import so we don't do it three times per
// test package.
// - Thorough overhaul of package documentation.
-// - Certain errors (e.g. parse error in x_test.go files, or failure to
-// import an initial package) still cause Load() to fail hard.
-// Fix that. (It's tricky because of the way x_test files are parsed
-// eagerly.)
import (
"errors"
@@ -200,6 +194,7 @@
"go/parser"
"go/token"
"os"
+ "sort"
"strings"
"sync"
"time"
@@ -215,7 +210,7 @@
// The zero value for Config is a ready-to-use default configuration.
type Config struct {
// Fset is the file set for the parser to use when loading the
- // program. If nil, it will be lazily initialized by any
+ // program. If nil, it may be lazily initialized by any
// method of Config.
Fset *token.FileSet
@@ -280,21 +275,19 @@
AllowErrors bool
// CreatePkgs specifies a list of non-importable initial
- // packages to create. Each element specifies a list of
- // parsed files to be type-checked into a new package, and a
- // path for that package. If the path is "", the package's
- // name will be used instead. The path needn't be globally
- // unique.
- //
- // The resulting packages will appear in the corresponding
- // elements of the Program.Created slice.
- CreatePkgs []CreatePkg
+ // packages to create. The resulting packages will appear in
+ // the corresponding elements of the Program.Created slice.
+ CreatePkgs []PkgSpec
// ImportPkgs specifies a set of initial packages to load from
// source. The map keys are package import paths, used to
- // locate the package relative to $GOROOT. The corresponding
- // values indicate whether to augment the package by *_test.go
- // files in a second pass.
+ // locate the package relative to $GOROOT.
+ //
+ // The map value indicates whether to load tests. If true, Load
+ // will add and type-check two lists of files to the package:
+ // non-test files followed by in-package *_test.go files. In
+ // addition, it will append the external test package (if any)
+ // to Program.Created.
ImportPkgs map[string]bool
// PackageCreated is a hook called when a types.Package
@@ -311,9 +304,14 @@
PackageCreated func(*types.Package)
}
-type CreatePkg struct {
- Path string // the import path of the resulting (non-importable) types.Package
- Files []*ast.File
+// A PkgSpec specifies a non-importable package to be created by Load.
+// Files are processed first, but typically only one of Files and
+// Filenames is provided. The path needn't be globally unique.
+//
+type PkgSpec struct {
+ Path string // import path ("" => use package declaration)
+ Files []*ast.File // ASTs of already-parsed files
+ Filenames []string // names of files to be parsed
}
// A Program is a Go program loaded from source or binary
@@ -321,8 +319,10 @@
type Program struct {
Fset *token.FileSet // the file set for this program
- // Created[i] contains the initial package whose ASTs were
- // supplied by Config.CreatePkgs[i].
+ // Created[i] contains the initial package whose ASTs or
+ // filenames were supplied by Config.CreatePkgs[i], followed by
+ // the external test package, if any, of each package in
+ // Config.ImportPkgs ordered by ImportPath.
Created []*PackageInfo
// Imported contains the initially imported packages,
@@ -376,8 +376,12 @@
return conf.Fset
}
-// ParseFile is a convenience function that invokes the parser using
-// the Config's FileSet, which is initialized if nil.
+// ParseFile is a convenience function (intended for testing) that invokes
+// the parser using the Config's FileSet, which is initialized if nil.
+//
+// src specifies the parser input as a string, []byte, or io.Reader, and
+// filename is its apparent name. If src is nil, the contents of
+// filename are read from the file system.
//
func (conf *Config) ParseFile(filename string, src interface{}) (*ast.File, error) {
// TODO(adonovan): use conf.build() etc like parseFiles does.
@@ -410,9 +414,6 @@
import path may denote two packages. (Whether this behaviour is
enabled is tool-specific, and may depend on additional flags.)
- Due to current limitations in the type-checker, only the first
- import path of the command line will contribute any tests.
-
A '--' argument terminates the list of packages.
`
@@ -424,7 +425,11 @@
// set of initial packages to be specified; see FromArgsUsage message
// for details.
//
-func (conf *Config) FromArgs(args []string, xtest bool) (rest []string, err error) {
+// Only superficial errors are reported at this stage; errors dependent
+// on I/O are detected during Load.
+//
+func (conf *Config) FromArgs(args []string, xtest bool) ([]string, error) {
+ var rest []string
for i, arg := range args {
if arg == "--" {
rest = args[i+1:]
@@ -441,53 +446,35 @@
return nil, fmt.Errorf("named files must be .go files: %s", arg)
}
}
- err = conf.CreateFromFilenames("", args...)
+ conf.CreateFromFilenames("", args...)
} else {
// Assume args are directories each denoting a
// package and (perhaps) an external test, iff xtest.
for _, arg := range args {
if xtest {
- err = conf.ImportWithTests(arg)
- if err != nil {
- break
- }
+ conf.ImportWithTests(arg)
} else {
conf.Import(arg)
}
}
}
- return
+ return rest, nil
}
-// CreateFromFilenames is a convenience function that parses the
-// specified *.go files and adds a package entry for them to
-// conf.CreatePkgs.
+// CreateFromFilenames is a convenience function that adds
+// a conf.CreatePkgs entry to create a package of the specified *.go
+// files.
//
-// It fails if any file could not be loaded or parsed.
-// TODO(adonovan): make it not return an error, by making CreatePkg
-// store filenames and defer parsing until Load.
-//
-func (conf *Config) CreateFromFilenames(path string, filenames ...string) error {
- files, errs := parseFiles(conf.fset(), conf.build(), nil, ".", filenames, conf.ParserMode)
- if len(errs) > 0 {
- return errs[0]
- }
- conf.CreateFromFiles(path, files...)
- return nil
+func (conf *Config) CreateFromFilenames(path string, filenames ...string) {
+ conf.CreatePkgs = append(conf.CreatePkgs, PkgSpec{Path: path, Filenames: filenames})
}
-// CreateFromFiles is a convenience function that adds a CreatePkgs
+// CreateFromFiles is a convenience function that adds a conf.CreatePkgs
// entry to create package of the specified path and parsed files.
//
-// Precondition: conf.Fset is non-nil and was the fileset used to parse
-// the files. (e.g. the files came from conf.ParseFile().)
-//
func (conf *Config) CreateFromFiles(path string, files ...*ast.File) {
- if conf.Fset == nil {
- panic("nil Fset")
- }
- conf.CreatePkgs = append(conf.CreatePkgs, CreatePkg{path, files})
+ conf.CreatePkgs = append(conf.CreatePkgs, PkgSpec{Path: path, Files: files})
}
// ImportWithTests is a convenience function that adds path to
@@ -500,45 +487,21 @@
// declaration, an additional package comprising just those files will
// be added to CreatePkgs.
//
-func (conf *Config) ImportWithTests(path string) error {
- if path == "unsafe" {
- return nil // ignore; not a real package
- }
- conf.Import(path)
-
- // Load the external test package.
- bp, err := conf.findSourcePackage(path)
- if err != nil {
- return err // package not found
- }
- xtestFiles, errs := conf.parsePackageFiles(bp, 'x')
- if len(errs) > 0 {
- // TODO(adonovan): fix: parse errors in x_test.go files
- // cause FromArgs() to fail completely.
- return errs[0] // I/O or parse error
- }
- if len(xtestFiles) > 0 {
- conf.CreateFromFiles(path+"_test", xtestFiles...)
- }
-
- // Mark the non-xtest package for augmentation with
- // in-package *_test.go files when we import it below.
- conf.ImportPkgs[path] = true
- return nil
-}
+func (conf *Config) ImportWithTests(path string) { conf.addImport(path, true) }
// Import is a convenience function that adds path to ImportPkgs, the
// set of initial packages that will be imported from source.
//
-func (conf *Config) Import(path string) {
+func (conf *Config) Import(path string) { conf.addImport(path, false) }
+
+func (conf *Config) addImport(path string, tests bool) {
if path == "unsafe" {
return // ignore; not a real package
}
if conf.ImportPkgs == nil {
conf.ImportPkgs = make(map[string]bool)
}
- // Subtle: adds value 'false' unless value is already true.
- conf.ImportPkgs[path] = conf.ImportPkgs[path] // unaugmented source package
+ conf.ImportPkgs[path] = conf.ImportPkgs[path] || tests
}
// PathEnclosingInterval returns the PackageInfo and ast.Node that
@@ -679,59 +642,91 @@
// -- loading proper (concurrent phase) --------------------------------
+ var errpkgs []string // packages that contained errors
+
// Load the initially imported packages and their dependencies,
// in parallel.
for _, ii := range imp.loadAll("", conf.ImportPkgs) {
if ii.err != nil {
- return nil, ii.err // failed to create package
+ conf.TypeChecker.Error(ii.err) // failed to create package
+ errpkgs = append(errpkgs, ii.path)
+ continue
}
prog.Imported[ii.info.Pkg.Path()] = ii.info
}
- // Augment the initial packages that need it.
+ // Augment the designated initial packages by their tests.
// Dependencies are loaded in parallel.
+ var xtestPkgs []*build.Package
for path, augment := range conf.ImportPkgs {
- if augment {
- // Find and create the actual package.
- bp, err := conf.findSourcePackage(path)
- if err != nil {
- // "Can't happen" because of previous loop.
- return nil, err // package not found
- }
-
- imp.importedMu.Lock() // (unnecessary, we're sequential here)
- info := imp.imported[path].info // must be non-nil, see above
- imp.importedMu.Unlock()
-
- files, errs := imp.conf.parsePackageFiles(bp, 't')
- for _, err := range errs {
- info.appendError(err)
- }
- // The test files augmenting package P cannot be imported,
- // but may import packages that import P,
- // so we must disable the cycle check.
- imp.addFiles(info, files, false)
+ if !augment {
+ continue
}
+
+ bp, err := conf.findSourcePackage(path)
+ if err != nil {
+ // Package not found, or can't even parse package declaration.
+ // Already reported by previous loop; ignore it.
+ continue
+ }
+
+ // Needs external test package?
+ if len(bp.XTestGoFiles) > 0 {
+ xtestPkgs = append(xtestPkgs, bp)
+ }
+
+ imp.importedMu.Lock() // (unnecessary, we're sequential here)
+ info := imp.imported[path].info // must be non-nil, see above
+ imp.importedMu.Unlock()
+
+ // Parse the in-package test files.
+ files, errs := imp.conf.parsePackageFiles(bp, 't')
+ for _, err := range errs {
+ info.appendError(err)
+ }
+
+ // The test files augmenting package P cannot be imported,
+ // but may import packages that import P,
+ // so we must disable the cycle check.
+ imp.addFiles(info, files, false)
}
- // CreatePkgs includes all external test packages,
- // so they must be processed after augmentation.
- // Dependencies are loaded in parallel.
- for _, create := range conf.CreatePkgs {
- path := create.Path
- if create.Path == "" && len(create.Files) > 0 {
- path = create.Files[0].Name.Name
- }
+ createPkg := func(path string, files []*ast.File, errs []error) {
info := imp.newPackageInfo(path)
- // Ad-hoc packages are non-importable; no cycle check is needed.
- imp.addFiles(info, create.Files, false)
+ for _, err := range errs {
+ info.appendError(err)
+ }
+
+ // Ad-hoc packages are non-importable,
+ // so no cycle check is needed.
+ // addFiles loads dependencies in parallel.
+ imp.addFiles(info, files, false)
prog.Created = append(prog.Created, info)
}
+ // Create packages specified by conf.CreatePkgs.
+ for _, cp := range conf.CreatePkgs {
+ files, errs := parseFiles(conf.fset(), conf.build(), nil, ".", cp.Filenames, conf.ParserMode)
+ files = append(files, cp.Files...)
+
+ path := cp.Path
+ if path == "" && len(files) > 0 {
+ path = files[0].Name.Name
+ }
+ createPkg(path, files, errs)
+ }
+
+ // Create external test packages.
+ sort.Sort(byImportPath(xtestPkgs))
+ for _, bp := range xtestPkgs {
+ files, errs := imp.conf.parsePackageFiles(bp, 'x')
+ createPkg(bp.ImportPath+"_test", files, errs)
+ }
+
// -- finishing up (sequential) ----------------------------------------
if len(prog.Imported)+len(prog.Created) == 0 {
- return nil, errors.New("no initial packages were specified")
+ return nil, errors.New("no initial packages were loaded")
}
// Create infos for indirectly imported packages.
@@ -749,7 +744,6 @@
if !conf.AllowErrors {
// Report errors in indirectly imported packages.
- var errpkgs []string
for _, info := range prog.AllPackages {
if len(info.Errors) > 0 {
errpkgs = append(errpkgs, info.Pkg.Path())
@@ -771,6 +765,12 @@
return prog, nil
}
+type byImportPath []*build.Package
+
+func (b byImportPath) Len() int { return len(b) }
+func (b byImportPath) Less(i, j int) bool { return b[i].ImportPath < b[j].ImportPath }
+func (b byImportPath) Swap(i, j int) { b[i], b[j] = b[j], b[i] }
+
// markErrorFreePackages sets the TransitivelyErrorFree flag on all
// applicable packages.
func markErrorFreePackages(allPackages map[*types.Package]*PackageInfo) {
diff --git a/go/loader/loader_test.go b/go/loader/loader_test.go
index 5cc3bdb..2972ff5 100644
--- a/go/loader/loader_test.go
+++ b/go/loader/loader_test.go
@@ -5,8 +5,8 @@
package loader_test
import (
- "fmt"
"go/build"
+ "reflect"
"sort"
"strings"
"sync"
@@ -16,111 +16,319 @@
"golang.org/x/tools/go/loader"
)
-func loadFromArgs(args []string) (prog *loader.Program, rest []string, err error) {
- conf := &loader.Config{}
- rest, err = conf.FromArgs(args, true)
- if err == nil {
- prog, err = conf.Load()
+// TestFromArgs checks that conf.FromArgs populates conf correctly.
+// It does no I/O.
+func TestFromArgs(t *testing.T) {
+ type result struct {
+ Err string
+ Rest []string
+ ImportPkgs map[string]bool
+ CreatePkgs []loader.PkgSpec
}
- return
+ for _, test := range []struct {
+ args []string
+ tests bool
+ want result
+ }{
+ // Mix of existing and non-existent packages.
+ {
+ args: []string{"nosuchpkg", "errors"},
+ want: result{
+ ImportPkgs: map[string]bool{"errors": false, "nosuchpkg": false},
+ },
+ },
+ // Same, with -test flag.
+ {
+ args: []string{"nosuchpkg", "errors"},
+ tests: true,
+ want: result{
+ ImportPkgs: map[string]bool{"errors": true, "nosuchpkg": true},
+ },
+ },
+ // Surplus arguments.
+ {
+ args: []string{"fmt", "errors", "--", "surplus"},
+ want: result{
+ Rest: []string{"surplus"},
+ ImportPkgs: map[string]bool{"errors": false, "fmt": false},
+ },
+ },
+ // Ad hoc package specified as *.go files.
+ {
+ args: []string{"foo.go", "bar.go"},
+ want: result{CreatePkgs: []loader.PkgSpec{{
+ Filenames: []string{"foo.go", "bar.go"},
+ }}},
+ },
+ // Mixture of *.go and import paths.
+ {
+ args: []string{"foo.go", "fmt"},
+ want: result{
+ Err: "named files must be .go files: fmt",
+ },
+ },
+ } {
+ var conf loader.Config
+ rest, err := conf.FromArgs(test.args, test.tests)
+ got := result{
+ Rest: rest,
+ ImportPkgs: conf.ImportPkgs,
+ CreatePkgs: conf.CreatePkgs,
+ }
+ if err != nil {
+ got.Err = err.Error()
+ }
+ if !reflect.DeepEqual(got, test.want) {
+ t.Errorf("FromArgs(%q) = %+v, want %+v", test.args, got, test.want)
+ }
+ }
}
-func TestLoadFromArgs(t *testing.T) {
- // Failed load: bad first import path causes parsePackageFiles to fail.
- args := []string{"nosuchpkg", "errors"}
- if _, _, err := loadFromArgs(args); err == nil {
- t.Errorf("loadFromArgs(%q) succeeded, want failure", args)
- } else {
- // cannot find package: ok.
- }
+func TestLoad_NoInitialPackages(t *testing.T) {
+ var conf loader.Config
- // Failed load: bad second import path proceeds to doImport0, which fails.
- args = []string{"errors", "nosuchpkg"}
- if _, _, err := loadFromArgs(args); err == nil {
- t.Errorf("loadFromArgs(%q) succeeded, want failure", args)
- } else {
- // cannot find package: ok
- }
+ const wantErr = "no initial packages were loaded"
- // Successful load.
- args = []string{"fmt", "errors", "--", "surplus"}
- prog, rest, err := loadFromArgs(args)
+ prog, err := conf.Load()
+ if err == nil {
+ t.Errorf("Load succeeded unexpectedly, want %q", wantErr)
+ } else if err.Error() != wantErr {
+ t.Errorf("Load failed with wrong error %q, want %q", err, wantErr)
+ }
+ if prog != nil {
+ t.Errorf("Load unexpectedly returned a Program")
+ }
+}
+
+func TestLoad_MissingInitialPackage(t *testing.T) {
+ var conf loader.Config
+ conf.Import("nosuchpkg")
+ conf.Import("errors")
+
+ const wantErr = "couldn't load packages due to errors: nosuchpkg"
+
+ prog, err := conf.Load()
+ if err == nil {
+ t.Errorf("Load succeeded unexpectedly, want %q", wantErr)
+ } else if err.Error() != wantErr {
+ t.Errorf("Load failed with wrong error %q, want %q", err, wantErr)
+ }
+ if prog != nil {
+ t.Errorf("Load unexpectedly returned a Program")
+ }
+}
+
+func TestLoad_MissingInitialPackage_AllowErrors(t *testing.T) {
+ var conf loader.Config
+ conf.AllowErrors = true
+ conf.Import("nosuchpkg")
+ conf.ImportWithTests("errors")
+
+ prog, err := conf.Load()
if err != nil {
- t.Fatalf("loadFromArgs(%q) failed: %s", args, err)
+ t.Errorf("Load failed unexpectedly: %v", err)
}
- if got, want := fmt.Sprint(rest), "[surplus]"; got != want {
- t.Errorf("loadFromArgs(%q) rest: got %s, want %s", args, got, want)
+ if prog == nil {
+ t.Fatalf("Load returned a nil Program")
}
- // Check list of Created packages.
- var pkgnames []string
- for _, info := range prog.Created {
- pkgnames = append(pkgnames, info.Pkg.Path())
+ if got, want := created(prog), "errors_test"; got != want {
+ t.Errorf("Created = %s, want %s", got, want)
}
- // All import paths may contribute tests.
- if got, want := fmt.Sprint(pkgnames), "[fmt_test errors_test]"; got != want {
- t.Errorf("Created: got %s, want %s", got, want)
+ if got, want := imported(prog), "errors"; got != want {
+ t.Errorf("Imported = %s, want %s", got, want)
+ }
+}
+
+func TestLoad_ParseError(t *testing.T) {
+ var conf loader.Config
+ conf.CreateFromFilenames("badpkg", "testdata/badpkgdecl.go")
+
+ const wantErr = "couldn't load packages due to errors: badpkg"
+
+ prog, err := conf.Load()
+ if err == nil {
+ t.Errorf("Load succeeded unexpectedly, want %q", wantErr)
+ } else if err.Error() != wantErr {
+ t.Errorf("Load failed with wrong error %q, want %q", err, wantErr)
+ }
+ if prog != nil {
+ t.Errorf("Load unexpectedly returned a Program")
+ }
+}
+
+func TestLoad_ParseError_AllowErrors(t *testing.T) {
+ var conf loader.Config
+ conf.AllowErrors = true
+ conf.CreateFromFilenames("badpkg", "testdata/badpkgdecl.go")
+
+ prog, err := conf.Load()
+ if err != nil {
+ t.Errorf("Load failed unexpectedly: %v", err)
+ }
+ if prog == nil {
+ t.Fatalf("Load returned a nil Program")
+ }
+ if got, want := created(prog), "badpkg"; got != want {
+ t.Errorf("Created = %s, want %s", got, want)
}
- // Check set of Imported packages.
- pkgnames = nil
- for path := range prog.Imported {
- pkgnames = append(pkgnames, path)
+ badpkg := prog.Created[0]
+ if len(badpkg.Files) != 1 {
+ t.Errorf("badpkg has %d files, want 1", len(badpkg.Files))
}
- sort.Strings(pkgnames)
- // All import paths may contribute tests.
- if got, want := fmt.Sprint(pkgnames), "[errors fmt]"; got != want {
- t.Errorf("Loaded: got %s, want %s", got, want)
+ wantErr := "testdata/badpkgdecl.go:1:34: expected 'package', found 'EOF'"
+ if !hasError(badpkg.Errors, wantErr) {
+ t.Errorf("badpkg.Errors = %v, want %s", badpkg.Errors, wantErr)
}
+}
+func TestLoad_FromSource_Success(t *testing.T) {
+ var conf loader.Config
+ conf.CreateFromFilenames("P", "testdata/a.go", "testdata/b.go")
+
+ prog, err := conf.Load()
+ if err != nil {
+ t.Errorf("Load failed unexpectedly: %v", err)
+ }
+ if prog == nil {
+ t.Fatalf("Load returned a nil Program")
+ }
+ if got, want := created(prog), "P"; got != want {
+ t.Errorf("Created = %s, want %s", got, want)
+ }
+}
+
+func TestLoad_FromImports_Success(t *testing.T) {
+ var conf loader.Config
+ conf.ImportWithTests("fmt")
+ conf.ImportWithTests("errors")
+
+ prog, err := conf.Load()
+ if err != nil {
+ t.Errorf("Load failed unexpectedly: %v", err)
+ }
+ if prog == nil {
+ t.Fatalf("Load returned a nil Program")
+ }
+ if got, want := created(prog), "errors_test fmt_test"; got != want {
+ t.Errorf("Created = %q, want %s", got, want)
+ }
+ if got, want := imported(prog), "errors fmt"; got != want {
+ t.Errorf("Imported = %s, want %s", got, want)
+ }
// Check set of transitive packages.
// There are >30 and the set may grow over time, so only check a few.
- all := map[string]struct{}{}
- for _, info := range prog.AllPackages {
- all[info.Pkg.Path()] = struct{}{}
+ want := map[string]bool{
+ "strings": true,
+ "time": true,
+ "runtime": true,
+ "testing": true,
+ "unicode": true,
}
- want := []string{"strings", "time", "runtime", "testing", "unicode"}
- for _, w := range want {
- if _, ok := all[w]; !ok {
- t.Errorf("AllPackages: want element %s, got set %v", w, all)
- }
+ for _, path := range all(prog) {
+ delete(want, path)
+ }
+ if len(want) > 0 {
+ t.Errorf("AllPackages is missing these keys: %q", keys(want))
}
}
-func TestLoadFromArgsSource(t *testing.T) {
- // mixture of *.go/non-go.
- args := []string{"testdata/a.go", "fmt"}
- prog, _, err := loadFromArgs(args)
+func TestLoad_MissingIndirectImport(t *testing.T) {
+ pkgs := map[string]string{
+ "a": `package a; import _ "b"`,
+ "b": `package b; import _ "c"`,
+ }
+ conf := loader.Config{
+ SourceImports: true,
+ Build: fakeContext(pkgs),
+ }
+ conf.Import("a")
+
+ const wantErr = "couldn't load packages due to errors: b"
+
+ prog, err := conf.Load()
if err == nil {
- t.Errorf("loadFromArgs(%q) succeeded, want failure", args)
- } else {
- // "named files must be .go files: fmt": ok
+ t.Errorf("Load succeeded unexpectedly, want %q", wantErr)
+ } else if err.Error() != wantErr {
+ t.Errorf("Load failed with wrong error %q, want %q", err, wantErr)
}
+ if prog != nil {
+ t.Errorf("Load unexpectedly returned a Program")
+ }
+}
- // successful load
- args = []string{"testdata/a.go", "testdata/b.go"}
- prog, _, err = loadFromArgs(args)
- if err != nil {
- t.Fatalf("loadFromArgs(%q) failed: %s", args, err)
- }
- if len(prog.Created) != 1 {
- t.Errorf("loadFromArgs(%q): got %d items, want 1", args, len(prog.Created))
- }
- if len(prog.Created) > 0 {
- path := prog.Created[0].Pkg.Path()
- if path != "P" {
- t.Errorf("loadFromArgs(%q): got %v, want [P]", prog.Created, path)
+func TestLoad_BadDependency_AllowErrors(t *testing.T) {
+ for _, test := range []struct {
+ descr string
+ pkgs map[string]string
+ wantPkgs string
+ }{
+
+ {
+ descr: "missing dependency",
+ pkgs: map[string]string{
+ "a": `package a; import _ "b"`,
+ "b": `package b; import _ "c"`,
+ },
+ wantPkgs: "a b",
+ },
+ {
+ descr: "bad package decl in dependency",
+ pkgs: map[string]string{
+ "a": `package a; import _ "b"`,
+ "b": `package b; import _ "c"`,
+ "c": `package`,
+ },
+ wantPkgs: "a b",
+ },
+ {
+ descr: "parse error in dependency",
+ pkgs: map[string]string{
+ "a": `package a; import _ "b"`,
+ "b": `package b; import _ "c"`,
+ "c": `package c; var x = `,
+ },
+ wantPkgs: "a b c",
+ },
+ } {
+ conf := loader.Config{
+ AllowErrors: true,
+ SourceImports: true,
+ Build: fakeContext(test.pkgs),
+ }
+ conf.Import("a")
+
+ prog, err := conf.Load()
+ if err != nil {
+ t.Errorf("%s: Load failed unexpectedly: %v", test.descr, err)
+ }
+ if prog == nil {
+ t.Fatalf("%s: Load returned a nil Program", test.descr)
+ }
+
+ if got, want := imported(prog), "a"; got != want {
+ t.Errorf("%s: Imported = %s, want %s", test.descr, got, want)
+ }
+ if got := all(prog); strings.Join(got, " ") != test.wantPkgs {
+ t.Errorf("%s: AllPackages = %s, want %s", test.descr, got, test.wantPkgs)
}
}
}
-// Simplifying wrapper around buildutil.FakeContext for single-file packages.
-func fakeContext(pkgs map[string]string) *build.Context {
- pkgs2 := make(map[string]map[string]string)
- for path, content := range pkgs {
- pkgs2[path] = map[string]string{"x.go": content}
- }
- return buildutil.FakeContext(pkgs2)
-}
+// TODO(adonovan): more Load tests:
+//
+// failures:
+// - to parse package decl of *_test.go files
+// - to parse package decl of external *_test.go files
+// - to parse whole of *_test.go files
+// - to parse whole of external *_test.go files
+// - to open a *.go file during import scanning
+// - to import from binary
+
+// features:
+// - InitialPackages
+// - PackageCreated hook
+// - TypeCheckFuncBodies hook
func TestTransitivelyErrorFreeFlag(t *testing.T) {
// Create an minimal custom build.Context
@@ -182,20 +390,11 @@
}
}
-func hasError(errors []error, substr string) bool {
- for _, err := range errors {
- if strings.Contains(err.Error(), substr) {
- return true
- }
- }
- return false
-}
-
-// Test that both syntax (scan/parse) and type errors are both recorded
+// Test that syntax (scan/parse), type, and loader errors are recorded
// (in PackageInfo.Errors) and reported (via Config.TypeChecker.Error).
func TestErrorReporting(t *testing.T) {
pkgs := map[string]string{
- "a": `package a; import _ "b"; var x int = false`,
+ "a": `package a; import (_ "b"; _ "c"); var x int = false`,
"b": `package b; 'syntax error!`,
}
conf := loader.Config{
@@ -229,6 +428,9 @@
if !hasError(info.Errors, "cannot convert false") {
t.Errorf("a.Errors = %v, want bool conversion (type) error", info.Errors)
}
+ if !hasError(info.Errors, "could not import c") {
+ t.Errorf("a.Errors = %v, want import (loader) error", info.Errors)
+ }
case "b":
if !hasError(info.Errors, "rune literal not terminated") {
t.Errorf("b.Errors = %v, want unterminated literal (syntax) error", info.Errors)
@@ -238,8 +440,9 @@
// Check errors reported via error handler.
if !hasError(allErrors, "cannot convert false") ||
- !hasError(allErrors, "rune literal not terminated") {
- t.Errorf("allErrors = %v, want both syntax and type errors", allErrors)
+ !hasError(allErrors, "rune literal not terminated") ||
+ !hasError(allErrors, "could not import c") {
+ t.Errorf("allErrors = %v, want syntax, type and loader errors", allErrors)
}
}
@@ -337,3 +540,60 @@
// - Test that in a legal test cycle, none of the symbols
// defined by augmentation are visible via import.
}
+
+// ---- utilities ----
+
+// Simplifying wrapper around buildutil.FakeContext for single-file packages.
+func fakeContext(pkgs map[string]string) *build.Context {
+ pkgs2 := make(map[string]map[string]string)
+ for path, content := range pkgs {
+ pkgs2[path] = map[string]string{"x.go": content}
+ }
+ return buildutil.FakeContext(pkgs2)
+}
+
+func hasError(errors []error, substr string) bool {
+ for _, err := range errors {
+ if strings.Contains(err.Error(), substr) {
+ return true
+ }
+ }
+ return false
+}
+
+func keys(m map[string]bool) (keys []string) {
+ for key := range m {
+ keys = append(keys, key)
+ }
+ sort.Strings(keys)
+ return
+}
+
+// Returns all loaded packages.
+func all(prog *loader.Program) []string {
+ var pkgs []string
+ for _, info := range prog.AllPackages {
+ pkgs = append(pkgs, info.Pkg.Path())
+ }
+ sort.Strings(pkgs)
+ return pkgs
+}
+
+// Returns initially imported packages, as a string.
+func imported(prog *loader.Program) string {
+ var pkgs []string
+ for _, info := range prog.Imported {
+ pkgs = append(pkgs, info.Pkg.Path())
+ }
+ sort.Strings(pkgs)
+ return strings.Join(pkgs, " ")
+}
+
+// Returns initially created packages, as a string.
+func created(prog *loader.Program) string {
+ var pkgs []string
+ for _, info := range prog.Created {
+ pkgs = append(pkgs, info.Pkg.Path())
+ }
+ return strings.Join(pkgs, " ")
+}
diff --git a/go/loader/stdlib_test.go b/go/loader/stdlib_test.go
index eb439a1..d14928a 100644
--- a/go/loader/stdlib_test.go
+++ b/go/loader/stdlib_test.go
@@ -40,9 +40,7 @@
ctxt.GOPATH = "" // disable GOPATH
conf := loader.Config{Build: &ctxt}
for _, path := range buildutil.AllPackages(conf.Build) {
- if err := conf.ImportWithTests(path); err != nil {
- t.Error(err)
- }
+ conf.ImportWithTests(path)
}
prog, err := conf.Load()
diff --git a/go/loader/testdata/badpkgdecl.go b/go/loader/testdata/badpkgdecl.go
new file mode 100644
index 0000000..1e39359
--- /dev/null
+++ b/go/loader/testdata/badpkgdecl.go
@@ -0,0 +1 @@
+// this file has no package decl
diff --git a/go/loader/util.go b/go/loader/util.go
index 1d782e1..1166c92 100644
--- a/go/loader/util.go
+++ b/go/loader/util.go
@@ -11,9 +11,10 @@
"go/token"
"io"
"os"
- "path/filepath"
"strconv"
"sync"
+
+ "golang.org/x/tools/go/buildutil"
)
// parseFiles parses the Go source files within directory dir and
@@ -27,21 +28,13 @@
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)
errors := make([]error, n)
for i, file := range files {
- if !isAbs(file) {
- file = joinPath(dir, file)
+ if !buildutil.IsAbsPath(ctxt, file) {
+ file = buildutil.JoinPath(ctxt, dir, file)
}
wg.Add(1)
go func(i int, file string) {
diff --git a/go/pointer/example_test.go b/go/pointer/example_test.go
index eecfd30..f6cca48 100644
--- a/go/pointer/example_test.go
+++ b/go/pointer/example_test.go
@@ -44,7 +44,8 @@
// Construct a loader.
conf := loader.Config{SourceImports: true}
- // Parse the input file.
+ // Parse the input file, a string.
+ // (Command-line tools should use conf.FromArgs.)
file, err := conf.ParseFile("myprog.go", myprog)
if err != nil {
fmt.Print(err) // parse error
diff --git a/go/ssa/interp/interp_test.go b/go/ssa/interp/interp_test.go
index 5d458f3..19f2ca0 100644
--- a/go/ssa/interp/interp_test.go
+++ b/go/ssa/interp/interp_test.go
@@ -340,9 +340,7 @@
// CreateTestMainPackage should return nil if there were no tests.
func TestNullTestmainPackage(t *testing.T) {
var conf loader.Config
- if err := conf.CreateFromFilenames("", "testdata/b_test.go"); err != nil {
- t.Fatalf("ParseFile failed: %s", err)
- }
+ conf.CreateFromFilenames("", "testdata/b_test.go")
iprog, err := conf.Load()
if err != nil {
t.Fatalf("CreatePackages failed: %s", err)
diff --git a/oracle/oracle.go b/oracle/oracle.go
index 60cc2be..4bb8a9d 100644
--- a/oracle/oracle.go
+++ b/oracle/oracle.go
@@ -322,7 +322,7 @@
// (and possibly its corresponding tests/production code).
// TODO(adonovan): set 'augment' based on which file list
// contains
- _ = conf.ImportWithTests(importPath) // ignore error
+ conf.ImportWithTests(importPath)
}
func pkgContainsFile(bp *build.Package, filename string) bool {
diff --git a/refactor/eg/eg_test.go b/refactor/eg/eg_test.go
index 6e9fc4b..bb96faf 100644
--- a/refactor/eg/eg_test.go
+++ b/refactor/eg/eg_test.go
@@ -72,9 +72,7 @@
"testdata/expr_type_mismatch.template",
} {
pkgname := strings.TrimSuffix(filepath.Base(filename), ".go")
- if err := conf.CreateFromFilenames(pkgname, filename); err != nil {
- t.Fatal(err)
- }
+ conf.CreateFromFilenames(pkgname, filename)
}
iprog, err := conf.Load()
if err != nil {
diff --git a/refactor/lexical/lexical_test.go b/refactor/lexical/lexical_test.go
index ada6595..1b772d6 100644
--- a/refactor/lexical/lexical_test.go
+++ b/refactor/lexical/lexical_test.go
@@ -37,11 +37,8 @@
SourceImports: true,
}
for _, path := range pkgs {
- if err := conf.ImportWithTests(path); err != nil {
- t.Error(err)
- }
+ conf.ImportWithTests(path)
}
-
iprog, err := conf.Load()
if err != nil {
t.Fatalf("Load failed: %v", err)
diff --git a/refactor/rename/rename.go b/refactor/rename/rename.go
index 6543685..fc76b32 100644
--- a/refactor/rename/rename.go
+++ b/refactor/rename/rename.go
@@ -365,9 +365,7 @@
}
for pkg := range pkgs {
- if err := conf.ImportWithTests(pkg); err != nil {
- return nil, err
- }
+ conf.ImportWithTests(pkg)
}
return conf.Load()
}
