diff --git a/cmd/eg/eg.go b/cmd/eg/eg.go index 0d9cf915b7..cc3ea643cb 100644 --- a/cmd/eg/eg.go +++ b/cmd/eg/eg.go @@ -61,9 +61,7 @@ func doMain() error { } // 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 2050780cd6..1901b30de1 100644 --- a/cmd/ssadump/main.go +++ b/cmd/ssadump/main.go @@ -203,7 +203,7 @@ func doMain() error { } 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 26a6ae2816..0fdda05a30 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 @@ package loader // 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 @@ import ( "go/parser" "go/token" "os" + "sort" "strings" "sync" "time" @@ -215,7 +210,7 @@ const trace = false // show timing info for type-checking // 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 @@ type Config struct { 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 @@ type Config struct { 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 CreatePkg struct { 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 @@ func (conf *Config) fset() *token.FileSet { 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 @@ It may take one of two forms: 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 @@ A '--' argument terminates the list of packages. // 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 @@ func (conf *Config) FromArgs(args []string, xtest bool) (rest []string, err erro 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 @@ func (conf *Config) CreateFromFiles(path string, files ...*ast.File) { // 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 @@ func (conf *Config) Load() (*Program, error) { // -- 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 @@ func (conf *Config) Load() (*Program, error) { 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 @@ func (conf *Config) Load() (*Program, error) { 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 5cc3bdbcbf..2972ff5218 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 @@ import ( "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 + } + 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) + } } - return } -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 TestTransitivelyErrorFreeFlag(t *testing.T) { } } -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 @@ func TestErrorReporting(t *testing.T) { 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 @@ func TestErrorReporting(t *testing.T) { // 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 @@ func TestCycles(t *testing.T) { // - 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 eb439a15f8..d14928a72c 100644 --- a/go/loader/stdlib_test.go +++ b/go/loader/stdlib_test.go @@ -40,9 +40,7 @@ func TestStdlib(t *testing.T) { 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 0000000000..1e393595c3 --- /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 1d782e1e48..1166c92219 100644 --- a/go/loader/util.go +++ b/go/loader/util.go @@ -11,9 +11,10 @@ import ( "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 @@ func parseFiles(fset *token.FileSet, ctxt *build.Context, displayPath func(strin 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 eecfd30ffd..f6cca48d2c 100644 --- a/go/pointer/example_test.go +++ b/go/pointer/example_test.go @@ -44,7 +44,8 @@ func main() { // 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 5d458f3054..19f2ca0840 100644 --- a/go/ssa/interp/interp_test.go +++ b/go/ssa/interp/interp_test.go @@ -340,9 +340,7 @@ func TestTestmainPackage(t *testing.T) { // 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 60cc2be5c7..4bb8a9d35e 100644 --- a/oracle/oracle.go +++ b/oracle/oracle.go @@ -322,7 +322,7 @@ func reduceScope(pos string, conf *loader.Config) { // (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 6e9fc4ba8c..bb96faf965 100644 --- a/refactor/eg/eg_test.go +++ b/refactor/eg/eg_test.go @@ -72,9 +72,7 @@ func Test(t *testing.T) { "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 ada65956bb..1b772d6ffa 100644 --- a/refactor/lexical/lexical_test.go +++ b/refactor/lexical/lexical_test.go @@ -37,11 +37,8 @@ func TestStdlib(t *testing.T) { 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 6543685457..fc76b32eae 100644 --- a/refactor/rename/rename.go +++ b/refactor/rename/rename.go @@ -365,9 +365,7 @@ func loadProgram(ctxt *build.Context, pkgs map[string]bool) (*loader.Program, er } for pkg := range pkgs { - if err := conf.ImportWithTests(pkg); err != nil { - return nil, err - } + conf.ImportWithTests(pkg) } return conf.Load() }