diff --git a/go/loader/importer_test.go b/go/loader/importer_test.go index 54aec5b0..8267b636 100644 --- a/go/loader/importer_test.go +++ b/go/loader/importer_test.go @@ -6,7 +6,11 @@ package loader_test import ( "fmt" + "go/ast" + "go/build" + "go/token" "sort" + "strings" "testing" "code.google.com/p/go.tools/go/loader" @@ -103,3 +107,80 @@ func TestLoadFromArgsSource(t *testing.T) { t.Errorf("loadFromArgs(%q): got %v, want [P]", prog.Created) } } + +func TestTransitivelyErrorFreeFlag(t *testing.T) { + conf := loader.Config{ + AllowTypeErrors: true, + SourceImports: true, + } + conf.Import("a") + + // Fake 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 string) (files []*ast.File, err error) { + if !strings.Contains(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 + } + defer func() { *pfn = saved }() + + prog, err := conf.Load() + if err != nil { + t.Errorf("Load failed: %s", err) + } + if prog == nil { + t.Fatalf("Load returnd nil *Program") + } + + for pkg, info := range prog.AllPackages { + var wantErr, wantTEF bool + switch pkg.Path() { + case "a", "b": + case "c": + wantErr = true + case "d", "e": + wantTEF = true + default: + t.Errorf("unexpected package: %q", pkg.Path()) + continue + } + + if (info.TypeError != nil) != wantErr { + if wantErr { + t.Errorf("Package %q.TypeError = nil, want error", pkg.Path()) + } else { + t.Errorf("Package %q has unexpected TypeError: %s", + pkg.Path(), info.TypeError) + } + } + + if info.TransitivelyErrorFree != wantTEF { + t.Errorf("Package %q.TransitivelyErrorFree=%t, want %t", + info.TransitivelyErrorFree, wantTEF) + } + } +} diff --git a/go/loader/loader.go b/go/loader/loader.go index 3fcd7f87..16c330a3 100644 --- a/go/loader/loader.go +++ b/go/loader/loader.go @@ -95,6 +95,7 @@ package loader // go/types; see bug 7114. // - s/path/importPath/g to avoid ambiguity with other meanings of // "path": a file name, a colon-separated directory list. +// - Thorough overhaul of package documentation. import ( "errors" @@ -158,6 +159,12 @@ type Config struct { // Otherwise &build.Default is used. Build *build.Context + // 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. + // If false, Load will fail if any package had a type error. + AllowTypeErrors 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 @@ -385,7 +392,7 @@ func (conf *Config) Import(path string) { // up to the AST root. It searches all ast.Files of all packages in prog. // exact is defined as for astutil.PathEnclosingInterval. // -// The result is (nil, nil, false) if not found. +// The zero value is returned if not found. // func (prog *Program) PathEnclosingInterval(start, end token.Pos) (pkg *PackageInfo, path []ast.Node, exact bool) { for _, info := range prog.AllPackages { @@ -431,8 +438,11 @@ type importInfo struct { // Load creates the initial packages specified by conf.{Create,Import}Pkgs, // loading their dependencies packages as needed. // -// On success, it returns a Program containing a PackageInfo for each -// package; all are well-typed. +// On success, Load returns a Program containing a PackageInfo for +// each package. On failure, it returns an error. +// +// If conf.AllowTypeErrors is set, a type error does not cause Load to +// fail, but is recorded in the PackageInfo.TypeError field. // // It is an error if no packages were loaded. // @@ -459,6 +469,8 @@ func (conf *Config) Load() (*Program, error) { for path := range conf.ImportPkgs { info, err := imp.importPackage(path) if err != nil { + // TODO(adonovan): don't abort Load just + // because of a parse error in one package. return nil, err // e.g. parse error (but not type error) } prog.Imported[path] = info @@ -473,19 +485,7 @@ func (conf *Config) Load() (*Program, error) { } if len(prog.Imported)+len(prog.Created) == 0 { - return nil, errors.New("no *.go source files nor packages were specified") - } - - // Report errors in indirectly imported packages. - var errpkgs []string - for _, info := range prog.AllPackages { - if info.err != nil { - errpkgs = append(errpkgs, info.Pkg.Path()) - } - } - if errpkgs != nil { - return nil, fmt.Errorf("couldn't load packages due to type errors: %s", - strings.Join(errpkgs, ", ")) + return nil, errors.New("no initial packages were specified") } // Create infos for indirectly imported packages. @@ -496,9 +496,66 @@ func (conf *Config) Load() (*Program, error) { } } + if !conf.AllowTypeErrors { + // Report errors in indirectly imported packages. + var errpkgs []string + for _, info := range prog.AllPackages { + if info.TypeError != nil { + errpkgs = append(errpkgs, info.Pkg.Path()) + } + } + if errpkgs != nil { + return nil, fmt.Errorf("couldn't load packages due to type errors: %s", + strings.Join(errpkgs, ", ")) + } + } + + markErrorFreePackages(prog.AllPackages) + return prog, nil } +// markErrorFreePackages sets the TransitivelyErrorFree flag on all +// applicable packages. +func markErrorFreePackages(allPackages map[*types.Package]*PackageInfo) { + // Build the transpose of the import graph. + importedBy := make(map[*types.Package]map[*types.Package]bool) + for P := range allPackages { + for _, Q := range P.Imports() { + clients, ok := importedBy[Q] + if !ok { + clients = make(map[*types.Package]bool) + importedBy[Q] = clients + } + clients[P] = true + } + } + + // Find all packages reachable from some error package. + reachable := make(map[*types.Package]bool) + var visit func(*types.Package) + visit = func(p *types.Package) { + if !reachable[p] { + reachable[p] = true + for q := range importedBy[p] { + visit(q) + } + } + } + for _, info := range allPackages { + if info.TypeError != nil { + visit(info.Pkg) + } + } + + // Mark the others as "transitively error-free". + for _, info := range allPackages { + if !reachable[info.Pkg] { + info.TransitivelyErrorFree = true + } + } +} + // build returns the effective build context. func (conf *Config) build() *build.Context { if conf.Build != nil { @@ -619,7 +676,7 @@ func (imp *importer) importFromSource(path string, which string) (*PackageInfo, // importable, i.e. no 'import' spec can resolve to it. // // createPackage never fails, but the resulting package may contain type -// errors; the first of these is recorded in PackageInfo.err. +// errors; the first of these is recorded in PackageInfo.TypeError. // func (imp *importer) createPackage(path string, files ...*ast.File) *PackageInfo { info := &PackageInfo{ @@ -640,10 +697,11 @@ func (imp *importer) createPackage(path string, files ...*ast.File) *PackageInfo tc.IgnoreFuncBodies = !f(path) } if tc.Error == nil { + // TODO(adonovan): also save errors in the PackageInfo? tc.Error = func(e error) { fmt.Fprintln(os.Stderr, e) } } tc.Import = imp.doImport // doImport wraps the user's importfn, effectively - info.Pkg, info.err = tc.Check(path, imp.conf.fset(), files, &info.Info) + info.Pkg, info.TypeError = tc.Check(path, imp.conf.fset(), files, &info.Info) imp.prog.AllPackages[info.Pkg] = info return info } diff --git a/go/loader/pkginfo.go b/go/loader/pkginfo.go index bb43ff18..dba4f0c7 100644 --- a/go/loader/pkginfo.go +++ b/go/loader/pkginfo.go @@ -18,11 +18,12 @@ import ( // Not mutated once constructed. // type PackageInfo struct { - Pkg *types.Package - Importable bool // true if 'import "Pkg.Path()"' would resolve to this - Files []*ast.File // abstract syntax for the package's files - err error // non-nil if the package had static errors - types.Info // type-checker deductions. + Pkg *types.Package + Importable bool // true if 'import "Pkg.Path()"' would resolve to this + TransitivelyErrorFree bool // true if Pkg and all its dependencies are free of errors + Files []*ast.File // abstract syntax for the package's files + TypeError error // non-nil if the package had type errors + types.Info // type-checker deductions. } func (info *PackageInfo) String() string { diff --git a/go/loader/util.go b/go/loader/util.go index 164ee66a..1bc4ea0c 100644 --- a/go/loader/util.go +++ b/go/loader/util.go @@ -21,7 +21,12 @@ import ( // 't': include in-package *_test.go source files (TestGoFiles) // 'x': include external *_test.go source files. (XTestGoFiles) // -func parsePackageFiles(ctxt *build.Context, fset *token.FileSet, path string, which string) ([]*ast.File, error) { +// 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 string) ([]*ast.File, error) { // Set the "!cgo" go/build tag, preferring (dummy) Go to // native C implementations of net.cgoLookupHost et al. ctxt2 := *ctxt diff --git a/go/ssa/create.go b/go/ssa/create.go index a16dc74f..cab8ba81 100644 --- a/go/ssa/create.go +++ b/go/ssa/create.go @@ -29,8 +29,8 @@ const ( GlobalDebug // Enable debug info for all packages ) -// Create returns a new SSA Program, creating all packages and all -// their members. +// Create returns a new SSA Program. An SSA Package is created for +// each transitively error-free package of iprog. // // Code for bodies of functions is not built until Build() is called // on the result. @@ -48,7 +48,9 @@ func Create(iprog *loader.Program, mode BuilderMode) *Program { } for _, info := range iprog.AllPackages { - prog.CreatePackage(info) + if info.TransitivelyErrorFree { + prog.CreatePackage(info) + } } return prog